Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-22474

Crumb must be sent with POST requests even when using authentication token

      If you are making a POST request from a tool like curl, and Jenkins is configured with a CrumbIssuer, you are required to obtain and pass back a valid crumb (preferably as an HTTP header) even when you are using an API token. This seems unnecessary, since the purpose of crumbs is to defend against CSRF attacks, which are possible only when the victim has a Jenkins session cookie in a browser or some browserlike client and unwittingly makes an HTTP request instigated by the attacker. But a request using basic authentication with an API token could not have come from such a client—you would not try to log in using an API token rather than using security realm-specific means, and in fact you cannot do so unless Jenkins sent a 401 response with a WWW-Authenticate header, which it almost never does.

      (loginError.jelly includes a 401 response without that header, which is probably wrong. BasicAuthenticationFilter, used with the legacy security realm, does send the challenge, though this does not seem to let you actually log in using an API token either—only pass through, like ApiTokenFilter would do.)

      Assuming that it is true that you cannot get a login session using an API token even if you tried, I would suggest that CrumbFilter check for req.getAttribute(ApiTokenProperty.class.getName()) instanceof User, which would be true if ApiTokenFilter has already run successfully, and in this case do nothing. (Or, for better modularity, ApiTokenFilter could implement a CrumbExclusion.) This would make the REST API significantly easier to use.

          [JENKINS-22474] Crumb must be sent with POST requests even when using authentication token

          Jesse Glick added a comment -

          Best if tokens are not retrievable, so there is no risk of /me/configure being retrieved by a cross-site script.

          Jesse Glick added a comment - Best if tokens are not retrievable, so there is no risk of /me/configure being retrieved by a cross-site script.

          As the ApiTokenFilter is deprecated now, I do the attribute setting in BasicHeaderApiTokenAuthenticator after having check the API Token.

          Concerning the retrievability, it will be addressed in JENKINS-32776.

          Wadeck Follonier added a comment - As the ApiTokenFilter is deprecated now, I do the attribute setting in BasicHeaderApiTokenAuthenticator after having check the API Token. Concerning the retrievability, it will be addressed in JENKINS-32776 .

          Code changed in jenkins
          User: Wadeck Follonier
          Path:
          core/src/main/java/jenkins/security/ApiCrumbExclusion.java
          core/src/main/java/jenkins/security/BasicHeaderApiTokenAuthenticator.java
          test/pom.xml
          test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java
          test/src/test/java/hudson/diagnosis/HudsonHomeDiskUsageMonitorTest.java
          test/src/test/java/hudson/model/AbstractProjectTest.java
          test/src/test/java/hudson/model/ExecutorTest.java
          test/src/test/java/hudson/model/ItemsTest.java
          test/src/test/java/hudson/model/JobTest.java
          test/src/test/java/hudson/model/PasswordParameterDefinitionTest.java
          test/src/test/java/hudson/model/ProjectTest.java
          test/src/test/java/hudson/model/QueueTest.java
          test/src/test/java/hudson/model/UserTest.java
          test/src/test/java/hudson/model/ViewTest.java
          test/src/test/java/hudson/security/ExtendedReadPermissionTest.java
          test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java
          test/src/test/java/hudson/security/LoginTest.java
          test/src/test/java/hudson/util/RobustReflectionConverterTest.java
          test/src/test/java/jenkins/model/JenkinsTest.java
          test/src/test/java/jenkins/security/ApiCrumbExclusionTest.java
          test/src/test/java/jenkins/security/ApiTokenPropertyTest.java
          test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java
          test/src/test/java/jenkins/security/Security380Test.java
          test/src/test/java/lib/form/PasswordTest.java
          test/src/test/resources/lib/form/PasswordTest/SecretNotPlainText/index.jelly
          test/src/test/resources/lib/form/PasswordTest/test1.jelly
          http://jenkins-ci.org/commit/jenkins/814d202716a6c61c7d371c6a62755d296fe199a5
          Log:
          JENKINS-22474 API Token does not require CSRF token (#3129)

          • in order to ease the use of the api, we are not requiring the request to have a crumb
          • in terms of security it's not a problem normally since the CSRF attacks use the cookies and in case of API Token, it's session-less / cookie-less
          • - adjust the license header
          • - add test for basic authentication
          • add test for login process
          • - add test for form submission using ui (htmlunit), not just login form
          • - modification requested by Jesse
          • - pom.xml update to use the last version of jenkins-test-harness (with the token helper methods)
          • beginning of the simplification of tests
          • - using the try-with-resource approach to ease readability
          • - using closure method now
          • - add missing login transformation
          • - add unit test
          • - use withToken
          • remove useless crumb for GET method
          • add fail (otherwise the assert in catch is not as useful as it could be)
          • another bunch of test cases
          • - for HudsonTestCase, some additional modifications are required: changing the view / different type of management for the variable inside the views
          • - small other tests
          • - last batch for the login method
          • - crumb is not more required since we are using API Token
          • - converting auth to ApiToken to avoid crumb method
          • - converting auth to ApiToken to avoid crumb method (second)
          • - remove usage of closure aware methods
          • - update the pom using the snapshot as adviced by Jesse
          • modifications on other class to adapt to the last modifications in JTH
          • - modifications requested during code review
          • - also put back my changes to the conflicted file
          • - correction of the merge

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Wadeck Follonier Path: core/src/main/java/jenkins/security/ApiCrumbExclusion.java core/src/main/java/jenkins/security/BasicHeaderApiTokenAuthenticator.java test/pom.xml test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java test/src/test/java/hudson/diagnosis/HudsonHomeDiskUsageMonitorTest.java test/src/test/java/hudson/model/AbstractProjectTest.java test/src/test/java/hudson/model/ExecutorTest.java test/src/test/java/hudson/model/ItemsTest.java test/src/test/java/hudson/model/JobTest.java test/src/test/java/hudson/model/PasswordParameterDefinitionTest.java test/src/test/java/hudson/model/ProjectTest.java test/src/test/java/hudson/model/QueueTest.java test/src/test/java/hudson/model/UserTest.java test/src/test/java/hudson/model/ViewTest.java test/src/test/java/hudson/security/ExtendedReadPermissionTest.java test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java test/src/test/java/hudson/security/LoginTest.java test/src/test/java/hudson/util/RobustReflectionConverterTest.java test/src/test/java/jenkins/model/JenkinsTest.java test/src/test/java/jenkins/security/ApiCrumbExclusionTest.java test/src/test/java/jenkins/security/ApiTokenPropertyTest.java test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java test/src/test/java/jenkins/security/Security380Test.java test/src/test/java/lib/form/PasswordTest.java test/src/test/resources/lib/form/PasswordTest/SecretNotPlainText/index.jelly test/src/test/resources/lib/form/PasswordTest/test1.jelly http://jenkins-ci.org/commit/jenkins/814d202716a6c61c7d371c6a62755d296fe199a5 Log: JENKINS-22474 API Token does not require CSRF token (#3129) JENKINS-22474 API Token does not require CSRF token in order to ease the use of the api, we are not requiring the request to have a crumb in terms of security it's not a problem normally since the CSRF attacks use the cookies and in case of API Token, it's session-less / cookie-less - adjust the license header - add test for basic authentication add test for login process - add test for form submission using ui (htmlunit), not just login form - modification requested by Jesse - pom.xml update to use the last version of jenkins-test-harness (with the token helper methods) beginning of the simplification of tests - using the try-with-resource approach to ease readability - using closure method now - add missing login transformation - add unit test - use withToken remove useless crumb for GET method add fail (otherwise the assert in catch is not as useful as it could be) another bunch of test cases - for HudsonTestCase, some additional modifications are required: changing the view / different type of management for the variable inside the views - small other tests - last batch for the login method - crumb is not more required since we are using API Token - converting auth to ApiToken to avoid crumb method - converting auth to ApiToken to avoid crumb method (second) - remove usage of closure aware methods - update the pom using the snapshot as adviced by Jesse modifications on other class to adapt to the last modifications in JTH - modifications requested during code review - also put back my changes to the conflicted file - correction of the merge

          Daniel Beck added a comment -

          Fixed towards 2.96.

          Daniel Beck added a comment - Fixed towards 2.96.

            wfollonier Wadeck Follonier
            jglick Jesse Glick
            Votes:
            6 Vote for this issue
            Watchers:
            11 Start watching this issue

              Created:
              Updated:
              Resolved: