Address PR comments.

rustls · Feb 11, 2023 · 2510646 · 2510646
1 parent 35b17bb
commit 2510646
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 33 deletions.
diff --git a/src/cert.rs b/src/cert.rs
@@ -95,38 +95,32 @@ pub(crate) fn parse_cert_internal<'a>(
             subject_alt_name: None,
         };
 
-        // mozilla::pkix allows the extensions to be omitted. It also includes
-        // special logic for handling critical Netscape Cert Type extensions.
-        // That has been intentionally omitted.
-
-        if tbs.at_end() {
-            return Ok(cert)
+        if !tbs.at_end() {
+            der::nested(
+                tbs,
+                der::Tag::ContextSpecificConstructed3,
+                Error::MalformedExtensions,
+                |tagged| {
+                    der::nested_of_mut(
+                        tagged,
+                        der::Tag::Sequence,
+                        der::Tag::Sequence,
+                        Error::BadDer,
+                        |extension| {
+                            let extn_id = der::expect_tag_and_get_value(extension, der::Tag::OID)?;
+                            let critical = der::optional_boolean(extension)?;
+                            let extn_value =
+                                der::expect_tag_and_get_value(extension, der::Tag::OctetString)?;
+                            match remember_extension(&mut cert, extn_id, extn_value)? {
+                                Understood::No if critical => Err(Error::UnsupportedCriticalExtension),
+                                _ => Ok(()),
+                            }
+                        },
+                    )
+                },
+            )?;
         }
 
-        der::nested(
-            tbs,
-            der::Tag::ContextSpecificConstructed3,
-            Error::MalformedExtensions,
-            |tagged| {
-                der::nested_of_mut(
-                    tagged,
-                    der::Tag::Sequence,
-                    der::Tag::Sequence,
-                    Error::BadDer,
-                    |extension| {
-                        let extn_id = der::expect_tag_and_get_value(extension, der::Tag::OID)?;
-                        let critical = der::optional_boolean(extension)?;
-                        let extn_value =
-                            der::expect_tag_and_get_value(extension, der::Tag::OctetString)?;
-                        match remember_extension(&mut cert, extn_id, extn_value)? {
-                            Understood::No if critical => Err(Error::UnsupportedCriticalExtension),
-                            _ => Ok(()),
-                        }
-                    },
-                )
-            },
-        )?;
-
         Ok(cert)
     })
 }

diff --git a/tests/cert_without_extensions.rs b/tests/cert_without_extensions.rs
@@ -20,7 +20,5 @@ fn cert_without_extensions_test() {
     // `openssl x509 -in cert_without_extensions.der -inform DER -text -noout`
     const CERT_WITHOUT_EXTENSIONS_DER: &[u8] = include_bytes!("cert_without_extensions.der");
 
-    assert!(
-        webpki::EndEntityCert::try_from(CERT_WITHOUT_EXTENSIONS_DER).is_ok()
-    );
+    assert!(webpki::EndEntityCert::try_from(CERT_WITHOUT_EXTENSIONS_DER).is_ok());
 }