• Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • git-client-plugin
    • None
    • Windows

      In https://github.com/jenkinsci/git-client-plugin/blob/master/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java

      Method createWindowsStandardAskpass() & createWindowsSshAskpass() create cmd.exe .bat batch files are created that echo the passwords to standard out. They use this method to encode the password:

      private String quoteWindowsCredentials(String str)

      { // Assumes the only meaningful character is %, this may be // insufficient.* return str.replace("%", "%%"); }

      This misses several characters are missed that also need to be quoted, see http://ss64.com/nt/syntax-esc.html for more specifics.

      Specifically the cmd.exe caret ^ escape character is needed to quote backslash, ampersand, right bracket, left bracket, caret, space and tab characters:
      ^\ ^& ^| ^> ^< ^^ ^ (note there is a space) and ^ (note there is a tab)

      This could easily be implemented by adding additional lines to the method for each of these extra characters.

      Currently when you use these characters in a Git password and run the job on windows you get very poor error messages saying that the fetch operation failed do to an error in ASKPASS.

          [JENKINS-40166] Passwords not quoted correctly in Windows

          Can't figure out how to link issues, please note JENKINS-38194 is a subset of this quoting issue (only referring to the ampersand character.)

          Frederick Staats added a comment - Can't figure out how to link issues, please note JENKINS-38194 is a subset of this quoting issue (only referring to the ampersand character.)

          Mark Waite added a comment -

          Git client plugin pull request PR207 has started the process of investigating the quoting problem. I think there is a better way which completely avoids the "shell escaping" problem. Refer to my master-PR207-exploring branch for an implementation which uses type (windows) and cat (unix) rather than echo.

          I think type and cat are better choices because the password can be written to a file without any escaping, then the cat (or type) command reports the contents of that file on a pipe to the git command.

          Mark Waite added a comment - Git client plugin pull request PR207 has started the process of investigating the quoting problem. I think there is a better way which completely avoids the "shell escaping" problem. Refer to my master-PR207-exploring branch for an implementation which uses type (windows) and cat (unix) rather than echo. I think type and cat are better choices because the password can be written to a file without any escaping, then the cat (or type) command reports the contents of that file on a pipe to the git command.

          Frederick Staats added a comment - - edited

          I agree that a temporary password file is a better fix than my first suggestion. I had only suggested the smallest possible change, not the most general change.

          Please note that all versions of Git for Windows ship with bash.exe out of the box in the same folder as git.exe (you can always use the path to git.exe as the path to bash.exe regardless of what %PATH% is set to).

          Type.exe has the problem that it is not always on the users path in Windows, but bash.exe is always next to git.exe. You can use bash's builtin file redirection and printf without having any other executable on your path:

          bash.exe --noprofile -c 'printf %s "$(<filenamehere)"'

          Ideally the passwords would never have to hit the disk unencrypted. In principle any non-encrypted secret that hits the file system is vulnerable to permanent storage and later unintended recovery, even if the the temporary file is deleted. In a perfect world we would pass a one time nonce that could be used to recover the password directly from Jenkins credential store using a machine local pipe, socket, or encrypted https endpoint. In the Windows world the OS kernel provides syscalls to store and retrieve secrets stored encrypted in the kernel which are Access Conrol List (ACL) protected so only specific accounts can access and decrypt them. These can be made temporary so that they don't outlive the lifetime of the process that created them. This also helps with preventing storing secrets in strings which in both the Java Virtual Machine and the .NET Common Runtime Environment are interned for the lifetime of a process and are vulnerable to memory based "debugging" and process memory dump attacks.

          The perfect being the tierney of the good just stopping using cmd.exe to echo unquoted passwords is an improvement.

          Frederick Staats added a comment - - edited I agree that a temporary password file is a better fix than my first suggestion. I had only suggested the smallest possible change, not the most general change. Please note that all versions of Git for Windows ship with bash.exe out of the box in the same folder as git.exe (you can always use the path to git.exe as the path to bash.exe regardless of what %PATH% is set to). Type.exe has the problem that it is not always on the users path in Windows, but bash.exe is always next to git.exe. You can use bash's builtin file redirection and printf without having any other executable on your path: bash.exe --noprofile -c 'printf %s "$(<filenamehere)"' Ideally the passwords would never have to hit the disk unencrypted. In principle any non-encrypted secret that hits the file system is vulnerable to permanent storage and later unintended recovery, even if the the temporary file is deleted. In a perfect world we would pass a one time nonce that could be used to recover the password directly from Jenkins credential store using a machine local pipe, socket, or encrypted https endpoint. In the Windows world the OS kernel provides syscalls to store and retrieve secrets stored encrypted in the kernel which are Access Conrol List (ACL) protected so only specific accounts can access and decrypt them. These can be made temporary so that they don't outlive the lifetime of the process that created them. This also helps with preventing storing secrets in strings which in both the Java Virtual Machine and the .NET Common Runtime Environment are interned for the lifetime of a process and are vulnerable to memory based "debugging" and process memory dump attacks. The perfect being the tierney of the good just stopping using cmd.exe to echo unquoted passwords is an improvement.

          This was a blocking issue for us, so I went ahead and implemented this simplest fix possible and submitted a pull request: https://github.com/jenkinsci/git-client-plugin/pull/231

          Using bash on Windows is probably a better answer, but was more interested in getting my builds up and running quickly

          Michael Letterle added a comment - This was a blocking issue for us, so I went ahead and implemented this simplest fix possible and submitted a pull request: https://github.com/jenkinsci/git-client-plugin/pull/231 Using bash on Windows is probably a better answer, but was more interested in getting my builds up and running quickly

          Mark Waite added a comment -

          If you'd like to test a prototype build, for a short time it will be available from the ci.jenkins.io server. Upload it manually, restart your Jenkins server, and see if it helps in your case.

          Mark Waite added a comment - If you'd like to test a prototype build, for a short time it will be available from the ci.jenkins.io server. Upload it manually, restart your Jenkins server, and see if it helps in your case.

          Mark Waite added a comment -

          Fixed in git client plugin 2.2.1 16 Jan 2016

          Mark Waite added a comment - Fixed in git client plugin 2.2.1 16 Jan 2016

            markewaite Mark Waite
            flstaats Frederick Staats
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: