Introduce a builder pattern for SslEchKeys + make set_ech_keys take a reference (#320)

Previously, set_ech_keys would consume the SslEchKeys struct to enforce
the requirement that the struct is immutable after initializing it on a
SSL_CTX. The problem with this is that it requires applications to
needlessly reallocate the SslEchKeys struct if they want to initialize
keys on multiple SSL_CTXs, which is a pretty common pattern. To work
around this, we introduce a builder (SslEchKeysBuilder) that requires
mutable access to add keys to the underlying struct. set_ech_keys takes
in a reference to SslEchKeys, which can only be made via consuming the
builder.

boring/src/ssl/ech.rs

@@ -1,11 +1,54 @@
 use crate::ffi;
-use foreign_types::{ForeignType, ForeignTypeRef};
+use foreign_types::ForeignType;
 use libc::c_int;
 
 use crate::error::ErrorStack;
 use crate::hpke::HpkeKey;
 use crate::{cvt_0i, cvt_p};
 
+pub struct SslEchKeysBuilder {
+    keys: SslEchKeys,
+}
+
+impl SslEchKeysBuilder {
+    pub fn new() -> Result<SslEchKeysBuilder, ErrorStack> {
+        unsafe {
+            ffi::init();
+            let keys = cvt_p(ffi::SSL_ECH_KEYS_new())?;
+
+            Ok(SslEchKeysBuilder::from_ptr(keys))
+        }
+    }
+
+    pub unsafe fn from_ptr(keys: *mut ffi::SSL_ECH_KEYS) -> Self {
+        Self {
+            keys: SslEchKeys::from_ptr(keys),
+        }
+    }
+
+    pub fn add_key(
+        &mut self,
+        is_retry_config: bool,
+        ech_config: &[u8],
+        key: HpkeKey,
+    ) -> Result<(), ErrorStack> {
+        unsafe {
+            cvt_0i(ffi::SSL_ECH_KEYS_add(
+                self.keys.as_ptr(),
+                is_retry_config as c_int,
+                ech_config.as_ptr(),
+                ech_config.len(),
+                key.as_ptr(),
+            ))
+            .map(|_| ())
+        }
+    }
+
+    pub fn build(self) -> SslEchKeys {
+        self.keys
+    }
+}
+
 foreign_type_and_impl_send_sync! {
     type CType = ffi::SSL_ECH_KEYS;
     fn drop = ffi::SSL_ECH_KEYS_free;
@@ -14,30 +57,7 @@ foreign_type_and_impl_send_sync! {
 }
 
 impl SslEchKeys {
-    pub fn new() -> Result<SslEchKeys, ErrorStack> {
+    pub fn builder() -> Result<SslEchKeysBuilder, ErrorStack> {
-        unsafe {
+        SslEchKeysBuilder::new()
-            ffi::init();
-            cvt_p(ffi::SSL_ECH_KEYS_new()).map(|p| SslEchKeys::from_ptr(p))
-        }
-    }
-}
-
-impl SslEchKeysRef {
-    pub fn add_key(
-        &mut self,
-        is_retry_config: bool,
-        ech_config: &[u8],
-        key: HpkeKey,
-    ) -> Result<(), ErrorStack> {
-        unsafe {
-            cvt_0i(ffi::SSL_ECH_KEYS_add(
-                self.as_ptr(),
-                is_retry_config as c_int,
-                ech_config.as_ptr(),
-                ech_config.len(),
-                key.as_ptr(),
-            ))
-            .map(|_| ())
-        }
     }
 }

boring/src/ssl/mod.rs

@@ -1960,7 +1960,7 @@ impl SslContextBuilder {
     /// threads.
     #[cfg(not(feature = "fips"))]
     #[corresponds(SSL_CTX_set1_ech_keys)]
-    pub fn set_ech_keys(&self, keys: SslEchKeys) -> Result<(), ErrorStack> {
+    pub fn set_ech_keys(&self, keys: &SslEchKeys) -> Result<(), ErrorStack> {
         unsafe { cvt(ffi::SSL_CTX_set1_ech_keys(self.as_ptr(), keys.as_ptr())).map(|_| ()) }
     }
 
@@ -2217,7 +2217,7 @@ impl SslContextRef {
     /// threads.
     #[cfg(not(feature = "fips"))]
     #[corresponds(SSL_CTX_set1_ech_keys)]
-    pub fn set_ech_keys(&self, keys: SslEchKeys) -> Result<(), ErrorStack> {
+    pub fn set_ech_keys(&self, keys: &SslEchKeys) -> Result<(), ErrorStack> {
         unsafe { cvt(ffi::SSL_CTX_set1_ech_keys(self.as_ptr(), keys.as_ptr())).map(|_| ()) }
     }
 }

boring/src/ssl/test/ech.rs

@@ -18,11 +18,12 @@ static ECH_KEY_2: &[u8] = include_bytes!("../../../test/echkey-2");
 fn bootstrap_ech(config: &[u8], key: &[u8], list: &[u8]) -> (Server, ClientSslBuilder) {
     let server = {
         let key = HpkeKey::dhkem_p256_sha256(key).unwrap();
-        let mut ech_keys = SslEchKeys::new().unwrap();
+        let mut ech_keys_builder = SslEchKeys::builder().unwrap();
-        ech_keys.add_key(true, config, key).unwrap();
+        ech_keys_builder.add_key(true, config, key).unwrap();
+        let ech_keys = ech_keys_builder.build();
 
         let mut builder = Server::builder();
-        builder.ctx().set_ech_keys(ech_keys).unwrap();
+        builder.ctx().set_ech_keys(&ech_keys).unwrap();
 
         builder.build()
     };