fix(connector): make certificate pinning real; disable Nagle

Security: with --verify-hostname off (the default), the old NoVerify
verifier accepted ANY server certificate — the CA loaded from --tls-ca
was never consulted, so the documented pinning was a no-op and the
connection was trivially MITM-able. Replace it with PinnedCertVerifier:
the presented end-entity cert must be byte-identical to a cert in the
--tls-ca file (any cert in a multi-cert PEM matches, supporting
rotation). Signature validation now uses the ring provider's full
algorithm set instead of a hardcoded 3-scheme list. Empty PEM files
fail fast instead of failing closed per-handshake.

The --verify-hostname path is unchanged (WebPki root-store validation).

Also: the third argument of connect_async_tls_with_config is
tungstenite's disable_nagle flag, not a verification toggle — we were
passing verify_hostname there, leaving Nagle ON for default users. Pass
true unconditionally, and disable Nagle on the plain ws:// path too;
socktop exchanges small request/response frames where Nagle only adds
latency.

Client now consumes the connector via a dual path+version dep so these
fixes are in local builds and CI before the crates.io publish (cargo
strips the path on publish). Connector version -> 1.51.0.

Verified E2E: agent A's cert connects to agent A; agent B's cert
against agent A fails the handshake (the rpi-worker-1 wrong-PEM
scenario); --verify-hostname against a 127.0.0.1 SAN still connects.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Cargo.lock

@@ -2422,7 +2422,7 @@ dependencies = [
  "ratatui",
  "serde",
  "serde_json",
- "socktop_connector 1.50.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "socktop_connector",
  "tempfile",
  "tokio",
  "unicode-width",
@@ -2462,7 +2462,7 @@ dependencies = [
 
 [[package]]
 name = "socktop_connector"
-version = "1.50.0"
+version = "1.51.0"
 dependencies = [
  "flate2",
  "futures-util",
@@ -2483,27 +2483,6 @@ dependencies = [
  "web-sys",
 ]
 
-[[package]]
-name = "socktop_connector"
-version = "1.50.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "61ea6a5733e71da6d5c94d23265b85f7041305bca51e6c33e7104464444047bc"
-dependencies = [
- "flate2",
- "futures-util",
- "prost",
- "prost-build",
- "protoc-bin-vendored",
- "rustls",
- "rustls-pemfile",
- "serde",
- "serde_json",
- "thiserror 2.0.17",
- "tokio",
- "tokio-tungstenite 0.24.0",
- "url",
-]
-
 [[package]]
 name = "stable_deref_trait"
 version = "1.2.1"

README.md

@@ -475,7 +475,7 @@ socktop --tls-ca /path/to/agent/cert.pem wss://HOST:8443/ws
 Notes:
 - Do not copy the private key off the server; only the cert.pem is needed by clients.
 - When --tls-ca/-t is supplied, the client auto‑upgrades ws:// to wss:// to avoid protocol mismatch.
-- Hostname (SAN) verification is DISABLED by default (the cert is still pinned). Use `--verify-hostname` to enable strict SAN checking.
+- Hostname (SAN) verification is DISABLED by default; instead the client PINS the certificate: the agent must present a cert byte-identical to one in your `--tls-ca` file (expiry is ignored in this mode — you pinned that exact cert). Use `--verify-hostname` to switch to strict chain + SAN validation instead.
 - You can run multiple clients with different cert paths by passing --tls-ca per invocation.
 
 ---

socktop/Cargo.toml

@@ -11,7 +11,7 @@ repository = "https://github.com/jasonwitty/socktop"
 
 [dependencies]
 # socktop connector for agent communication
-socktop_connector = "1.50.0"
+socktop_connector = { version = "1.51.0", path = "../socktop_connector" }
 
 tokio = { workspace = true }
 futures-util = { workspace = true }

socktop_connector/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "socktop_connector"
-version = "1.50.0"
+version = "1.51.0"
 edition = "2024"
 license = "MIT"
 description = "WebSocket connector library for socktop agent communication"

socktop_connector/src/networking/connection.rs

@@ -6,7 +6,7 @@ use crate::error::{ConnectorError, Result};
 use std::io::BufReader;
 use std::sync::Arc;
 use tokio_tungstenite::tungstenite::client::IntoClientRequest;
-use tokio_tungstenite::{MaybeTlsStream, WebSocketStream, connect_async};
+use tokio_tungstenite::{MaybeTlsStream, WebSocketStream};
 use url::Url;
 
 #[cfg(feature = "tls")]
@@ -15,7 +15,7 @@ use {
     rustls::{
         DigitallySignedStruct, RootCertStore, SignatureScheme,
         client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier},
-        crypto::ring,
+        crypto::{WebPkiSupportedAlgorithms, ring},
         pki_types::{CertificateDer, ServerName, UnixTime},
     },
     rustls_pemfile::Item,
@@ -64,7 +64,8 @@ async fn connect_without_ca_and_config(url: &str, config: &ConnectorConfig) -> R
         );
     }
 
-    let (ws, _) = connect_async(req).await?;
+    // `true` disables Nagle: small request/response frames, latency matters.
+    let (ws, _) = tokio_tungstenite::connect_async_with_config(req, None, true).await?;
     Ok(ws)
 }
 
