cloudflare / boring

BoringSSL bindings for the Rust programming language.
354 stars 107 forks source link

Expose set_permute_extensions #247

Closed mstyura closed 2 months ago

rushilmehra commented 2 months ago

Let's add the SSL variant as well https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_set_permute_extensions

mstyura commented 2 months ago

@rushilmehra do you mean adding it into

impl SslRef {
 ///...
}

?

rushilmehra commented 2 months ago

Yeah, that's the one. Is this API incompatible with FIPS?

mstyura commented 2 months ago

There was compilation failure reported by CI in initial commit. CI reported there is no ffi::SSL_CTX_set_permute_extensions function. I don't know much about fips, but what I've googled it seems like it could be explained by the absence of certification of the recent enough version of boring ssl. I see that the SSL_CTX_set_permute_extensions was introduced on 14 June 2021 (https://boringssl-review.googlesource.com/c/boringssl/+/48045), while according to documentation the most recent certification was done 29 April 2021 (https://boringssl.googlesource.com/boringssl/+/master/crypto/fipsmodule/FIPS.md)

rushilmehra commented 2 months ago

Yeah this is because the fips feature builds with a separate version of boringssl: https://github.com/google/boringssl/tree/853ca1ea1168dff08011e5d42d94609cc0ca2e27 which doesn't know about these APIs. I suppose gating it to non-fips is fine, but let's leave a comment in both places describing why it's gated to non-fips. Once we upgrade the submoduled fips commit we can remove these gates

rushilmehra commented 2 months ago

Merged, thanks @mstyura

mstyura commented 2 months ago

Thank you, @rushilmehra, for reviewing and providing valuable feedback. May I kindly ask you to take a look at another one of my PRs? If necessary, I can open a separate issue for further discussion, as the proposed changes are slightly bigger compared to this PR. I initially missed the contributing guidelines, which prefer an issue in addition to a PR when the changes are non-trivial.