Skip to content

rework dns_names helper, remove alloc req. #178

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.

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
merged 22 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4e2ef1b
subject_name: remove comment about required feature
cpu Sep 13, 2023
839cd93
lib: remove unneeded manual docsrs cfg_attr
cpu Sep 13, 2023
0682cec
tests: remove unnecessary pub visibility
cpu Sep 14, 2023
12cd0ea
tests: rename data -> cert_der in expect_cert_dns_names
cpu Sep 14, 2023
f5ace6d
end_entity: rework dns_names to return iter of &str
cpu Sep 18, 2023
de8b2ec
tests: simplify `expect_cert_dns_names`
cpu Sep 18, 2023
8d60950
tests: move `no_subject_alt_names` test above helper
cpu Sep 18, 2023
2350c2e
tests: rework `no_subject_alt_names` test
cpu Sep 18, 2023
22cebce
tests: clean up unneeded bindings
cpu Sep 14, 2023
7ffbabe
end_entity: fix `dns_names` rustdoc comment
cpu Sep 14, 2023
8bf4fa9
end_entity: make `dns_names` infallible
cpu Sep 18, 2023
807df74
lib: rm alloc req. for `dns_names`
cpu Sep 18, 2023
be4dba6
subject_name: tidy up `list_cert_dns_names`
cpu Sep 18, 2023
4332b8c
signed_data: fix code block missing close marker
cpu Sep 18, 2023
415c387
Move InvalidDnsNameError definition down below usage
djc Sep 20, 2023
7aa0f8d
Move GeneralDnsNameRef definition to the top
djc Sep 20, 2023
d0522c0
Refactor faux-bool enum definition for AllowWildcards
djc Sep 20, 2023
b11327b
Use as_str() method to reference DnsNameRef contents
djc Sep 20, 2023
abf970c
Use as_str() method to reference WildcardDnsNameRef contents
djc Sep 20, 2023
f470c07
Use as_str() method to reference GeneralDnsNameRef contents
djc Sep 20, 2023
1e92abf
Remove API that is now unused
djc Sep 20, 2023
64fb1b7
cert: move `list_cert_dns_names` to `Cert`
cpu Sep 20, 2023
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
23 changes: 23 additions & 0 deletions src/cert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
use crate::der::{self, DerIterator, FromDer, Tag, CONSTRUCTED, CONTEXT_SPECIFIC};
use crate::error::{DerTypeId, Error};
use crate::signed_data::SignedData;
use crate::subject_name::{GeneralName, NameIterator, WildcardDnsNameRef};
use crate::x509::{remember_extension, set_extension_once, DistributionPointName, Extension};
use crate::DnsNameRef;

/// A parsed X509 certificate.
pub struct Cert<'a> {
Expand Down Expand Up @@ -136,6 +138,27 @@ impl<'a> Cert<'a> {
self.subject.as_slice_less_safe()
}

pub(crate) fn valid_dns_names(&self) -> impl Iterator<Item = &str> {
NameIterator::new(Some(self.subject), self.subject_alt_name).filter_map(|result| {
let presented_id = match result.ok()? {
GeneralName::DnsName(presented) => presented,
_ => return None,
};

// if the name could be converted to a DNS name, return it; otherwise,
// keep going.
match DnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()) {
Ok(dns_name) => Some(dns_name.as_str()),
Err(_) => {
match WildcardDnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()) {
Ok(wildcard_dns_name) => Some(wildcard_dns_name.as_str()),
Err(_) => None,
}
}
}
})
}

