diff --git a/Cargo.lock b/Cargo.lock index ce98f84..fc63ee3 100644 --- a/Cargo.lock +++ b/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" diff --git a/README.md b/README.md index 4a44831..fd2cffd 100644 --- a/README.md +++ b/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. --- diff --git a/socktop/Cargo.toml b/socktop/Cargo.toml index 1707a4e..fd1f71f 100644 --- a/socktop/Cargo.toml +++ b/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 } diff --git a/socktop_connector/Cargo.toml b/socktop_connector/Cargo.toml index 7ff30c6..240cdc6 100644 --- a/socktop_connector/Cargo.toml +++ b/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" diff --git a/socktop_connector/src/networking/connection.rs b/socktop_connector/src/networking/connection.rs index 8ba2ece..efa894d 100644 --- a/socktop_connector/src/networking/connection.rs +++ b/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)] - struct NoVerify; - impl ServerCertVerifier for NoVerify { - fn verify_server_cert( - &self, - _end_entity: &CertificateDer<'_>, - _intermediates: &[CertificateDer<'_>], - _server_name: &ServerName, - _ocsp_response: &[u8], - _now: UnixTime, - ) -> std::result::Result { - Ok(ServerCertVerified::assertion()) - } - fn verify_tls12_signature( - &self, - _message: &[u8], - _cert: &CertificateDer<'_>, - _dss: &DigitallySignedStruct, - ) -> std::result::Result { - Ok(HandshakeSignatureValid::assertion()) - } - fn verify_tls13_signature( - &self, - _message: &[u8], - _cert: &CertificateDer<'_>, - _dss: &DigitallySignedStruct, - ) -> std::result::Result { - Ok(HandshakeSignatureValid::assertion()) - } - fn supported_verify_schemes(&self) -> Vec { - 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. + // Default mode: certificate PINNING without hostname verification. + // The server must present a certificate byte-identical to one in the + // --tls-ca file. This intentionally ignores expiry and chain building + // (the operator pinned this exact cert), but unlike a blanket accept + // it makes MITM certs fail the handshake. + cfg.dangerous() + .set_certificate_verifier(Arc::new(PinnedCertVerifier::new(der_certs))); } 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>, + algorithms: WebPkiSupportedAlgorithms, +} + +#[cfg(feature = "tls")] +impl PinnedCertVerifier { + fn new(pinned: Vec>) -> 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 { + 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 { + rustls::crypto::verify_tls12_signature(message, cert, dss, &self.algorithms) + } + fn verify_tls13_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> std::result::Result { + rustls::crypto::verify_tls13_signature(message, cert, dss, &self.algorithms) + } + fn supported_verify_schemes(&self) -> Vec { + 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)); + } +}