10 ms·

Travis CI Security Advisory: Secured Environment Variables

87 points by edmorley 9y ago

lawnchair_larry 9y ago
Secrets in environment variables is such a bad security anti-pattern, and it seems to be getting more popular.
- phalangion 9y ago
  What is a better pattern?
  - karlmdavis 9y ago
    I'd love to know, too. Best as I can tell, the only really secure option is to read them in from STDIN. Depending on how the process is running, it may also be reasonably secure to read the secrets from files. But getting this right is tricky, and really prone to human error: all it takes is one errant chmod/chown to remove the security. Honestly, given the challenges of those options, keeping secrets in environment variables seems like a reasonable compromise to me.
    - lawnchair_larry 9y ago
      Even putting them in text files is far better than environment variables. Consider how often people log environment variables or even leave phpinfo() lying around. As someone who does penetration tests, I can tell you that it's far more common to gain access to environment variables than it is to read files on the server. The difference between services that didn't have secrets there and the ones that did was a low severity finding that was barely worth logging, and a full remote compromise.
      - eropple 9y ago
        How often people log environment variables? Maybe the places I've worked are outliers, but `puts ENV.inspect` or the use of PHP wouldn't pass code review in my prior gigs, nor would they be things I'd write today. My experience is literally the opposite--somebody forgetting a chmod is a lot more common than somebody leaking environment variables like that (leaving it up to somebody to figure out a really smart way to read `/proc/pid/environ` or something).
        lgas 9y ago
        I suspect the vast majority of software development organizations do not do code reviews, so you're already in the minority.
        eropple 9y ago
        Distressingly likely. Point taken.
        lawnchair_larry 9y ago
        Even if they do, they 1) Don't reject PHP wholesale and 2) Don't require that the devops person trying to find a bug go through code review when deploying a debugging tool.
        spc476 9y ago
        I've written code that does that [1] to log information to help diagnose crashes in long running daemons. Now, the dumping of environment variables is optional but it has been helpful to me in isolating issues with some CGI based programs (as Apache passes critical information to the script in environment variables). Of course, it was in a situation where core dumps are not generated so anything related to a crash is helpful. [1] https://github.com/spc476/CGILib/blob/master/src/crashreport.c https://github.com/spc476/CGILib/blob/master/src/crashreport...
  - jacobwg 9y ago
    I've seen secrets stored in special files, or personally I prefer something like https://www.vaultproject.io/ https://www.vaultproject.io/, which may still use environment variables, but they can potentially be passed in in a more secure, controlled way.
  - kyrra 9y ago
    Encrypt the data at rest with an encryption appliance (HSM). Or use a key management service[0][1] to store encryption keys that can be used to decrypt your at-rest data. [0] https://cloud.google.com/kms/ https://cloud.google.com/kms/ [1] https://aws.amazon.com/kms/ https://aws.amazon.com/kms/
    - tehlike 9y ago
      +1 with kms you could implement access control for different jobs.
  - draw_down 9y ago
    I think there are services designed to store and provide secrets... maybe one of those.
  - jacques_chester 9y ago
    Secrets or credential management is hard, but the first step is to centralise. Many folk use Vault. There's also Knox, KeyWhiz and I forget some others. I've been a secrets-management product team (CredHub) for several months now. We've looked at different ways of shuttling secrets but really, it's going to be specific to the context. For example, one job our software does is to hand credentials to a trusted BOSH director during deployments. That's basically done at this point and works very nicely from an operator perspective. But then when we look at handing secrets to applications, or getting secrets to CI, it's a bit trickier. We use Concourse a lot and for Concourse the next major track of work centres entirely around creating a secrets-management layer that backs onto secret-management systems. Disclosure: At the moment I work on CredHub on behalf of Pivotal.
    - eropple 9y ago
      Not applicable for y'all, but for our AWS people, I'll cape up for Credstash[1] (or its moral equivalent, Sneaker[2], but I prefer the use of a Dynamo table to S3 for this). Uses IAM to ensure secret access and offers revisioning. Plus it's super easy to work with. My normal stack, using my own Auster[3] for orchestration, uses an offline, file-based secrets store (usually an encrypted USB key in production) for stuff like database root passwords that don't need to be online, then push database passwords into Credstash with encryption contexts (the KMS thing that makes IAM effective for this). My Chef cookbooks happily slurp in credstashed secrets via rcredstash, now that the PR to make it work with parameterized KMS keys has hit, and provisioning is very straightforward. (There are a lot of proper nouns in this paragraph, but the nice thing is that each component handles its own business and so there aren't many vertical concerns.) CI secret access isn't a thing in the systems I develop (unit tests don't need them, integration tests get a spun-up environment that provisions its own secrets), but you could provide access with a bog-standard token machine. [1] https://github.com/fugue/credstash https://github.com/fugue/credstash [2] https://github.com/codahale/sneaker https://github.com/codahale/sneaker [3] https://github.com/eropple/auster https://github.com/eropple/auster
    - dogecoinbase 9y ago
      Secrets or credential management is hard, but the first step is to centralise. Ah yes, the "all eggs, one basket" approach to secret management. This is the correct approach, if you are trying to sell a platform -- gets you lock-in, and if you fail to keep secrets secure, you were going to blow up anyways, so the business risk management dictates that you should shoot for the moon and risk your client's data in the hopes of getting traction.
- tomjakubowski 9y ago
  Storing secrets elsewhere wouldn't have prevented this specific problem, where git potentially leaked credentials (in URL form) to the build log.
- geofft 9y ago
  Storing secrets elsewhere (for some value of elsewhere that may or may not be better than environment variables) would not have helped this vulnerability: even if the OAuth token were in a file, and it were copied into .git/config, git would still have printed the URL on a failed push. But storing secrets in environment variables isn't a universally bad thing (which is what I interpret "anti-pattern" to mean). It's a bad thing in certain contexts, including OSes (e.g., AIX and OpenBSD) where environment variables are readable by any local user. But there are certainly uses of it where it's more secure than any alternative (e.g., mosh uses $MOSH_KEY for this and it's pretty sound), and except for the OSes where environment variables are intentionally world-readable, most attacks that would let you read an environment variable would also let you read any other place to store a secret, including a file, a pipe, or a special kernel thing like Linux keyrings.
  - lawnchair_larry 9y ago
    I think you have some faulty assumptions. Environment variables are not usually disclosed because somebody got shell access. It's very common to print them out while debugging, but not common to print arbitrary files containing your credentials. I have yet to see phpinfo() print ~/.ssh/id_rsa to the world for example, but I sure have pulled a lot of passwords out of it.
- optimuspaul 9y ago
  While I agree with this statement I don't know of any other way to do so with Travis-CI.
andrewvc 9y ago
While I love travis for what it is, this is a foreseeable result here. At the very least they need to add a failsafe that checks all outgoing logs for any secure tokens and replaces them with '*' or something. If you sign up to play a game of whack-a-mole you will lose eventually.
- wwwigham 9y ago
  That's actually exactly what the stated mitigation they've implemented is, if I've read it correctly. Though they're working on building it into their VM images in order to reduce the memory overhead of the filter.
- tehlike 9y ago
  What if the token was encrypted or altered in a way simple find replace wouldnt work?
  - ReverseCold 9y ago
    Even just encoding it differently (base64, bin, hex, etc) would work against that.
0x0 9y ago
When I submitted a pull request to a random open source project, I was surprised to see a travis build kick off immediately. Is there a chance for rogue pull requests to contain (build) code that dumps out travis environment secrets? I didn't explore this but obviously the code is being built by scripts that are part of the commit.
- roidrage 9y ago
  That wouldn't be possible, and that's independent of the security issue we've disclosed in the post. For pull requests Travis CI has long had security measures in place to prevent this scenario from happening: https://docs.travis-ci.com/user/pull-requests#Pull-Requests-and-Security-Restrictions https://docs.travis-ci.com/user/pull-requests#Pull-Requests-...
  - tehlike 9y ago
    Doesnt this still make it potentially available in case some malicious/unmalicious coder leaves some console debugging out?
    - nothrabannosir 9y ago
      Only if you merge it in. The point is the secure environment variables are not available at all in the fork build. The bash oneliner they show is to help you run scripts which won't crash if they don't have those env vars available, not to "hide them" by running a test script which doesn't use them.
      - tehlike 9y ago
        I know many instances where code reviews didnt catch log statement in huge binaries.
        nothrabannosir 9y ago
        Right, but now you're in the "review of a PR didn't catch malicious code" boat. At which point, you've got bigger problems than leaking env vars in your CI. Not to dismiss it---it's just a different point.
deleted 9y ago
[deleted]
mintplant 9y ago
> As this fix runs in Ruby, it results in larger memory overhead. To address this, we are working on an executable binary that will be installed as part of the build-image. Work on this is expected to be complete by the end of the week. Run it through sed?
- jacques_chester 9y ago
  We hit this in Cloud Foundry Buildpacks, where all the builds and logs are completely public[0]. The team wrote and added Concourse-Filter[1]. You stream logs through it. It compares what's going past to current environment variables. Anytime something goes past that looks like one of the environment variables, it blanks it out. You can set a whitelist for things you're not fussed about. Disclosure: I have twice worked on Buildpacks on behalf of Pivotal. [0] https://buildpacks.ci.cf-app.com/ https://buildpacks.ci.cf-app.com/ [1] https://github.com/pivotal-cf-experimental/concourse-filter https://github.com/pivotal-cf-experimental/concourse-filter
arekkas 9y ago
Did they bother to notify the repositories affected by this? Could not find anything.
- mattcoles 9y ago
  > We have been in close communications with GitHub since we began working on this incident, and have shared impacted tokens with GitHub for their records. GitHub will revoke token access for the affected tokens and will contact token owners to notify them. Both Travis CI and GitHub recommend affected owners revoke their own access tokens and create new tokens immediately. Seems like both Github and Travis did.
arekkas 9y ago
Did they bother to notify the repositories affected by this? Could not find anything.

87 points by edmorley 9y ago

lawnchair_larry 9y ago

Secrets in environment variables is such a bad security anti-pattern, and it seems to be getting more popular.

phalangion 9y ago
What is a better pattern?
- karlmdavis 9y ago
  I'd love to know, too. Best as I can tell, the only really secure option is to read them in from STDIN. Depending on how the process is running, it may also be reasonably secure to read the secrets from files. But getting this right is tricky, and really prone to human error: all it takes is one errant chmod/chown to remove the security. Honestly, given the challenges of those options, keeping secrets in environment variables seems like a reasonable compromise to me.
  - lawnchair_larry 9y ago
    Even putting them in text files is far better than environment variables. Consider how often people log environment variables or even leave phpinfo() lying around. As someone who does penetration tests, I can tell you that it's far more common to gain access to environment variables than it is to read files on the server. The difference between services that didn't have secrets there and the ones that did was a low severity finding that was barely worth logging, and a full remote compromise.
    - eropple 9y ago
      How often people log environment variables? Maybe the places I've worked are outliers, but `puts ENV.inspect` or the use of PHP wouldn't pass code review in my prior gigs, nor would they be things I'd write today. My experience is literally the opposite--somebody forgetting a chmod is a lot more common than somebody leaking environment variables like that (leaving it up to somebody to figure out a really smart way to read `/proc/pid/environ` or something).
      - lgas 9y ago
        I suspect the vast majority of software development organizations do not do code reviews, so you're already in the minority.
        eropple 9y ago
        Distressingly likely. Point taken.
        lawnchair_larry 9y ago
        Even if they do, they 1) Don't reject PHP wholesale and 2) Don't require that the devops person trying to find a bug go through code review when deploying a debugging tool.
      - spc476 9y ago
        I've written code that does that [1] to log information to help diagnose crashes in long running daemons. Now, the dumping of environment variables is optional but it has been helpful to me in isolating issues with some CGI based programs (as Apache passes critical information to the script in environment variables). Of course, it was in a situation where core dumps are not generated so anything related to a crash is helpful. [1] https://github.com/spc476/CGILib/blob/master/src/crashreport.c https://github.com/spc476/CGILib/blob/master/src/crashreport...
