From 211a288b8e26b28a747ca1fcc6d24876f236c14d Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 22 Sep 2014 15:40:19 -0400 Subject: [PATCH] Backed out changeset 40d6ccba44f1 (bug 1045973) --- security/pkix/lib/pkixcert.cpp | 101 ++++++++++++++++----------------- 1 file changed, 50 insertions(+), 51 deletions(-) diff --git a/security/pkix/lib/pkixcert.cpp b/security/pkix/lib/pkixcert.cpp index 1491496a3bc9..48ceb117246c 100644 --- a/security/pkix/lib/pkixcert.cpp +++ b/security/pkix/lib/pkixcert.cpp @@ -126,64 +126,63 @@ BackCert::Init() // 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); + // 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; } - } - - // 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; + // 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);