Consolidate HashSetAllow and AllowedPublicKeys types (#19755)

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:

consensus/core/src/network/tonic_tls.rs

@@ -1,19 +1,22 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::collections::BTreeSet;
-
+use crate::context::Context;
 use consensus_config::{AuthorityIndex, NetworkKeyPair};
-use fastcrypto::ed25519::Ed25519PublicKey;
+use sui_tls::AllowPublicKeys;
 use tokio_rustls::rustls::{ClientConfig, ServerConfig};
 
-use crate::context::Context;
-
 pub(crate) fn create_rustls_server_config(
     context: &Context,
     network_keypair: NetworkKeyPair,
 ) -> ServerConfig {
-    let allower = AllowedPublicKeys::new(context);
+    let allower = AllowPublicKeys::new(
+        context
+            .committee
+            .authorities()
+            .map(|(_i, a)| a.network_key.clone().into_inner())
+            .collect(),
+    );
     let verifier = sui_tls::ClientCertVerifier::new(allower, certificate_server_name(context));
     // TODO: refactor to use key bytes
     let self_signed_cert = sui_tls::SelfSignedCertificate::new(
@@ -56,30 +59,6 @@ pub(crate) fn create_rustls_client_config(
     tls_config
 }
 
-// Checks if the public key from a TLS certificate belongs to one of the validators.
-#[derive(Debug)]
-struct AllowedPublicKeys {
-    // TODO: refactor to use key bytes
-    keys: BTreeSet<Ed25519PublicKey>,
-}
-
-impl AllowedPublicKeys {
-    fn new(context: &Context) -> Self {
-        let keys = context
-            .committee
-            .authorities()
-            .map(|(_i, a)| a.network_key.clone().into_inner())
-            .collect();
-        Self { keys }
-    }
-}
-
-impl sui_tls::Allower for AllowedPublicKeys {
-    fn allowed(&self, key: &Ed25519PublicKey) -> bool {
-        self.keys.contains(key)
-    }
-}
-
 fn certificate_server_name(context: &Context) -> String {
     format!("consensus_epoch_{}", context.committee.epoch())
 }

crates/sui-tls/Cargo.toml

@@ -9,6 +9,7 @@ publish = false
 
 [dependencies]
 anyhow.workspace = true
+arc-swap.workspace = true
 ed25519.workspace = true
 pkcs8.workspace = true
 rcgen.workspace = true

crates/sui-tls/src/lib.rs

@@ -10,14 +10,16 @@ pub const SUI_VALIDATOR_SERVER_NAME: &str = "sui";
 pub use acceptor::{TlsAcceptor, TlsConnectionInfo};
 pub use certgen::SelfSignedCertificate;
 pub use verifier::{
-    public_key_from_certificate, AllowAll, Allower, ClientCertVerifier, HashSetAllow,
-    ServerCertVerifier, ValidatorAllowlist,
+    public_key_from_certificate, AllowAll, AllowPublicKeys, Allower, ClientCertVerifier,
+    ServerCertVerifier,
 };
 
 pub use rustls;
 
 #[cfg(test)]
 mod tests {
+    use std::collections::BTreeSet;
+
     use super::*;
     use fastcrypto::ed25519::Ed25519KeyPair;
     use fastcrypto::traits::KeyPair;
@@ -100,23 +102,16 @@ mod tests {
         let allowed = Ed25519KeyPair::generate(&mut rng);
         let disallowed = Ed25519KeyPair::generate(&mut rng);
 
-        let allowed_public_key = allowed.public().to_owned();
+        let allowed_public_keys = BTreeSet::from([allowed.public().to_owned()]);
         let allowed_cert = SelfSignedCertificate::new(allowed.private(), SUI_VALIDATOR_SERVER_NAME);
 
         let disallowed_cert =
             SelfSignedCertificate::new(disallowed.private(), SUI_VALIDATOR_SERVER_NAME);
 
-        let mut allowlist = HashSetAllow::new();
+        let allowlist = AllowPublicKeys::new(allowed_public_keys);
         let verifier =
             ClientCertVerifier::new(allowlist.clone(), SUI_VALIDATOR_SERVER_NAME.to_string());
 
-        // Add our public key to the allower
-        allowlist
-            .inner_mut()
-            .write()
-            .unwrap()
-            .insert(allowed_public_key);
-
         // The allowed cert passes validation
         verifier
             .verify_client_cert(&allowed_cert.rustls_certificate(), &[], UnixTime::now())
@@ -132,7 +127,7 @@ mod tests {
         );
 
         // After removing the allowed public key from the set it now fails validation
-        allowlist.inner_mut().write().unwrap().clear();
+        allowlist.update(BTreeSet::new());
         let err = verifier
             .verify_client_cert(&allowed_cert.rustls_certificate(), &[], UnixTime::now())
             .unwrap_err();
@@ -149,17 +144,10 @@ mod tests {
         let public_key = keypair.public().to_owned();
         let cert = SelfSignedCertificate::new(keypair.private(), "not-sui");
 
-        let mut allowlist = HashSetAllow::new();
+        let allowlist = AllowPublicKeys::new(BTreeSet::from([public_key.clone()]));
         let client_verifier =
             ClientCertVerifier::new(allowlist.clone(), SUI_VALIDATOR_SERVER_NAME.to_string());
 
-        // Add our public key to the allower
-        allowlist
-            .inner_mut()
-            .write()
-            .unwrap()
-            .insert(public_key.clone());
-
         // Allowed public key but the server-name in the cert is not the required "sui"
         let err = client_verifier
             .verify_client_cert(&cert.rustls_certificate(), &[], UnixTime::now())
@@ -210,7 +198,7 @@ mod tests {
             .build()
             .unwrap();
 
-        let mut allowlist = HashSetAllow::new();
+        let allowlist = AllowPublicKeys::new(BTreeSet::new());
         let tls_config =
             ClientCertVerifier::new(allowlist.clone(), SUI_VALIDATOR_SERVER_NAME.to_string())
                 .rustls_server_config(
@@ -240,11 +228,7 @@ mod tests {
         client.get(&server_url).send().await.unwrap_err();
 
         // Insert the client's public key into the allowlist and verify the request is successful
-        allowlist
-            .inner_mut()
-            .write()
-            .unwrap()
-            .insert(client_public_key.clone());
+        allowlist.update(BTreeSet::from([client_public_key.clone()]));
 
         let res = client.get(&server_url).send().await.unwrap();
         let body = res.text().await.unwrap();

crates/sui-tls/src/verifier.rs

@@ -1,6 +1,7 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+use arc_swap::ArcSwap;
 use fastcrypto::ed25519::Ed25519PublicKey;
 use fastcrypto::traits::ToFromBytes;
 use rustls::crypto::WebPkiSupportedAlgorithms;
@@ -10,10 +11,8 @@ use rustls::pki_types::ServerName;
 use rustls::pki_types::SignatureVerificationAlgorithm;
 use rustls::pki_types::TrustAnchor;
 use rustls::pki_types::UnixTime;
-use std::{
-    collections::HashSet,
-    sync::{Arc, RwLock},
-};
+use std::collections::BTreeSet;
+use std::sync::Arc;
 
 static SUPPORTED_SIG_ALGS: &[&dyn SignatureVerificationAlgorithm] = &[webpki::ring::ED25519];
 
@@ -22,13 +21,12 @@ static SUPPORTED_ALGORITHMS: WebPkiSupportedAlgorithms = WebPkiSupportedAlgorith
     mapping: &[(rustls::SignatureScheme::ED25519, SUPPORTED_SIG_ALGS)],
 };
 
-pub type ValidatorAllowlist = Arc<RwLock<HashSet<Ed25519PublicKey>>>;
-
 /// The Allower trait provides an interface for callers to inject decsions whether
 /// to allow a cert to be verified or not.  This does not prform actual cert validation
 /// it only acts as a gatekeeper to decide if we should even try.  For example, we may want
 /// to filter our actions to well known public keys.
 pub trait Allower: std::fmt::Debug + Send + Sync {
+    // TODO: change allower interface to use raw key bytes.
     fn allowed(&self, key: &Ed25519PublicKey) -> bool;
 }
 
@@ -42,32 +40,28 @@ impl Allower for AllowAll {
     }
 }
 
-/// HashSetAllow restricts keys to those that are found in the member set. non-members will not be
-/// allowed.
+/// AllowPublicKeys restricts keys to those that are found in the member set. non-members will
+/// not be allowed.
 #[derive(Debug, Clone, Default)]
-pub struct HashSetAllow {
-    inner: ValidatorAllowlist,
+pub struct AllowPublicKeys {
+    inner: Arc<ArcSwap<BTreeSet<Ed25519PublicKey>>>,
 }
 
-impl HashSetAllow {
-    pub fn new() -> Self {
-        let inner = Arc::new(RwLock::new(HashSet::new()));
-        Self { inner }
-    }
-    /// Get a reference to the inner service
-    pub fn inner(&self) -> &ValidatorAllowlist {
-        &self.inner
+impl AllowPublicKeys {
+    pub fn new(allowed: BTreeSet<Ed25519PublicKey>) -> Self {
+        Self {
+            inner: Arc::new(ArcSwap::from_pointee(allowed)),
+        }
     }
 
-    /// Get a mutable reference to the inner service
-    pub fn inner_mut(&mut self) -> &mut ValidatorAllowlist {
-        &mut self.inner
+    pub fn update(&self, new_allowed: BTreeSet<Ed25519PublicKey>) {
+        self.inner.store(Arc::new(new_allowed));
     }
 }
 
-impl Allower for HashSetAllow {
+impl Allower for AllowPublicKeys {
     fn allowed(&self, key: &Ed25519PublicKey) -> bool {
-        self.inner.read().unwrap().contains(key)
+        self.inner.load().contains(key)
     }
 }