From 56b857b26976d7d3fb287ba18d7ab96bd09f96ac Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Mon, 9 Sep 2024 08:06:01 -0600 Subject: [PATCH] x509-cert: make `RelativeDistinguishedName` field private Like #1508 did to `RdnSequence`, this makes the inner field of the `RelativeDistinguishedName` struct private, instead explicitly delegating the `iter`, `len`, and `push` methods. --- x509-cert/src/builder/profile/cabf.rs | 6 +++--- x509-cert/src/builder/profile/cabf/tls.rs | 5 ++--- x509-cert/src/name.rs | 24 ++++++++++++++++++++++- x509-cert/tests/certificate.rs | 4 ++-- x509-cert/tests/certreq.rs | 4 ++-- x509-cert/tests/name.rs | 16 +++++++-------- x509-cert/tests/pkix_extensions.rs | 4 ++-- x509-cert/tests/trust_anchor_format.rs | 12 ++++++------ 8 files changed, 47 insertions(+), 28 deletions(-) diff --git a/x509-cert/src/builder/profile/cabf.rs b/x509-cert/src/builder/profile/cabf.rs index fed669475..a8b02ac0c 100644 --- a/x509-cert/src/builder/profile/cabf.rs +++ b/x509-cert/src/builder/profile/cabf.rs @@ -44,11 +44,11 @@ pub fn check_names_encoding(name: &Name, multiple_allowed: bool) -> Result<()> { let mut seen = HashSet::new(); for rdn in name.iter() { - if rdn.0.len() != 1 { + if rdn.len() != 1 { return Err(Error::NonUniqueRdn); } - for atv in rdn.0.iter() { + for atv in rdn.iter() { if !multiple_allowed && !seen.insert(atv.oid) { return Err(Error::NonUniqueATV); } @@ -88,7 +88,7 @@ pub fn ca_certificate_naming(subject: &Name) -> Result<()> { check_names_encoding(subject, false)?; for rdn in subject.iter() { - for atv in rdn.0.iter() { + for atv in rdn.iter() { if !allowed.remove(&atv.oid) { return Err(Error::InvalidAttribute { oid: atv.oid }); } diff --git a/x509-cert/src/builder/profile/cabf/tls.rs b/x509-cert/src/builder/profile/cabf/tls.rs index 9e4d87300..142f8cce2 100644 --- a/x509-cert/src/builder/profile/cabf/tls.rs +++ b/x509-cert/src/builder/profile/cabf/tls.rs @@ -148,8 +148,7 @@ impl CertificateType { .iter() .filter_map(|rdn| { let out = SetOfVec::::from_iter( - rdn.0 - .iter() + rdn.iter() .filter(|attr_value| attr_value.oid == rfc4519::COUNTRY_NAME) .cloned(), ) @@ -157,7 +156,7 @@ impl CertificateType { Some(RelativeDistinguishedName(out)) }) - .filter(|rdn| !rdn.0.is_empty()) + .filter(|rdn| !rdn.is_empty()) .collect(); let subject: Name = rdns.into(); diff --git a/x509-cert/src/name.rs b/x509-cert/src/name.rs index 6fc03c357..2da628b41 100644 --- a/x509-cert/src/name.rs +++ b/x509-cert/src/name.rs @@ -153,7 +153,29 @@ pub type DistinguishedName = RdnSequence; /// [RFC 5280 Section 4.1.2.4]: https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.4 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] #[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct RelativeDistinguishedName(pub SetOfVec); +pub struct RelativeDistinguishedName(pub(crate) SetOfVec); + +impl RelativeDistinguishedName { + /// Is this [`RelativeDistinguishedName`] empty? + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Iterate over this [`RelativeDistinguishedName`]. + pub fn iter(&self) -> impl Iterator { + self.0.iter() + } + + /// Length of this [`RelativeDistinguishedName`]. + pub fn len(&self) -> usize { + self.0.len() + } + + /// Insert an [`AttributeTypeAndValue`] into this [`RelativeDistinguishedName`]. Must be unique. + pub fn insert(&mut self, item: AttributeTypeAndValue) -> Result<(), der::Error> { + self.0.insert(item) + } +} /// Parse a [`RelativeDistinguishedName`] string. /// diff --git a/x509-cert/tests/certificate.rs b/x509-cert/tests/certificate.rs index 0aaf284f1..f6beb4765 100644 --- a/x509-cert/tests/certificate.rs +++ b/x509-cert/tests/certificate.rs @@ -241,7 +241,7 @@ fn decode_cert() { let mut counter = 0; let i = cert.tbs_certificate().issuer().iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "2.5.4.6"); @@ -296,7 +296,7 @@ fn decode_cert() { counter = 0; let i = cert.tbs_certificate().subject().iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { // Yes, this cert features RDNs encoded in reverse order if 0 == counter { diff --git a/x509-cert/tests/certreq.rs b/x509-cert/tests/certreq.rs index 78b7308fc..903387ad0 100644 --- a/x509-cert/tests/certreq.rs +++ b/x509-cert/tests/certreq.rs @@ -37,7 +37,7 @@ fn decode_rsa_2048_der() { // Check all the RDNs. assert_eq!(cr.info.subject.len(), NAMES.len()); for (name, (oid, val)) in cr.info.subject.iter().zip(NAMES) { - let kind = name.0.get(0).unwrap(); + let kind = name.iter().next().unwrap(); let value = match kind.value.tag() { Tag::Utf8String => Utf8StringRef::try_from(&kind.value).unwrap().as_str(), Tag::PrintableString => PrintableStringRef::try_from(&kind.value).unwrap().as_str(), @@ -45,7 +45,7 @@ fn decode_rsa_2048_der() { }; assert_eq!(kind.oid, oid.parse().unwrap()); - assert_eq!(name.0.len(), 1); + assert_eq!(name.len(), 1); assert_eq!(value, *val); } diff --git a/x509-cert/tests/name.rs b/x509-cert/tests/name.rs index c968c4987..e0763b5ef 100644 --- a/x509-cert/tests/name.rs +++ b/x509-cert/tests/name.rs @@ -36,7 +36,7 @@ fn decode_name() { let mut counter = 0; let i = rdn1a.iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "2.5.4.6"); @@ -99,7 +99,7 @@ fn decode_rdn() { // : } let rdn1 = RelativeDistinguishedName::from_der(&hex!("310B3009060355040613025553")[..]).unwrap(); - let i = rdn1.0.iter(); + let i = rdn1.iter(); for atav in i { let oid = atav.oid; assert_eq!(oid.to_string(), "2.5.4.6"); @@ -125,7 +125,7 @@ fn decode_rdn() { &hex!("311F300A060355040A0C03313233301106035504030C0A4A4F484E20534D495448")[..], ) .unwrap(); - let mut i = rdn2a.0.iter(); + let mut i = rdn2a.iter(); let atav1a = i.next().unwrap(); let oid2 = atav1a.oid; assert_eq!(oid2.to_string(), "2.5.4.10"); @@ -143,8 +143,8 @@ fn decode_rdn() { assert_eq!(utf8a.to_string(), "JOHN SMITH"); let mut from_scratch = RelativeDistinguishedName::default(); - assert!(from_scratch.0.insert(atav1a.clone()).is_ok()); - assert!(from_scratch.0.insert(atav2a.clone()).is_ok()); + assert!(from_scratch.insert(atav1a.clone()).is_ok()); + assert!(from_scratch.insert(atav2a.clone()).is_ok()); let reencoded = from_scratch.to_der().unwrap(); assert_eq!( reencoded, @@ -152,9 +152,7 @@ fn decode_rdn() { ); let mut from_scratch2 = RelativeDistinguishedName::default(); - assert!(from_scratch2.0.insert_ordered(atav2a.clone()).is_ok()); - // fails when caller adds items not in DER lexicographical order - assert!(from_scratch2.0.insert_ordered(atav1a.clone()).is_err()); + assert!(from_scratch2.insert(atav2a.clone()).is_ok()); // allow out-of-order RDNs (see: RustCrypto/formats#625) assert!(RelativeDistinguishedName::from_der( @@ -357,7 +355,7 @@ fn rdns_serde() { let rdns = RdnSequence::from_der(&der).unwrap(); for (l, r) in brdns.iter().zip(rdns.iter()) { - for (ll, rr) in l.0.iter().zip(r.0.iter()) { + for (ll, rr) in l.iter().zip(r.iter()) { assert_eq!(ll, rr); } diff --git a/x509-cert/tests/pkix_extensions.rs b/x509-cert/tests/pkix_extensions.rs index cb6620508..41622f5be 100644 --- a/x509-cert/tests/pkix_extensions.rs +++ b/x509-cert/tests/pkix_extensions.rs @@ -583,7 +583,7 @@ fn decode_cert() { let mut counter = 0; let i = cert.tbs_certificate().issuer().iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "2.5.4.6"); @@ -634,7 +634,7 @@ fn decode_cert() { counter = 0; let i = cert.tbs_certificate().subject().iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "2.5.4.6"); diff --git a/x509-cert/tests/trust_anchor_format.rs b/x509-cert/tests/trust_anchor_format.rs index 1343864e7..2f824b18e 100644 --- a/x509-cert/tests/trust_anchor_format.rs +++ b/x509-cert/tests/trust_anchor_format.rs @@ -92,7 +92,7 @@ fn decode_ta1() { counter = 0; let i = cert_path.ta_name.iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "2.5.4.6"); @@ -169,7 +169,7 @@ fn decode_ta2() { let mut counter = 0; let i = cert_path.ta_name.iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "2.5.4.6"); @@ -216,7 +216,7 @@ fn decode_ta2() { GeneralName::DirectoryName(dn) => { let i = dn.iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "2.5.4.6"); @@ -296,7 +296,7 @@ fn decode_ta3() { let mut counter = 0; let i = cert_path.ta_name.iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "2.5.4.6"); @@ -343,7 +343,7 @@ fn decode_ta3() { GeneralName::DirectoryName(dn) => { let i = dn.iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "2.5.4.6"); @@ -416,7 +416,7 @@ fn decode_ta4() { let mut counter = 0; let i = cert_path.ta_name.iter(); for rdn in i { - let i1 = rdn.0.iter(); + let i1 = rdn.iter(); for atav in i1 { if 0 == counter { assert_eq!(atav.oid.to_string(), "0.9.2342.19200300.100.1.25");