rust: bssl-tls: Split Methods trait into connection and context variants Signed-off-by: Xiangfei Ding <xfding@google.com> Change-Id: If2fdf0339f1fa6375aad3b7e17158b9f6a6a6964 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/99607 Reviewed-by: Rudolf Polzer <rpolzer@google.com> Reviewed-by: Adam Langley <agl@google.com> Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/rust/bssl-tls/src/connection/methods.rs b/rust/bssl-tls/src/connection/methods.rs index dc227ab..6e71049 100644 --- a/rust/bssl-tls/src/connection/methods.rs +++ b/rust/bssl-tls/src/connection/methods.rs
@@ -31,6 +31,7 @@ use crate::{ Methods, + MethodsRef, PrivateKeyMethods, VerifyCertificateMethods, abort_on_panic, @@ -97,7 +98,7 @@ } } -impl<Mode: HasTlsConnectionMethod> Methods for RustConnectionMethods<Mode> { +impl<Mode: HasTlsConnectionMethod> MethodsRef for RustConnectionMethods<Mode> { unsafe extern "C" fn from_ssl<'a>(ssl: *mut bssl_sys::SSL) -> Option<&'a Self> { unsafe { // Safety: `ssl` is originated from `TlsConnection::from_ssl`. @@ -106,7 +107,23 @@ !methods.is_null(), "connection method should have been attached at construction time" ); - // Safety: `ctx` is originated from `Box::into_raw` + // Safety: `methods` is originated from `Box::into_raw` + Some(&*(methods as *const RustConnectionMethods<Mode>)) + } + } +} + +impl<Mode: HasTlsConnectionMethod> Methods for RustConnectionMethods<Mode> { + unsafe extern "C" fn from_ssl<'a>(ssl: *mut bssl_sys::SSL) -> Option<&'a mut Self> { + unsafe { + // Safety: `ssl` is originated from `TlsConnection::from_ssl`. + let methods = bssl_sys::SSL_get_ex_data(ssl, Mode::registration()); + debug_assert!( + !methods.is_null(), + "connection method should have been attached at construction time" + ); + // Safety: `methods` is originated from `Box::into_raw`. + // The caller must ensure exclusive access to the connection. Some(&mut *(methods as *mut RustConnectionMethods<Mode>)) } }
diff --git a/rust/bssl-tls/src/context/credentials.rs b/rust/bssl-tls/src/context/credentials.rs index 69d9f27..48ebd44 100644 --- a/rust/bssl-tls/src/context/credentials.rs +++ b/rust/bssl-tls/src/context/credentials.rs
@@ -39,7 +39,7 @@ cert_cb, early_callback::{ EarlyCallback, - early_select_cert_cb, // + early_cb, // }, // }, errors::Error, @@ -176,7 +176,7 @@ // Safety: we only install our own vtable. bssl_sys::SSL_CTX_set_select_certificate_cb( ctx, - Some(early_select_cert_cb::<M, super::methods::RustContextMethods<M>>), + Some(early_cb::<M, super::methods::RustContextMethods<M>>), ); } self.get_context_methods().early_callback_handler = Some(Box::new(handler) as _);
diff --git a/rust/bssl-tls/src/context/methods.rs b/rust/bssl-tls/src/context/methods.rs index 8bce0a0..5b94700 100644 --- a/rust/bssl-tls/src/context/methods.rs +++ b/rust/bssl-tls/src/context/methods.rs
@@ -22,7 +22,7 @@ use crate::{ EarlyCallbackMethods, - Methods, + MethodsRef, PrivateKeyMethods, VerifyCertificateMethods, context::{ @@ -65,7 +65,7 @@ } } -impl<M: HasTlsContextMethod> Methods for RustContextMethods<M> { +impl<M: HasTlsContextMethod> MethodsRef for RustContextMethods<M> { unsafe extern "C" fn from_ssl<'a>(ssl: *mut bssl_sys::SSL) -> Option<&'a Self> { unsafe { // Safety: `ssl` must be still valid by BoringSSL invariant. @@ -75,8 +75,8 @@ } // Safety: `ctx` is originated from `TlsContext::new_inner`. let methods = bssl_sys::SSL_CTX_get_ex_data(ctx, M::registration()); - // Safety: `ctx` is originated from `Box::into_raw` - Some(&mut *(methods as *mut RustContextMethods<_>)) + // Safety: `methods` is originated from `Box::into_raw` + Some(&*(methods as *const RustContextMethods<_>)) } } }
diff --git a/rust/bssl-tls/src/credentials/early_callback.rs b/rust/bssl-tls/src/credentials/early_callback.rs index 94352fd..fd498c9 100644 --- a/rust/bssl-tls/src/credentials/early_callback.rs +++ b/rust/bssl-tls/src/credentials/early_callback.rs
@@ -274,7 +274,8 @@ fn process(&self, client_hello: &mut ClientHello<'_, M>) -> EarlyCallbackResult; } -pub(crate) unsafe extern "C" fn early_select_cert_cb<Mode, MethodsT>( +// For this callback, MethodsT could only be RustContextMethods<Mode>. +pub(crate) unsafe extern "C" fn early_cb<Mode, MethodsT>( client_hello: *const bssl_sys::SSL_CLIENT_HELLO, ) -> bssl_sys::ssl_select_cert_result_t where
diff --git a/rust/bssl-tls/src/credentials/methods.rs b/rust/bssl-tls/src/credentials/methods.rs index 11bc865..fa0900e 100644 --- a/rust/bssl-tls/src/credentials/methods.rs +++ b/rust/bssl-tls/src/credentials/methods.rs
@@ -25,7 +25,7 @@ use once_cell::sync::Lazy; use crate::{ - Methods, + MethodsRef, PrivateKeyMethods, abort_on_panic, connection::methods::private_key_op_from_ssl, @@ -64,7 +64,7 @@ pub(crate) private_key_methods: Option<Box<dyn PrivateKeyDelegate>>, } -impl Methods for RustCredentialMethods { +impl MethodsRef for RustCredentialMethods { unsafe extern "C" fn from_ssl<'a>(ssl: *mut bssl_sys::SSL) -> Option<&'a Self> { unsafe { // Safety: `ssl` is valid per BoringSSL invariant. @@ -78,7 +78,7 @@ return None; } // Safety: `cred` is originated from `Box::into_raw`. - Some(&*(methods as *mut RustCredentialMethods)) + Some(&*(methods as *const RustCredentialMethods)) } } }
diff --git a/rust/bssl-tls/src/lib.rs b/rust/bssl-tls/src/lib.rs index 8eb233b..32f8135 100644 --- a/rust/bssl-tls/src/lib.rs +++ b/rust/bssl-tls/src/lib.rs
@@ -64,21 +64,38 @@ list.iter().any(|elem| !seen.insert(elem)) } +/// Method table accessor for connection-level state. +/// +/// Returns `&'a mut Self` because connection state is exclusively owned. +#[allow(unused)] // This trait will be used by certificate selection callback. pub(crate) trait Methods { - /// Safety: `ssl` must outlive `'a` and it must be passed in from BoringSSL - /// through vtable calls. + /// Safety: + /// - `ssl` must outlive `'a` and it must be passed in from BoringSSL + /// through vtable calls. + /// - the returned method table **must not** be aliased. + unsafe extern "C" fn from_ssl<'a>(ssl: *mut bssl_sys::SSL) -> Option<&'a mut Self>; +} + +/// Shared method table accessor. +/// +/// Returns `&'a Self` for shared (non-exclusive) access to the method table. +/// Used by context, connection, and credential method tables for read-only lookups. +pub(crate) trait MethodsRef { + /// Safety: + /// - `ssl` must outlive `'a` and it must be passed in from BoringSSL + /// through vtable calls. unsafe extern "C" fn from_ssl<'a>(ssl: *mut bssl_sys::SSL) -> Option<&'a Self>; } -pub(crate) trait PrivateKeyMethods: Methods { +pub(crate) trait PrivateKeyMethods: MethodsRef { fn private_key_methods(&self) -> Option<&dyn credentials::PrivateKeyDelegate>; } -pub(crate) trait VerifyCertificateMethods: Methods { +pub(crate) trait VerifyCertificateMethods: MethodsRef { fn verify_certificate_methods(&self) -> Option<&dyn credentials::VerifyCertificate>; } -pub(crate) trait EarlyCallbackMethods<M>: Methods { +pub(crate) trait EarlyCallbackMethods<M>: MethodsRef { fn early_callback_handler(&self) -> Option<&dyn credentials::early_callback::EarlyCallback<M>>; }
diff --git a/rust/bssl-tls/src/methods.rs b/rust/bssl-tls/src/methods.rs index 228ac08..0efd790 100644 --- a/rust/bssl-tls/src/methods.rs +++ b/rust/bssl-tls/src/methods.rs
@@ -19,12 +19,9 @@ c_void, // }; -use crate::{ - Methods, - abort_on_panic, // -}; +use crate::abort_on_panic; -pub(crate) unsafe extern "C" fn drop_box_rust_methods<M: Methods>( +pub(crate) unsafe extern "C" fn drop_box_rust_methods<M>( _parent: *mut c_void, ptr: *mut c_void, _ad: *mut bssl_sys::CRYPTO_EX_DATA,