@@ -85,7 +86,12 @@ async fn connect_with_ca_and_config(
             der_certs.push(der);
         }
     }
-    root.add_parsable_certificates(der_certs);
+    if der_certs.is_empty() {
+        return Err(ConnectorError::protocol_error(format!(
+            "no certificates found in --tls-ca file: {ca_path}"
+        )));
+    }
+    root.add_parsable_certificates(der_certs.iter().cloned());
 
     let mut cfg = ClientConfig::builder()
         .with_root_certificates(root)
@@ -114,57 +120,89 @@ async fn connect_with_ca_and_config(
     }
 
     if !config.verify_hostname {
-        #[derive(Debug)]
+        // Default mode: certificate PINNING without hostname verification.
-        struct NoVerify;
+        // The server must present a certificate byte-identical to one in the
-        impl ServerCertVerifier for NoVerify {
+        // --tls-ca file. This intentionally ignores expiry and chain building
-            fn verify_server_cert(
+        // (the operator pinned this exact cert), but unlike a blanket accept
-                &self,
+        // it makes MITM certs fail the handshake.
-                _end_entity: &CertificateDer<'_>,
+        cfg.dangerous()
-                _intermediates: &[CertificateDer<'_>],
+            .set_certificate_verifier(Arc::new(PinnedCertVerifier::new(der_certs)));
-                _server_name: &ServerName,
-                _ocsp_response: &[u8],
-                _now: UnixTime,
-            ) -> std::result::Result<ServerCertVerified, rustls::Error> {
-                Ok(ServerCertVerified::assertion())
-            }
-            fn verify_tls12_signature(
-                &self,
-                _message: &[u8],
-                _cert: &CertificateDer<'_>,
-                _dss: &DigitallySignedStruct,
-            ) -> std::result::Result<HandshakeSignatureValid, rustls::Error> {
-                Ok(HandshakeSignatureValid::assertion())
-            }
-            fn verify_tls13_signature(
-                &self,
-                _message: &[u8],
-                _cert: &CertificateDer<'_>,
-                _dss: &DigitallySignedStruct,
-            ) -> std::result::Result<HandshakeSignatureValid, rustls::Error> {
-                Ok(HandshakeSignatureValid::assertion())
-            }
-            fn supported_verify_schemes(&self) -> Vec<SignatureScheme> {
-                vec![
-                    SignatureScheme::ECDSA_NISTP256_SHA256,
-                    SignatureScheme::ED25519,
-                    SignatureScheme::RSA_PSS_SHA256,
-                ]
-            }
-        }
-        cfg.dangerous().set_certificate_verifier(Arc::new(NoVerify));
-        // Note: hostname verification disabled (default). Set SOCKTOP_VERIFY_NAME=1 to enable strict SAN checking.
     }
     let cfg = Arc::new(cfg);
+    // Third argument is tungstenite's `disable_nagle`: always true — socktop
+    // exchanges small request/response frames where Nagle only adds latency.
     let (ws, _) = tokio_tungstenite::connect_async_tls_with_config(
         req,
         None,
-        config.verify_hostname,
+        true,
         Some(Connector::Rustls(cfg)),
     )
     .await?;
     Ok(ws)
 }
 