- jacobwg 9y ago
  I've seen secrets stored in special files, or personally I prefer something like https://www.vaultproject.io/ https://www.vaultproject.io/, which may still use environment variables, but they can potentially be passed in in a more secure, controlled way.
- kyrra 9y ago
  Encrypt the data at rest with an encryption appliance (HSM). Or use a key management service[0][1] to store encryption keys that can be used to decrypt your at-rest data. [0] https://cloud.google.com/kms/ https://cloud.google.com/kms/ [1] https://aws.amazon.com/kms/ https://aws.amazon.com/kms/
  - tehlike 9y ago
    +1 with kms you could implement access control for different jobs.
- draw_down 9y ago
  I think there are services designed to store and provide secrets... maybe one of those.
- jacques_chester 9y ago
  Secrets or credential management is hard, but the first step is to centralise. Many folk use Vault. There's also Knox, KeyWhiz and I forget some others. I've been a secrets-management product team (CredHub) for several months now. We've looked at different ways of shuttling secrets but really, it's going to be specific to the context. For example, one job our software does is to hand credentials to a trusted BOSH director during deployments. That's basically done at this point and works very nicely from an operator perspective. But then when we look at handing secrets to applications, or getting secrets to CI, it's a bit trickier. We use Concourse a lot and for Concourse the next major track of work centres entirely around creating a secrets-management layer that backs onto secret-management systems. Disclosure: At the moment I work on CredHub on behalf of Pivotal.
  - eropple 9y ago
    Not applicable for y'all, but for our AWS people, I'll cape up for Credstash[1] (or its moral equivalent, Sneaker[2], but I prefer the use of a Dynamo table to S3 for this). Uses IAM to ensure secret access and offers revisioning. Plus it's super easy to work with. My normal stack, using my own Auster[3] for orchestration, uses an offline, file-based secrets store (usually an encrypted USB key in production) for stuff like database root passwords that don't need to be online, then push database passwords into Credstash with encryption contexts (the KMS thing that makes IAM effective for this). My Chef cookbooks happily slurp in credstashed secrets via rcredstash, now that the PR to make it work with parameterized KMS keys has hit, and provisioning is very straightforward. (There are a lot of proper nouns in this paragraph, but the nice thing is that each component handles its own business and so there aren't many vertical concerns.) CI secret access isn't a thing in the systems I develop (unit tests don't need them, integration tests get a spun-up environment that provisions its own secrets), but you could provide access with a bog-standard token machine. [1] https://github.com/fugue/credstash https://github.com/fugue/credstash [2] https://github.com/codahale/sneaker https://github.com/codahale/sneaker [3] https://github.com/eropple/auster https://github.com/eropple/auster
  - dogecoinbase 9y ago
    Secrets or credential management is hard, but the first step is to centralise. Ah yes, the "all eggs, one basket" approach to secret management. This is the correct approach, if you are trying to sell a platform -- gets you lock-in, and if you fail to keep secrets secure, you were going to blow up anyways, so the business risk management dictates that you should shoot for the moon and risk your client's data in the hopes of getting traction.
tomjakubowski 9y ago
Storing secrets elsewhere wouldn't have prevented this specific problem, where git potentially leaked credentials (in URL form) to the build log.
geofft 9y ago
Storing secrets elsewhere (for some value of elsewhere that may or may not be better than environment variables) would not have helped this vulnerability: even if the OAuth token were in a file, and it were copied into .git/config, git would still have printed the URL on a failed push. But storing secrets in environment variables isn't a universally bad thing (which is what I interpret "anti-pattern" to mean). It's a bad thing in certain contexts, including OSes (e.g., AIX and OpenBSD) where environment variables are readable by any local user. But there are certainly uses of it where it's more secure than any alternative (e.g., mosh uses $MOSH_KEY for this and it's pretty sound), and except for the OSes where environment variables are intentionally world-readable, most attacks that would let you read an environment variable would also let you read any other place to store a secret, including a file, a pipe, or a special kernel thing like Linux keyrings.
- lawnchair_larry 9y ago
  I think you have some faulty assumptions. Environment variables are not usually disclosed because somebody got shell access. It's very common to print them out while debugging, but not common to print arbitrary files containing your credentials. I have yet to see phpinfo() print ~/.ssh/id_rsa to the world for example, but I sure have pulled a lot of passwords out of it.
optimuspaul 9y ago
While I agree with this statement I don't know of any other way to do so with Travis-CI.

andrewvc 9y ago

While I love travis for what it is, this is a foreseeable result here. At the very least they need to add a failsafe that checks all outgoing logs for any secure tokens and replaces them with '*' or something. If you sign up to play a game of whack-a-mole you will lose eventually.

wwwigham 9y ago
That's actually exactly what the stated mitigation they've implemented is, if I've read it correctly. Though they're working on building it into their VM images in order to reduce the memory overhead of the filter.
tehlike 9y ago
What if the token was encrypted or altered in a way simple find replace wouldnt work?
- ReverseCold 9y ago
  Even just encoding it differently (base64, bin, hex, etc) would work against that.

0x0 9y ago

When I submitted a pull request to a random open source project, I was surprised to see a travis build kick off immediately. Is there a chance for rogue pull requests to contain (build) code that dumps out travis environment secrets? I didn't explore this but obviously the code is being built by scripts that are part of the commit.

roidrage 9y ago
That wouldn't be possible, and that's independent of the security issue we've disclosed in the post. For pull requests Travis CI has long had security measures in place to prevent this scenario from happening: https://docs.travis-ci.com/user/pull-requests#Pull-Requests-and-Security-Restrictions https://docs.travis-ci.com/user/pull-requests#Pull-Requests-...
- tehlike 9y ago
  Doesnt this still make it potentially available in case some malicious/unmalicious coder leaves some console debugging out?
  - nothrabannosir 9y ago
    Only if you merge it in. The point is the secure environment variables are not available at all in the fork build. The bash oneliner they show is to help you run scripts which won't crash if they don't have those env vars available, not to "hide them" by running a test script which doesn't use them.
    - tehlike 9y ago
      I know many instances where code reviews didnt catch log statement in huge binaries.
      - nothrabannosir 9y ago
        Right, but now you're in the "review of a PR didn't catch malicious code" boat. At which point, you've got bigger problems than leaking env vars in your CI. Not to dismiss it---it's just a different point.

deleted 9y ago

[deleted]

mintplant 9y ago

> As this fix runs in Ruby, it results in larger memory overhead. To address this, we are working on an executable binary that will be installed as part of the build-image. Work on this is expected to be complete by the end of the week. Run it through sed?

jacques_chester 9y ago
We hit this in Cloud Foundry Buildpacks, where all the builds and logs are completely public[0]. The team wrote and added Concourse-Filter[1]. You stream logs through it. It compares what's going past to current environment variables. Anytime something goes past that looks like one of the environment variables, it blanks it out. You can set a whitelist for things you're not fussed about. Disclosure: I have twice worked on Buildpacks on behalf of Pivotal. [0] https://buildpacks.ci.cf-app.com/ https://buildpacks.ci.cf-app.com/ [1] https://github.com/pivotal-cf-experimental/concourse-filter https://github.com/pivotal-cf-experimental/concourse-filter

arekkas 9y ago

Did they bother to notify the repositories affected by this? Could not find anything.

mattcoles 9y ago
> We have been in close communications with GitHub since we began working on this incident, and have shared impacted tokens with GitHub for their records. GitHub will revoke token access for the affected tokens and will contact token owners to notify them. Both Travis CI and GitHub recommend affected owners revoke their own access tokens and create new tokens immediately. Seems like both Github and Travis did.

arekkas 9y ago

Did they bother to notify the repositories affected by this? Could not find anything.