Backed out changeset 40d6ccba44f1 (bug 1045973)
authorRichard Barnes <rbarnes@mozilla.com>
Mon, 22 Sep 2014 15:40:19 -0400
changeset 206587 ed8de2cc24f30bef0cd08e8e0a36a372eec96a5e
parent 206586 cfa50eefce84e971a2f330d05025a14f08429019
child 206588 3c7c479d5040ed723d1adf0a392e61e7378a0a43
push id27532
push userkwierso@gmail.com
push dateTue, 23 Sep 2014 01:57:26 +0000
treeherdermozilla-central@790f41c631cc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1045973
milestone35.0a1
backs out40d6ccba44f1610b61a84dbe9acd08f472907991
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 40d6ccba44f1 (bug 1045973)
security/pkix/lib/pkixcert.cpp
--- a/security/pkix/lib/pkixcert.cpp
+++ b/security/pkix/lib/pkixcert.cpp
@@ -121,74 +121,73 @@ BackCert::Init()
   if (rv != Success) {
     return rv;
   }
 
   static const uint8_t CSC = der::CONTEXT_SPECIFIC | der::CONSTRUCTED;
 
   // RFC 5280 says: "These fields MUST only appear if the version is 2 or 3
   // (Section 4.1.2.1). These fields MUST NOT appear if the version is 1."
-  // However, for compatibility reasons, we will parse v1/v2 certificates 
-  // according to the v3 rules, and thus accept these fields. The same
-  // rules also apply to "v4" certificates, since these are typically
-  // actually v3 certificates, but generated by software with an off-by-one
-  // error.
+  if (version != der::Version::v1) {
 
-  // Ignore issuerUniqueID if present.
-  if (tbsCertificate.Peek(CSC | 1)) {
-    rv = der::ExpectTagAndSkipValue(tbsCertificate, CSC | 1);
-    if (rv != Success) {
-      return rv;
+    // Ignore issuerUniqueID if present.
+    if (tbsCertificate.Peek(CSC | 1)) {
+      rv = der::ExpectTagAndSkipValue(tbsCertificate, CSC | 1);
+      if (rv != Success) {
+        return rv;
+      }
     }
-  }
 
-  // Ignore subjectUniqueID if present.
-  if (tbsCertificate.Peek(CSC | 2)) {
-    rv = der::ExpectTagAndSkipValue(tbsCertificate, CSC | 2);
-    if (rv != Success) {
-      return rv;
+    // Ignore subjectUniqueID if present.
+    if (tbsCertificate.Peek(CSC | 2)) {
+      rv = der::ExpectTagAndSkipValue(tbsCertificate, CSC | 2);
+      if (rv != Success) {
+        return rv;
+      }
     }
   }
 
-  // RFC 5280 only defines the use of extensions for v3 certificates.
-  // However, for compatibility reasons, we treat v1, v2, v3, and v4 in
-  // the same way, allowing and parsing extensions in all cases.
-  rv = der::OptionalExtensions(tbsCertificate, CSC | 3,
-                               bind(&BackCert::RememberExtension, this, _1,
-                                    _2, _3, _4));
-  if (rv != Success) {
-    return rv;
-  }
-
-  // The Netscape Certificate Type extension is an obsolete
-  // Netscape-proprietary mechanism that we ignore in favor of the standard
-  // extensions. However, some CAs have issued certificates with the Netscape
-  // Cert Type extension marked critical. Thus, for compatibility reasons, we
-  // "understand" this extension by ignoring it when it is not critical, and
-  // by ensuring that the equivalent standardized extensions are present when
-  // it is marked critical, based on the assumption that the information in
-  // the Netscape Cert Type extension is consistent with the information in
-  // the standard extensions.
-  //
-  // Here is a mapping between the Netscape Cert Type extension and the
-  // standard extensions:
-  //
-  // Netscape Cert Type  |  BasicConstraints.cA  |  Extended Key Usage
-  // --------------------+-----------------------+----------------------
-  // SSL Server          |  false                |  id_kp_serverAuth
-  // SSL Client          |  false                |  id_kp_clientAuth
-  // S/MIME Client       |  false                |  id_kp_emailProtection
-  // Object Signing      |  false                |  id_kp_codeSigning
-  // SSL Server CA       |  true                 |  id_pk_serverAuth
-  // SSL Client CA       |  true                 |  id_kp_clientAuth
-  // S/MIME CA           |  true                 |  id_kp_emailProtection
-  // Object Signing CA   |  true                 |  id_kp_codeSigning
-  if (criticalNetscapeCertificateType.GetLength() > 0 &&
-      (basicConstraints.GetLength() == 0 || extKeyUsage.GetLength() == 0)) {
-    return Result::ERROR_UNKNOWN_CRITICAL_EXTENSION;
+  // Extensions were added in v3, so only accept extensions in v3 certificates.
+  // v4 certificates are not defined but there are some certificates issued
+  // with v4 that expect v3 decoding. For compatibility reasons we handle them
+  // as v3 certificates.
+  if (version == der::Version::v3 || version == der::Version::v4) {
+    rv = der::OptionalExtensions(tbsCertificate, CSC | 3,
+                                 bind(&BackCert::RememberExtension, this, _1,
+                                      _2, _3, _4));
+    if (rv != Success) {
+      return rv;
+    }
+    // The Netscape Certificate Type extension is an obsolete
+    // Netscape-proprietary mechanism that we ignore in favor of the standard
+    // extensions. However, some CAs have issued certificates with the Netscape
+    // Cert Type extension marked critical. Thus, for compatibility reasons, we
+    // "understand" this extension by ignoring it when it is not critical, and
+    // by ensuring that the equivalent standardized extensions are present when
+    // it is marked critical, based on the assumption that the information in
+    // the Netscape Cert Type extension is consistent with the information in
+    // the standard extensions.
+    //
+    // Here is a mapping between the Netscape Cert Type extension and the
+    // standard extensions:
+    //
+    // Netscape Cert Type  |  BasicConstraints.cA  |  Extended Key Usage
+    // --------------------+-----------------------+----------------------
+    // SSL Server          |  false                |  id_kp_serverAuth
+    // SSL Client          |  false                |  id_kp_clientAuth
+    // S/MIME Client       |  false                |  id_kp_emailProtection
+    // Object Signing      |  false                |  id_kp_codeSigning
+    // SSL Server CA       |  true                 |  id_pk_serverAuth
+    // SSL Client CA       |  true                 |  id_kp_clientAuth
+    // S/MIME CA           |  true                 |  id_kp_emailProtection
+    // Object Signing CA   |  true                 |  id_kp_codeSigning
+    if (criticalNetscapeCertificateType.GetLength() > 0 &&
+        (basicConstraints.GetLength() == 0 || extKeyUsage.GetLength() == 0)) {
+      return Result::ERROR_UNKNOWN_CRITICAL_EXTENSION;
+    }
   }
 
   return der::End(tbsCertificate);
 }
 
 // XXX: The second value is of type |const Input&| instead of type |Input| due
 // to limitations in our std::bind polyfill.
 Result