diff --git a/README.md b/README.md index a80129d8..90267304 100644 --- a/README.md +++ b/README.md @@ -213,7 +213,7 @@ or ``` $ cargo run --bin tlsclient-mio -- --http expired.badssl.com -TLS error: WebPkiError(CertExpired, ValidateServerCert) +TLS error: InvalidCertificate(Expired) Connection closed ``` diff --git a/bogo/config.json b/bogo/config.json index e82697e4..5464d846 100644 --- a/bogo/config.json +++ b/bogo/config.json @@ -174,6 +174,9 @@ "ALPNServer-EmptyProtocolName-TLS-TLS12": ":PEER_MISBEHAVIOUR:", "ALPNServer-EmptyProtocolName-TLS-TLS13": ":PEER_MISBEHAVIOUR:", "Verify-ServerAuth-SignatureType": ":PEER_MISBEHAVIOUR:", + "Verify-ClientAuth-SignatureType": ":BAD_SIGNATURE:", + "Verify-ServerAuth-SignatureType-TLS13": ":BAD_SIGNATURE:", + "Verify-ClientAuth-SignatureType-TLS13": ":BAD_SIGNATURE:", "ClientAuth-Enforced": ":PEER_MISBEHAVIOUR:", "ServerAuth-Enforced": ":PEER_MISBEHAVIOUR:", "UnofferedExtension-Client": ":PEER_MISBEHAVIOUR:", @@ -359,6 +362,7 @@ "Downgrade-TLS10-Client": ":HANDSHAKE_FAILURE:", "Downgrade-TLS10-Server": ":INCOMPATIBLE:", "UnsupportedCurve": ":PEER_MISBEHAVIOUR:", + "ECDSACurveMismatch-Verify-TLS13": ":BAD_SIGNATURE:", "SecondServerHelloNoVersion-TLS13": ":PEER_MISBEHAVIOUR:", "SecondServerHelloWrongVersion-TLS13": ":INCOMPATIBLE:", "TooManyChangeCipherSpec-Client-TLS13": ":PEER_MISBEHAVIOUR:", diff --git a/examples/tests/badssl.rs b/examples/tests/badssl.rs index d22dc5e2..346862d0 100644 --- a/examples/tests/badssl.rs +++ b/examples/tests/badssl.rs @@ -33,9 +33,7 @@ mod online { fn expired() { connect("expired.badssl.com") .fails() - .expect( - r#"TLS error: InvalidCertificateData\("invalid peer certificate: CertExpired"\)"#, - ) + .expect(r#"TLS error: InvalidCertificate\(Expired\)"#) .go() .unwrap(); } @@ -44,7 +42,7 @@ mod online { fn wrong_host() { connect("wrong.host.badssl.com") .fails() - .expect(r#"TLS error: InvalidCertificateData\("invalid peer certificate: CertNotValidForName"\)"#) + .expect(r#"TLS error: InvalidCertificate\(NotValidForName\)"#) .go() .unwrap(); } @@ -53,9 +51,7 @@ mod online { fn self_signed() { connect("self-signed.badssl.com") .fails() - .expect( - r#"TLS error: InvalidCertificateData\("invalid peer certificate: UnknownIssuer"\)"#, - ) + .expect(r#"TLS error: InvalidCertificate\(UnknownIssuer\)"#) .go() .unwrap(); } @@ -122,9 +118,7 @@ mod online { fn sha1_2016() { connect("sha1-2016.badssl.com") .fails() - .expect( - r#"TLS error: InvalidCertificateData\("invalid peer certificate: CertExpired"\)"#, - ) + .expect(r#"TLS error: InvalidCertificate\(Expired\)"#) .go() .unwrap(); } diff --git a/rustls/examples/internal/bogo_shim.rs b/rustls/examples/internal/bogo_shim.rs index b329f4b6..e6cccd67 100644 --- a/rustls/examples/internal/bogo_shim.rs +++ b/rustls/examples/internal/bogo_shim.rs @@ -13,7 +13,7 @@ use rustls::internal::msgs::persist; use rustls::quic::{self, ClientQuicExt, QuicExt, ServerQuicExt}; use rustls::server::ClientHello; use rustls::ProtocolVersion; -use rustls::{ClientConnection, Connection, ServerConnection, Side}; +use rustls::{CertificateError, ClientConnection, Connection, ServerConnection, Side}; use std::env; use std::fs; @@ -628,9 +628,11 @@ fn handle_err(err: rustls::Error) -> ! { Error::AlertReceived(AlertDescription::DecompressionFailure) => { quit_err(":SSLV3_ALERT_DECOMPRESSION_FAILURE:") } - Error::InvalidCertificateEncoding => quit(":CANNOT_PARSE_LEAF_CERT:"), - Error::InvalidCertificateSignature => quit(":BAD_SIGNATURE:"), - Error::InvalidCertificateSignatureType => quit(":WRONG_SIGNATURE_TYPE:"), + Error::InvalidCertificate(CertificateError::BadEncoding) => { + quit(":CANNOT_PARSE_LEAF_CERT:") + } + Error::InvalidCertificate(CertificateError::BadSignature) => quit(":BAD_SIGNATURE:"), + Error::InvalidCertificate(e) => quit(&format!(":BAD_CERT: ({:?})", e)), Error::PeerSentOversizedRecord => quit(":DATA_LENGTH_TOO_LONG:"), _ => { println_err!("unhandled error: {:?}", err); diff --git a/rustls/examples/internal/trytls_shim.rs b/rustls/examples/internal/trytls_shim.rs index 5bed489f..5e8d04f5 100644 --- a/rustls/examples/internal/trytls_shim.rs +++ b/rustls/examples/internal/trytls_shim.rs @@ -78,11 +78,9 @@ fn communicate( if let Err(err) = client.process_new_packets() { return match err { - Error::InvalidCertificateData(_) - | Error::InvalidCertificateSignature - | Error::InvalidCertificateSignatureType - | Error::InvalidCertificateEncoding - | Error::AlertReceived(_) => Ok(Verdict::Reject(err)), + Error::InvalidCertificate(_) | Error::AlertReceived(_) => { + Ok(Verdict::Reject(err)) + } _ => Err(From::from(format!("{:?}", err))), }; } diff --git a/rustls/src/anchors.rs b/rustls/src/anchors.rs index 4caf3e7d..57eee1e5 100644 --- a/rustls/src/anchors.rs +++ b/rustls/src/anchors.rs @@ -3,6 +3,7 @@ use crate::key; use crate::log::{debug, trace}; use crate::msgs::handshake::{DistinguishedName, DistinguishedNames}; use crate::x509; +use crate::{CertificateError, Error}; /// A trust anchor, commonly known as a "Root Certificate." #[derive(Debug, Clone)] @@ -101,8 +102,9 @@ impl RootCertStore { } /// Add a single DER-encoded certificate to the store. - pub fn add(&mut self, der: &key::Certificate) -> Result<(), webpki::Error> { - let ta = webpki::TrustAnchor::try_from_cert_der(&der.0)?; + pub fn add(&mut self, der: &key::Certificate) -> Result<(), Error> { + let ta = webpki::TrustAnchor::try_from_cert_der(&der.0) + .map_err(|_| Error::InvalidCertificate(CertificateError::BadEncoding))?; let ota = OwnedTrustAnchor::from_subject_spki_name_constraints( ta.subject, ta.spki, diff --git a/rustls/src/cipher.rs b/rustls/src/cipher.rs index b595ca68..b4cde3b9 100644 --- a/rustls/src/cipher.rs +++ b/rustls/src/cipher.rs @@ -87,7 +87,7 @@ struct InvalidMessageEncrypter {} impl MessageEncrypter for InvalidMessageEncrypter { fn encrypt(&self, _m: BorrowedPlainMessage, _seq: u64) -> Result { - Err(Error::General("encrypt not yet available".to_string())) + Err(Error::EncryptError) } } diff --git a/rustls/src/client/hs.rs b/rustls/src/client/hs.rs index c5828aa6..b49707d2 100644 --- a/rustls/src/client/hs.rs +++ b/rustls/src/client/hs.rs @@ -3,7 +3,7 @@ use crate::bs_debug; use crate::check::inappropriate_handshake_message; use crate::conn::{CommonState, ConnectionRandoms, State}; use crate::enums::{CipherSuite, ProtocolVersion}; -use crate::error::{Error, PeerIncompatible, PeerMisbehaved}; +use crate::error::{CertificateError, Error, PeerIncompatible, PeerMisbehaved}; use crate::hash_hs::HandshakeHashBuffer; use crate::kx; #[cfg(feature = "logging")] @@ -814,7 +814,7 @@ impl State for ExpectServerHelloOrHelloRetryRequest { pub(super) fn send_cert_error_alert(common: &mut CommonState, err: Error) -> Error { match err { - Error::InvalidCertificateEncoding => { + Error::InvalidCertificate(CertificateError::BadEncoding) => { common.send_fatal_alert(AlertDescription::DecodeError); } Error::PeerMisbehaved(_) => { diff --git a/rustls/src/error.rs b/rustls/src/error.rs index 7b3fd462..a547ec6b 100644 --- a/rustls/src/error.rs +++ b/rustls/src/error.rs @@ -3,10 +3,11 @@ use crate::rand; use std::error::Error as StdError; use std::fmt; +use std::sync::Arc; use std::time::SystemTimeError; /// rustls reports protocol errors using this type. -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, Clone)] pub enum Error { /// We received a TLS message that isn't valid right now. /// `expect_types` lists the message types we can expect right now. @@ -60,17 +61,11 @@ pub enum Error { /// We received a fatal alert. This means the peer is unhappy. AlertReceived(AlertDescription), - /// We received an invalidly encoded certificate from the peer. - InvalidCertificateEncoding, - - /// We received a certificate with invalid signature type. - InvalidCertificateSignatureType, - - /// We received a certificate with invalid signature. - InvalidCertificateSignature, - - /// We received a certificate which includes invalid data. - InvalidCertificateData(String), + /// We saw an invalid certificate. + /// + /// The contained error is from the certificate validation trait + /// implementation. + InvalidCertificate(CertificateError), /// The presented SCT(s) were invalid. InvalidSct(sct::Error), @@ -188,7 +183,7 @@ impl From for Error { #[non_exhaustive] #[allow(missing_docs)] -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, Clone)] /// The set of cases where we failed to make a connection because a peer /// doesn't support a TLS version/feature we require. /// @@ -222,6 +217,58 @@ impl From for Error { } } +#[non_exhaustive] +#[derive(Debug, Clone)] +/// The ways in which certificate validators can express errors. +/// +/// Note that the rustls TLS protocol code interprets specifically these +/// error codes to send specific TLS alerts. Therefore, if a +/// custom certificate validator uses incorrect errors the library as +/// a whole will send alerts that do not match the standard (this is usually +/// a minor issue, but could be misleading). +pub enum CertificateError { + /// The certificate is not correctly encoded. + BadEncoding, + + /// The current time is after the `notAfter` time in the certificate. + Expired, + + /// The current time is before the `notBefore` time in the certificate. + NotValidYet, + + /// The certificate contains an extension marked critical, but it was + /// not processed by the certificate validator. + UnhandledCriticalExtension, + + /// The certificate chain is not issued by a known root certificate. + UnknownIssuer, + + /// A certificate is not correctly signed by the key of its alleged + /// issuer. + BadSignature, + + /// The subject names in an end-entity certificate do not include + /// the expected name. + NotValidForName, + + /// Any other error. + /// + /// This can be used by custom verifiers to expose the underlying error + /// (where they are not better described by the more specific errors + /// above). + /// + /// It is also used by the default verifier in case its error is + /// not covered by the above common cases. + Other(Arc), +} + +impl From for Error { + #[inline] + fn from(e: CertificateError) -> Self { + Self::InvalidCertificate(e) + } +} + fn join(items: &[T]) -> String { items .iter() @@ -257,17 +304,8 @@ impl fmt::Display for Error { Self::PeerIncompatible(ref why) => write!(f, "peer is incompatible: {:?}", why), Self::PeerMisbehaved(ref why) => write!(f, "peer misbehaved: {:?}", why), Self::AlertReceived(ref alert) => write!(f, "received fatal alert: {:?}", alert), - Self::InvalidCertificateEncoding => { - write!(f, "invalid peer certificate encoding") - } - Self::InvalidCertificateSignatureType => { - write!(f, "invalid peer certificate signature type") - } - Self::InvalidCertificateSignature => { - write!(f, "invalid peer certificate signature") - } - Self::InvalidCertificateData(ref reason) => { - write!(f, "invalid peer certificate contents: {}", reason) + Self::InvalidCertificate(ref err) => { + write!(f, "invalid peer certificate: {:?}", err) } Self::CorruptMessage => write!(f, "received corrupt message"), Self::NoCertificatesPresented => write!(f, "peer sent no certificates"), @@ -328,10 +366,7 @@ mod tests { super::PeerIncompatible::Tls12NotOffered.into(), super::PeerMisbehaved::UnsolicitedCertExtension.into(), Error::AlertReceived(AlertDescription::ExportRestriction), - Error::InvalidCertificateEncoding, - Error::InvalidCertificateSignatureType, - Error::InvalidCertificateSignature, - Error::InvalidCertificateData("Data".into()), + super::CertificateError::Expired.into(), Error::InvalidSct(sct::Error::MalformedSct), Error::General("undocumented error".to_string()), Error::FailedToGetCurrentTime, @@ -352,7 +387,7 @@ mod tests { fn rand_error_mapping() { use super::rand; let err: Error = rand::GetRandomFailed.into(); - assert_eq!(err, Error::FailedToGetRandomBytes); + assert!(matches!(err, Error::FailedToGetRandomBytes)); } #[test] @@ -363,6 +398,6 @@ mod tests { .duration_since(SystemTime::now()) .unwrap_err(); let err: Error = time_error.into(); - assert_eq!(err, Error::FailedToGetCurrentTime); + assert!(matches!(err, Error::FailedToGetCurrentTime)); } } diff --git a/rustls/src/lib.rs b/rustls/src/lib.rs index c1611c0f..8def5798 100644 --- a/rustls/src/lib.rs +++ b/rustls/src/lib.rs @@ -166,7 +166,7 @@ //! therefore call `client.process_new_packets()` which parses and processes the messages. //! Any error returned from `process_new_packets` is fatal to the connection, and will tell you //! why. For example, if the server's certificate is expired `process_new_packets` will -//! return `Err(WebPkiError(CertExpired, ValidateServerCert))`. From this point on, +//! return `Err(InvalidCertificate(Expired))`. From this point on, //! `process_new_packets` will not do any new work and will return that error continually. //! //! You can extract newly received data by calling `client.reader()` (which implements the @@ -371,7 +371,7 @@ pub use crate::conn::{ CommonState, Connection, ConnectionCommon, IoState, Reader, Side, SideData, Writer, }; pub use crate::enums::{CipherSuite, ProtocolVersion, SignatureScheme}; -pub use crate::error::{Error, PeerIncompatible, PeerMisbehaved}; +pub use crate::error::{CertificateError, Error, PeerIncompatible, PeerMisbehaved}; pub use crate::key::{Certificate, PrivateKey}; pub use crate::key_log::{KeyLog, NoKeyLog}; pub use crate::key_log_file::KeyLogFile; diff --git a/rustls/src/msgs/deframer.rs b/rustls/src/msgs/deframer.rs index ea523f0b..02008689 100644 --- a/rustls/src/msgs/deframer.rs +++ b/rustls/src/msgs/deframer.rs @@ -624,7 +624,7 @@ mod tests { ); let mut rl = RecordLayer::new(); - assert_eq!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage); + assert!(matches!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage)); } #[test] @@ -636,7 +636,7 @@ mod tests { ); let mut rl = RecordLayer::new(); - assert_eq!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage); + assert!(matches!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage)); } #[test] @@ -648,7 +648,7 @@ mod tests { ); let mut rl = RecordLayer::new(); - assert_eq!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage); + assert!(matches!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage)); } #[test] @@ -676,9 +676,9 @@ mod tests { ); let mut rl = RecordLayer::new(); - assert_eq!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage); + assert!(matches!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage)); // CorruptMessage has been fused - assert_eq!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage); + assert!(matches!(d.pop(&mut rl).unwrap_err(), Error::CorruptMessage)); } #[test] diff --git a/rustls/src/tls12/cipher.rs b/rustls/src/tls12/cipher.rs index 2910c20c..55b13800 100644 --- a/rustls/src/tls12/cipher.rs +++ b/rustls/src/tls12/cipher.rs @@ -155,7 +155,7 @@ impl MessageEncrypter for GcmMessageEncrypter { self.enc_key .seal_in_place_separate_tag(nonce, aad, &mut payload[GCM_EXPLICIT_NONCE_LEN..]) .map(|tag| payload.extend(tag.as_ref())) - .map_err(|_| Error::General("encrypt failed".to_string()))?; + .map_err(|_| Error::EncryptError)?; Ok(OpaqueMessage { typ: msg.typ, @@ -225,7 +225,7 @@ impl MessageEncrypter for ChaCha20Poly1305MessageEncrypter { self.enc_key .seal_in_place_append_tag(nonce, aad, &mut buf) - .map_err(|_| Error::General("encrypt failed".to_string()))?; + .map_err(|_| Error::EncryptError)?; Ok(OpaqueMessage { typ: msg.typ, diff --git a/rustls/src/verify.rs b/rustls/src/verify.rs index a67e5d47..cf05fca9 100644 --- a/rustls/src/verify.rs +++ b/rustls/src/verify.rs @@ -3,7 +3,7 @@ use std::fmt; use crate::anchors::{OwnedTrustAnchor, RootCertStore}; use crate::client::ServerName; use crate::enums::SignatureScheme; -use crate::error::{Error, PeerMisbehaved}; +use crate::error::{CertificateError, Error, PeerMisbehaved}; use crate::key::Certificate; #[cfg(feature = "logging")] use crate::log::{debug, trace, warn}; @@ -104,7 +104,7 @@ pub trait ServerCertVerifier: Send + Sync { /// /// Note that none of the certificates have been parsed yet, so it is the responsibility of /// the implementor to handle invalid data. It is recommended that the implementor returns - /// [`Error::InvalidCertificateEncoding`] when these cases are encountered. + /// [`Error::InvalidCertificate(CertificateError::BadEncoding)`] when these cases are encountered. /// /// `scts` contains the Signed Certificate Timestamps (SCTs) the server /// sent with the end-entity certificate, if any. @@ -254,7 +254,7 @@ pub trait ClientCertVerifier: Send + Sync { /// /// Note that none of the certificates have been parsed yet, so it is the responsibility of /// the implementor to handle invalid data. It is recommended that the implementor returns - /// [`Error::InvalidCertificateEncoding`] when these cases are encountered. + /// [`Error::InvalidCertificate(CertificateError::BadEncoding)`] when these cases are encountered. fn verify_client_cert( &self, end_entity: &Certificate, @@ -617,12 +617,16 @@ impl ClientCertVerifier for AllowAnyAnonymousOrAuthenticatedClient { fn pki_error(error: webpki::Error) -> Error { use webpki::Error::*; match error { - BadDer | BadDerTime => Error::InvalidCertificateEncoding, - InvalidSignatureForPublicKey => Error::InvalidCertificateSignature, - UnsupportedSignatureAlgorithm | UnsupportedSignatureAlgorithmForPublicKey => { - Error::InvalidCertificateSignatureType - } - e => Error::InvalidCertificateData(format!("invalid peer certificate: {}", e)), + BadDer | BadDerTime => CertificateError::BadEncoding.into(), + CertNotValidYet => CertificateError::NotValidYet.into(), + CertExpired | InvalidCertValidity => CertificateError::Expired.into(), + UnknownIssuer => CertificateError::UnknownIssuer.into(), + CertNotValidForName => CertificateError::NotValidForName.into(), + + InvalidSignatureForPublicKey + | UnsupportedSignatureAlgorithm + | UnsupportedSignatureAlgorithmForPublicKey => CertificateError::BadSignature.into(), + _ => CertificateError::Other(Arc::new(error)).into(), } } diff --git a/rustls/tests/api.rs b/rustls/tests/api.rs index 55d3f210..8d802d97 100644 --- a/rustls/tests/api.rs +++ b/rustls/tests/api.rs @@ -18,7 +18,10 @@ use rustls::quic::{self, ClientQuicExt, QuicExt, ServerQuicExt}; use rustls::server::{AllowAnyAnonymousOrAuthenticatedClient, ClientHello, ResolvesServerCert}; #[cfg(feature = "secret_extraction")] use rustls::ConnectionTrafficSecrets; -use rustls::{sign, ConnectionCommon, Error, KeyLog, PeerIncompatible, PeerMisbehaved, SideData}; +use rustls::{ + sign, CertificateError, ConnectionCommon, Error, KeyLog, PeerIncompatible, PeerMisbehaved, + SideData, +}; use rustls::{CipherSuite, ProtocolVersion, SignatureScheme}; use rustls::{ClientConfig, ClientConnection}; use rustls::{ServerConfig, ServerConnection}; @@ -32,7 +35,7 @@ fn alpn_test_error( server_protos: Vec>, client_protos: Vec>, agreed: Option<&[u8]>, - expected_error: Option, + expected_error: impl Fn() -> Option, ) { let mut server_config = make_server_config(KeyType::Rsa); server_config.alpn_protocols = server_protos; @@ -51,12 +54,12 @@ fn alpn_test_error( let error = do_handshake_until_error(&mut client, &mut server); assert_eq!(client.alpn_protocol(), agreed); assert_eq!(server.alpn_protocol(), agreed); - assert_eq!(error.err(), expected_error); + assert_debug_eq(error.err(), expected_error()); } } fn alpn_test(server_protos: Vec>, client_protos: Vec>, agreed: Option<&[u8]>) { - alpn_test_error(server_protos, client_protos, agreed, None) + alpn_test_error(server_protos, client_protos, agreed, || None) } #[test] @@ -75,7 +78,7 @@ fn alpn() { vec![b"server-proto".to_vec()], vec![b"client-proto".to_vec()], None, - Some(ErrorFromPeer::Server(Error::NoApplicationProtocol)), + || Some(ErrorFromPeer::Server(Error::NoApplicationProtocol)), ); // server chooses preference @@ -90,7 +93,7 @@ fn alpn() { vec![b"PROTO".to_vec()], vec![b"proto".to_vec()], None, - Some(ErrorFromPeer::Server(Error::NoApplicationProtocol)), + || Some(ErrorFromPeer::Server(Error::NoApplicationProtocol)), ); } @@ -186,75 +189,75 @@ fn check_read(reader: &mut dyn io::Read, bytes: &[u8]) { #[test] fn config_builder_for_client_rejects_empty_kx_groups() { - assert_eq!( + assert_debug_eq( ClientConfig::builder() .with_safe_default_cipher_suites() .with_kx_groups(&[]) .with_safe_default_protocol_versions() .err(), - Some(Error::General("no kx groups configured".into())) + Some(Error::General("no kx groups configured".into())), ); } #[test] fn config_builder_for_client_rejects_empty_cipher_suites() { - assert_eq!( + assert_debug_eq( ClientConfig::builder() .with_cipher_suites(&[]) .with_safe_default_kx_groups() .with_safe_default_protocol_versions() .err(), - Some(Error::General("no usable cipher suites configured".into())) + Some(Error::General("no usable cipher suites configured".into())), ); } #[cfg(feature = "tls12")] #[test] fn config_builder_for_client_rejects_incompatible_cipher_suites() { - assert_eq!( + assert_debug_eq( ClientConfig::builder() .with_cipher_suites(&[rustls::cipher_suite::TLS13_AES_256_GCM_SHA384]) .with_safe_default_kx_groups() .with_protocol_versions(&[&rustls::version::TLS12]) .err(), - Some(Error::General("no usable cipher suites configured".into())) + Some(Error::General("no usable cipher suites configured".into())), ); } #[test] fn config_builder_for_server_rejects_empty_kx_groups() { - assert_eq!( + assert_debug_eq( ServerConfig::builder() .with_safe_default_cipher_suites() .with_kx_groups(&[]) .with_safe_default_protocol_versions() .err(), - Some(Error::General("no kx groups configured".into())) + Some(Error::General("no kx groups configured".into())), ); } #[test] fn config_builder_for_server_rejects_empty_cipher_suites() { - assert_eq!( + assert_debug_eq( ServerConfig::builder() .with_cipher_suites(&[]) .with_safe_default_kx_groups() .with_safe_default_protocol_versions() .err(), - Some(Error::General("no usable cipher suites configured".into())) + Some(Error::General("no usable cipher suites configured".into())), ); } #[cfg(feature = "tls12")] #[test] fn config_builder_for_server_rejects_incompatible_cipher_suites() { - assert_eq!( + assert_debug_eq( ServerConfig::builder() .with_cipher_suites(&[rustls::cipher_suite::TLS13_AES_256_GCM_SHA384]) .with_safe_default_kx_groups() .with_protocol_versions(&[&rustls::version::TLS12]) .err(), - Some(Error::General("no usable cipher suites configured".into())) + Some(Error::General("no usable cipher suites configured".into())), ); } @@ -873,11 +876,11 @@ fn client_checks_server_certificate_with_given_name() { let mut server = ServerConnection::new(Arc::clone(&server_config)).unwrap(); let err = do_handshake_until_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( err, - Err(ErrorFromPeer::Client(Error::InvalidCertificateData( - "invalid peer certificate: CertNotValidForName".into(), - ))) + Err(ErrorFromPeer::Client(Error::InvalidCertificate( + CertificateError::NotValidForName, + ))), ); } } @@ -943,9 +946,9 @@ fn client_cert_resolve() { let (mut client, mut server) = make_pair_for_arc_configs(&Arc::new(client_config), &server_config); - assert_eq!( + assert_debug_eq( do_handshake_until_error(&mut client, &mut server), - Err(ErrorFromPeer::Server(Error::NoCertificatesPresented)) + Err(ErrorFromPeer::Server(Error::NoCertificatesPresented)), ); } } @@ -1849,11 +1852,11 @@ fn server_exposes_offered_sni_even_if_resolver_fails() { assert_eq!(None, server.sni_hostname()); transfer(&mut client, &mut server); - assert_eq!( + assert_debug_eq( server.process_new_packets(), Err(Error::General( - "no server certificate chain resolved".to_string() - )) + "no server certificate chain resolved".to_string(), + )), ); assert_eq!(Some("thisdoesnotexist.com"), server.sni_hostname()); } @@ -1880,17 +1883,17 @@ fn sni_resolver_works() { let mut client1 = ClientConnection::new(Arc::new(make_client_config(kt)), dns_name("localhost")).unwrap(); let err = do_handshake_until_error(&mut client1, &mut server1); - assert_eq!(err, Ok(())); + assert_debug_eq(err, Ok(())); let mut server2 = ServerConnection::new(Arc::clone(&server_config)).unwrap(); let mut client2 = ClientConnection::new(Arc::new(make_client_config(kt)), dns_name("notlocalhost")).unwrap(); let err = do_handshake_until_error(&mut client2, &mut server2); - assert_eq!( + assert_debug_eq( err, Err(ErrorFromPeer::Server(Error::General( - "no server certificate chain resolved".into() - ))) + "no server certificate chain resolved".into(), + ))), ); } @@ -1901,28 +1904,28 @@ fn sni_resolver_rejects_wrong_names() { let signing_key = sign::RsaSigningKey::new(&kt.get_key()).unwrap(); let signing_key: Arc = Arc::new(signing_key); - assert_eq!( - Ok(()), + assert_debug_eq( resolver.add( "localhost", - sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()) - ) + sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()), + ), + Ok(()), ); - assert_eq!( - Err(Error::General( - "The server certificate is not valid for the given name".into() - )), + assert_debug_eq( resolver.add( "not-localhost", - sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()) - ) + sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()), + ), + Err(Error::General( + "The server certificate is not valid for the given name".into(), + )), ); - assert_eq!( - Err(Error::General("Bad DNS name".into())), + assert_debug_eq( resolver.add( "not ascii 🦀", - sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()) - ) + sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()), + ), + Err(Error::General("Bad DNS name".into())), ); } @@ -1933,12 +1936,12 @@ fn sni_resolver_lower_cases_configured_names() { let signing_key = sign::RsaSigningKey::new(&kt.get_key()).unwrap(); let signing_key: Arc = Arc::new(signing_key); - assert_eq!( - Ok(()), + assert_debug_eq( resolver.add( "LOCALHOST", - sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()) - ) + sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()), + ), + Ok(()), ); let mut server_config = make_server_config(kt); @@ -1949,7 +1952,7 @@ fn sni_resolver_lower_cases_configured_names() { let mut client1 = ClientConnection::new(Arc::new(make_client_config(kt)), dns_name("localhost")).unwrap(); let err = do_handshake_until_error(&mut client1, &mut server1); - assert_eq!(err, Ok(())); + assert_debug_eq(err, Ok(())); } #[test] @@ -1960,12 +1963,12 @@ fn sni_resolver_lower_cases_queried_names() { let signing_key = sign::RsaSigningKey::new(&kt.get_key()).unwrap(); let signing_key: Arc = Arc::new(signing_key); - assert_eq!( - Ok(()), + assert_debug_eq( resolver.add( "localhost", - sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()) - ) + sign::CertifiedKey::new(kt.get_chain(), signing_key.clone()), + ), + Ok(()), ); let mut server_config = make_server_config(kt); @@ -1976,7 +1979,7 @@ fn sni_resolver_lower_cases_queried_names() { let mut client1 = ClientConnection::new(Arc::new(make_client_config(kt)), dns_name("LOCALHOST")).unwrap(); let err = do_handshake_until_error(&mut client1, &mut server1); - assert_eq!(err, Ok(())); + assert_debug_eq(err, Ok(())); } #[test] @@ -1986,25 +1989,29 @@ fn sni_resolver_rejects_bad_certs() { let signing_key = sign::RsaSigningKey::new(&kt.get_key()).unwrap(); let signing_key: Arc = Arc::new(signing_key); - assert_eq!( - Err(Error::General( - "No end-entity certificate in certificate chain".into() + assert_debug_eq( + resolver + .add( + "localhost", + sign::CertifiedKey::new(vec![], signing_key.clone()), + ) + .err(), + Some(Error::General( + "No end-entity certificate in certificate chain".into(), )), - resolver.add( - "localhost", - sign::CertifiedKey::new(vec![], signing_key.clone()) - ) ); let bad_chain = vec![rustls::Certificate(vec![0xa0])]; - assert_eq!( - Err(Error::General( - "End-entity certificate in certificate chain is syntactically invalid".into() + assert_debug_eq( + resolver + .add( + "localhost", + sign::CertifiedKey::new(bad_chain, signing_key.clone()), + ) + .err(), + Some(Error::General( + "End-entity certificate in certificate chain is syntactically invalid".into(), )), - resolver.add( - "localhost", - sign::CertifiedKey::new(bad_chain, signing_key.clone()) - ) ); } @@ -2014,34 +2021,34 @@ fn do_exporter_test(client_config: ClientConfig, server_config: ServerConfig) { let (mut client, mut server) = make_pair_for_configs(client_config, server_config); - assert_eq!( + assert_debug_eq( + client.export_keying_material(&mut client_secret, b"label", Some(b"context")), Err(Error::HandshakeNotComplete), - client.export_keying_material(&mut client_secret, b"label", Some(b"context")) ); - assert_eq!( + assert_debug_eq( + server.export_keying_material(&mut server_secret, b"label", Some(b"context")), Err(Error::HandshakeNotComplete), - server.export_keying_material(&mut server_secret, b"label", Some(b"context")) ); do_handshake(&mut client, &mut server); - assert_eq!( + assert_debug_eq( + client.export_keying_material(&mut client_secret, b"label", Some(b"context")), Ok(()), - client.export_keying_material(&mut client_secret, b"label", Some(b"context")) ); - assert_eq!( + assert_debug_eq( + server.export_keying_material(&mut server_secret, b"label", Some(b"context")), Ok(()), - server.export_keying_material(&mut server_secret, b"label", Some(b"context")) ); assert_eq!(client_secret.to_vec(), server_secret.to_vec()); - assert_eq!( + assert_debug_eq( + client.export_keying_material(&mut client_secret, b"label", None), Ok(()), - client.export_keying_material(&mut client_secret, b"label", None) ); assert_ne!(client_secret.to_vec(), server_secret.to_vec()); - assert_eq!( + assert_debug_eq( + server.export_keying_material(&mut server_secret, b"label", None), Ok(()), - server.export_keying_material(&mut server_secret, b"label", None) ); assert_eq!(client_secret.to_vec(), server_secret.to_vec()); } @@ -3271,11 +3278,11 @@ mod test_quic { ServerConnection::new_quic(server_config, quic::Version::V1, server_params.into()) .unwrap(); - assert_eq!( + assert_debug_eq( step(&mut client, &mut server) .err() .unwrap(), - Error::NoApplicationProtocol + Error::NoApplicationProtocol, ); assert_eq!( @@ -3400,11 +3407,11 @@ mod test_quic { server .read_tls(&mut buf.as_slice()) .unwrap(); - assert_eq!( - server.process_new_packets().err(), - Some(Error::PeerMisbehaved( - PeerMisbehaved::MissingQuicTransportParameters - )) + assert_debug_eq( + server.process_new_packets(), + Err(Error::PeerMisbehaved( + PeerMisbehaved::MissingQuicTransportParameters, + )), ); } @@ -3466,10 +3473,10 @@ mod test_quic { server .read_tls(&mut buf.as_slice()) .unwrap(); - assert_eq!( - server.process_new_packets().err(), - Some(Error::PeerIncompatible( - PeerIncompatible::SupportedVersionsExtensionRequired + assert_debug_eq( + server.process_new_packets(), + Err(Error::PeerIncompatible( + PeerIncompatible::SupportedVersionsExtensionRequired, )), ); } @@ -4030,22 +4037,22 @@ fn check_client_max_fragment_size(size: usize) -> Option { #[test] fn bad_client_max_fragment_sizes() { - assert_eq!( + assert_debug_eq( check_client_max_fragment_size(31), - Some(Error::BadMaxFragmentSize) + Some(Error::BadMaxFragmentSize), ); - assert_eq!(check_client_max_fragment_size(32), None); - assert_eq!(check_client_max_fragment_size(64), None); - assert_eq!(check_client_max_fragment_size(1460), None); - assert_eq!(check_client_max_fragment_size(0x4000), None); - assert_eq!(check_client_max_fragment_size(0x4005), None); - assert_eq!( + assert_debug_eq(check_client_max_fragment_size(32), None); + assert_debug_eq(check_client_max_fragment_size(64), None); + assert_debug_eq(check_client_max_fragment_size(1460), None); + assert_debug_eq(check_client_max_fragment_size(0x4000), None); + assert_debug_eq(check_client_max_fragment_size(0x4005), None); + assert_debug_eq( check_client_max_fragment_size(0x4006), - Some(Error::BadMaxFragmentSize) + Some(Error::BadMaxFragmentSize), ); - assert_eq!( + assert_debug_eq( check_client_max_fragment_size(0xffff), - Some(Error::BadMaxFragmentSize) + Some(Error::BadMaxFragmentSize), ); } @@ -4087,11 +4094,11 @@ fn test_server_rejects_duplicate_sni_names() { let (client, server) = make_pair(KeyType::Rsa); let (mut client, mut server) = (client.into(), server.into()); transfer_altered(&mut client, duplicate_sni_payload, &mut server); - assert_eq!( + assert_debug_eq( server.process_new_packets(), Err(Error::PeerMisbehaved( - PeerMisbehaved::DuplicateServerNameTypes - )) + PeerMisbehaved::DuplicateServerNameTypes, + )), ); } @@ -4116,11 +4123,11 @@ fn test_server_rejects_empty_sni_extension() { let (client, server) = make_pair(KeyType::Rsa); let (mut client, mut server) = (client.into(), server.into()); transfer_altered(&mut client, empty_sni_payload, &mut server); - assert_eq!( + assert_debug_eq( server.process_new_packets(), Err(Error::PeerMisbehaved( - PeerMisbehaved::ServerNameMustContainOneHostName - )) + PeerMisbehaved::ServerNameMustContainOneHostName, + )), ); } @@ -4147,11 +4154,11 @@ fn test_server_rejects_clients_without_any_kx_group_overlap() { let (client, server) = make_pair(KeyType::Rsa); let (mut client, mut server) = (client.into(), server.into()); transfer_altered(&mut client, different_kx_group, &mut server); - assert_eq!( + assert_debug_eq( server.process_new_packets(), Err(Error::PeerIncompatible( - PeerIncompatible::NoKxGroupsInCommon - )) + PeerIncompatible::NoKxGroupsInCommon, + )), ); } @@ -4172,11 +4179,11 @@ fn test_client_rejects_illegal_tls13_ccs() { let (mut server, mut client) = (server.into(), client.into()); transfer_altered(&mut server, corrupt_ccs, &mut client); - assert_eq!( + assert_debug_eq( client.process_new_packets(), Err(Error::PeerMisbehaved( - PeerMisbehaved::IllegalMiddleboxChangeCipherSpec - )) + PeerMisbehaved::IllegalMiddleboxChangeCipherSpec, + )), ); } @@ -4282,9 +4289,9 @@ fn test_acceptor() { .kind(), io::ErrorKind::Other, ); - assert_eq!( + assert_debug_eq( acceptor.accept().err(), - Some(Error::General("Acceptor polled after completion".into())) + Some(Error::General("Acceptor polled after completion".into())), ); let mut acceptor = Acceptor::default(); diff --git a/rustls/tests/client_cert_verifier.rs b/rustls/tests/client_cert_verifier.rs index 5af9c2ac..5953d53d 100644 --- a/rustls/tests/client_cert_verifier.rs +++ b/rustls/tests/client_cert_verifier.rs @@ -5,9 +5,10 @@ mod common; use crate::common::{ - dns_name, do_handshake_until_both_error, do_handshake_until_error, get_client_root_store, - make_client_config_with_versions, make_client_config_with_versions_with_auth, - make_pair_for_arc_configs, ErrorFromPeer, KeyType, ALL_KEY_TYPES, + assert_debug_eq, dns_name, do_handshake_until_both_error, do_handshake_until_error, + get_client_root_store, make_client_config_with_versions, + make_client_config_with_versions_with_auth, make_pair_for_arc_configs, ErrorFromPeer, KeyType, + ALL_KEY_TYPES, }; use rustls::client::WebPkiVerifier; use rustls::internal::msgs::base::PayloadU16; @@ -71,7 +72,7 @@ fn client_verifier_works() { let (mut client, mut server) = make_pair_for_arc_configs(&Arc::new(client_config.clone()), &server_config); let err = do_handshake_until_error(&mut client, &mut server); - assert_eq!(err, Ok(())); + assert_debug_eq(err, Ok(())); } } } @@ -101,11 +102,11 @@ fn client_verifier_no_schemes() { let (mut client, mut server) = make_pair_for_arc_configs(&Arc::new(client_config.clone()), &server_config); let err = do_handshake_until_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( err, Err(ErrorFromPeer::Client(Error::CorruptMessagePayload( - ContentType::Handshake - ))) + ContentType::Handshake, + ))), ); } } @@ -131,14 +132,14 @@ fn client_verifier_no_root() { let mut client = ClientConnection::new(Arc::new(client_config), dns_name("notlocalhost")).unwrap(); let errs = do_handshake_until_both_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( errs, Err(vec![ ErrorFromPeer::Server(Error::General( - "client rejected by client_auth_root_subjects".into() + "client rejected by client_auth_root_subjects".into(), )), - ErrorFromPeer::Client(Error::AlertReceived(AlertDescription::AccessDenied)) - ]) + ErrorFromPeer::Client(Error::AlertReceived(AlertDescription::AccessDenied)), + ]), ); } } @@ -164,14 +165,14 @@ fn client_verifier_no_auth_no_root() { let mut client = ClientConnection::new(Arc::new(client_config), dns_name("notlocalhost")).unwrap(); let errs = do_handshake_until_both_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( errs, Err(vec![ ErrorFromPeer::Server(Error::General( - "client rejected by client_auth_root_subjects".into() + "client rejected by client_auth_root_subjects".into(), )), - ErrorFromPeer::Client(Error::AlertReceived(AlertDescription::AccessDenied)) - ]) + ErrorFromPeer::Client(Error::AlertReceived(AlertDescription::AccessDenied)), + ]), ); } } @@ -203,14 +204,14 @@ fn client_verifier_no_auth_yes_root() { let mut client = ClientConnection::new(Arc::new(client_config), dns_name("localhost")).unwrap(); let errs = do_handshake_until_both_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( errs, Err(vec![ ErrorFromPeer::Server(Error::NoCertificatesPresented), ErrorFromPeer::Client(Error::AlertReceived( - AlertDescription::CertificateRequired - )) - ]) + AlertDescription::CertificateRequired, + )), + ]), ); } } @@ -242,9 +243,9 @@ fn client_verifier_fails_properly() { let mut client = ClientConnection::new(Arc::new(client_config), dns_name("localhost")).unwrap(); let err = do_handshake_until_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( err, - Err(ErrorFromPeer::Server(Error::General("test err".into()))) + Err(ErrorFromPeer::Server(Error::General("test err".into()))), ); } } @@ -276,14 +277,14 @@ fn client_verifier_must_determine_client_auth_requirement_to_continue() { let mut client = ClientConnection::new(Arc::new(client_config), dns_name("localhost")).unwrap(); let errs = do_handshake_until_both_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( errs, Err(vec![ ErrorFromPeer::Server(Error::General( - "client rejected by client_auth_mandatory".into() + "client rejected by client_auth_mandatory".into(), )), - ErrorFromPeer::Client(Error::AlertReceived(AlertDescription::AccessDenied)) - ]) + ErrorFromPeer::Client(Error::AlertReceived(AlertDescription::AccessDenied)), + ]), ); } } diff --git a/rustls/tests/common/mod.rs b/rustls/tests/common/mod.rs index 63e6ba8a..7e589720 100644 --- a/rustls/tests/common/mod.rs +++ b/rustls/tests/common/mod.rs @@ -397,7 +397,7 @@ pub fn do_handshake( (to_server, to_client) } -#[derive(PartialEq, Debug)] +#[derive(Debug)] pub enum ErrorFromPeer { Client(Error), Server(Error), @@ -471,3 +471,10 @@ impl io::Read for FailsReads { Err(io::Error::from(self.errkind)) } } + +pub fn assert_debug_eq(err: T, expect: T) +where + T: std::fmt::Debug, +{ + assert_eq!(format!("{:?}", err), format!("{:?}", expect)); +} diff --git a/rustls/tests/server_cert_verifier.rs b/rustls/tests/server_cert_verifier.rs index bbe130b4..097f5e42 100644 --- a/rustls/tests/server_cert_verifier.rs +++ b/rustls/tests/server_cert_verifier.rs @@ -4,7 +4,7 @@ mod common; use crate::common::{ - do_handshake, do_handshake_until_both_error, make_client_config_with_versions, + assert_debug_eq, do_handshake, do_handshake_until_both_error, make_client_config_with_versions, make_pair_for_arc_configs, make_server_config, ErrorFromPeer, ALL_KEY_TYPES, }; use rustls::client::{ @@ -53,12 +53,12 @@ fn client_can_override_certificate_verification_and_reject_certificate() { let (mut client, mut server) = make_pair_for_arc_configs(&Arc::new(client_config), &server_config); let errs = do_handshake_until_both_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( errs, Err(vec![ ErrorFromPeer::Client(Error::CorruptMessage), - ErrorFromPeer::Server(Error::AlertReceived(AlertDescription::BadCertificate)) - ]) + ErrorFromPeer::Server(Error::AlertReceived(AlertDescription::BadCertificate)), + ]), ); } } @@ -82,12 +82,12 @@ fn client_can_override_certificate_verification_and_reject_tls12_signatures() { let (mut client, mut server) = make_pair_for_arc_configs(&Arc::new(client_config), &server_config); let errs = do_handshake_until_both_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( errs, Err(vec![ ErrorFromPeer::Client(Error::CorruptMessage), - ErrorFromPeer::Server(Error::AlertReceived(AlertDescription::BadCertificate)) - ]) + ErrorFromPeer::Server(Error::AlertReceived(AlertDescription::BadCertificate)), + ]), ); } } @@ -109,12 +109,12 @@ fn client_can_override_certificate_verification_and_reject_tls13_signatures() { let (mut client, mut server) = make_pair_for_arc_configs(&Arc::new(client_config), &server_config); let errs = do_handshake_until_both_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( errs, Err(vec![ ErrorFromPeer::Client(Error::CorruptMessage), - ErrorFromPeer::Server(Error::AlertReceived(AlertDescription::BadCertificate)) - ]) + ErrorFromPeer::Server(Error::AlertReceived(AlertDescription::BadCertificate)), + ]), ); } } @@ -135,14 +135,14 @@ fn client_can_override_certificate_verification_and_offer_no_signature_schemes() let (mut client, mut server) = make_pair_for_arc_configs(&Arc::new(client_config), &server_config); let errs = do_handshake_until_both_error(&mut client, &mut server); - assert_eq!( + assert_debug_eq( errs, Err(vec![ ErrorFromPeer::Server(Error::PeerIncompatible( - rustls::PeerIncompatible::NoSignatureSchemesInCommon + rustls::PeerIncompatible::NoSignatureSchemesInCommon, )), ErrorFromPeer::Client(Error::AlertReceived(AlertDescription::HandshakeFailure)), - ]) + ]), ); } }