/// Returns an iterator over the certificate's cRLDistributionPoints extension values, if any.
pub(crate) fn crl_distribution_points(
&self,
Expand Down
21 changes: 8 additions & 13 deletions src/end_entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ use pki_types::{CertificateDer, SignatureVerificationAlgorithm, TrustAnchor, Uni

use crate::crl::RevocationOptions;
use crate::error::Error;
#[cfg(feature = "alloc")]
use crate::subject_name::GeneralDnsNameRef;
use crate::subject_name::{self, SubjectNameRef};
use crate::verify_cert::{self, KeyUsage};
use crate::{cert, signed_data};
Expand Down Expand Up @@ -150,14 +148,13 @@ impl<'a> EndEntityCert<'a> {
)
}

/// Returns a list of the DNS names provided in the subject alternative names extension
/// Returns a list of valid DNS names provided in the subject alternative names extension
///
/// This function must not be used to implement custom DNS name verification.
/// Verification functions are already provided as `verify_is_valid_for_dns_name`
/// and `verify_is_valid_for_at_least_one_dns_name`.
#[cfg(feature = "alloc")]
pub fn dns_names(&'a self) -> Result<impl Iterator<Item = GeneralDnsNameRef<'a>>, Error> {
subject_name::list_cert_dns_names(self)
/// Checking that a certificate is valid for a given subject name should always be done with
/// [EndEntityCert::verify_is_valid_for_subject_name].
pub fn dns_names(&'a self) -> impl Iterator<Item = &'a str> {
self.inner.valid_dns_names()
}
}

Expand Down Expand Up @@ -214,10 +211,8 @@ mod tests {
let cert =
EndEntityCert::try_from(der).expect("should parse end entity certificate correctly");

let mut names = cert
.dns_names()
.expect("should get all DNS names correctly for end entity cert");
assert_eq!(names.next().map(<&str>::from), Some(name));
assert_eq!(names.next().map(<&str>::from), None);
let mut names = cert.dns_names();
assert_eq!(names.next(), Some(name));
assert_eq!(names.next(), None);
}
}
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ pub use {

pub use pki_types as types;

#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
#[cfg(feature = "alloc")]
pub use {
crl::{OwnedCertRevocationList, OwnedRevokedCert},
Expand Down
1 change: 1 addition & 0 deletions src/signed_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ impl<'a> SignedData<'a> {
/// signatureAlgorithm AlgorithmIdentifier,
/// signatureValue BIT STRING
/// }
/// ```
///
/// OCSP responses (RFC 6960) look like this:
/// ```ASN.1
Expand Down
117 changes: 35 additions & 82 deletions src/subject_name/dns_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,37 +63,14 @@ impl AsRef<str> for DnsName {
#[derive(Clone, Copy, Eq, PartialEq, Hash)]
pub struct DnsNameRef<'a>(pub(crate) &'a [u8]);

impl AsRef<str> for DnsNameRef<'_> {
#[inline]
fn as_ref(&self) -> &str {
// The unwrap won't fail because DnsNameRef are guaranteed to be ASCII
// and ASCII is a subset of UTF-8.
core::str::from_utf8(self.0).unwrap()
}
}

/// An error indicating that a `DnsNameRef` could not built because the input
/// is not a syntactically-valid DNS Name.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct InvalidDnsNameError;

impl core::fmt::Display for InvalidDnsNameError {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
write!(f, "{:?}", self)
}
}

#[cfg(feature = "std")]
impl ::std::error::Error for InvalidDnsNameError {}

impl<'a> DnsNameRef<'a> {
/// Constructs a `DnsNameRef` from the given input if the input is a
/// syntactically-valid DNS name.
pub fn try_from_ascii(dns_name: &'a [u8]) -> Result<Self, InvalidDnsNameError> {
if !is_valid_dns_id(
untrusted::Input::from(dns_name),
IdRole::Reference,
AllowWildcards::No,
Wildcards::Deny,
) {
return Err(InvalidDnsNameError);
}
Expand All @@ -110,10 +87,15 @@ impl<'a> DnsNameRef<'a> {
/// Constructs a `DnsName` from this `DnsNameRef`
#[cfg(feature = "alloc")]
pub fn to_owned(&self) -> DnsName {
// DnsNameRef is already guaranteed to be valid ASCII, which is a
// subset of UTF-8.
let s: &str = (*self).into();
DnsName(s.to_ascii_lowercase())
// DnsNameRef is already guaranteed to be valid ASCII, which is subset of UTF-8.
DnsName(self.as_str().to_ascii_lowercase())
}

/// Yields a reference to the DNS name as a `&str`.
pub fn as_str(&self) -> &'a str {
// The unwrap won't fail because `DnsNameRef` values are guaranteed to be ASCII and ASCII
// is a subset of UTF-8.
core::str::from_utf8(self.0).unwrap()
}
}

Expand All @@ -132,33 +114,6 @@ impl core::fmt::Debug for DnsNameRef<'_> {
}
}

impl<'a> From<DnsNameRef<'a>> for &'a str {
fn from(DnsNameRef(d): DnsNameRef<'a>) -> Self {
// The unwrap won't fail because DnsNameRefs are guaranteed to be ASCII
// and ASCII is a subset of UTF-8.
core::str::from_utf8(d).unwrap()
}
}

/// A DNS name that may be either a DNS name identifier presented by a server (which may include
/// wildcards), or a DNS name identifier referenced by a client for matching purposes (wildcards
/// not permitted).
pub enum GeneralDnsNameRef<'name> {
/// a reference to a DNS name that may be used for matching purposes.
DnsName(DnsNameRef<'name>),
/// a reference to a presented DNS name that may include a wildcard.
Wildcard(WildcardDnsNameRef<'name>),
}

impl<'a> From<GeneralDnsNameRef<'a>> for &'a str {
fn from(d: GeneralDnsNameRef<'a>) -> Self {
match d {
GeneralDnsNameRef::DnsName(name) => name.into(),
GeneralDnsNameRef::Wildcard(name) => name.into(),
}
}
}

/// A reference to a DNS Name presented by a server that may include a wildcard.
///
/// A `WildcardDnsNameRef` is guaranteed to be syntactically valid. The validity rules
Expand All @@ -172,27 +127,28 @@ impl<'a> From<GeneralDnsNameRef<'a>> for &'a str {
/// [RFC 5280 Section 7.2]: https://tools.ietf.org/html/rfc5280#section-7.2
/// [RFC 6125 Section 4.1]: https://www.rfc-editor.org/rfc/rfc6125#section-4.1
#[derive(Clone, Copy, Eq, PartialEq, Hash)]
pub struct WildcardDnsNameRef<'a>(&'a [u8]);
pub(crate) struct WildcardDnsNameRef<'a>(&'a [u8]);

impl<'a> WildcardDnsNameRef<'a> {
/// Constructs a `WildcardDnsNameRef` from the given input if the input is a
/// syntactically-valid DNS name.
pub fn try_from_ascii(dns_name: &'a [u8]) -> Result<Self, InvalidDnsNameError> {
pub(crate) fn try_from_ascii(dns_name: &'a [u8]) -> Result<Self, InvalidDnsNameError> {
if !is_valid_dns_id(
untrusted::Input::from(dns_name),
IdRole::Reference,
AllowWildcards::Yes,
Wildcards::Allow,
) {
return Err(InvalidDnsNameError);
}

Ok(Self(dns_name))
}

/// Constructs a `WildcardDnsNameRef` from the given input if the input is a
/// syntactically-valid DNS name.
pub fn try_from_ascii_str(dns_name: &'a str) -> Result<Self, InvalidDnsNameError> {
Self::try_from_ascii(dns_name.as_bytes())
/// Yields a reference to the DNS name as a `&str`.
pub(crate) fn as_str(&self) -> &'a str {
// The unwrap won't fail because a `WildcardDnsNameRef` is guaranteed to be ASCII and
// ASCII is a subset of UTF-8.
core::str::from_utf8(self.0).unwrap()
}
}

Expand All @@ -211,23 +167,20 @@ impl core::fmt::Debug for WildcardDnsNameRef<'_> {
}
}

impl<'a> From<WildcardDnsNameRef<'a>> for &'a str {
fn from(WildcardDnsNameRef(d): WildcardDnsNameRef<'a>) -> Self {
// The unwrap won't fail because WildcardDnsNameRef are guaranteed to be ASCII
// and ASCII is a subset of UTF-8.
core::str::from_utf8(d).unwrap()
}
}
/// An error indicating that a `DnsNameRef` could not built because the input
/// is not a syntactically-valid DNS Name.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct InvalidDnsNameError;

