Add some extra certificate parsing errors for policy qualifiers.

Bug: 634443
Change-Id: I415c4d968b29325307401bf1c8b23c24ba977d37
Reviewed-on: https://chromium-review.googlesource.com/597409Reviewed-by: Matt Mueller <mattm@chromium.org>
Commit-Queue: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491478}

net/cert/internal/certificate_policies.cc

@@ -31,6 +31,10 @@ DEFINE_CERT_ERROR_ID(kPolicyInformationTrailingData,
                      "PolicyInformation has trailing data");
 DEFINE_CERT_ERROR_ID(kFailedParsingPolicyQualifiers,
                      "Failed parsing policy qualifiers");
+DEFINE_CERT_ERROR_ID(kMissingQualifier,
+                     "PolicyQualifierInfo is missing qualifier");
+DEFINE_CERT_ERROR_ID(kPolicyQualifierInfoTrailingData,
+                     "PolicyQualifierInfo has trailing data");
 
 // -- policyQualifierIds for Internet policy qualifiers
 //
@@ -90,11 +94,15 @@ bool ParsePolicyQualifiers(bool restrict_to_known_qualifiers,
     //      qualifier          ANY DEFINED BY policyQualifierId }
     der::Tag tag;
     der::Input value;
-    if (!policy_information_parser.ReadTagAndValue(&tag, &value))
+    if (!policy_information_parser.ReadTagAndValue(&tag, &value)) {
+      errors->AddError(kMissingQualifier);
       return false;
+    }
     // Should not have trailing data after qualifier.
-    if (policy_information_parser.HasMore())
+    if (policy_information_parser.HasMore()) {
+      errors->AddError(kPolicyQualifierInfoTrailingData);
       return false;
+    }
   }
   return true;
 }

net/cert/internal/parsed_certificate_unittest.cc

@@ -141,7 +141,8 @@ TEST(ParsedCertificateTest, BadKeyUsage) {
   ASSERT_FALSE(ParseCertificateFromFile("bad_key_usage.pem", {}));
 }
 
-// TODO(eroman): What is wrong with policy qualifiers?
+// Parses a certificate that has a PolicyQualifierInfo that is missing the
+// qualifier field.
 TEST(ParsedCertificateTest, BadPolicyQualifiers) {
   ASSERT_FALSE(ParseCertificateFromFile("bad_policy_qualifiers.pem", {}));
 }
@@ -151,7 +152,8 @@ TEST(ParsedCertificateTest, BadSignatureAlgorithmOid) {
   ASSERT_FALSE(ParseCertificateFromFile("bad_signature_algorithm_oid.pem", {}));
 }
 
-// TODO(eroman): What is wrong with the validity?
+//  The validity encodes time as UTCTime but following the BER rules rather than
+//  DER rules (i.e. YYMMDDHHMMZ instead of YYMMDDHHMMSSZ).
 TEST(ParsedCertificateTest, BadValidity) {
   ASSERT_FALSE(ParseCertificateFromFile("bad_validity.pem", {}));
 }

net/data/parse_certificate_unittest/bad_policy_qualifiers.pem

@@ -21,9 +21,10 @@ jr/a+66npWEWLdJRS+jYpciCP7Bs4CQ3KqoEmRagV0Q/kk923429y/2YCfLa+E7PWy57z6glaV1
 xcGy1rNRBQkfkzAR96isuvSTTV+GpXi/VbYaQFOwhkmyrr0udUT5RuPk7+4ca2ebEJYHZ0=
 -----END CERTIFICATE-----
 
+ERROR: PolicyQualifierInfo is missing qualifier
 ERROR: Failed parsing policy qualifiers
 ERROR: Failed parsing certificate policies
 
 -----BEGIN ERRORS-----
-RVJST1I6IEZhaWxlZCBwYXJzaW5nIHBvbGljeSBxdWFsaWZpZXJzCkVSUk9SOiBGYWlsZWQgcGFyc2luZyBjZXJ0aWZpY2F0ZSBwb2xpY2llcwo=
+RVJST1I6IFBvbGljeVF1YWxpZmllckluZm8gaXMgbWlzc2luZyBxdWFsaWZpZXIKRVJST1I6IEZhaWxlZCBwYXJzaW5nIHBvbGljeSBxdWFsaWZpZXJzCkVSUk9SOiBGYWlsZWQgcGFyc2luZyBjZXJ0aWZpY2F0ZSBwb2xpY2llcwo=
 -----END ERRORS-----