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

Upgrading workflow-cps to 2.85 or later leaks credentials in ui

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      When viewing logs credentials are hidden as expected. But when in blueocean or viewing logs in the branch builds overview the "title" used to be "Shell Script (self time 3s)" but now includes the command but doesn't remove the secret when using withVault. 

      withCredentials will have a title of "echo ${TEXT} | wc" Good

      withVault will have a title of "echo cred123 | wc " Bad

      I don't know if this is an issue with workflow-cps or one of the hashicorp-vault plugins but it seem like there was a change in workflow-cps:2.85 that added the command to the title and now I can see credentials.

      Currently using
      hashicorp-vault-pipeline:1.3
      hashicorp-vault-plugin:3.6.1

        Attachments

          Issue Links

            Activity

            Hide
            carroll Carroll Chiou added a comment -

            So if the vault credential is appearing in Blue Ocean (and most likely pipeline steps), that would mean that workflow-cps is unable to find the credential variable within the list of sensitive environment variables.

            The change in workflow-cps 2.85+ is that it is using the new workflow-step-api API (2.23) for registering sensitive variables. It appears the hashicorp plugin is adding the sensitive variables via Context.env.

            Console masking would still work since the plugin is doing that on its own.

            In order for workflow-cps to properly identify a sensitive variable, it needs to be registered as a sensitive environment variable. This is done by adding the sensitive variables to an EnvironmentExpander and then merging it to the current environment variables.

            See the credential-binding plugin as an example here and here

            Show
            carroll Carroll Chiou added a comment - So if the vault credential is appearing in Blue Ocean (and most likely pipeline steps), that would mean that workflow-cps is unable to find the credential variable within the list of sensitive environment variables. The change in workflow-cps 2.85+ is that it is using the new workflow-step-api API (2.23) for registering sensitive variables. It appears the hashicorp plugin is adding the sensitive variables via Context.env . Console masking would still work since the plugin is doing that on its own. In order for workflow-cps to properly identify a sensitive variable, it needs to be registered as a sensitive environment variable. This is done by adding the sensitive variables to an EnvironmentExpander and then merging it to the current environment variables. See the credential-binding plugin as an example here and here
            Hide
            adamphillips Adam added a comment -

            Joseph Petersen and Tobias Larscheid as top contributors to hashicorp-vault-plugin is this a known issue or something that you can help with? 

            Show
            adamphillips Adam added a comment - Joseph Petersen  and Tobias Larscheid as top contributors to hashicorp-vault-plugin is this a known issue or something that you can help with? 
            Hide
            duemir Denys Digtiar added a comment -

            It looks like Groovy String interpolation might be at fault here, see String interpolation

            Show
            duemir Denys Digtiar added a comment - It looks like Groovy String interpolation might be at fault here, see  String interpolation
            Hide
            tobilarscheid Tobias Larscheid added a comment -

            Hi Adam, my time with the vault plugin is long ago! Nevertheless I was the one originally implementing the log masking part, see this class: https://github.com/jenkinsci/hashicorp-vault-plugin/blob/master/src/main/java/com/datapipe/jenkins/vault/log/MaskingConsoleLogFilter.java

            Unfortunately I have not enough (developer) experience with blue ocean changes. I assume there must be a better, more general way of telling Jenkins that a certain string is a credential and should not be rendered anywhere in the UI. In hindsight my log filter feels pretty hand crafted and should definitely be replaced by some kind of jenkins build in functionality, if such exists.

            Show
            tobilarscheid Tobias Larscheid added a comment - Hi Adam , my time with the vault plugin is long ago! Nevertheless I was the one originally implementing the log masking part, see this class: https://github.com/jenkinsci/hashicorp-vault-plugin/blob/master/src/main/java/com/datapipe/jenkins/vault/log/MaskingConsoleLogFilter.java Unfortunately I have not enough (developer) experience with blue ocean changes. I assume there must be a better, more general way of telling Jenkins that a certain string is a credential and should not be rendered anywhere in the UI. In hindsight my log filter feels pretty hand crafted and should definitely be replaced by some kind of jenkins build in functionality, if such exists.
            Hide
            jwminton Jasen Minton added a comment -

            This bug is a regression in org.jenkinsci.plugins.workflow.cps.actions.ArgumentsActionImpl.java

            I think it's important to note that with this regression security sensitive content is now getting saved out to the $JENKINS_HOME/jobs files. This isn't a transitory UI leak that goes away with the server. It is the file based content that drives both the BlueOcean and standard UI's.

            Effectively, pipeline execution status is getting saved to XML job files and ArgumentsActionImpl is supposed to be doing the text masking before security sensitive content gets saved.

            In 2.84 during ArgumentsActionImpl.sanitizeObjectAndRecordMutation execution, it called isStringSafe and used a white list to determine whether or not to mask secrets. 

            In 2.85+ this is replaced with an attempt to match a list of variables from an EnvironmentExpander to mask. If no list is given, no variables are masked. Its not clear to me at all how the vault should have provided the list of variables to mask. However, the resulting code is that there is no list of sensitive variables to block and with the white list capability removed it doesn't appear possible to mask any content so all content is now getting saved to the XML files in $JENKINS_HOME/jobs that constitute what both UI's present later.

            A possible fix that worked for me is that if the list of sensitive variables is size zero, reinstate the white list filter. Here it is laid into a 2.87 version as an example: ArgumentsActionImpl.java

            I initially assumed this would be a vault bug, but I just didn't see how or where to supply the list of sensitive variables to the cps plugin to pick up and use in an EnvironmentExpander plus this API changed out from underneath it so the fallback shouldn't be a complete lack of masking that which used to be masked, which is the case 2.85+

            Show
            jwminton Jasen Minton added a comment - This bug is a regression in org.jenkinsci.plugins.workflow.cps.actions.ArgumentsActionImpl.java I think it's important to note that with this regression security sensitive content is now getting saved out to the $JENKINS_HOME/jobs files. This isn't a transitory UI leak that goes away with the server. It is the file based content that drives both the BlueOcean and standard UI's. Effectively, pipeline execution status is getting saved to XML job files and ArgumentsActionImpl is supposed to be doing the text masking before security sensitive content gets saved. In 2.84 during ArgumentsActionImpl.sanitizeObjectAndRecordMutation execution, it called isStringSafe and used a white list to determine whether or not to mask secrets.  In 2.85+ this is replaced with an attempt to match a list of variables from an EnvironmentExpander to mask. If no list is given, no variables are masked. Its not clear to me at all how the vault should have provided the list of variables to mask. However, the resulting code is that there is no list of sensitive variables to block and with the white list capability removed it doesn't appear possible to mask any content so all content is now getting saved to the XML files in $JENKINS_HOME/jobs that constitute what both UI's present later. A possible fix that worked for me is that if the list of sensitive variables is size zero, reinstate the white list filter. Here it is laid into a 2.87 version as an example:  ArgumentsActionImpl.java I initially assumed this would be a vault bug, but I just didn't see how or where to supply the list of sensitive variables to the cps plugin to pick up and use in an EnvironmentExpander plus this API changed out from underneath it so the fallback shouldn't be a complete lack of masking that which used to be masked, which is the case 2.85+
            Hide
            carroll Carroll Chiou added a comment - - edited

            So to clarify and expand my original post, what the hashicorp plugin does is add sensitive variables through Context.env. This is not the recommended way to add environment variables. The recommended way is to use the EnvironmentExpander.

            From a quick look, I think the vault can provide the list of sensitive variables by using the EnvironmentExpander in the same place that it adds the sensitive variables via Context.env. You can see the the credentials-binding plugin do that here and here. You should not need to change ArgumentsActionImpl.

            As for console masking, I think the same chunk of code can help. That way the plugin doesn't have to try and roll its own console filtering,.

            Show
            carroll Carroll Chiou added a comment - - edited So to clarify and expand my original post , what the hashicorp plugin does is add sensitive variables through Context.env . This is not the recommended way to add environment variables. The recommended way is to use the EnvironmentExpander . From a quick look, I think the vault can provide the list of sensitive variables by using the EnvironmentExpander in the same place that it adds the sensitive variables via Context.env . You can see the the credentials-binding plugin do that here and here . You should not need to change ArgumentsActionImpl. As for console masking, I think the same chunk of code can help . That way the plugin doesn't have to try and roll its own console filtering,.
            Hide
            jwminton Jasen Minton added a comment -

            The Context.env that you're referencing is not a StepContext. It's a public static final inner class named Context inherited from jenkins.tasks.SimpleBuildWrapper. The VaultBuildWrapper extends SimpleBuildWrapper which extends hudson.tasks.BuildWrapper. For VaultBuildWrapper to have access to org.jenkinsci.plugins.workflow.steps.EnvironmentExpander and a meaningful org.jenkinsci.plugins.workflow.steps.StepContext it would need to be managed and reimplemented as a org.jenkinsci.plugins.workflow.steps.Step subclass, unless I'm missing something and sorry if I am but I'm not seeing how to use Step and StepContext class based api from a BuildWrapper based subclass.

            As a type of BuildWrapper, there does exist API to manage the list of sensitive variable names. It's consistent and meaningful use seems to be optional however.

            I'm also very uneasy about this security sensitive information that was previously getting masked and not saved now getting saved to jobs XML files. If this is deemed a secure operation, please let me know the reference so I can pass it along, otherwise this really should be getting escalated as a greater security risk. Sorry if I'm being an alarmist but better err on the side of safety. 

            Show
            jwminton Jasen Minton added a comment - The Context.env that you're referencing is not a StepContext. It's a public static final inner class named Context inherited from jenkins.tasks.SimpleBuildWrapper. The VaultBuildWrapper extends SimpleBuildWrapper which extends hudson.tasks.BuildWrapper. For VaultBuildWrapper to have access to org.jenkinsci.plugins.workflow.steps.EnvironmentExpander and a meaningful org.jenkinsci.plugins.workflow.steps.StepContext it would need to be managed and reimplemented as a org.jenkinsci.plugins.workflow.steps.Step subclass, unless I'm missing something and sorry if I am but I'm not seeing how to use Step and StepContext class based api from a BuildWrapper based subclass. As a type of BuildWrapper, there does exist API to manage the list of sensitive variable names. It's consistent and meaningful use seems to be optional however. I'm also very uneasy about this security sensitive information that was previously getting masked and not saved now getting saved to jobs XML files. If this is deemed a secure operation, please let me know the reference so I can pass it along, otherwise this really should be getting escalated as a greater security risk. Sorry if I'm being an alarmist but better err on the side of safety. 
            Hide
            carroll Carroll Chiou added a comment - - edited

            Whoops, you're right, I missed that. Ideally this plugin should be updated to be used as step if we are using it in pipeline. I do see that you can use withCredentials to pass the vault secrets. withCredentials would be the recommended way to pass any secrets through pipeline. Does masking fail when using that method?

            Are there things that the withVault step does that withCredentials does not do? Ideally there should be no need for the plugin to try and add secrets on its own or even do its own console masking.

            I assume there must be a better, more general way of telling Jenkins that a certain string is a credential and should not be rendered anywhere in the UI.

            comment-403591
            That would be using the credentials-binding plugin.

            Show
            carroll Carroll Chiou added a comment - - edited Whoops, you're right, I missed that. Ideally this plugin should be updated to be used as step if we are using it in pipeline. I do see that you can use withCredentials to pass the vault secrets. withCredentials would be the recommended way to pass any secrets through pipeline. Does masking fail when using that method? Are there things that the withVault step does that withCredentials does not do? Ideally there should be no need for the plugin to try and add secrets on its own or even do its own console masking. I assume there must be a better, more general way of telling Jenkins that a certain string is a credential and should not be rendered anywhere in the UI. comment-403591 That would be using the credentials-binding plugin.

              People

              Assignee:
              scddev Dietmar Scheidl
              Reporter:
              adamphillips Adam
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: