Skip to content

replace openssl dependency with pem#272

Closed
nornagon wants to merge 1 commit intomozilla:masterfrom
nornagon:pem
Closed

replace openssl dependency with pem#272
nornagon wants to merge 1 commit intomozilla:masterfrom
nornagon:pem

Conversation

@nornagon
Copy link
Copy Markdown

@nornagon nornagon commented Aug 10, 2018

fixes #223

@nornagon
Copy link
Copy Markdown
Author

nornagon commented Aug 10, 2018

NB. I haven't tested this against actual GCS. But it compiles :D

@luser
Copy link
Copy Markdown
Contributor

luser commented Aug 28, 2018

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?

@luser
Copy link
Copy Markdown
Contributor

luser commented Aug 28, 2018

If this is landable we could get rid of the silly "all-windows" feature set:

sccache/Cargo.toml

Line 108 in 55210fe

all-windows = ["redis", "s3", "memcached", "azure"]

sccache/appveyor.yml

Line 19 in 55210fe

- cmd: cargo build %RELEASE% --verbose --features="all-windows %EXTRA_FEATURES%"

That's only there because it was too much work to build openssl for Windows, so we're not building gcs in Windows CI.

@nornagon
Copy link
Copy Markdown
Author

nornagon commented Aug 29, 2018 via email

@luser
Copy link
Copy Markdown
Contributor

luser commented Aug 30, 2018

I tested this out locally and it doesn't seem to work, unfortunately. ☹️ I created a GCS bucket and first tested sccache master with it, where it successfully stored a cache entry and was able to retrieve it as a cache hit. Then I built this PR and tried compiling the same source with the same GCS bucket and got the following error in the log:

TRACE:sccache::compiler::compiler: [hello.o]: Hash key: 3be4f5a0edf0f289a81522dde151d2a08d2bbe412ee4af60d48c83f6c7e93c72e5910ce9074232e5fa2b157f2eeeeeaeb3876f3d9f7b008e6e2593da9ca447a7
WARN:sccache::cache::gcs: Got GCS error: Error(Msg("Invalid Key"), State { next_error: None, backtrace: Some(stack backtrace:

<...>

   8:        0x10ef5cdc3 - sccache::cache::gcs::GCSCredentialProvider::credentials::{{closure}}::h4b9a4ddff862ad98
                        at src/cache/gcs.rs:309

Line 309 is here:

sccache/src/cache/gcs.rs

Line 309 in b086c38

Err(e) => Err(e.to_string().into()),

@luser luser mentioned this pull request Oct 11, 2018
Merged
fxb
fxb reviewed Oct 11, 2018
View reviewed changes
src/cache/gcs.rs
&jwt::Header::new(jwt::Algorithm::RS256),
&jwt_claims,
&binary_key,
&pem_cert.contents,
Copy link
Copy Markdown
Contributor

@fxb fxb Oct 11, 2018

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.

Copy link
Copy Markdown
Contributor

@fxb fxb Oct 11, 2018

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?

Copy link
Copy Markdown
Collaborator

@drahnr drahnr Nov 9, 2018 •
edited
Loading

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.

This was referenced Feb 4, 2019
Closed
Merged
@luser
Copy link
Copy Markdown
Contributor

luser commented Feb 8, 2019

I merge #367 which fixes this in a different way. Sorry we couldn't get this PR to work!

@luser luser closed this Feb 8, 2019
@drahnr drahnr mentioned this pull request Nov 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@drahnr drahnr drahnr left review comments

+1 more reviewer

@fxb fxb fxb left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Look into removing the openssl dependency for gcs

4 participants

@nornagon @luser @fxb @drahnr