-
Notifications
You must be signed in to change notification settings - Fork 553
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
replace openssl dependency with pem #272
Conversation
NB. I haven't tested this against actual GCS. But it compiles :D |
Thanks for the patch! I'm in the middle of trying to get some large pull requests merged, but once those are taken care of I will merge this. It would be good to have someone test against GCS before landing this, though. @cramertj: do you have time for that? |
If this is landable we could get rid of the silly "all-windows" feature set: Line 108 in 55210fe
Line 19 in 55210fe
That's only there because it was too much work to build openssl for Windows, so we're not building gcs in Windows CI. |
Nice! Sounds like a good follow-up PR.
…On Tue, Aug 28, 2018 at 17:42 Ted Mielczarek ***@***.***> wrote:
If this is landable we could get rid of the silly "all-windows" feature
set:
https://github.com/mozilla/sccache/blob/55210fe1bb61615fdc3aaede47dbe399ddc05794/Cargo.toml#L108
https://github.com/mozilla/sccache/blob/55210fe1bb61615fdc3aaede47dbe399ddc05794/appveyor.yml#L19
That's only there because it was too much work to build openssl for
Windows, so we're not building gcs in Windows CI.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#272 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAKjAB1wPTd2SB7bHvulTqkQKpgpz8I7ks5uVXLrgaJpZM4V42nN>
.
|
I tested this out locally and it doesn't seem to work, unfortunately.
Line 309 is here: Line 309 in b086c38
|
|
||
let auth_request_jwt = jwt::encode( | ||
&jwt::Header::new(jwt::Algorithm::RS256), | ||
&jwt_claims, | ||
&binary_key, | ||
&pem_cert.contents, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks to me like the private key is never converted to DER format, like it was above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or is that just the same as decoding the base64 to binary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fxb that's right (assuming you stripped the --- BEGIN...
---END..
pre/post ambles first), there is a good overview at https://knowledge.digicert.com/generalinformation/INFO4448.html about the differences.
I merge #367 which fixes this in a different way. Sorry we couldn't get this PR to work! |
fixes #223