-
Notifications
You must be signed in to change notification settings - Fork 968
Remove dependency on OpenSSL #8768
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
Conversation
f9eed07 to
08de065
Compare
| # The lighthouse/key_legacy.p12 file is generated specifically for macOS because the default `openssl pkcs12` encoding | ||
| # algorithm in OpenSSL v3 is not compatible with the PKCS algorithm used by the Apple Security Framework. The client | ||
| # side (using the reqwest crate) relies on the Apple Security Framework to parse PKCS files. | ||
| # We don't need to generate web3signer/key_legacy.p12 because the compatibility issue doesn't occur on the web3signer | ||
| # side. It seems that web3signer (Java) uses its own implementation to parse PKCS files. | ||
| # See https://github.com/sigp/lighthouse/issues/6442#issuecomment-2469252651 |
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.
I believe now that we are using pure Rust, we no longer rely on the Apple Security Framework and so can support OpenSSL v3 even on MacOS
ackintosh
left a comment
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.
Looks good to me!
|
Can we add openssl to the list of banned crates here? Lines 7 to 19 in edba56b
|
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.
LGTM. The web3 signer tests pass so seem like this is good to go.
dapplion
left a comment
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.
LGTM
Merge Queue StatusRule:
This pull request spent 31 minutes 52 seconds in the queue, including 30 minutes 41 seconds running CI. Required conditions to merge
|
Issue Addressed
#8756
Proposed Changes
Only the Web3Signer actually needs OpenSSL in order to parse PKCS12 certificates. This updates the function to instead manually parse the cert (using the
p12-keystorecrate) and converts it to aPEMcertificate (using thepemcrate) which can be directly converted to areqwest::tls::Identityas this can be done directly inrustls.Additional Info
There may be some situations where a .p12 certificate could be strangely formatted (for example, if not generated by openssl) and break this function where it would succeed before. I'm not sure how to quantify the risks here but we could consider adding more test cases.