Skip to content

Expand certificate validation error message improvements #18933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zanieb merged 1 commit into from
Apr 9, 2026
+133 −84
Merged

Expand certificate validation error message improvements #18933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
209 changes: 130 additions & 79 deletions crates/uv-client/src/tls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,71 @@ pub(crate) struct DiagnosticCertificate(CertificateDer<'static>);

impl DiagnosticCertificate {
fn parse(&self) -> Option<X509Certificate<'_>> {
let (_, certificate) = X509Certificate::from_der(self.0.as_ref()).ok()?;
Some(certificate)
match X509Certificate::from_der(self.0.as_ref()) {
Ok((_, certificate)) => Some(certificate),
Err(err) => {
debug!("Failed to parse certificate for improved validation message: {err:?}");
None
}
}
}
}

#[derive(Debug)]
pub(crate) enum TlsConfigurationError {
UnsupportedCriticalExtension {
source: CertificateSource,
certificate: DiagnosticCertificate,
},
InvalidTrustAnchor {
source: CertificateSource,
certificate: DiagnosticCertificate,
error: WebPkiError,
},
pub(crate) struct TlsConfigurationError {
source: CertificateSource,
certificate: DiagnosticCertificate,
reason: InvalidTrustAnchorReason,
}

#[derive(Debug)]
pub(crate) enum InvalidTrustAnchorReason {
UnsupportedCriticalExtension,
BadDer,
BadDerTime,
EmptyEkuExtension,
ExtensionValueInvalid,
MalformedExtensions,
TrailingData,
UnsupportedCertVersion,
Other(WebPkiError),
}

impl InvalidTrustAnchorReason {
fn from_webpki_error(error: WebPkiError) -> Self {
match error {
WebPkiError::UnsupportedCriticalExtension => Self::UnsupportedCriticalExtension,
WebPkiError::BadDer => Self::BadDer,
WebPkiError::BadDerTime => Self::BadDerTime,
WebPkiError::EmptyEkuExtension => Self::EmptyEkuExtension,
WebPkiError::ExtensionValueInvalid => Self::ExtensionValueInvalid,
WebPkiError::MalformedExtensions => Self::MalformedExtensions,
WebPkiError::TrailingData(_) => Self::TrailingData,
WebPkiError::UnsupportedCertVersion => Self::UnsupportedCertVersion,
error => Self::Other(error),
}
}

fn message(&self) -> Option<&'static str> {
match self {
Self::UnsupportedCriticalExtension => None,
Self::BadDer => Some("malformed DER certificate"),
Self::BadDerTime => Some("malformed certificate time"),
Self::EmptyEkuExtension => Some("empty extended key usage extension"),
Self::ExtensionValueInvalid => Some("invalid certificate extension value"),
Self::MalformedExtensions => Some("malformed certificate extensions"),
Self::TrailingData => Some("trailing data in DER certificate"),
Self::UnsupportedCertVersion => Some("unsupported certificate version"),
Self::Other(_) => None,
}
}

fn error(&self) -> Option<&WebPkiError> {
match self {
Self::Other(error) => Some(error),
_ => None,
}
}
}

