Skip to content
This repository has been archived by the owner on Feb 10, 2021. It is now read-only.

Switch to Travis's new authentication/integrity check scheme #43

Open
Blackbaud-BobbyEarl opened this issue Aug 18, 2016 · 10 comments
Open
Labels

Comments

@Blackbaud-BobbyEarl
Copy link
Contributor

To my knowledge, our setup/configuration hasn't changed, but we now consistently get "Received Travis request with incorrect hash!"

This in turn causes the status / comment to not be updated / posted on the PR. Looking at Bootstrap, it appears maybe you're seeing the same behavior?

@Blackbaud-BobbyEarl
Copy link
Contributor Author

I believe this is related. travis-ci/travis-ci#6435

@bobholt
Copy link

bobholt commented Jan 18, 2017

On August 31 Travis announced that they would be dropping support for the Authorization header and instead using the Signature header as described here.

I don't see an accompanying change to Savage. Is it no longer working anywhere?

@Blackbaud-BobbyEarl
Copy link
Contributor Author

Looking at twbs/bootstrap#21736, as an example, we can see that the statuses from Savage are not being updated prior to merging. :-(

@cvrebert
Copy link
Collaborator

Your analysis is correct. I haven't had much time for Savage lately, and so the Authorization header logic is outdated relative to Travis's awesome bugfix.

@Blackbaud-BobbyEarl
Copy link
Contributor Author

Certainly not your burden to carry alone @cvrebert. I took a rough stab at implementing the update authorization technique, but ultimately ran out of time due to my lack of Scala knowledge.

@cvrebert
Copy link
Collaborator

Yeah, PRs welcomed. I'm happy to translate to Scala from Java if necessary.

@bobholt
Copy link

bobholt commented Jan 18, 2017

I also have no Scala knowledge (yet), but am willing to put a few hours into it.

@bobholt
Copy link

bobholt commented Jan 18, 2017

I've waded out about as far as I can, I think. I opened a WIP PR against my own branch that makes the changes that were obvious to me and lays out TODOs for what I think is left. Anybody is welcome to try to take it from there (no guarantees that Base64 stuff works), but I'll keep plugging away at it as I get time if no one else does: https://github.com/bobholt/savage/pull/1/files

@cvrebert cvrebert added the bug label Jan 19, 2017
@cvrebert
Copy link
Collaborator

Assuming we pull in BouncyCastle for the public-key crypto, the verification part should start with doing Signature.getInstance("SHA1WithRSA", "BC") (based on Travis's code samples, they're using SHA-1 + RSA) and then you follow the dance explained in https://docs.oracle.com/javase/tutorial/security/apisign/vstep4.html

I am less clear on how to read Travis's public key into a PublicKey object.

@cvrebert
Copy link
Collaborator

Okay, think I've got the crypto part all figured out now. PR incoming.

cvrebert added a commit that referenced this issue Jan 22, 2017
To be used for Travis's new authentication mechanism
Refs #43
cvrebert added a commit that referenced this issue Jan 22, 2017
To be used for Travis's new authentication mechanism
Refs #43
cvrebert added a commit that referenced this issue Jan 22, 2017
…stle

To be used for Travis's new authentication mechanism
Refs #43
cvrebert added a commit that referenced this issue Jan 22, 2017
…stle (#53)

To be used for Travis's new authentication mechanism
Refs #43
@cvrebert cvrebert changed the title auth.isValid consistently failing Switch to Travis's new authentication/integrity check scheme Jan 22, 2017
cvrebert added a commit that referenced this issue Jan 24, 2017
This theoretically addresses #43,
though ideally we should fetch Travis' public key ourselves
rather than requiring the user to copy it into the settings file themself.
cvrebert added a commit that referenced this issue Jan 24, 2017
This theoretically addresses #43,
though ideally we should fetch Travis' public key ourselves
rather than requiring the user to copy it into the settings file themself.
cvrebert added a commit that referenced this issue Jan 24, 2017
This theoretically addresses #43,
though ideally we should fetch Travis' public key ourselves
rather than requiring the user to copy it into the settings file themself.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants