Skip to content

Clean up name api #182

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

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 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
cd7bbfd
end_entity: make `dns_names` infallible
cpu Sep 18, 2023
5f4ec11
lib: rm alloc req. for `dns_names`
cpu Sep 18, 2023
9754177
subject_name: tidy up `list_cert_dns_names`
cpu Sep 18, 2023
006f878
signed_data: fix code block missing close marker
cpu Sep 18, 2023
d8c05e1
Move InvalidDnsNameError definition down below usage
djc Sep 20, 2023
221fd4a
Move GeneralDnsNameRef definition to the top
djc Sep 20, 2023
4e169bb
Refactor faux-bool enum definition for AllowWildcards
djc Sep 20, 2023
51f23a4
Use as_str() method to reference DnsNameRef contents
djc Sep 20, 2023
941589c
Use as_str() method to reference WildcardDnsNameRef contents
djc Sep 20, 2023
d5bc8e9
Use as_str() method to reference GeneralDnsNameRef contents
djc Sep 20, 2023
27a1234
Remove API that is now unused
djc 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
17 changes: 6 additions & 11 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 @@ -153,10 +151,9 @@ impl<'a> EndEntityCert<'a> {
/// Returns a list of the 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> {
/// 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> {
subject_name::list_cert_dns_names(self)
}
}
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
9 changes: 3 additions & 6 deletions src/subject_name/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,8 @@
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

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

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

Expand All @@ -31,6 +28,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, list_cert_dns_names, verify_cert_subject_name, GeneralName,
};
60 changes: 18 additions & 42 deletions src/subject_name/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@
// 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::vec::Vec;

use super::dns_name::{self, DnsNameRef};
#[cfg(feature = "alloc")]
use super::dns_name::{GeneralDnsNameRef, WildcardDnsNameRef};
use super::dns_name::{self, DnsNameRef, WildcardDnsNameRef};
use super::ip_address::{self, IpAddrRef};
use super::name::SubjectNameRef;
use crate::der::{self, FromDer};
Expand All @@ -29,7 +24,7 @@ pub(crate) fn verify_cert_dns_name(
dns_name: DnsNameRef,
) -> Result<(), Error> {
let cert = cert.inner();
let dns_name = untrusted::Input::from(dns_name.as_ref().as_bytes());
let dns_name = untrusted::Input::from(dns_name.as_str().as_bytes());
NameIterator::new(Some(cert.subject), cert.subject_alt_name)
.find_map(|result| {
let name = match result {
Expand Down Expand Up @@ -317,45 +312,26 @@ impl<'a> Iterator for NameIterator<'a> {
}
}

#[cfg(feature = "alloc")]
pub(crate) fn list_cert_dns_names<'names>(
cert: &'names crate::EndEntityCert<'names>,
) -> Result<impl Iterator<Item = GeneralDnsNameRef<'names>>, Error> {
) -> impl Iterator<Item = &'names str> {
let cert = &cert.inner();
let mut names = Vec::new();

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

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

let dns_name = DnsNameRef::try_from_ascii(presented_id.as_slice_less_safe())
.map(GeneralDnsNameRef::DnsName)
.or_else(|_| {
WildcardDnsNameRef::try_from_ascii(presented_id.as_slice_less_safe())
.map(GeneralDnsNameRef::Wildcard)
});

// if the name could be converted to a DNS name, add it; otherwise,
// keep going.
if let Ok(name) = dns_name {
names.push(name)
}

None
});
NameIterator::new(Some(cert.subject), cert.subject_alt_name).filter_map(|result| {
let presented_id = match result.ok()? {
GeneralName::DnsName(presented) => presented,
_ => return None,
};

match result {
Some(err) => Err(err),
_ => Ok(names.into_iter()),
}
// 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,
},
}
})
}

// It is *not* valid to derive `Eq`, `PartialEq, etc. for this type. In
Expand Down
Loading