impl TlsConfigurationError {
Expand All @@ -65,88 +114,90 @@ impl TlsConfigurationError {
error: WebPkiError,
cert: &CertificateDer<'_>,
) -> Self {
let certificate = DiagnosticCertificate(cert.clone().into_owned());
match error {
WebPkiError::UnsupportedCriticalExtension => Self::UnsupportedCriticalExtension {
source,
certificate,
},
error => Self::InvalidTrustAnchor {
source,
certificate,
error,
},
Self {
source,
certificate: DiagnosticCertificate(cert.clone().into_owned()),
reason: InvalidTrustAnchorReason::from_webpki_error(error),
}
}
}

fn format_trust_anchor_detail(
reason: &InvalidTrustAnchorReason,
certificate: Option<&X509Certificate<'_>>,
) -> Option<String> {
match reason {
InvalidTrustAnchorReason::UnsupportedCertVersion => certificate.map(|certificate| {
format!(
"unsupported certificate version `{}`",
certificate.version()
)
}),
InvalidTrustAnchorReason::ExtensionValueInvalid => None,
_ => None,
}
}

impl Display for TlsConfigurationError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::UnsupportedCriticalExtension {
source,
certificate,
} => {
write!(
f,
"certificate in `{}` (from `{}`) uses an unsupported critical extension",
source.path().simplified_display(),
source.env_var()
)?;
if let Some(certificate) = certificate.parse() {
let subject = certificate.subject();
if subject.iter_attributes().next().is_some() {
write!(f, " on certificate `{subject}`")?;
}
let critical_extensions = certificate
.iter_extensions()
.filter(|extension| extension.critical)
.map(|extension| extension.oid.to_owned())
.collect::<Vec<_>>();
if let [critical_extension] = critical_extensions.as_slice() {
write!(f, "; critical extension: `{critical_extension}`")?;
} else if !critical_extensions.is_empty() {
write!(
f,
"; critical extensions: {}",
critical_extensions
.iter()
.map(|oid| format!("`{oid}`"))
.join(", ")
)?;
}
}
Ok(())
write!(
f,
"certificate in `{}` (from `{}`) ",
self.source.path().simplified_display(),
self.source.env_var()
)?;
match &self.reason {
InvalidTrustAnchorReason::UnsupportedCriticalExtension => {
write!(f, "uses an unsupported critical extension")?;
}
_ => {
write!(f, "could not be used as a trust anchor")?;
}
Self::InvalidTrustAnchor {
source,
certificate,
..
} => {
write!(
f,
"certificate in `{}` (from `{}`) could not be used as a trust anchor",
source.path().simplified_display(),
source.env_var()
)?;
if let Some(certificate) = certificate.parse() {
let subject = certificate.subject();
if subject.iter_attributes().next().is_some() {
write!(f, " on certificate `{subject}`")?;
}
}

let parsed_certificate = self.certificate.parse();
if let Some(certificate) = parsed_certificate.as_ref() {
let subject = certificate.subject();
if subject.iter_attributes().next().is_some() {
// Avoid rendering empty subject DNs.
write!(f, " on certificate `{subject}`")?;
}
if let InvalidTrustAnchorReason::UnsupportedCriticalExtension = &self.reason {
let critical_extensions = certificate
.iter_extensions()
.filter(|extension| extension.critical)
.map(|extension| extension.oid.to_owned())
.collect::<Vec<_>>();
if let [critical_extension] = critical_extensions.as_slice() {
write!(f, "; critical extension: `{critical_extension}`")?;
} else if !critical_extensions.is_empty() {
write!(
f,
"; critical extensions: {}",
critical_extensions
.iter()
.map(|oid| format!("`{oid}`"))
.join(", ")
)?;
}
Ok(())
}
}

let detailed_reason = format_trust_anchor_detail(&self.reason, parsed_certificate.as_ref())
.or_else(|| self.reason.message().map(str::to_owned));
if let Some(detailed_reason) = detailed_reason {
write!(f, ": {detailed_reason}")?;
}

Ok(())
}
}

impl std::error::Error for TlsConfigurationError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Self::UnsupportedCriticalExtension { .. } => None,
Self::InvalidTrustAnchor { error, .. } => Some(error),
}
self.reason
.error()
.map(|error| error as &(dyn std::error::Error + 'static))
}
}

Expand Down
8 changes: 3 additions & 5 deletions crates/uv-client/tests/it/ssl_certs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ Caused by: certificate in `[TMP]/ca.pem` (from `SSL_CERT_FILE`) uses an unsuppor
}

/// An invalid trust anchor in `SSL_CERT_FILE` returns a builder error with a
/// generic trust-anchor message.
/// more specific validation message.
#[tokio::test]
async fn test_ssl_cert_file_invalid_trust_anchor_returns_error() -> Result<()> {
let cert = TestCertificate::new_with_duplicate_basic_constraints_ca_extension()?;
Expand All @@ -524,16 +524,14 @@ async fn test_ssl_cert_file_invalid_trust_anchor_returns_error() -> Result<()> {
.expect_err("expected client build to fail");

let source = err.source().expect("expected client build error source");
let next_source = source.source().expect("expected trust anchor validation cause");
let display = format!("{err}\nCaused by: {source}\nCaused by: {next_source}");
let display = format!("{err}\nCaused by: {source}");

with_settings!({
filters => vec![(temp_dir_filter.as_str(), "[TMP]/")]
}, {
assert_snapshot!(display, @r#"
failed to build HTTP client
Caused by: certificate in `[TMP]/ca.pem` (from `SSL_CERT_FILE`) could not be used as a trust anchor on certificate `CN=uv-test-ca, O=Astral Software Inc.`
Caused by: ExtensionValueInvalid
Caused by: certificate in `[TMP]/ca.pem` (from `SSL_CERT_FILE`) could not be used as a trust anchor on certificate `CN=uv-test-ca, O=Astral Software Inc.`: invalid certificate extension value
"#);
});
})
Expand Down