+/// Accepts exactly the certificates the user pinned via `--tls-ca`, nothing else.
+///
+/// Used when hostname verification is off (the default for self-signed
+/// home-lab certs). Signature validation still runs with the ring provider's
+/// full algorithm set; only the certificate identity check is replaced —
+/// by an exact DER comparison against the pinned certificate(s).
+#[cfg(feature = "tls")]
+#[derive(Debug)]
+struct PinnedCertVerifier {
+    pinned: Vec<CertificateDer<'static>>,
+    algorithms: WebPkiSupportedAlgorithms,
+}
+
+#[cfg(feature = "tls")]
+impl PinnedCertVerifier {
+    fn new(pinned: Vec<CertificateDer<'static>>) -> Self {
+        Self {
+            pinned,
+            algorithms: ring::default_provider().signature_verification_algorithms,
+        }
+    }
+}
+
+#[cfg(feature = "tls")]
+impl ServerCertVerifier for PinnedCertVerifier {
+    fn verify_server_cert(
+        &self,
+        end_entity: &CertificateDer<'_>,
+        _intermediates: &[CertificateDer<'_>],
+        _server_name: &ServerName,
+        _ocsp_response: &[u8],
+        _now: UnixTime,
+    ) -> std::result::Result<ServerCertVerified, rustls::Error> {
+        if self.pinned.iter().any(|p| p == end_entity) {
+            Ok(ServerCertVerified::assertion())
+        } else {
+            Err(rustls::Error::InvalidCertificate(
+                rustls::CertificateError::ApplicationVerificationFailure,
+            ))
+        }
+    }
+    fn verify_tls12_signature(
+        &self,
+        message: &[u8],
+        cert: &CertificateDer<'_>,
+        dss: &DigitallySignedStruct,
+    ) -> std::result::Result<HandshakeSignatureValid, rustls::Error> {
+        rustls::crypto::verify_tls12_signature(message, cert, dss, &self.algorithms)
+    }
+    fn verify_tls13_signature(
+        &self,
+        message: &[u8],
+        cert: &CertificateDer<'_>,
+        dss: &DigitallySignedStruct,
+    ) -> std::result::Result<HandshakeSignatureValid, rustls::Error> {
+        rustls::crypto::verify_tls13_signature(message, cert, dss, &self.algorithms)
+    }
+    fn supported_verify_schemes(&self) -> Vec<SignatureScheme> {
+        self.algorithms.supported_schemes()
+    }
+}
+
 #[cfg(not(feature = "tls"))]
 async fn connect_with_ca_and_config(
     _url: &str,
@@ -181,3 +219,73 @@ async fn connect_with_ca_and_config(
 fn ensure_crypto_provider() {
     let _ = ring::default_provider().install_default();
 }
+
+#[cfg(all(test, feature = "tls"))]
+mod tests {
+    use super::*;
+
+    fn verifier(pinned: &[&[u8]]) -> PinnedCertVerifier {
+        let _ = ring::default_provider().install_default();
+        PinnedCertVerifier::new(
+            pinned
+                .iter()
+                .map(|b| CertificateDer::from(b.to_vec()))
+                .collect(),
+        )
+    }
+
+    fn verify(v: &PinnedCertVerifier, presented: &[u8]) -> bool {
+        v.verify_server_cert(
+            &CertificateDer::from(presented.to_vec()),
+            &[],
+            &ServerName::try_from("agent.test").unwrap(),
+            &[],
+            UnixTime::now(),
+        )
+        .is_ok()
+    }
+
+    /// The regression this verifier exists to prevent: the old NoVerify
+    /// accepted ANY certificate when hostname verification was off, so the
+    /// documented pinning was a no-op. The pinned cert must be accepted and
+    /// every other cert rejected.
+    #[test]
+    fn only_the_pinned_certificate_is_accepted() {
+        let v = verifier(&[b"pinned-cert-der"]);
+        assert!(verify(&v, b"pinned-cert-der"));
+        assert!(!verify(&v, b"some-mitm-cert"), "unpinned cert accepted");
+        assert!(!verify(&v, b""), "empty cert accepted");
+    }
+
+    /// A --tls-ca file may hold several certs (e.g. during rotation); any of
+    /// them must satisfy the pin.
+    #[test]
+    fn any_cert_in_a_multi_cert_pem_satisfies_the_pin() {
+        let v = verifier(&[b"old-cert", b"new-cert"]);
+        assert!(verify(&v, b"old-cert"));
+        assert!(verify(&v, b"new-cert"));
+        assert!(!verify(&v, b"third-party-cert"));
+    }
+
+    /// Fail closed: an empty pin set must reject everything rather than
+    /// falling back to accept-all.
+    #[test]
+    fn an_empty_pin_set_rejects_all_certificates() {
+        let v = verifier(&[]);
+        assert!(!verify(&v, b"anything"));
+    }
+
+    /// Signature schemes come from the real provider, not a hardcoded list —
+    /// an agent using e.g. RSA-PKCS1 must still be able to handshake.
+    #[test]
+    fn signature_schemes_come_from_the_provider() {
+        let v = verifier(&[b"x"]);
+        let schemes = v.supported_verify_schemes();
+        assert!(
+            schemes.len() > 3,
+            "suspiciously short scheme list: {schemes:?}"
+        );
+        assert!(schemes.contains(&SignatureScheme::RSA_PKCS1_SHA256));
+        assert!(schemes.contains(&SignatureScheme::ECDSA_NISTP256_SHA256));
+    }
+}