Skip to content

Commit

Permalink
Add context in name validation errors
Browse files Browse the repository at this point in the history
  • Loading branch information
djc committed Dec 22, 2024
1 parent 33d3d30 commit c821e9b
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 31 deletions.
21 changes: 21 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@

use core::fmt;
use core::ops::ControlFlow;
#[cfg(feature = "alloc")]
use alloc::vec::Vec;
#[cfg(feature = "alloc")]
use alloc::string::String;

use pki_types::ServerName;

/// An error that occurs during certificate validation or name validation.
#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -127,6 +133,20 @@ pub enum Error {
/// Trailing data was found while parsing DER-encoded input for the named type.
TrailingData(DerTypeId),

/// The certificate is not valid for the name it is being validated for.
///
/// This variant is only used when `alloc` is enabled. When `alloc` is disabled,
/// `CertNotValidForName` is used instead.
#[cfg(feature = "alloc")]
UnexpectedCertName {
/// Expected server name.
expected: ServerName<'static>,
/// The names presented in the end entity certificate.
///
/// Unlike `expected`, these names might contain wildcard labels.
presented: Vec<String>,
},

/// A valid issuer for the certificate could not be found.
UnknownIssuer,

Expand Down Expand Up @@ -225,6 +245,7 @@ impl Error {
Self::PathLenConstraintViolated => 220,
Self::CaUsedAsEndEntity | Self::EndEntityUsedAsCa => 210,
Self::IssuerNotCrlSigner => 200,
Self::UnexpectedCertName { .. } => 280,

// Errors related to supported features used in an invalid way.
Self::InvalidCertValidity => 190,
Expand Down
59 changes: 43 additions & 16 deletions src/subject_name/dns_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,61 @@
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

#[cfg(feature = "alloc")]
use alloc::string::String;
use core::fmt::Write;

#[cfg(feature = "alloc")]
use pki_types::ServerName;
use pki_types::{DnsName, InvalidDnsNameError};

use super::verify::{GeneralName, NameIterator};
use crate::{Cert, Error};

pub(crate) fn verify_dns_names(reference: &DnsName<'_>, cert: &Cert<'_>) -> Result<(), Error> {
let dns_name = untrusted::Input::from(reference.as_ref().as_bytes());
NameIterator::new(Some(cert.subject), cert.subject_alt_name)
.find_map(|result| {
let name = match result {
Ok(name) => name,
Err(err) => return Some(Err(err)),
};
let result = NameIterator::new(Some(cert.subject), cert.subject_alt_name).find_map(|result| {
let name = match result {
Ok(name) => name,
Err(err) => return Some(Err(err)),
};

let presented_id = match name {
GeneralName::DnsName(presented) => presented,
_ => return None,
};
let presented_id = match name {
GeneralName::DnsName(presented) => presented,
_ => return None,
};

match presented_id_matches_reference_id(presented_id, IdRole::Reference, dns_name) {
Ok(true) => Some(Ok(())),
Ok(false) | Err(Error::MalformedDnsIdentifier) => None,
Err(e) => Some(Err(e)),
}
match presented_id_matches_reference_id(presented_id, IdRole::Reference, dns_name) {
Ok(true) => Some(Ok(())),
Ok(false) | Err(Error::MalformedDnsIdentifier) => None,
Err(e) => Some(Err(e)),
}
});

match result {
Some(result) => return result,
#[cfg(feature = "alloc")]
None => {}
#[cfg(not(feature = "alloc"))]
None => return Err(Error::CertNotValidForName),
}

// Try to yield a more useful error. To avoid allocating on the happy path,
// we reconstruct the same `NameIterator` and replay it.
#[cfg(feature = "alloc")]
{
Err(Error::UnexpectedCertName {
expected: ServerName::DnsName(reference.to_owned()),
presented: NameIterator::new(Some(cert.subject), cert.subject_alt_name)
.filter_map(|result| match result.ok()? {
GeneralName::DnsName(presented) => {
String::from_utf8(presented.as_slice_less_safe().to_vec()).ok()
}
_ => return None,
})
.collect(),
})
.unwrap_or(Err(Error::CertNotValidForName))
}
}

/// A reference to a DNS Name presented by a server that may include a wildcard.
Expand Down
56 changes: 41 additions & 15 deletions src/subject_name/ip_address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

#[cfg(feature = "alloc")]
use alloc::string::String;

use pki_types::IpAddr;
#[cfg(feature = "alloc")]
use pki_types::ServerName;

use super::verify::{GeneralName, NameIterator};
use crate::{Cert, Error};
Expand All @@ -23,24 +28,45 @@ pub(crate) fn verify_ip_address_names(reference: &IpAddr, cert: &Cert<'_>) -> Re
IpAddr::V6(ip) => untrusted::Input::from(ip.as_ref()),
};

NameIterator::new(None, cert.subject_alt_name)
.find_map(|result| {
let name = match result {
Ok(name) => name,
Err(err) => return Some(Err(err)),
};
let result = NameIterator::new(None, cert.subject_alt_name).find_map(|result| {
let name = match result {
Ok(name) => name,
Err(err) => return Some(Err(err)),
};

let presented_id = match name {
GeneralName::IpAddress(presented) => presented,
_ => return None,
};
let presented_id = match name {
GeneralName::IpAddress(presented) => presented,
_ => return None,
};

match presented_id_matches_reference_id(presented_id, ip_address) {
true => Some(Ok(())),
false => None,
}
match presented_id_matches_reference_id(presented_id, ip_address) {
true => Some(Ok(())),
false => None,
}
});

match result {
Some(result) => return result,
#[cfg(feature = "alloc")]
None => {}
#[cfg(not(feature = "alloc"))]
None => Err(Error::CertNotValidForName),
}

#[cfg(feature = "alloc")]
{
Err(Error::UnexpectedCertName {
expected: ServerName::from(*reference),
presented: NameIterator::new(None, cert.subject_alt_name)
.filter_map(|result| match result.ok()? {
GeneralName::IpAddress(presented) => {
String::from_utf8(presented.as_slice_less_safe().to_vec()).ok()
}
_ => None,
})
.collect(),
})
.unwrap_or(Err(Error::CertNotValidForName))
}
}

// https://tools.ietf.org/html/rfc5280#section-4.2.1.6 says:
Expand Down

0 comments on commit c821e9b

Please sign in to comment.