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

Pipeline: Groovy Plugin [SECURITY-2463] excessive path length

      With this update, workspace subfolders are created with a 64-character randomized name.  This can result in path length limit violations in established Jenkins pipeline projects on a Windows system, which has a default max path of 260 characters.  The code generating this long folder name appears to be from commit SECURITY-2463/SECURITY-2595.  It looks like the supplied length integer of '32' is resulting in a a 64-character folder name.  Can the length integer be reduced to '8' or '16' for better compatibility with Windows systems?  The same code was introduced in the Pipeline: Shared Groovy Libraries and Pipeline: Multibranch plugins as well.

       

       

          [JENKINS-67836] Pipeline: Groovy Plugin [SECURITY-2463] excessive path length

          It seems CpsScmFlowDefinition does not create this 64-character subfolder if it can use lightweight checkout instead. Which it probably cannot if the SCM needs to merge a pull request.

          Kalle Niemitalo added a comment - It seems CpsScmFlowDefinition does not create this 64-character subfolder if it can use lightweight checkout instead. Which it probably cannot if the SCM needs to merge a pull request.

          Jesse Glick added a comment -

          Can the length integer be reduced

          Probably not without subverting the security fix, right dnusbaum?

          if the SCM needs to merge a pull request

          In GitHub? Try head rather than merge branch discovery mode.

          Jesse Glick added a comment - Can the length integer be reduced Probably not without subverting the security fix, right dnusbaum ? if the SCM needs to merge a pull request In GitHub? Try head rather than merge branch discovery mode.

          HMACConfidentialKey.mac uses Util.toHexString on a 256-bit MAC, resulting in 64 characters. If this were switched to Base32, then it would be 52 characters with the same security. Base64 would be 43 characters but would lose some security on case-insensitive file systems.

          Kalle Niemitalo added a comment - HMACConfidentialKey.mac uses Util.toHexString on a 256-bit MAC, resulting in 64 characters. If this were switched to Base32, then it would be 52 characters with the same security. Base64 would be 43 characters but would lose some security on case-insensitive file systems.

          On Windows, it would also be possible to use non-ASCII characters in the directory name. If there were 16384 usable case-insensitive code points in the Basic Multilingual Plane, then each code point would hold 14 bits, and 256 bits would fit in 19 code points. However, I don't think this would be a good idea, because:

          • The code points would have to be chosen carefully, to ensure that file systems won't treat any of them as equivalent, even if using future versions of Unicode. Any mistakes here would weaken the security somewhat.
          • The implementation that maps binary values to code points would either be somewhat complex (a binary search on ranges with strides) or need a large array of constants.
          • Jenkins administrators would have difficulty typing the directory names, or even listing them if the font does not support the characters.
          • Unusual characters in directory names might hit bugs similar to JENKINS-48493. Not that bug specifically but similar ones elsewhere.

          Kalle Niemitalo added a comment - On Windows, it would also be possible to use non-ASCII characters in the directory name. If there were 16384 usable case-insensitive code points in the Basic Multilingual Plane, then each code point would hold 14 bits, and 256 bits would fit in 19 code points. However, I don't think this would be a good idea, because: The code points would have to be chosen carefully, to ensure that file systems won't treat any of them as equivalent, even if using future versions of Unicode. Any mistakes here would weaken the security somewhat. The implementation that maps binary values to code points would either be somewhat complex (a binary search on ranges with strides) or need a large array of constants. Jenkins administrators would have difficulty typing the directory names, or even listing them if the font does not support the characters. Unusual characters in directory names might hit bugs similar to JENKINS-48493 . Not that bug specifically but similar ones elsewhere.

          Jesse Glick added a comment -

          For comparison, we used to have similar problems with the workspace paths of Pipeline branch projects, which were resolved (I hope!) by switching to a different scheme whereby a truncated yet human-readable version of the branch name becomes the directory name but conflicts are explicitly prevented by means of an index file listing branches and corresponding directories. See https://github.com/jenkinsci/branch-api-plugin/pull/129.

          Jesse Glick added a comment - For comparison, we used to have similar problems with the workspace paths of Pipeline branch projects, which were resolved (I hope!) by switching to a different scheme whereby a truncated yet human-readable version of the branch name becomes the directory name but conflicts are explicitly prevented by means of an index file listing branches and corresponding directories. See https://github.com/jenkinsci/branch-api-plugin/pull/129 .

          Is the confidentiality of HMACConfidentialKey here intended only to prevent an attacker from searching for hash collisions offline?

          Kalle Niemitalo added a comment - Is the confidentiality of HMACConfidentialKey here intended only to prevent an attacker from searching for hash collisions offline?

          Devin Nusbaum added a comment -

          Can the length integer be reduced

          It would reduce the level of security.

          Is the confidentiality of HMACConfidentialKey here intended only to prevent an attacker from searching for hash collisions offline?

          Yes.

          I think that setting up an index file along the lines of https://github.com/jenkinsci/branch-api-plugin/pull/129 like Jesse mentioned (or maybe repurposing the *-name.txt sibling files that are already being created) and then using a heavily truncated version of the hash as the default directory name (maybe 8 characters), adding characters as needed, would probably be the best way to resolve this. These directories are typically per-job or even per-build (for pipeline-groovy-lib, so collisions should be relatively rare.

          Any fix for this would need to be duplicated across workflow-cps, pipeline-groovy-lib, and workflow-multibranch. Depending on the complexity, perhaps it would make sense to expose some kind of utility API for this in workflow-cps or some other plugin.

          Devin Nusbaum added a comment - Can the length integer be reduced It would reduce the level of security. Is the confidentiality of HMACConfidentialKey here intended only to prevent an attacker from searching for hash collisions offline? Yes. I think that setting up an index file along the lines of https://github.com/jenkinsci/branch-api-plugin/pull/129 like Jesse mentioned (or maybe repurposing the *-name.txt sibling files that are already being created) and then using a heavily truncated version of the hash as the default directory name (maybe 8 characters), adding characters as needed, would probably be the best way to resolve this. These directories are typically per-job or even per-build (for pipeline-groovy-lib , so collisions should be relatively rare. Any fix for this would need to be duplicated across workflow-cps , pipeline-groovy-lib , and workflow-multibranch . Depending on the complexity, perhaps it would make sense to expose some kind of utility API for this in workflow-cps or some other plugin.

          Does an attack require a hash collision between an attacker-controlled SCM key and a trusted SCM key, or can it also abuse a hash collision between two attacker-controlled SCM keys? In the former case, the hash doesn't need as many bits, I think.

          Kalle Niemitalo added a comment - Does an attack require a hash collision between an attacker-controlled SCM key and a trusted SCM key, or can it also abuse a hash collision between two attacker-controlled SCM keys? In the former case, the hash doesn't need as many bits, I think.

          Devin Nusbaum added a comment - - edited

          Unfortunately, it is the latter case. We are attempting to prevent collisions between any two attacker-controlled SCMs. See CpsScmFlowDefinitionTest.checkoutDirectoriesAreNotReusedByDifferentScms for a test that demonstrates the kind of attack we are concerned about.

          I filed https://github.com/jenkinsci/workflow-cps-plugin/pull/548 as an untested draft PR that might fix the issue. Feel free to try it out if it passes CI. I am not sure when I will have time to look into this in more detail.

          Devin Nusbaum added a comment - - edited Unfortunately, it is the latter case. We are attempting to prevent collisions between any two attacker-controlled SCMs. See CpsScmFlowDefinitionTest.checkoutDirectoriesAreNotReusedByDifferentScms for a test that demonstrates the kind of attack we are concerned about. I filed https://github.com/jenkinsci/workflow-cps-plugin/pull/548 as an untested draft PR that might fix the issue. Feel free to try it out if it passes CI. I am not sure when I will have time to look into this in more detail.

            Unassigned Unassigned
            mpalla 61115anon
            Votes:
            2 Vote for this issue
            Watchers:
            6 Start watching this issue

              Created:
              Updated: