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

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

    XMLWordPrintable

Details

    Description

      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.

       

       

      Attachments

        Issue Links

          Activity

            jglick 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.

            jglick 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?

            kon Kalle Niemitalo added a comment - Is the confidentiality of HMACConfidentialKey here intended only to prevent an attacker from searching for hash collisions offline?
            dnusbaum 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.

            dnusbaum 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.

            kon 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.
            dnusbaum 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.

            dnusbaum 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.

            People

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

              Dates

                Created:
                Updated: