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

Encrypted vars for PRs (from forks) #1946

Open
vojtajina opened this Issue Feb 7, 2014 · 38 comments

Comments

Projects
None yet
@vojtajina

vojtajina commented Feb 7, 2014

We (Angular, jQuery, and most likely any other open source project) need to be able to use BrowserStack/SauceLab browsers from Travis, even from pull requests from forks.

@santiycr mentioned the solution (based on private/public keys) has been already decided. Can we get some rough plan on how and when this will be implemented? We are happy to help as this is important feature for us.

@henrikhodne @jzaefferer @IgorMinar @jlipps @dhimil @markelog

@roidrage

This comment has been minimized.

Show comment
Hide comment
@roidrage

roidrage Feb 7, 2014

Member

Could you elaborate a bit more on what kind of solution has already been decided on?

Member

roidrage commented Feb 7, 2014

Could you elaborate a bit more on what kind of solution has already been decided on?

@jlipps

This comment has been minimized.

Show comment
Hide comment
@jlipps

jlipps Feb 11, 2014

Hi guys. Just got the scoop from @sourishkrout who had been working a bit on this at Sauce beforehand. Here's the basic idea:

Currently, usernames/accesskeys are encrypted as secure tokens in .travis.yml. These are decrypted on Travis and used to populate environment variables, which can in turn be used to start jobs on Sauce. The security risk for allowing PRs to decrypt these tokens is that they could be output by anyone who submits a PR.

The proposed solution is as follows:

  • secure tokens continue to encrypt usernames/accesskeys
  • accesskeys are not decrypted to env vars on Travis.
  • instead, they are HMAC-encoded along with two other bits of information:
    • a secret key owned by travis and known only by Sauce
    • a creation timestamp
  • this new token is put into the appropriate env var, and can be used to make authenticated requests to Sauce

On the Sauce side, we can decode this HMAC with the use of the user's accesskey (which we know) and retrieve the timestamp and travis key. This will validate that the request came from Travis and that it is OK to trust the timestamp. Thus trusted, we'll continue to authenticate requests with that HMAC until a certain amount of time has elapsed since the timestamp (proposal: the max duration of a Travis job).

This will involve work on both the Travis and Sauce sides. I'm happy to lead the effort at Sauce if @henrikhodne or someone else can do the work on the Travis side.

Also, I'm a newb when it comes to HMAC so I'm not at all clear on how we can decode the timestamp and the Travis key to verify the request. Sounds like it should be pretty straightforward to figure out, though if anyone has guidance I'm happy to listen.

jlipps commented Feb 11, 2014

Hi guys. Just got the scoop from @sourishkrout who had been working a bit on this at Sauce beforehand. Here's the basic idea:

Currently, usernames/accesskeys are encrypted as secure tokens in .travis.yml. These are decrypted on Travis and used to populate environment variables, which can in turn be used to start jobs on Sauce. The security risk for allowing PRs to decrypt these tokens is that they could be output by anyone who submits a PR.

The proposed solution is as follows:

  • secure tokens continue to encrypt usernames/accesskeys
  • accesskeys are not decrypted to env vars on Travis.
  • instead, they are HMAC-encoded along with two other bits of information:
    • a secret key owned by travis and known only by Sauce
    • a creation timestamp
  • this new token is put into the appropriate env var, and can be used to make authenticated requests to Sauce

On the Sauce side, we can decode this HMAC with the use of the user's accesskey (which we know) and retrieve the timestamp and travis key. This will validate that the request came from Travis and that it is OK to trust the timestamp. Thus trusted, we'll continue to authenticate requests with that HMAC until a certain amount of time has elapsed since the timestamp (proposal: the max duration of a Travis job).

This will involve work on both the Travis and Sauce sides. I'm happy to lead the effort at Sauce if @henrikhodne or someone else can do the work on the Travis side.