impl AsRef<str> for WildcardDnsNameRef<'_> {
#[inline]
fn as_ref(&self) -> &str {
// The unwrap won't fail because WildcardDnsNameRef are guaranteed to be ASCII
// and ASCII is a subset of UTF-8.
core::str::from_utf8(self.0).unwrap()
impl core::fmt::Display for InvalidDnsNameError {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
write!(f, "{:?}", self)
}
}

#[cfg(feature = "std")]
impl ::std::error::Error for InvalidDnsNameError {}

pub(super) fn presented_id_matches_reference_id(
presented_dns_id: untrusted::Input,
reference_dns_id: untrusted::Input,
Expand Down Expand Up @@ -371,11 +324,11 @@ fn presented_id_matches_reference_id_internal(
reference_dns_id_role: IdRole,
reference_dns_id: untrusted::Input,
) -> Result<bool, Error> {
if !is_valid_dns_id(presented_dns_id, IdRole::Presented, AllowWildcards::Yes) {
if !is_valid_dns_id(presented_dns_id, IdRole::Presented, Wildcards::Allow) {
return Err(Error::MalformedDnsIdentifier);
}

if !is_valid_dns_id(reference_dns_id, reference_dns_id_role, AllowWildcards::No) {
if !is_valid_dns_id(reference_dns_id, reference_dns_id_role, Wildcards::Deny) {
return Err(match reference_dns_id_role {
IdRole::NameConstraint => Error::MalformedNameConstraint,
_ => Error::MalformedDnsIdentifier,
Expand Down Expand Up @@ -506,9 +459,9 @@ fn ascii_lower(b: u8) -> u8 {
}

#[derive(Clone, Copy, PartialEq)]
enum AllowWildcards {
No,
Yes,
enum Wildcards {
Deny,
Allow,
}

#[derive(Clone, Copy, PartialEq)]
Expand All @@ -531,7 +484,7 @@ enum IdRole {
fn is_valid_dns_id(
hostname: untrusted::Input,
id_role: IdRole,
allow_wildcards: AllowWildcards,
allow_wildcards: Wildcards,
) -> bool {
// https://blogs.msdn.microsoft.com/oldnewthing/20120412-00/?p=7873/
if hostname.len() > 253 {
Expand All @@ -552,7 +505,7 @@ fn is_valid_dns_id(
// Only presented IDs are allowed to have wildcard labels. And, like
// Chromium, be stricter than RFC 6125 requires by insisting that a
// wildcard label consist only of '*'.
let is_wildcard = allow_wildcards == AllowWildcards::Yes && input.peek(b'*');
let is_wildcard = allow_wildcards == Wildcards::Allow && input.peek(b'*');
let mut is_first_byte = !is_wildcard;
if is_wildcard {
if input.read_byte() != Ok(b'*') || input.read_byte() != Ok(b'.') {
Expand Down
10 changes: 4 additions & 6 deletions src/subject_name/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

mod dns_name;
#[cfg(feature = "alloc")]
pub(crate) use dns_name::GeneralDnsNameRef;
pub(super) use dns_name::WildcardDnsNameRef;
pub use dns_name::{DnsNameRef, InvalidDnsNameError};

/// Requires the `alloc` feature.
#[cfg(feature = "alloc")]
pub use dns_name::DnsName;

Expand All @@ -31,6 +29,6 @@ pub use ip_address::{AddrParseError, IpAddrRef};
pub use ip_address::IpAddr;

mod verify;
#[cfg(feature = "alloc")]
pub(super) use verify::list_cert_dns_names;
pub(super) use verify::{check_name_constraints, verify_cert_subject_name, GeneralName};
pub(super) use verify::{
check_name_constraints, verify_cert_subject_name, GeneralName, NameIterator,
};
Loading