x509-cert: make RelativeDistinguishedName field private (#1510)

tarcieri · web-flow · commit 94b00b7b0710 · 2024-09-09T08:16:27.000-06:00
Like #1508 did to `RdnSequence`, this makes the inner field of the `RelativeDistinguishedName` struct private, instead explicitly delegating the `iter`, `len`, and `push` methods.
diff --git 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
@@ -148,16 +148,15 @@ impl CertificateType {
             .iter()
             .filter_map(|rdn| {
                 let out = SetOfVec::<AttributeTypeAndValue>::from_iter(
-                    rdn.0
-                        .iter()
+                    rdn.iter()
                         .filter(|attr_value| attr_value.oid == rfc4519::COUNTRY_NAME)
                         .cloned(),
                 )
                 .ok()?;
 
                 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
@@ -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<AttributeTypeAndValue>);
+pub struct RelativeDistinguishedName(pub(crate) SetOfVec<AttributeTypeAndValue>);
+
+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<Item = &AttributeTypeAndValue> {
+        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
@@ -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
@@ -37,15 +37,15 @@ 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(),
             _ => panic!("unexpected tag"),
         };
 
         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
@@ -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,18 +143,16 @@ 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,
         &hex!("311F300A060355040A0C03313233301106035504030C0A4A4F484E20534D495448")
     );
 
     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
@@ -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
@@ -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");

Original file line number	Diff line number	Diff line change
`@@ -44,11 +44,11 @@ pub fn check_names_encoding(name: &Name, multiple_allowed: bool) -> Result<()> {`
`44`	`44`	`let mut seen = HashSet::new();`
`45`	`45`
`46`	`46`	`for rdn in name.iter() {`
`47`		`- if rdn.0.len() != 1 {`
	`47`	`+ if rdn.len() != 1 {`
`48`	`48`	`return Err(Error::NonUniqueRdn);`
`49`	`49`	`}`
`50`	`50`
`51`		`- for atv in rdn.0.iter() {`
	`51`	`+ for atv in rdn.iter() {`
`52`	`52`	`if !multiple_allowed && !seen.insert(atv.oid) {`
`53`	`53`	`return Err(Error::NonUniqueATV);`
`54`	`54`	`}`
`@@ -88,7 +88,7 @@ pub fn ca_certificate_naming(subject: &Name) -> Result<()> {`
`88`	`88`	`check_names_encoding(subject, false)?;`
`89`	`89`
`90`	`90`	`for rdn in subject.iter() {`
`91`		`- for atv in rdn.0.iter() {`
	`91`	`+ for atv in rdn.iter() {`
`92`	`92`	`if !allowed.remove(&atv.oid) {`
`93`	`93`	`return Err(Error::InvalidAttribute { oid: atv.oid });`
`94`	`94`	`}`