diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index ea54a6e25..45278a30e 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -1203,16 +1203,18 @@ impl SslContextBuilder { } } - /// Use [`set_cert_store_builder`] or [`set_cert_store_ref`] instead. + /// Replaces the context's certificate store, and keeps it immutable. + /// + /// This method allows sharing the `X509Store`, but calls to `cert_store_mut` will panic. /// - /// Replaces the context's certificate store. + /// Use [`set_cert_store_builder`] to set a mutable cert store + /// (there's no way to have both sharing and mutability). #[corresponds(SSL_CTX_set_cert_store)] - #[deprecated(note = "Use set_cert_store_builder or set_cert_store_ref instead")] pub fn set_cert_store(&mut self, cert_store: X509Store) { #[cfg(feature = "rpk")] assert!(!self.is_rpk, "This API is not supported for RPK"); - self.has_shared_cert_store = false; + self.has_shared_cert_store = true; unsafe { ffi::SSL_CTX_set_cert_store(self.as_ptr(), cert_store.into_ptr()); } @@ -1235,14 +1237,7 @@ impl SslContextBuilder { /// This method allows sharing the `X509Store`, but calls to `cert_store_mut` will panic. #[corresponds(SSL_CTX_set_cert_store)] pub fn set_cert_store_ref(&mut self, cert_store: &X509Store) { - #[cfg(feature = "rpk")] - assert!(!self.is_rpk, "This API is not supported for RPK"); - - self.has_shared_cert_store = true; - unsafe { - ffi::X509_STORE_up_ref(cert_store.as_ptr()); - ffi::SSL_CTX_set_cert_store(self.as_ptr(), cert_store.as_ptr()); - } + self.set_cert_store(cert_store.to_owned()); } /// Controls read ahead behavior. @@ -1771,7 +1766,8 @@ impl SslContextBuilder { assert!( !self.has_shared_cert_store, - "Shared X509Store can't be mutated. Make a new store" + "Shared X509Store can't be mutated. Use set_cert_store_builder() instead of set_cert_store() + or completely finish building the cert store setting it." ); // OTOH, it's not safe to return a shared &X509Store when the builder owns it exclusively diff --git a/boring/src/x509/store.rs b/boring/src/x509/store.rs index 1f6d839b2..e7621e739 100644 --- a/boring/src/x509/store.rs +++ b/boring/src/x509/store.rs @@ -134,6 +134,23 @@ foreign_type_and_impl_send_sync! { pub struct X509Store; } +impl ToOwned for X509StoreRef { + type Owned = X509Store; + + fn to_owned(&self) -> X509Store { + unsafe { + ffi::X509_STORE_up_ref(self.as_ptr()); + X509Store::from_ptr(self.as_ptr()) + } + } +} + +impl Clone for X509Store { + fn clone(&self) -> X509Store { + (**self).to_owned() + } +} + impl X509StoreRef { /// **Warning: this method is unsound** /// @@ -160,12 +177,15 @@ impl X509StoreRef { } #[test] -#[allow(dead_code)] -// X509Store must not implement Clone because `SslContextBuilder::cert_store_mut` lets -// you get a mutable reference to a store that could have been cloned before being -// passed to `SslContextBuilder::set_cert_store`. -fn no_clone_for_x509store() { - trait MustNotImplementClone {} - impl MustNotImplementClone for T {} - impl MustNotImplementClone for X509Store {} +#[should_panic = "Shared X509Store can't be mutated"] +fn set_cert_store_pevents_mutability() { + use crate::ssl::*; + + let mut ctx = SslContext::builder(SslMethod::tls()).unwrap(); + let store = X509StoreBuilder::new().unwrap().build(); + + ctx.set_cert_store(store.clone()); + + // This is bad. + let _aliased_store = ctx.cert_store_mut(); }