New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secret variables are not secure #4284

Closed
ChristophB125 opened this Issue May 9, 2017 · 7 comments

Comments

Projects
None yet
3 participants
@ChristophB125
Copy link

ChristophB125 commented May 9, 2017

It is very easy for a normal user with no admin rights to get the value of any secret variable. Let the secret variable name be secretVariable and its value secretValue. Then in a PowerShell script simply put the following code:
!$(secretVariable)
The console log then contains the value of the secret variable 4 times in plain text:

2017-05-09T20:09:44.0463696Z ##[error]!secretValue : The term '!secretValue' is not recognized as the name of a cmdlet, function, script file, or operable
program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
At D:\a_temp\2591431a-31f8-4247-84d2-f5891bacdbe0.ps1:7 char:1

  • !secretValue
  •   + CategoryInfo          : ObjectNotFound: (!secretValue:String) [], CommandNotFoundException
      + FullyQualifiedErrorId : CommandNotFoundException
    
    
    

2017-05-09T20:09:44.2671418Z ##[error]Process completed with exit code 0 and had 1 error(s) written to the error stream.

@ericsciple

This comment has been minimized.

Copy link
Member

ericsciple commented May 9, 2017

@ChristophB125 what agent version are you on? i believe we scrub error messages in current agent version

@ericsciple

This comment has been minimized.

Copy link
Member

ericsciple commented May 9, 2017

...also note, if you have edit permission to the definition, then you are the admin of that definition.

@ChristophB125

This comment has been minimized.

Copy link

ChristophB125 commented May 9, 2017

@EricJizbaMSFT : I am using the 'Hosted' agent. One does not need to have permission to edit the build definition. Every user can see the name of a secret variable (first step) and then a user who can commit code to the repository can simply inject the following line into a PowerShell script that gets run by a build step to extract the variable without even breaking the build or anyone noticing
try{!$(secretVariable)}catch{}; $command=$error[0].Exception.CommandName; Write-Host "The value of the secret variable is $($command.Substring(1,$command.Length-1))"
Also why are secret variables not secure strings or something similar? When using them to create credential objects using ConvertTo-SecureString one has to use the -AsPlainText -Force option, which triggers the PSAvoidUsingPlainTextForPassword PSScriptanalyzer warning.

@bryanmacfarlane

This comment has been minimized.

Copy link
Member

bryanmacfarlane commented May 10, 2017

Secret variables used to ensure that string (1) does not show up in the definition after initially inputting it (2) does not show up in the web UI or logs when downloading it and (3) stored securely and encrypted in our strong box outside of standard definition storage.

Of course it does not prevent users that can contribute to the automation from getting at the secret. That is the point of automation. If I can contribute to the automation, and the automation can get at the secret (that is the point right? for the task to use the secret) then that contribution can email it off, save it off etc...

The point is not to prevent the automation authors from getting at their own values. The point is so if the logs are downloaded and sent around or someone with view permission, doesn't accidentally see it.

@ericsciple

This comment has been minimized.

Copy link
Member

ericsciple commented May 15, 2017

@ChristophB125 the steps you describe do not repro on the hosted agent. Here is what I did:

  1. Added a secret variable: mysecretvariable with value mysecretvalue
  2. Added an inline ps script: !$(mysecretvariable)
  3. Queued against the hosted agent
  4. The secret value in the error message is masked (replaced with asterisks) in all places: web console log, downloaded build logs zip, and the error issue on the build summary page.

The agent's only responsibility is to scrub the issue text and logs before posting back to the server.

@ChristophB125

This comment has been minimized.

Copy link

ChristophB125 commented May 15, 2017

@ericsciple : I had another try over the weekend and I originally though that you had fixed it anyway because the log errors are now masked for me as well. It seems to me that there are different versions of the Hosted agent in the cloud. As far as I could see, the logic of the agent is to do a simple string replacement of the secret in the log because if I do a Write-Host my secretvalue (assuming I know the value already), it gets masked as well. I can kind of understand Bryian's point but I like the level of safety that you have now with the masking of log errors as well so that at least accidentally one cannot see the secret. As he rightly argues, there will always be a way to get the password simply because the object type is a string. What I would like to see is the possibility to have the secret variable already in the form of a secure string, which would be the safest way of creating credential objects.

@ericsciple

This comment has been minimized.

Copy link
Member

ericsciple commented May 15, 2017

I see. So for the powershell task specifically, provide inputs on the task UI that map into a secure string variable or pscredential variable inside the powershell runspace. I'll forward the feedback along to the PM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment