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

Pipeline Clone fails on zOS with HTTPS ASKPASS credentials

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed (View Workflow)
    • Priority: Critical
    • Resolution: Fixed
    • Component/s: git-client-plugin
    • Labels:
      None
    • Environment:
      Jenkins:2.222.3
      git-client: 3.3.1
      git-plugin: 4.3.0
      z/OS R2.4
      git 2.14
    • Similar Issues:
    • Released As:
      3.4.0

      Description

      Similar to the case https://issues.jenkins-ci.org/browse/JENKINS-63146 but for HTTPS, the encoding of the password used by ASKPASS is coming across in UTF8 instead of IBM-1047.

       
      When attempting to clone a repository using an ASKPASS for HTTPS in a pipeline, the clone fails with
      fatal: Authentication failed for 'URL...'
      This is not urgent for our team, but have been asked to open this by our colleagues on a different project who are experiencing this situation, which we can reproduce on our z/OS machine.

        Attachments

          Issue Links

            Activity

            Hide
            rsbeckerca Randall Becker added a comment -

            It looks like a similar fix might make sense, perhaps using the same or variant of the property key from Issue #63146:

            private File createPasswordFile(StandardUsernamePasswordCredentials userPass) throws IOException {
            File passwordFile = createTempFile("password", ".txt");
            try (PrintWriter w = new PrintWriter(passwordFile, "UTF-8")) {| |w.println(Secret.toString(userPass.getPassword()));| |}
            return passwordFile;
            }

            Instead of using "UTF-8", use it by default, but if org.jenkinsci.plugins.gitclient.CliGitAPIImpl.password.file.encoding is supplied, the similar approach from createPassphraseFile as in Commit f3ccd696c705589d0011ec69c170afb6148df9b1.

            Show
            rsbeckerca Randall Becker added a comment - It looks like a similar fix might make sense, perhaps using the same or variant of the property key from Issue #63146: private File createPasswordFile(StandardUsernamePasswordCredentials userPass) throws IOException { File passwordFile = createTempFile("password", ".txt"); try (PrintWriter w = new PrintWriter(passwordFile, "UTF-8")) {| |w.println(Secret.toString(userPass.getPassword()));| |} return passwordFile; } Instead of using "UTF-8", use it by default, but if  org.jenkinsci.plugins.gitclient.CliGitAPIImpl. password .file.encoding is supplied, the similar approach from createPassphraseFile as in Commit f3ccd696c705589d0011ec69c170afb6148df9b1.
            Hide
            markewaite Mark Waite added a comment -

            If that team is willing to validate a build, I'm willing to consider providing the build based on that idea Randall Becker. I won't provide a prototype build unless they agree to test the build and report the results of the test. I have no access to zOS environments so I must rely on those with that access to test changes for zOS.

            Show
            markewaite Mark Waite added a comment - If that team is willing to validate a build, I'm willing to consider providing the build based on that idea Randall Becker . I won't provide a prototype build unless they agree to test the build and report the results of the test. I have no access to zOS environments so I must rely on those with that access to test changes for zOS.
            Hide
            rsbeckerca Randall Becker added a comment -

            I will test and report on the results of the changes as soon as practical (very soon). I also will keep the test environment around for future as long as I have a z/OS machine at my disposal - which my company is graciously paying for.

            Show
            rsbeckerca Randall Becker added a comment - I will test and report on the results of the changes as soon as practical (very soon). I also will keep the test environment around for future as long as I have a z/OS machine at my disposal - which my company is graciously paying for.
            Hide
            dbehm Dennis Behm added a comment -

            I have access to a z/OS machine as well, and would volunteer to test the fix.

            Show
            dbehm Dennis Behm added a comment - I have access to a z/OS machine as well, and would volunteer to test the fix.
            Hide
            markewaite Mark Waite added a comment -

            I extended the change from JENKINS-63146 for this case as well. See the pull request at https://ci.jenkins.io/job/Plugins/job/git-client-plugin/view/change-requests/job/PR-585/ for a build that should be testable. Documentation of the properties is included in the plugin README

            Show
            markewaite Mark Waite added a comment - I extended the change from JENKINS-63146 for this case as well. See the pull request at https://ci.jenkins.io/job/Plugins/job/git-client-plugin/view/change-requests/job/PR-585/ for a build that should be testable. Documentation of the properties is included in the plugin README
            Hide
            rsbeckerca Randall Becker added a comment -

            The test passes!
            using GIT_ASKPASS to set credentials
            Using password charset 'IBM1047'
            > git fetch --tags --progress – https://user@bitbucket.org/proj/repo.git +refs/heads/:refs/remotes/origin/ # timeout=10 > git rev-parse refs/remotes/origin/httpsscm_test^{commit} # timeout=10
            Checking out Revision b13718a8aa8083112cea241dcb6b753dff202af1 (refs/remotes/origin/httpsscm_test)
            Thank you!

             

            Show
            rsbeckerca Randall Becker added a comment - The test passes! using GIT_ASKPASS to set credentials Using password charset 'IBM1047' > git fetch --tags --progress – https://user@bitbucket.org/proj/repo.git +refs/heads/ :refs/remotes/origin/ # timeout=10 > git rev-parse refs/remotes/origin/httpsscm_test^{commit} # timeout=10 Checking out Revision b13718a8aa8083112cea241dcb6b753dff202af1 (refs/remotes/origin/httpsscm_test) Thank you!  
            Hide
            dbehm Dennis Behm added a comment -

            Hi Mark,

             

            I think there is a small issue with the code here:

            https://github.com/jenkinsci/git-client-plugin/blob/2fc643d12aeb02d587933619754ce9f38d7ae3da/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java#L2191

            The condition tests password.file.encoding but takes credentials.file.encoding as charset. So, this is why my test failed with FATAL: Null charset name, while I only supplied the password.file.encoding property.

            What about using credentials.file.encoding for both scenarios - I think this would work.

            Thank you

            Show
            dbehm Dennis Behm added a comment - Hi Mark,   I think there is a small issue with the code here: https://github.com/jenkinsci/git-client-plugin/blob/2fc643d12aeb02d587933619754ce9f38d7ae3da/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java#L2191 The condition tests  password.file.encoding but takes credentials.file.encoding as charset. So, this is why my test failed with  FATAL: Null charset name,  while I only supplied the password.file.encoding property. What about using  credentials.file.encoding  for both scenarios - I think this would work. Thank you
            Hide
            dbehm Dennis Behm added a comment -

            To continue with the test I have supplied password and credentials file encoding environment variables. In the http scenario, you also need to take care of the username file in createUsernameFile(..) 
            https://github.com/jenkinsci/git-client-plugin/blob/2fc643d12aeb02d587933619754ce9f38d7ae3da/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java#L2181

            Show
            dbehm Dennis Behm added a comment - To continue with the test I have supplied password and credentials file encoding environment variables. In the http scenario, you also need to take care of the username file in createUsernameFile(..)  https://github.com/jenkinsci/git-client-plugin/blob/2fc643d12aeb02d587933619754ce9f38d7ae3da/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java#L2181
            Hide
            rsbeckerca Randall Becker added a comment - - edited

            I concur with Dennis. Our test case supplied the user id in the URI, so there was no need to encode the user name.  There is also a problem in createPasswordFile, where the second calculation of the property name refers to "credentials" instead of "password". This should be added also (having trouble with the mark-up, but you get the idea):

            private File createUsernameFile(StandardUsernamePasswordCredentials userPass) throws IOException {
                String charset = "UTF-8";
                if (isZos() && System.getProperty(CliGitAPIImpl.class.getName() + ".username.file.encoding") != null) {
                    charset = Charset.forName(System.getProperty(CliGitAPIImpl.class.getName() + "username.file.encoding")).toString();
                    listener.getLogger().println("Using username charset '" + charset + "'");
                }

                File usernameFile = createTempFile("username", ".txt");
                try (PrintWriter w = new PrintWriter(usernameFile, charset)) {
                    w.println(userPass.getUsername());
                }
                return usernameFile;
            }

            Show
            rsbeckerca Randall Becker added a comment - - edited I concur with Dennis. Our test case supplied the user id in the URI, so there was no need to encode the user name.  There is also a problem in createPasswordFile, where the second calculation of the property name refers to "credentials" instead of "password". This should be added also (having trouble with the mark-up, but you get the idea): private File createUsernameFile(StandardUsernamePasswordCredentials userPass) throws IOException {     String charset = "UTF-8";     if (isZos() && System.getProperty(CliGitAPIImpl.class.getName() + ".username.file.encoding") != null) {         charset = Charset.forName(System.getProperty(CliGitAPIImpl.class.getName() + "username.file.encoding")).toString();         listener.getLogger().println("Using username charset '" + charset + "'");     }     File usernameFile = createTempFile("username", ".txt");     try (PrintWriter w = new PrintWriter(usernameFile, charset)) {         w.println(userPass.getUsername());     }     return usernameFile; }
            Hide
            markewaite Mark Waite added a comment -

            In the spirit of "don't repeat yourself" and to fix the bug that you detected, I've submitted https://github.com/jenkinsci/git-client-plugin/pull/598 that updates the property names so that they can all be documented in a single block in the README.

            Randall Becker and Dennis Behm could you review the proposed code change in the pull request?

            Show
            markewaite Mark Waite added a comment - In the spirit of "don't repeat yourself" and to fix the bug that you detected, I've submitted https://github.com/jenkinsci/git-client-plugin/pull/598 that updates the property names so that they can all be documented in a single block in the README. Randall Becker and Dennis Behm could you review the proposed code change in the pull request?
            Hide
            markewaite Mark Waite added a comment -

            Released in git client plugin 3.4.0 with the properties documented in plugin properties

            Show
            markewaite Mark Waite added a comment - Released in git client plugin 3.4.0 with the properties documented in plugin properties
            Hide
            dbehm Dennis Behm added a comment -

            Hi Mark, Mark Waite

            thanks for shipping this so quickly. I already applied it in our environment. There is a defect in computeCredentialFileCharset.
            https://github.com/jenkinsci/git-client-plugin/blob/3c437b8c783e388bc13b9fa79833d6b0c5813544/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java#L2198

            java.nio.charset.UnsupportedCharsetException: org.jenkinsci.plugins.gitclient.CliGitAPIImpl.user.name.file.encoding

                        String charset = Charset.forName(property).toString();

            should be 

             

            String charset = Charset.forName(System.getProperty(property)).toString();
            
            Show
            dbehm Dennis Behm added a comment - Hi Mark, Mark Waite thanks for shipping this so quickly. I already applied it in our environment. There is a defect in  computeCredentialFileCharset . https://github.com/jenkinsci/git-client-plugin/blob/3c437b8c783e388bc13b9fa79833d6b0c5813544/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java#L2198 java.nio.charset.UnsupportedCharsetException: org.jenkinsci.plugins.gitclient.CliGitAPIImpl.user.name.file.encoding String charset = Charset.forName(property).toString(); should be    String charset = Charset.forName( System .getProperty(property)).toString();
            Hide
            markewaite Mark Waite added a comment -

            Wow, how many ways can I make mistakes when I write code? Sorry about that. I'll fix that bug and deliver a new release.

            Show
            markewaite Mark Waite added a comment - Wow, how many ways can I make mistakes when I write code? Sorry about that. I'll fix that bug and deliver a new release.
            Hide
            rsbeckerca Randall Becker added a comment -

            I missed that one also.

            Aside: there is a new JGit release coming also, with a fix to file system resolution calculation for z/OS (it was reporting 0)

            Show
            rsbeckerca Randall Becker added a comment - I missed that one also. Aside: there is a new JGit release coming also, with a fix to file system resolution calculation for z/OS (it was reporting 0)
            Hide
            markewaite Mark Waite added a comment -

            I released git client plugin 3.4.1 with the fix. Would like confirmation that it works for you.

            A release with a new version of JGit will have to wait until they deliver that new release. I didn't see any indications of a planned release date, so I assume it will part of an upcoming quarterly JGit release.

            Show
            markewaite Mark Waite added a comment - I released git client plugin 3.4.1 with the fix. Would like confirmation that it works for you. A release with a new version of JGit will have to wait until they deliver that new release. I didn't see any indications of a planned release date, so I assume it will part of an upcoming quarterly JGit release.
            Hide
            rsbeckerca Randall Becker added a comment -

            I'm monitoring JGit on this. Will report on it. Thanks for the version number for release. I'll give it a shot this weekend.

            Show
            rsbeckerca Randall Becker added a comment - I'm monitoring JGit on this. Will report on it. Thanks for the version number for release. I'll give it a shot this weekend.
            Hide
            rsbeckerca Randall Becker added a comment - - edited

            Our SSH and HTTPS tests both pass on 3.4.1. Thank you.

            Show
            rsbeckerca Randall Becker added a comment - - edited Our SSH and HTTPS tests both pass on 3.4.1. Thank you.
            Hide
            dbehm Dennis Behm added a comment -

            Thank you Mark. I can confirm, that it works!

            Show
            dbehm Dennis Behm added a comment - Thank you Mark. I can confirm, that it works!

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              rsbeckerca Randall Becker
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: