Add set_cert_verify_callback (SSL_CTX_set_cert_verify)
#287
Conversation
|
failing build looks related to: #285 |
| /// | ||
| /// This method panics if this `SslContext` is associated with a RPK context. | ||
| #[corresponds(SSL_CTX_set_cert_verify_callback)] | ||
| pub fn set_cert_verify_callback<F>(&mut self, callback: F) |
There was a problem hiding this comment.
You can unset by passing NULL, but that's not possible with this binding.
There was a problem hiding this comment.
it seems we either change this to Option or add another function that unsets this callback. Changing this to Option seems to make Rust unable to reason about arguments to this function when it is called, requiring type annotations (which are onerous when you just want to pass None).
| #[cfg(feature = "rpk")] | ||
| assert!(!self.is_rpk, "This API is not supported for RPK"); | ||
|
|
||
| self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback); |
There was a problem hiding this comment.
set_verify_callback uses
// this needs to be in an Arc since the callback can register a new callback!
self.replace_ex_data(Ssl::cached_ex_index(), Arc::new(callback));
There was a problem hiding this comment.
from my digging, this is copy-pasta. see this comment by @nox https://github.com/cloudflare/boring/blob/master/boring/src/ssl/callbacks.rs#L200-L202
I agree that it is impossible to get a mutable reference to an SslRef or SslContextRef, so I don't see why this needs to be an Arc. I am happy to make it one just to be safe, though
There was a problem hiding this comment.
Add a comment with the reference.
semaj-cf
force-pushed
the
expose-set-cert-verify-callback
branch
2 times, most recently
from
f1e0e77 to
d18dca5
Compare
boring/src/ssl/callbacks.rs
Outdated
| @@ -64,6 +64,34 @@ where | |||
| unsafe { raw_custom_verify_callback(ssl, out_alert, callback) } | |||
| } | |||
|
|
|||
| pub(super) unsafe extern "C" fn cert_verify<F>( | |||
There was a problem hiding this comment.
| pub(super) unsafe extern "C" fn cert_verify<F>( | |
| pub(super) unsafe extern "C" fn raw_cert_verify<F>( |
semaj-cf
force-pushed
the
expose-set-cert-verify-callback
branch
2 times, most recently
from
e7e68e5 to
aa7431f
Compare
Add a wrapper for `SSL_CTX_set_cert_verify`, which allows consumers to override the default certificate verification behavior. The binding resembles `SSL_CTX_set_verify`'s. See https://docs.openssl.org/master/man3/SSL_CTX_set_cert_verify_callback/ for more details.
semaj-cf
force-pushed
the
expose-set-cert-verify-callback
branch
from
aa7431f to
921ebbd
Compare
Add a wrapper for
SSL_CTX_set_cert_verify, which allows consumers to override the default certificate verification behavior.The binding resembles
SSL_CTX_set_verify's.See
https://docs.openssl.org/master/man3/SSL_CTX_set_cert_verify_callback/ for more details.