Also, I'm a newb when it comes to HMAC so I'm not at all clear on how we can decode the timestamp and the Travis key to verify the request. Sounds like it should be pretty straightforward to figure out, though if anyone has guidance I'm happy to listen.

@sourishkrout

This comment has been minimized.

Show comment
Hide comment
@sourishkrout

sourishkrout Feb 12, 2014

It may be worth utilizing JWT to do the implementation of authentication (http://self-issued.info/docs/draft-ietf-oauth-json-web-token.html).

sourishkrout commented Feb 12, 2014

It may be worth utilizing JWT to do the implementation of authentication (http://self-issued.info/docs/draft-ietf-oauth-json-web-token.html).

@IgorMinar

This comment has been minimized.

Show comment
Hide comment
@IgorMinar

IgorMinar Feb 12, 2014

@jlipps I like this a lot. We discussed this issue on the angular team and came up with the same solution. As you said, it does require work on both travis and sauce, but it can be done generically enough that other providers could utilize the solution to exchange secret information with either of you.

IgorMinar commented Feb 12, 2014

@jlipps I like this a lot. We discussed this issue on the angular team and came up with the same solution. As you said, it does require work on both travis and sauce, but it can be done generically enough that other providers could utilize the solution to exchange secret information with either of you.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer commented Feb 14, 2014

@drogus

This comment has been minimized.

Show comment
Hide comment
@drogus

drogus Feb 14, 2014

Member

I like the idea of signed requests with expire time. We could also make this a public API, so others may also use it for their purpose.

I think this would also be a good idea to allow to scope such requests to not give a full API access for Travis.

Member

drogus commented Feb 14, 2014

I like the idea of signed requests with expire time. We could also make this a public API, so others may also use it for their purpose.

I think this would also be a good idea to allow to scope such requests to not give a full API access for Travis.

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Feb 14, 2014

Member

@drogus how would this work as a public API? From my understanding, what is being requested would be specific to each provider. Eg. CodeClimate, Coveralls, Browserstack and Sauce Labs would all require different key pairs for encrypting the tokens for them to decrypt.

I'm not sure what you mean by the second paragraph.

As for this suggested approach, it is something we are going to look into. I'll update this ticket next week with more info once we have had a chance to investigate the impact and complexity involved within our codebase.

Member

joshk commented Feb 14, 2014

@drogus how would this work as a public API? From my understanding, what is being requested would be specific to each provider. Eg. CodeClimate, Coveralls, Browserstack and Sauce Labs would all require different key pairs for encrypting the tokens for them to decrypt.

I'm not sure what you mean by the second paragraph.

As for this suggested approach, it is something we are going to look into. I'll update this ticket next week with more info once we have had a chance to investigate the impact and complexity involved within our codebase.

@drogus

This comment has been minimized.

Show comment
Hide comment
@drogus

drogus Feb 14, 2014

Member

@drogus how would this work as a public API? From my understanding, what is being requested would be specific to each provider. Eg. CodeClimate, Coveralls, Browserstack and Sauce Labs would all require different key pairs for encrypting the tokens for them to decrypt.

What I meant by this is to make this provider agnostic. What we're discussing here is to encrypt access key before sending it to the external provider using saved secret. I think that we can store such secrets agnostically, for example add it to repository settings.

I'm not saying that this is the way to go and I'm not sure how would details look like, but we should at least discuss an agnostic approach to this.

I'm not sure what you mean by the second paragraph.

I meant that Sauce Labs should probably scope such requests to not allow to use full API from Travis, ie. not give access to changing user details etc.

Member

drogus commented Feb 14, 2014

@drogus how would this work as a public API? From my understanding, what is being requested would be specific to each provider. Eg. CodeClimate, Coveralls, Browserstack and Sauce Labs would all require different key pairs for encrypting the tokens for them to decrypt.

What I meant by this is to make this provider agnostic. What we're discussing here is to encrypt access key before sending it to the external provider using saved secret. I think that we can store such secrets agnostically, for example add it to repository settings.

I'm not saying that this is the way to go and I'm not sure how would details look like, but we should at least discuss an agnostic approach to this.

I'm not sure what you mean by the second paragraph.

I meant that Sauce Labs should probably scope such requests to not allow to use full API from Travis, ie. not give access to changing user details etc.

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Feb 14, 2014

Member

The key/secret for Travis/Sauce would be per provider. Eg. Travis and CodeClimate would have a pair, so would Travis and Coveralls. The key/secret wouldn't be per repo.

Member

joshk commented Feb 14, 2014

The key/secret for Travis/Sauce would be per provider. Eg. Travis and CodeClimate would have a pair, so would Travis and Coveralls. The key/secret wouldn't be per repo.

@drogus

This comment has been minimized.

Show comment
Hide comment
@drogus

drogus Feb 14, 2014

Member

The key/secret for Travis/Sauce would be per provider. Eg. Travis and CodeClimate would have a pair, so would Travis and Coveralls. The key/secret wouldn't be per repo.

I think that's something we need to discuss. I was thinking about a secret, which you can configure per user or per repository on Travis CI. I agree that if we're talking about a global secret which is held by us and Sauce Labs, then it will be hard to do it in a transparent way.

I think that the problem with this discussion is that we're possibly talking about different things. Let's hold a bit with discussing any details further before we have a concrete proposal of how exactly it would look like (ie. if we use global token, how do we exchange it with Sauce Labs etc.). Would you like to lay it out?

Member

drogus commented Feb 14, 2014

The key/secret for Travis/Sauce would be per provider. Eg. Travis and CodeClimate would have a pair, so would Travis and Coveralls. The key/secret wouldn't be per repo.

I think that's something we need to discuss. I was thinking about a secret, which you can configure per user or per repository on Travis CI. I agree that if we're talking about a global secret which is held by us and Sauce Labs, then it will be hard to do it in a transparent way.

I think that the problem with this discussion is that we're possibly talking about different things. Let's hold a bit with discussing any details further before we have a concrete proposal of how exactly it would look like (ie. if we use global token, how do we exchange it with Sauce Labs etc.). Would you like to lay it out?

@IgorMinar

This comment has been minimized.

Show comment
Hide comment
@IgorMinar

IgorMinar Feb 17, 2014

I think that the solution can be generic. This is what I had in mind:

1/ travis user sets key1 via travis UI (key1 can be sauce key or some other key) - this is the secret never to be transmitted during build
2/ kick off a build
3/ travis will compute someHashingFn(repo + buildNumber + timestamp + key1) and will expose the result as KEY1_HASHED or similarly named env variable. (the buildNumber and timestamp also need to be exposed as ENV variables) (someHashingFn can be HMAC md5, or something similar)
4/ when our build scripts tries to create a tunnel to sauce and get hold of a browser instead of the real key, we'll use KEY1_HASHED and append repo, buildNumber and timestamp as exposed via ENV variables (but not key1)
5/ sauce/browser stack can then compute someHashingFn(repo + buildNumber + timestamp + key1) and compare it to KEY1_HASHED we sent. If they match, this hash hasn't been used yet and timestamp is within X seconds from now then we are good to go.

Since key1 is not being transmitted, but both travis and sauce know what it is, they both can compute KEY1_HASHED. If they both compute the same value, it means that the request is authentic. The buildNumber and timestamp can then be used to prevent replay attacks.

I used sauce as the example here, but in reality any provider willing to implement the verification mechanism on their end could use this.

PS: I believe that Amazon S3 uses a very similar scheme for sending credentials over plain HTTP.

IgorMinar commented Feb 17, 2014

I think that the solution can be generic. This is what I had in mind:

1/ travis user sets key1 via travis UI (key1 can be sauce key or some other key) - this is the secret never to be transmitted during build
2/ kick off a build
3/ travis will compute someHashingFn(repo + buildNumber + timestamp + key1) and will expose the result as KEY1_HASHED or similarly named env variable. (the buildNumber and timestamp also need to be exposed as ENV variables) (someHashingFn can be HMAC md5, or something similar)
4/ when our build scripts tries to create a tunnel to sauce and get hold of a browser instead of the real key, we'll use KEY1_HASHED and append repo, buildNumber and timestamp as exposed via ENV variables (but not key1)
5/ sauce/browser stack can then compute someHashingFn(repo + buildNumber + timestamp + key1) and compare it to KEY1_HASHED we sent. If they match, this hash hasn't been used yet and timestamp is within X seconds from now then we are good to go.

Since key1 is not being transmitted, but both travis and sauce know what it is, they both can compute KEY1_HASHED. If they both compute the same value, it means that the request is authentic. The buildNumber and timestamp can then be used to prevent replay attacks.

I used sauce as the example here, but in reality any provider willing to implement the verification mechanism on their end could use this.

PS: I believe that Amazon S3 uses a very similar scheme for sending credentials over plain HTTP.

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Feb 17, 2014

Member

Yes, we actually use the S3 signature feature for our caching solution. This would require coordination with Sauce Labs.

Member

rkh commented Feb 17, 2014

Yes, we actually use the S3 signature feature for our caching solution. This would require coordination with Sauce Labs.

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Feb 18, 2014

Member

We are looking into a similar solution and will update this issue when we've had an opportunity to discuss this internally.

Member

joshk commented Feb 18, 2014

We are looking into a similar solution and will update this issue when we've had an opportunity to discuss this internally.

@nakula

This comment has been minimized.

Show comment
Hide comment
@nakula

nakula Feb 18, 2014

vendor = BrowserStack/sauce/Coveralls/CodeClimate

@IgorMinar I suppose we can optimize further, and keep secret keys as it is. When travis current decodes them, they can re-encode as per your way, and then send it across. And, vendor does the appropriate authentication.

But, issue I see is, it very much restrict developers to only use vendor made plugins/extensions/tools. So, if I develop some tool on top of one of the vendors APIs, I will have to handle travis separately in my tool, which does not sounds great.

Right now, I think @jlipps idea works, but yes travis will have to create and share secret key with each vendor and restricts new vendors.

nakula commented Feb 18, 2014

vendor = BrowserStack/sauce/Coveralls/CodeClimate

@IgorMinar I suppose we can optimize further, and keep secret keys as it is. When travis current decodes them, they can re-encode as per your way, and then send it across. And, vendor does the appropriate authentication.

But, issue I see is, it very much restrict developers to only use vendor made plugins/extensions/tools. So, if I develop some tool on top of one of the vendors APIs, I will have to handle travis separately in my tool, which does not sounds great.

Right now, I think @jlipps idea works, but yes travis will have to create and share secret key with each vendor and restricts new vendors.

@IgorMinar

This comment has been minimized.

Show comment
Hide comment
@IgorMinar

IgorMinar Feb 20, 2014

@nakula if the protocol is open and documented then any vendor can implement it.

The only other auth protocol that I know of that could work here is oAuth, but that might be even bigger pain in the butt to use. But maybe that's not the case, I'm not sure. Someone needs to dive in and research this.

IgorMinar commented Feb 20, 2014

@nakula if the protocol is open and documented then any vendor can implement it.

The only other auth protocol that I know of that could work here is oAuth, but that might be even bigger pain in the butt to use. But maybe that's not the case, I'm not sure. Someone needs to dive in and research this.

@nakula

This comment has been minimized.

Show comment
Hide comment
@nakula

nakula Feb 24, 2014

@IgorMinar vendor implementation is not the issue.

Anyone building new projects on top of vendors will have to implement it too.. eg. if someone plans to make karma-browserstack or karma-sauce, they will have to implement this integration (assuming vendors, travis implement it once initially)

@joshk any thoughts?

nakula commented Feb 24, 2014

@IgorMinar vendor implementation is not the issue.

Anyone building new projects on top of vendors will have to implement it too.. eg. if someone plans to make karma-browserstack or karma-sauce, they will have to implement this integration (assuming vendors, travis implement it once initially)

@joshk any thoughts?

@santiycr

This comment has been minimized.

Show comment
Hide comment
@santiycr

santiycr commented Mar 27, 2014

ping?

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Mar 27, 2014

Member

Hey @santiycr

Email me at josh at travis-ci dot com and we can talk more about this.

Member

joshk commented Mar 27, 2014

Hey @santiycr

Email me at josh at travis-ci dot com and we can talk more about this.

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Apr 28, 2014

Contributor

Is there any update on this?

Contributor

markelog commented Apr 28, 2014

Is there any update on this?

@roidrage

This comment has been minimized.

Show comment
Hide comment
@roidrage

roidrage Apr 28, 2014

Member

There isn't, sorry.

Member

roidrage commented Apr 28, 2014

There isn't, sorry.

@amitu

This comment has been minimized.

Show comment
Hide comment
@amitu

amitu Jun 15, 2014

I have another scheme:

  1. Each test running on travis has a environment variable TRAVIS_NONCE set. This variable contains a random opaque string (len <= 32).
  2. There exists a http service, nonces.travis-ci.org, which accepts ?nonce= and returns {"repo": "amitu/bslt", "ip": "<ip of travis vm running test"}/{"error": "test over/invalid nonce/etc"}.

BrowserStack/Sauce/other vendors will provide UI to their customers to authorize travis by selecting repo names. Travis-Nonce would be passed to vendor as key/header/whatever, and vendor makes a GET request to nonces.travis-ci.org to confirm that the session-start request is coming from correct ip and is from authorized github repo, and will let it run.

amitu commented Jun 15, 2014

I have another scheme:

  1. Each test running on travis has a environment variable TRAVIS_NONCE set. This variable contains a random opaque string (len <= 32).
  2. There exists a http service, nonces.travis-ci.org, which accepts ?nonce= and returns {"repo": "amitu/bslt", "ip": "<ip of travis vm running test"}/{"error": "test over/invalid nonce/etc"}.

BrowserStack/Sauce/other vendors will provide UI to their customers to authorize travis by selecting repo names. Travis-Nonce would be passed to vendor as key/header/whatever, and vendor makes a GET request to nonces.travis-ci.org to confirm that the session-start request is coming from correct ip and is from authorized github repo, and will let it run.

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Jun 16, 2014

Member

I'm sorry but i don't think this solution will work for us as it means VMs need to have a registered IP address, and is fairly complicated to implement.

Member

joshk commented Jun 16, 2014

I'm sorry but i don't think this solution will work for us as it means VMs need to have a registered IP address, and is fairly complicated to implement.

@amitu

This comment has been minimized.

Show comment
Hide comment
@amitu

amitu Jun 17, 2014

Should have clarified: they needed not be the IP address of the VM. The IP address vendor is supposed to get when a network request comes to vendors server from a program running on VM needed be there.

Rationale: The only reason I put IP address there is to avoid a vector that someone initiates a build by doing a dummy pull request on an authorised open source repo, and handing off the nonce to their server and using that to run unrelated tests.

But: There is nothing stopping from having their request route through travis VM despite the IP address restriction, so I suppose this mitigation is also not bullet proof.

So therefore: I suppose the solution would letting github / repo administrator blocking users who are sending dummy pull requests, instead of us worrying about it, and drop the IP address requirement altogether. Another alternative could be: {"repo": "amitu/bslt", "committer": "$github username of who created the pull request$"}, and vendor can block committers if needed.

amitu commented Jun 17, 2014

Should have clarified: they needed not be the IP address of the VM. The IP address vendor is supposed to get when a network request comes to vendors server from a program running on VM needed be there.

Rationale: The only reason I put IP address there is to avoid a vector that someone initiates a build by doing a dummy pull request on an authorised open source repo, and handing off the nonce to their server and using that to run unrelated tests.

But: There is nothing stopping from having their request route through travis VM despite the IP address restriction, so I suppose this mitigation is also not bullet proof.

So therefore: I suppose the solution would letting github / repo administrator blocking users who are sending dummy pull requests, instead of us worrying about it, and drop the IP address requirement altogether. Another alternative could be: {"repo": "amitu/bslt", "committer": "$github username of who created the pull request$"}, and vendor can block committers if needed.

naomiblack referenced this issue in angular/angular.js Jun 29, 2014

chore: set up Sauce Labs with Travis
This should not affect the Jenkins build at all.

Now, the Travis build uses Chrome on Sauce Labs, which in theory gives us opportunity to use any
browser/platform that Sauce Labs offers.
@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Sep 13, 2014

It looks like this discussion has stalled, due to the complexity of a 100% secure solution.

I'd like to propose something simpler that would solve this problem for many open source projects using BrowserStack and Saucelabs, with a good enough approach. The proposal is the option for individual projects to opt into exposing secure environment variables to pull requests. This could be a toggle in the settings just like the one for "Build pull requests". It could also be a toggle next to the existing "Display value in build logs" toggle when adding Environment Variables through repo settings (instead of adding them to .travis.yml).

The attack vector for this is still the same: Someone can send a trivial pull request that echos environment variables to the Travis build console. Our defense against this attack would be to invalidate the BrowserStack or Saucelabs account and switch to a new one. Thanks to notifications sent for pull requests, the time window for abusing the account would be pretty small.

Without this option, the only solution seems to be to put the credentials directly in the source code in the GitHub repository (maybe with a bit of obfuscation). This would make it trivial for anyone to abuse the credentials, without us getting any direct notification about it.

In either case, Saucelabs and BrowserStack don't seem to mind about credentials leaking, but for our own testing needs I heavily prefer the opt-in to at least require some effort on the attacker side, before they get a chance to interrupt our own tests by emptying our available workers.

jzaefferer commented Sep 13, 2014

It looks like this discussion has stalled, due to the complexity of a 100% secure solution.

I'd like to propose something simpler that would solve this problem for many open source projects using BrowserStack and Saucelabs, with a good enough approach. The proposal is the option for individual projects to opt into exposing secure environment variables to pull requests. This could be a toggle in the settings just like the one for "Build pull requests". It could also be a toggle next to the existing "Display value in build logs" toggle when adding Environment Variables through repo settings (instead of adding them to .travis.yml).

The attack vector for this is still the same: Someone can send a trivial pull request that echos environment variables to the Travis build console. Our defense against this attack would be to invalidate the BrowserStack or Saucelabs account and switch to a new one. Thanks to notifications sent for pull requests, the time window for abusing the account would be pretty small.

Without this option, the only solution seems to be to put the credentials directly in the source code in the GitHub repository (maybe with a bit of obfuscation). This would make it trivial for anyone to abuse the credentials, without us getting any direct notification about it.

In either case, Saucelabs and BrowserStack don't seem to mind about credentials leaking, but for our own testing needs I heavily prefer the opt-in to at least require some effort on the attacker side, before they get a chance to interrupt our own tests by emptying our available workers.

@jrjohnson

This comment has been minimized.

Show comment
Hide comment
@jrjohnson

jrjohnson Sep 15, 2014

@jzaefferer I couldn't agree more. Right now all of our sauce credentials are in the clear, but forcing a PR to expose them would make attacks far easier to detect. 👍 for this solution.

jrjohnson commented Sep 15, 2014

@jzaefferer I couldn't agree more. Right now all of our sauce credentials are in the clear, but forcing a PR to expose them would make attacks far easier to detect. 👍 for this solution.

@nakula

This comment has been minimized.

Show comment
Hide comment
@nakula

nakula commented Sep 17, 2014

👍

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Sep 22, 2014

@roidrage @BanzaiMan is there anything I can do to move this forward?

jzaefferer commented Sep 22, 2014

@roidrage @BanzaiMan is there anything I can do to move this forward?

@roidrage

This comment has been minimized.

Show comment
Hide comment
@roidrage

roidrage Sep 22, 2014

Member

Not yet, sorry. We'll revisit this soon and see how/if we fit it in with our product roadmap.

Member

roidrage commented Sep 22, 2014

Not yet, sorry. We'll revisit this soon and see how/if we fit it in with our product roadmap.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Nov 4, 2014

@roidrage did this find its way on the product roadmap?

jzaefferer commented Nov 4, 2014

@roidrage did this find its way on the product roadmap?

@roidrage

This comment has been minimized.

Show comment
Hide comment
@roidrage

roidrage Nov 4, 2014

Member

Not yet, sorry. At this point we're likely looking at some time next year. We've been chatting with Sauce how we can make this happen, but don't have anything concrete yet.

Member

roidrage commented Nov 4, 2014

Not yet, sorry. At this point we're likely looking at some time next year. We've been chatting with Sauce how we can make this happen, but don't have anything concrete yet.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Nov 4, 2014

The need to chat with Sauce sounds like you're still referring to the original discussion here, not to my later suggestion posted above (#1946 (comment)). If that's the case, could you take a look at that comment and see if this proposal is more realistic in the short term?

jzaefferer commented Nov 4, 2014

The need to chat with Sauce sounds like you're still referring to the original discussion here, not to my later suggestion posted above (#1946 (comment)). If that's the case, could you take a look at that comment and see if this proposal is more realistic in the short term?

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Nov 5, 2014

Member

Hi @jzaefferer

I'm sorry but the comment you refer to is not a solution we are currently investigating.

You can achieve exactly what you have described without us having to add anything to Travis CI itself.

We are still interested in working with Sauce Labs on implementing a full solution, but as @roidrage said, this won't be until sometime next year.

Thanks for taking the time to propose the alternative solution.

Member

joshk commented Nov 5, 2014

Hi @jzaefferer

I'm sorry but the comment you refer to is not a solution we are currently investigating.

You can achieve exactly what you have described without us having to add anything to Travis CI itself.

We are still interested in working with Sauce Labs on implementing a full solution, but as @roidrage said, this won't be until sometime next year.

Thanks for taking the time to propose the alternative solution.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Nov 5, 2014

You can achieve exactly what you have described without us having to add anything to Travis CI itself.

It works for all pushes except for pull requests, which is exactly what this issue is about. Without changes from Travis CI, the only option is to put credentials directly into the git repository. As I wrote above: "The proposal is the option for individual projects to opt into exposing secure environment variables to pull requests." That would make a difference and should be much simpler to implement, compared to full solutions previously discussed.

jzaefferer commented Nov 5, 2014

You can achieve exactly what you have described without us having to add anything to Travis CI itself.

It works for all pushes except for pull requests, which is exactly what this issue is about. Without changes from Travis CI, the only option is to put credentials directly into the git repository. As I wrote above: "The proposal is the option for individual projects to opt into exposing secure environment variables to pull requests." That would make a difference and should be much simpler to implement, compared to full solutions previously discussed.

@joshk

This comment has been minimized.

Show comment
Hide comment
@joshk

joshk Nov 6, 2014

Member

Like you said with:

Without this option, the only solution seems to be to put the credentials directly in the source code in the GitHub repository (maybe with a bit of obfuscation). This would make it trivial for anyone to abuse the credentials, without us getting any direct notification about it.

This solution is pretty much as good as allowing secure env vars to be allowed in PRs.

On top of this, you say:

The attack vector for this is still the same: Someone can send a trivial pull request that echos environment variables to the Travis build console. Our defense against this attack would be to invalidate the BrowserStack or Saucelabs account and switch to a new one. Thanks to notifications sent for pull requests, the time window for abusing the account would be pretty small.

What about when you are sleeping? Or if the PR submitter devises a smart way to POST the creds somewhere and not output anything to the log file?

So many people do not check a PR as soon as it is created, so at least to me, this proposal does not provide as much value as we would spend adding and maintaining it.

We would ask instead that you give us time to talk to Sauce Labs more so we can come back with a complete solution.

Member

joshk commented Nov 6, 2014

Like you said with:

Without this option, the only solution seems to be to put the credentials directly in the source code in the GitHub repository (maybe with a bit of obfuscation). This would make it trivial for anyone to abuse the credentials, without us getting any direct notification about it.

This solution is pretty much as good as allowing secure env vars to be allowed in PRs.

On top of this, you say:

The attack vector for this is still the same: Someone can send a trivial pull request that echos environment variables to the Travis build console. Our defense against this attack would be to invalidate the BrowserStack or Saucelabs account and switch to a new one. Thanks to notifications sent for pull requests, the time window for abusing the account would be pretty small.

What about when you are sleeping? Or if the PR submitter devises a smart way to POST the creds somewhere and not output anything to the log file?

So many people do not check a PR as soon as it is created, so at least to me, this proposal does not provide as much value as we would spend adding and maintaining it.

We would ask instead that you give us time to talk to Sauce Labs more so we can come back with a complete solution.

@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Nov 6, 2014

Or if the PR submitter devises a smart way to POST the creds somewhere and not output anything to the log file?

The diff of the PR would make it pretty obvious, the Travis output itself doesn't matter that much.

So many people do not check a PR as soon as it is created, so at least to me, this proposal does not provide as much value as we would spend adding and maintaining it.

It would be a big improvement over putting credentials directly in the repo, where there would be no notification at all of the attack.

We would ask instead that you give us time to talk to Sauce Labs more so we can come back with a complete solution.

Alright, I think it was worth a try, but I also understand that the value/effort ratio isn't good enough. When you work on a proper solution, please also talk to BrowserStack (@nakula, support@browserstack.com).

jzaefferer commented Nov 6, 2014

Or if the PR submitter devises a smart way to POST the creds somewhere and not output anything to the log file?

The diff of the PR would make it pretty obvious, the Travis output itself doesn't matter that much.

So many people do not check a PR as soon as it is created, so at least to me, this proposal does not provide as much value as we would spend adding and maintaining it.

It would be a big improvement over putting credentials directly in the repo, where there would be no notification at all of the attack.

We would ask instead that you give us time to talk to Sauce Labs more so we can come back with a complete solution.

Alright, I think it was worth a try, but I also understand that the value/effort ratio isn't good enough. When you work on a proper solution, please also talk to BrowserStack (@nakula, support@browserstack.com).

@nakula

This comment has been minimized.

Show comment
Hide comment
@nakula

nakula commented Nov 6, 2014

+1

@kratsg

This comment has been minimized.

Show comment
Hide comment
@kratsg

kratsg commented Nov 22, 2014

+1

@travis-ci travis-ci locked and limited conversation to collaborators Nov 22, 2014

@BanzaiMan BanzaiMan self-assigned this Aug 3, 2016

@BanzaiMan

This comment has been minimized.

Show comment
Hide comment
@BanzaiMan

BanzaiMan Oct 13, 2016

Member

Sauce Connect can make use of the secret API key on PR builds from forks using the jwt addon now.

Member

BanzaiMan commented Oct 13, 2016

Sauce Connect can make use of the secret API key on PR builds from forks using the jwt addon now.

nijel referenced this issue in phpmyadmin/phpmyadmin Aug 22, 2017

Merge pull request #13602 from mauriciofauth/tracking
Refactor tracking functions to static methods

@DrTorte DrTorte added the locked label Apr 2, 2018

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