bug 989516 - mozilla::pkix: temporarily allow improper basicConstraint:cA encodings r=cviecco

security/pkix/lib/pkixcheck.cpp

@@ -188,7 +188,13 @@ DecodeBasicConstraints(const SECItem* encodedBasicConstraints,
   }
 
   bool isCA = false;
-  if (der::OptionalBoolean(input, isCA) != der::Success) {
+  // TODO(bug 989518): cA is by default false. According to DER, default
+  // values must not be explicitly encoded in a SEQUENCE. So, if this
+  // value is present and false, it is an encoding error. However, Go Daddy
+  // has issued many certificates with this improper encoding, so we can't
+  // enforce this yet (hence passing true for allowInvalidExplicitEncoding
+  // to der::OptionalBoolean).
+  if (der::OptionalBoolean(input, true, isCA) != der::Success) {
     return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
   }
   basicConstraints.isCA = isCA;

security/pkix/lib/pkixder.h

@@ -374,15 +374,18 @@ Boolean(Input& input, /*out*/ bool& value)
 
 // This is for any BOOLEAN DEFAULT FALSE.
 // (If it is present and false, this is a bad encoding.)
+// TODO(bug 989518): For compatibility reasons, in some places we allow
+// invalid encodings with the explicit default value.
 inline Result
-OptionalBoolean(Input& input, /*out*/ bool& value)
+OptionalBoolean(Input& input, bool allowInvalidExplicitEncoding,
+                /*out*/ bool& value)
 {
   value = false;
   if (input.Peek(BOOLEAN)) {
     if (Boolean(input, value) != Success) {
       return Failure;
     }
-    if (!value) {
+    if (!allowInvalidExplicitEncoding && !value) {
       return Fail(SEC_ERROR_BAD_DER);
     }
   }