[tls] add ServerCertVerifier (MystenLabs#17625)

## Description Reuse the self signed client cert verification logic for verifying self signed server cert. Allow flexible server name. Allow getting public key from cert to be used outside of sui-tls crate. ## Test plan CI --- ## 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:
clayford08 · May 10, 2024 · 6d50133 · 6d50133
1 parent 98bee1c
commit 6d50133
Show file tree

Hide file tree

Showing 4 changed files with 228 additions and 61 deletions.
diff --git a/crates/sui-proxy/src/admin.rs b/crates/sui-proxy/src/admin.rs
@@ -19,7 +19,10 @@ use std::io::BufReader;
 use std::net::{IpAddr, SocketAddr};
 use std::sync::Arc;
 use std::time::Duration;
-use sui_tls::{rustls::ServerConfig, AllowAll, CertVerifier, SelfSignedCertificate, TlsAcceptor};
+use sui_tls::SUI_VALIDATOR_SERVER_NAME;
+use sui_tls::{
+    rustls::ServerConfig, AllowAll, ClientCertVerifier, SelfSignedCertificate, TlsAcceptor,
+};
 use tokio::signal;
 use tower::ServiceBuilder;
 use tower_http::{
@@ -232,7 +235,7 @@ pub fn create_server_cert_default_allow(
 ) -> Result<ServerConfig, sui_tls::rustls::Error> {
     let CertKeyPair(server_certificate, _) = generate_self_cert(hostname);
 
-    CertVerifier::new(AllowAll).rustls_server_config(
+    ClientCertVerifier::new(AllowAll, SUI_VALIDATOR_SERVER_NAME.to_string()).rustls_server_config(
         vec![server_certificate.rustls_certificate()],
         server_certificate.rustls_private_key(),
     )
@@ -256,9 +259,10 @@ pub fn create_server_cert_enforce_peer(
     })?;
     let allower = SuiNodeProvider::new(dynamic_peers.url, dynamic_peers.interval, static_peers);
     allower.poll_peer_list();
-    let c = CertVerifier::new(allower.clone()).rustls_server_config(
-        load_certs(&certificate_path),
-        load_private_key(&private_key_path),
-    )?;
+    let c = ClientCertVerifier::new(allower.clone(), SUI_VALIDATOR_SERVER_NAME.to_string())
+        .rustls_server_config(
+            load_certs(&certificate_path),
+            load_private_key(&private_key_path),
+        )?;
     Ok((c, Some(allower)))
 }
diff --git a/crates/sui-proxy/src/lib.rs b/crates/sui-proxy/src/lib.rs
@@ -49,7 +49,7 @@ mod tests {
     use protobuf::RepeatedField;
     use std::net::TcpListener;
     use std::time::Duration;
-    use sui_tls::{CertVerifier, TlsAcceptor};
+    use sui_tls::{ClientCertVerifier, TlsAcceptor};
 
     async fn run_dummy_remote_write(listener: TcpListener) {
         /// i accept everything, send me the trash
@@ -91,12 +91,15 @@ mod tests {
 
         // init the tls config and allower
         let mut allower = SuiNodeProvider::new("".into(), Duration::from_secs(30), vec![]);
-        let tls_config = CertVerifier::new(allower.clone())
-            .rustls_server_config(
-                vec![server_priv_cert.rustls_certificate()],
-                server_priv_cert.rustls_private_key(),
-            )
-            .unwrap();
+        let tls_config = ClientCertVerifier::new(
+            allower.clone(),
+            sui_tls::SUI_VALIDATOR_SERVER_NAME.to_string(),
+        )
+        .rustls_server_config(
+            vec![server_priv_cert.rustls_certificate()],
+            server_priv_cert.rustls_private_key(),
+        )
+        .unwrap();
 
         let client = admin::make_reqwest_client(
             RemoteWriteConfig {

diff --git a/crates/sui-tls/src/lib.rs b/crates/sui-tls/src/lib.rs
@@ -9,7 +9,10 @@ pub const SUI_VALIDATOR_SERVER_NAME: &str = "sui";
 
 pub use acceptor::{TlsAcceptor, TlsConnectionInfo};
 pub use certgen::SelfSignedCertificate;
-pub use verifier::{AllowAll, Allower, CertVerifier, HashSetAllow, ValidatorAllowlist};
+pub use verifier::{
+    public_key_from_certificate, AllowAll, Allower, ClientCertVerifier, HashSetAllow,
+    ServerCertVerifier, ValidatorAllowlist,
+};
 
 pub use rustls;
 
@@ -18,7 +21,8 @@ mod tests {
     use super::*;
     use fastcrypto::ed25519::Ed25519KeyPair;
     use fastcrypto::traits::KeyPair;
-    use rustls::server::ClientCertVerifier;
+    use rustls::client::ServerCertVerifier as _;
+    use rustls::server::ClientCertVerifier as _;
 
     #[test]
     fn verify_allowall() {
@@ -30,7 +34,7 @@ mod tests {
         let random_cert_alice =
             SelfSignedCertificate::new(disallowed.private(), SUI_VALIDATOR_SERVER_NAME);
 
-        let verifier = CertVerifier::new(AllowAll);
+        let verifier = ClientCertVerifier::new(AllowAll, SUI_VALIDATOR_SERVER_NAME.to_string());
 
         // The bob passes validation
         verifier
@@ -51,6 +55,49 @@ mod tests {
             .unwrap();
     }
 
+    #[test]
+    fn verify_server_cert() {
+        let mut rng = rand::thread_rng();
+        let allowed = Ed25519KeyPair::generate(&mut rng);
+        let disallowed = Ed25519KeyPair::generate(&mut rng);
+        let allowed_public_key = allowed.public().to_owned();
+        let random_cert_bob =
+            SelfSignedCertificate::new(allowed.private(), SUI_VALIDATOR_SERVER_NAME);
+        let random_cert_alice =
+            SelfSignedCertificate::new(disallowed.private(), SUI_VALIDATOR_SERVER_NAME);
+
+        let verifier =
+            ServerCertVerifier::new(allowed_public_key, SUI_VALIDATOR_SERVER_NAME.to_string());
+
+        // The bob passes validation
+        verifier
+            .verify_server_cert(
+                &random_cert_bob.rustls_certificate(),
+                &[],
+                &rustls::ServerName::try_from("example.com").unwrap(),
+                &mut Vec::<&[u8]>::new().iter().cloned(),
+                &[],
+                std::time::SystemTime::now(),
+            )
+            .unwrap();
+
+        // The alice does not pass validation
+        let err = verifier
+            .verify_server_cert(
+                &random_cert_alice.rustls_certificate(),
+                &[],
+                &rustls::ServerName::try_from("example.com").unwrap(),
+                &mut Vec::<&[u8]>::new().iter().cloned(),
+                &[],
+                std::time::SystemTime::now(),
+            )
+            .unwrap_err();
+        assert!(
+            matches!(err, rustls::Error::General(_)),
+            "Actual error: {err:?}"
+        );
+    }
+
     #[test]
     fn verify_hashset() {
         let mut rng = rand::thread_rng();
@@ -64,7 +111,8 @@ mod tests {
             SelfSignedCertificate::new(disallowed.private(), SUI_VALIDATOR_SERVER_NAME);
 
         let mut allowlist = HashSetAllow::new();
-        let verifier = CertVerifier::new(allowlist.clone());
+        let verifier =
+            ClientCertVerifier::new(allowlist.clone(), SUI_VALIDATOR_SERVER_NAME.to_string());
 
         // Add our public key to the allower
         allowlist
@@ -83,23 +131,31 @@ mod tests {
             .unwrap();
 
         // The disallowed cert fails validation
-        verifier
+        let err = verifier
             .verify_client_cert(
                 &disallowed_cert.rustls_certificate(),
                 &[],
                 std::time::SystemTime::now(),
             )
             .unwrap_err();
+        assert!(
+            matches!(err, rustls::Error::General(_)),
+            "Actual error: {err:?}"
+        );
 
         // After removing the allowed public key from the set it now fails validation
         allowlist.inner_mut().write().unwrap().clear();
-        verifier
+        let err = verifier
             .verify_client_cert(
                 &allowed_cert.rustls_certificate(),
                 &[],
                 std::time::SystemTime::now(),
             )
             .unwrap_err();
+        assert!(
+            matches!(err, rustls::Error::General(_)),
+            "Actual error: {err:?}"
+        );
     }
 
     #[test]
@@ -110,19 +166,49 @@ mod tests {
         let cert = SelfSignedCertificate::new(keypair.private(), "not-sui");
 
         let mut allowlist = HashSetAllow::new();
-        let verifier = CertVerifier::new(allowlist.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);
+        allowlist
+            .inner_mut()
+            .write()
+            .unwrap()
+            .insert(public_key.clone());
 
         // Allowed public key but the server-name in the cert is not the required "sui"
-        verifier
+        let err = client_verifier
             .verify_client_cert(
                 &cert.rustls_certificate(),
                 &[],
                 std::time::SystemTime::now(),
             )
             .unwrap_err();
+        assert_eq!(
+            err,
+            rustls::Error::InvalidCertificate(rustls::CertificateError::NotValidForName),
+            "Actual error: {err:?}"
+        );
+
+        let server_verifier =
+            ServerCertVerifier::new(public_key, SUI_VALIDATOR_SERVER_NAME.to_string());
+
+        // Allowed public key but the server-name in the cert is not the required "sui"
+        let err = server_verifier
+            .verify_server_cert(
+                &cert.rustls_certificate(),
+                &[],
+                &rustls::ServerName::try_from("example.com").unwrap(),
+                &mut Vec::<&[u8]>::new().iter().cloned(),
+                &[],
+                std::time::SystemTime::now(),
+            )
+            .unwrap_err();
+        assert_eq!(
+            err,
+            rustls::Error::InvalidCertificate(rustls::CertificateError::NotValidForName),
+            "Actual error: {err:?}"
+        );
     }
 
     #[tokio::test]
@@ -146,12 +232,13 @@ mod tests {
             .unwrap();
 
         let mut allowlist = HashSetAllow::new();
-        let tls_config = CertVerifier::new(allowlist.clone())
-            .rustls_server_config(
-                vec![server_certificate.rustls_certificate()],
-                server_certificate.rustls_private_key(),
-            )
-            .unwrap();
+        let tls_config =
+            ClientCertVerifier::new(allowlist.clone(), SUI_VALIDATOR_SERVER_NAME.to_string())
+                .rustls_server_config(
+                    vec![server_certificate.rustls_certificate()],
+                    server_certificate.rustls_private_key(),
+                )
+                .unwrap();
 
         async fn handler(tls_info: axum::Extension<TlsConnectionInfo>) -> String {
             tls_info.public_key().unwrap().to_string()