rust: bssl-tls: Unstage the callback uninstallers As it turns out, uninstallers do not encourage correct use of connection builders. As it has shown limited benefit, we decided to unstage this API. Update-Note: We will only support callback installation. If there is a need to temporarily disable callback effects, please configure the callbacks at connection flight time to return success codes to by-pass. Signed-off-by: Xiangfei Ding <xfding@google.com> Change-Id: Ia1db939e1ebe8ec3864b2800860d15e96a6a6964 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/100527 Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/rust/bssl-tls-tokio/src/tests/transport.rs b/rust/bssl-tls-tokio/src/tests/transport.rs index 519d810..7ef13b8 100644 --- a/rust/bssl-tls-tokio/src/tests/transport.rs +++ b/rust/bssl-tls-tokio/src/tests/transport.rs
@@ -35,7 +35,9 @@ use bssl_tls::context::TlsContextBuilder; let mut server_builder = TlsContextBuilder::new_tls(); - server_builder.with_credential(super::server_credential()).unwrap(); + server_builder + .with_credential(super::server_credential()) + .unwrap(); let server_conn = server_builder.build().new_server_connection().build(); let mut client_builder = TlsContextBuilder::new_tls();
diff --git a/rust/bssl-tls-tokio/tests/hyper.rs b/rust/bssl-tls-tokio/tests/hyper.rs index 294026a..4c42c43 100644 --- a/rust/bssl-tls-tokio/tests/hyper.rs +++ b/rust/bssl-tls-tokio/tests/hyper.rs
@@ -17,8 +17,8 @@ use bssl_tls::alpn::H2; use bssl_tls::context::TlsContextBuilder; use bssl_tls::credentials::{Certificate, TlsCredential, TlsCredentialBuilder}; -use bssl_tls_tokio::hyper::HyperBsslConnector; use bssl_tls_tokio::TokioTlsExt; +use bssl_tls_tokio::hyper::HyperBsslConnector; use bssl_x509::{ certificates::X509Certificate, keys::PrivateKey,
diff --git a/rust/bssl-tls/src/connection/credentials.rs b/rust/bssl-tls/src/connection/credentials.rs index a3eae89..eb07f03 100644 --- a/rust/bssl-tls/src/connection/credentials.rs +++ b/rust/bssl-tls/src/connection/credentials.rs
@@ -191,7 +191,7 @@ /// Set certificate selection callback on **server** side. pub fn with_server_side_certificate_callback<T: 'static + ServerCertificateSelector<M>>( &mut self, - cb: Option<T>, + cb: T, ) -> &mut Self { self.as_in_handshake() .set_server_side_certificate_callback(cb); @@ -207,7 +207,7 @@ /// Set certificate selection callback on **client** side. pub fn with_client_side_certificate_callback<T: 'static + ClientCertificateSelector<M>>( &mut self, - cb: Option<T>, + cb: T, ) -> &mut Self { self.as_in_handshake() .set_client_side_certificate_callback(cb); @@ -309,30 +309,18 @@ /// Set certificate selection callback on **server** side. pub fn set_server_side_certificate_callback<T: 'static + ServerCertificateSelector<M>>( &mut self, - cb: Option<T>, + cb: T, ) -> &mut Self { let conn = self.ptr(); let methods = self.0.get_connection_methods(); - if let Some(cb) = cb { - methods.server_cert_cb = Some(Box::new(cb) as _); - methods.server_cert_cb_installed = true; - unsafe { - // Safety: we only install our own vtable. - bssl_sys::SSL_set_cert_cb( - conn, - Some(select_cert_cb::<super::methods::RustConnectionMethods<M>, M>), - core::ptr::null_mut(), - ); - } - } else { - methods.server_cert_cb = None; - methods.server_cert_cb_installed = false; - if !methods.client_cert_cb_installed { - unsafe { - // Safety: we only uninstall the vtable. - bssl_sys::SSL_set_cert_cb(conn, None, core::ptr::null_mut()); - } - } + methods.server_cert_cb = Some(Box::new(cb) as _); + unsafe { + // Safety: we only install our own vtable. + bssl_sys::SSL_set_cert_cb( + conn, + Some(select_cert_cb::<super::methods::RustConnectionMethods<M>, M>), + core::ptr::null_mut(), + ); } self } @@ -346,30 +334,18 @@ /// Set certificate selection callback on **client** side. pub fn set_client_side_certificate_callback<T: 'static + ClientCertificateSelector<M>>( &mut self, - cb: Option<T>, + cb: T, ) -> &mut Self { let conn = self.ptr(); let methods = self.0.get_connection_methods(); - if let Some(cb) = cb { - methods.client_cert_cb = Some(Box::new(cb) as _); - methods.client_cert_cb_installed = true; - unsafe { - // Safety: we only install our own vtable. - bssl_sys::SSL_set_cert_cb( - conn, - Some(select_cert_cb::<super::methods::RustConnectionMethods<M>, M>), - core::ptr::null_mut(), - ); - } - } else { - methods.client_cert_cb = None; - methods.client_cert_cb_installed = false; - if !methods.server_cert_cb_installed { - unsafe { - // Safety: we only uninstall the vtable. - bssl_sys::SSL_set_cert_cb(conn, None, core::ptr::null_mut()); - } - } + methods.client_cert_cb = Some(Box::new(cb) as _); + unsafe { + // Safety: we only install our own vtable. + bssl_sys::SSL_set_cert_cb( + conn, + Some(select_cert_cb::<super::methods::RustConnectionMethods<M>, M>), + core::ptr::null_mut(), + ); } self }
diff --git a/rust/bssl-tls/src/connection/methods.rs b/rust/bssl-tls/src/connection/methods.rs index 6202732..37acb5d 100644 --- a/rust/bssl-tls/src/connection/methods.rs +++ b/rust/bssl-tls/src/connection/methods.rs
@@ -75,8 +75,6 @@ pub handshake_complete: Option<Box<dyn super::lifecycle::HandshakeComplete>>, pub server_cert_cb: Option<Box<dyn ServerCertificateSelector<Mode>>>, pub client_cert_cb: Option<Box<dyn ClientCertificateSelector<Mode>>>, - pub server_cert_cb_installed: bool, - pub client_cert_cb_installed: bool, /// A mailbox to propagate IO retrying reasons. pub pending_reason: Option<TlsRetryReason>, } @@ -90,8 +88,6 @@ handshake_complete: None, server_cert_cb: None, client_cert_cb: None, - server_cert_cb_installed: false, - client_cert_cb_installed: false, pending_reason: None, } }
diff --git a/rust/bssl-tls/src/context/credentials.rs b/rust/bssl-tls/src/context/credentials.rs index 63ca379..c1deab5 100644 --- a/rust/bssl-tls/src/context/credentials.rs +++ b/rust/bssl-tls/src/context/credentials.rs
@@ -253,26 +253,18 @@ /// Set certificate selection callback on **client** side. pub fn with_client_side_certificate_callback<T: 'static + ClientCertificateSelector<M>>( &mut self, - cb: Option<T>, + cb: T, ) -> &mut Self { let ctx = self.ptr(); let methods = self.get_context_methods(); - if let Some(cb) = cb { - methods.client_cert_cb = Some(Box::new(cb) as _); - unsafe { - // Safety: we only install our own vtable. - bssl_sys::SSL_CTX_set_cert_cb( - ctx, - Some(select_cert_cb::<RustContextMethods<M>, M>), - null_mut(), - ); - } - } else { - methods.client_cert_cb = None; - unsafe { - // Safety: we only uninstall the vtable. - bssl_sys::SSL_CTX_set_cert_cb(ctx, None, null_mut()); - } + methods.client_cert_cb = Some(Box::new(cb) as _); + unsafe { + // Safety: we only install our own vtable. + bssl_sys::SSL_CTX_set_cert_cb( + ctx, + Some(select_cert_cb::<RustContextMethods<M>, M>), + null_mut(), + ); } self } @@ -280,26 +272,18 @@ /// Set certificate selection callback on **server** side. pub fn with_server_side_certificate_callback<T: 'static + ServerCertificateSelector<M>>( &mut self, - cb: Option<T>, + cb: T, ) -> &mut Self { let ctx = self.ptr(); let methods = self.get_context_methods(); - if let Some(cb) = cb { - methods.server_cert_cb = Some(Box::new(cb) as _); - unsafe { - // Safety: we only install our own vtable. - bssl_sys::SSL_CTX_set_cert_cb( - ctx, - Some(select_cert_cb::<RustContextMethods<M>, M>), - null_mut(), - ); - } - } else { - methods.server_cert_cb = None; - unsafe { - // Safety: we only uninstall the vtable. - bssl_sys::SSL_CTX_set_cert_cb(ctx, None, null_mut()); - } + methods.server_cert_cb = Some(Box::new(cb) as _); + unsafe { + // Safety: we only install our own vtable. + bssl_sys::SSL_CTX_set_cert_cb( + ctx, + Some(select_cert_cb::<RustContextMethods<M>, M>), + null_mut(), + ); } self }
diff --git a/rust/bssl-tls/src/credentials.rs b/rust/bssl-tls/src/credentials.rs index aa24387..106df17 100644 --- a/rust/bssl-tls/src/credentials.rs +++ b/rust/bssl-tls/src/credentials.rs
@@ -159,22 +159,14 @@ /// This will override the `TlsConnection` private key delegate. pub fn with_private_key_delegate<T: 'static + PrivateKeyDelegate>( &mut self, - key_method: Option<T>, + key_method: T, ) -> &mut Self { let cred = self.ptr(); - if let Some(key_method) = key_method { - unsafe { - // Safety: we only install our own vtable. - bssl_sys::SSL_CREDENTIAL_set_private_key_method(cred, methods::PRIVATE_KEY_METHODS); - } - self.get_credential_methods().private_key_methods = Some(Box::new(key_method) as _); - } else { - unsafe { - // Safety: we only uninstall the vtable. - bssl_sys::SSL_CREDENTIAL_set_private_key_method(cred, core::ptr::null()); - } - self.get_credential_methods().private_key_methods.take(); + unsafe { + // Safety: we only install our own vtable. + bssl_sys::SSL_CREDENTIAL_set_private_key_method(cred, methods::PRIVATE_KEY_METHODS); } + self.get_credential_methods().private_key_methods = Some(Box::new(key_method) as _); self }
diff --git a/rust/bssl-tls/src/credentials/tests.rs b/rust/bssl-tls/src/credentials/tests.rs index c745173..15f1586 100644 --- a/rust/bssl-tls/src/credentials/tests.rs +++ b/rust/bssl-tls/src/credentials/tests.rs
@@ -710,7 +710,7 @@ let mut server_ctx_builder = TlsContextBuilder::new_tls(); let server_cert_store = load_trust_store(Trust::SslClient); server_ctx_builder - .with_server_side_certificate_callback(Some(server_selector)) + .with_server_side_certificate_callback(server_selector) .with_certificate_store(&server_cert_store) .set_ca_names(vec![DistinguishedName::from_bytes(TEST_CA_DN, None)?]); let server_ctx = server_ctx_builder.build(); @@ -718,7 +718,7 @@ let mut client_ctx_builder = TlsContextBuilder::new_tls(); let client_cert_store = load_trust_store(Trust::SslServer); client_ctx_builder - .with_client_side_certificate_callback(Some(client_selector)) + .with_client_side_certificate_callback(client_selector) .with_certificate_store(&client_cert_store) .set_ca_names(vec![DistinguishedName::from_bytes(TEST_CA_DN, None)?]); let client_ctx = client_ctx_builder.build(); @@ -833,13 +833,13 @@ let mut server_conn_builder = server_ctx.new_server_connection(); server_conn_builder .with_certificate_verification_mode(CertificateVerificationMode::PeerCertMandatory) - .with_server_side_certificate_callback(Some(server_selector)); + .with_server_side_certificate_callback(server_selector); let server_conn = server_conn_builder.build(); let mut client_conn_builder = client_ctx.new_client_connection(); client_conn_builder .with_certificate_verification_mode(CertificateVerificationMode::PeerCertMandatory) - .with_client_side_certificate_callback(Some(client_selector)); + .with_client_side_certificate_callback(client_selector); let mut client_conn = client_conn_builder.build(); client_conn
diff --git a/rust/bssl-tls/src/tests/credentials.rs b/rust/bssl-tls/src/tests/credentials.rs index 06e8fc5..d74c210 100644 --- a/rust/bssl-tls/src/tests/credentials.rs +++ b/rust/bssl-tls/src/tests/credentials.rs
@@ -110,9 +110,9 @@ let mut builder = TlsCredentialBuilder::new(); builder .with_certificate_chain(&[server_cert, ca])? - .with_private_key_delegate(Some(crate::credentials::AsyncPrivateKeyDelegateAdapter( + .with_private_key_delegate(crate::credentials::AsyncPrivateKeyDelegateAdapter( private_key_method, - ))); + )); builder.build().unwrap() }; server_ctx_builder.with_credential(server_cred)?;