Rework certificate errors

rustls now has its own `CertificateError` type that aims to cover
most common errors with certificate validation.
This commit is contained in:
Joseph Birr-Pixton 2023-01-24 15:56:20 +00:00
parent 600a8f2a32
commit a1ee6d28ac
10 changed files with 99 additions and 61 deletions

View File

@ -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
```

View File

@ -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:",

View File

@ -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();
}

View File

@ -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);

View File

@ -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))),
};
}

View File

@ -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<ClientConnectionData> 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(_) => {

View File

@ -60,17 +60,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),
@ -222,6 +216,51 @@ impl From<PeerIncompatible> for Error {
}
}
#[non_exhaustive]
#[derive(Debug, PartialEq, 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.
Other,
}
impl From<CertificateError> for Error {
#[inline]
fn from(e: CertificateError) -> Self {
Self::InvalidCertificate(e)
}
}
fn join<T: fmt::Debug>(items: &[T]) -> String {
items
.iter()
@ -257,17 +296,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 +358,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,

View File

@ -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;

View File

@ -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,22 @@ 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
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(),
e => {
crate::log::warn!(
"webpki error {:?} being converted to CertificateError::Other",
e
);
CertificateError::Other.into()
}
e => Error::InvalidCertificateData(format!("invalid peer certificate: {}", e)),
}
}

View File

@ -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};
@ -875,8 +878,8 @@ fn client_checks_server_certificate_with_given_name() {
let err = do_handshake_until_error(&mut client, &mut server);
assert_eq!(
err,
Err(ErrorFromPeer::Client(Error::InvalidCertificateData(
"invalid peer certificate: CertNotValidForName".into(),
Err(ErrorFromPeer::Client(Error::InvalidCertificate(
CertificateError::NotValidForName
)))
);
}