-
-
Notifications
You must be signed in to change notification settings - Fork 749
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
Expose Poly1305 bindings on libressl and boringssl #1998
Conversation
The LibreSSL build is red. I am pretty suspicious of adding support of APIs that OpenSSL doesn't support to the rust-openssl library. |
d900a18
to
55ee0da
Compare
Fixed, I forgot to add the header to
OpenSSL does have support for it (source), it's just that you can also use |
Right, so this PR is adding support of APIs that OpenSSL doesn't support. |
I think what @facutuesca is saying is that OpenSSL does support these APIs, but just chooses to expose them under different symbol names: OpenSSL exposes them as From a quick glance, the symbol name is the only difference: OpenSSL's versions are public and have the same parameters/parameter order. I'm not sure if that distinction is important for you, but I'd argue that this does mean that the same API is supported by all 3 here 🙂. Perhaps what we could do is re-export them all under the same symbol, if that's preferable? |
Oh sure, if OpenSSL just uses a different name for the function then it seems fine to expose, but I'd want to include those as well. |
Makes sense! @facutuesca, let's expose those OpenSSL symbols as well in this PR (under a corresponding Edit: From convo, we should also add a safe interface for these APIs to this PR. |
We just realized a hiccup: OpenSSL does have these APIs and the symbols aren't hidden, but they're included in a header that isn't part of OpenSSL's normal build-installed headers (i.e., Given that, there's a sense in which these aren't part of the OpenSSL public API (even if they support them); apologies for the confusion. @sfackler how would you prefer to move forwards here? We could probably bind directly to OpenSSL's symbols, but I wanted to confirm that that'd be okay with you as a matter of crate/library policy. |
Ah that is unfortunate. I definitely wouldn't want to bind to non-public APIs. Is there a reason you can't work through the EVP interface on all implementations? |
Yeah, Boring and Libre don't support Poly1305 through the EVP interface. |
smh |
Maybe that's a good reason to only expose the raw bindings in |
This PR exposes the Poly1305 API from LibreSSL (source) and BoringSSL (source)
The API was already exposed for Boring through
bssl-sys
(when building with theunstable_boringssl
feature), but was not present in normal (bindgen
) builds. I did not add a handwritten version for BoringSSL, because from what I could see Boring is either built withbindgen
orbssl-sys
bindings, not the manually-written ones.For LibreSSL, both the handwritten and the
bindgen
bindings should work now.