Convert tests that parse an Extensions sequence to instead

net/cert/internal/parse_certificate_unittest.cc

Index: net/cert/internal/parse_certificate_unittest.cc
diff --git a/net/cert/internal/parse_certificate_unittest.cc b/net/cert/internal/parse_certificate_unittest.cc
index 00f4acfed2ef56a7293782d4f0e281f71f5dc0d8..493638989eeafb1442ceb69b4d5b106fcc731220 100644
--- a/net/cert/internal/parse_certificate_unittest.cc
+++ b/net/cert/internal/parse_certificate_unittest.cc
@@ -418,107 +418,34 @@ TEST(ParseCertificateTest, ExtensionCritical3) {
   ASSERT_FALSE(ParseCertificateFromFile("extension_critical_3.pem"));
 }
 
-// Runs a test for extensions parsing. The input file is a PEM file which
-// contains a DER-encoded Extensions sequence, as well as the expected value
-// for each contained extension.
-void EnsureParsingExtensionsSucceeds(
-    const std::string& file_name,
-    std::map<der::Input, ParsedExtension>* extensions,
-    std::string* data) {
-  const PemBlockMapping mappings[] = {
-      // Test Input.
-      {"EXTENSIONS", data},
-  };
-
-  ASSERT_TRUE(ReadTestDataFromPemFile(GetFilePath(file_name), mappings));
-  ASSERT_TRUE(ParseExtensions(der::Input(data), extensions));
-}
-
-// Runs a test that verifies extensions parsing fails. The input file is a PEM
-// file which contains a DER-encoded Extensions sequence.
-void EnsureParsingExtensionsFails(const std::string& file_name) {
-  std::string data;
-
-  const PemBlockMapping mappings[] = {
-      {"EXTENSIONS", &data},
-  };
-
-  std::map<der::Input, ParsedExtension> extensions;
-  ASSERT_TRUE(ReadTestDataFromPemFile(GetFilePath(file_name), mappings));
-  ASSERT_FALSE(ParseExtensions(der::Input(&data), &extensions));
-}
-
 // Parses an Extensions that is an empty sequence.
-TEST(ParseExtensionsTest, EmptySequence) {
-  EnsureParsingExtensionsFails("extensions_empty_sequence.pem");
+TEST(ParseCertificateTest, ExtensionsEmptySequence) {
+  ASSERT_FALSE(ParseCertificateFromFile("extensions_empty_sequence.pem"));
 }
 
 // Parses an Extensions that is not a sequence.
-TEST(ParseExtensionsTest, NotSequence) {
-  EnsureParsingExtensionsFails("extensions_not_sequence.pem");
+TEST(ParseCertificateTest, ExtensionsNotSequence) {
+  ASSERT_FALSE(ParseCertificateFromFile("extensions_not_sequence.pem"));
 }
 
 // Parses an Extensions that has data after the sequence.
-TEST(ParseExtensionsTest, DataAfterSequence) {
-  EnsureParsingExtensionsFails("extensions_data_after_sequence.pem");
+TEST(ParseCertificateTest, ExtensionsDataAfterSequence) {
+  ASSERT_FALSE(ParseCertificateFromFile("extensions_data_after_sequence.pem"));
 }
 
 // Parses an Extensions that contains duplicated key usages.
-TEST(ParseExtensionsTest, DuplicateKeyUsage) {
-  EnsureParsingExtensionsFails("extensions_duplicate_key_usage.pem");
-}
-
-// Parses an Extensions that contains an unknown critical extension.
-TEST(ParseExtensionsTest, UnknownCritical) {

[mattm, 2016/09/23 01:54:32]

Were these tests supposed to be removed?

[eroman, 2016/09/23 01:58:05]

Yes -- they are basically duplicates with extension_critical.pem and
extension_not_critical.pem (kind of happend when I changed those from basic
constraints to an unknown extension in the previous patch).

There are several other duplicate tests that were the result of testing
indepently at the various layers. Which I don't think are all that useful.

-  std::string data;
-  std::map<der::Input, ParsedExtension> extensions;
-  EnsureParsingExtensionsSucceeds("extensions_unknown_critical.pem",
-                                  &extensions, &data);
-
-  ASSERT_EQ(1u, extensions.size());
-  auto iter = extensions.find(DavidBenOid());
-  ASSERT_TRUE(iter != extensions.end());
-  EXPECT_TRUE(iter->second.critical);
-  EXPECT_EQ(4u, iter->second.value.Length());
-}
-
-// Parses an Extensions that contains an unknown non-critical extension.
-TEST(ParseExtensionsTest, UnknownNonCritical) {
-  std::string data;
-  std::map<der::Input, ParsedExtension> extensions;
-  EnsureParsingExtensionsSucceeds("extensions_unknown_non_critical.pem",
-                                  &extensions, &data);
-
-  ASSERT_EQ(1u, extensions.size());
-  auto iter = extensions.find(DavidBenOid());
-  ASSERT_TRUE(iter != extensions.end());
-  EXPECT_FALSE(iter->second.critical);
-  EXPECT_EQ(4u, iter->second.value.Length());
-}
-
-// Parses an Extensions that contains a basic constraints.
-TEST(ParseExtensionsTest, BasicConstraints) {
-  std::string data;
-  std::map<der::Input, ParsedExtension> extensions;
-  EnsureParsingExtensionsSucceeds("extensions_basic_constraints.pem",
-                                  &extensions, &data);
-
-  ASSERT_EQ(1u, extensions.size());
-
-  auto iter = extensions.find(BasicConstraintsOid());
-  ASSERT_TRUE(iter != extensions.end());
-  EXPECT_TRUE(iter->second.critical);
-  EXPECT_EQ(2u, iter->second.value.Length());
+TEST(ParseCertificateTest, ExtensionsDuplicateKeyUsage) {
+  ASSERT_FALSE(ParseCertificateFromFile("extensions_duplicate_key_usage.pem"));
 }
 
 // Parses an Extensions that contains an extended key usages.
-TEST(ParseExtensionsTest, ExtendedKeyUsage) {
-  std::string data;
-  std::map<der::Input, ParsedExtension> extensions;
-  EnsureParsingExtensionsSucceeds("extensions_extended_key_usage.pem",
-                                  &extensions, &data);
+TEST(ParseCertificateTest, ExtendedKeyUsage) {
+  scoped_refptr<ParsedCertificate> cert =
+      ParseCertificateFromFile("extended_key_usage.pem");
+  ASSERT_TRUE(cert);
 
-  ASSERT_EQ(1u, extensions.size());
+  const auto& extensions = cert->unparsed_extensions();
+  ASSERT_EQ(3u, extensions.size());
 
   auto iter = extensions.find(ExtKeyUsageOid());
   ASSERT_TRUE(iter != extensions.end());
@@ -527,28 +454,24 @@ TEST(ParseExtensionsTest, ExtendedKeyUsage) {
 }
 
 // Parses an Extensions that contains a key usage.
-TEST(ParseExtensionsTest, KeyUsage) {
-  std::string data;
-  std::map<der::Input, ParsedExtension> extensions;
-  EnsureParsingExtensionsSucceeds("extensions_key_usage.pem", &extensions,
-                                  &data);
-
-  ASSERT_EQ(1u, extensions.size());
+TEST(ParseCertificateTest, KeyUsage) {
+  scoped_refptr<ParsedCertificate> cert =
+      ParseCertificateFromFile("key_usage.pem");
+  ASSERT_TRUE(cert);
 
-  auto iter = extensions.find(KeyUsageOid());
-  ASSERT_TRUE(iter != extensions.end());
-  EXPECT_TRUE(iter->second.critical);
-  EXPECT_EQ(4u, iter->second.value.Length());
+  ASSERT_TRUE(cert->has_key_usage());
+  const uint8_t kExpectedValue[] = {0xA0};

[mattm, 2016/09/23 01:54:32]

maybe dumb question, but why is it {a0} and not {05 a0}?

[eroman, 2016/09/23 02:14:10]

I will make this clearer in the code, and also add an assertion for the 0x05
(number of unused bits).

Explanation:

The "value" of the BIT string's DER is 05 0a

05 -- the number of unused bits
a0 -- the actual bits

So in other words, it is a bit-string of 3 bits, "101" (digitalSignature and
keyEncipherment I believe).

The comparison below is using BitString::bytes(), hence doesn't include the 5
counter.

I will improve the expectations to also check the first 3 bits directly.

Thanks for pointing out this confusion.

[eroman, 2016/09/23 02:21:39]

Done.

+  EXPECT_EQ(der::Input(kExpectedValue), cert->key_usage().bytes());
 }
 
 // Parses an Extensions that contains a policies extension.
-TEST(ParseExtensionsTest, Policies) {
-  std::string data;
-  std::map<der::Input, ParsedExtension> extensions;
-  EnsureParsingExtensionsSucceeds("extensions_policies.pem", &extensions,
-                                  &data);
+TEST(ParseCertificateTest, Policies) {
+  scoped_refptr<ParsedCertificate> cert =
+      ParseCertificateFromFile("policies.pem");
+  ASSERT_TRUE(cert);
 
-  ASSERT_EQ(1u, extensions.size());
+  const auto& extensions = cert->unparsed_extensions();
+  ASSERT_EQ(3u, extensions.size());
 
   auto iter = extensions.find(CertificatePoliciesOid());
   ASSERT_TRUE(iter != extensions.end());
@@ -557,40 +480,29 @@ TEST(ParseExtensionsTest, Policies) {
 }
 
 // Parses an Extensions that contains a subjectaltname extension.
-TEST(ParseExtensionsTest, SubjectAltName) {
-  std::string data;
-  std::map<der::Input, ParsedExtension> extensions;
-  EnsureParsingExtensionsSucceeds("extensions_subject_alt_name.pem",
-                                  &extensions, &data);
-
-  ASSERT_EQ(1u, extensions.size());
+TEST(ParseCertificateTest, SubjectAltName) {
+  scoped_refptr<ParsedCertificate> cert =
+      ParseCertificateFromFile("subject_alt_name.pem");
+  ASSERT_TRUE(cert);
 
-  auto iter = extensions.find(SubjectAltNameOid());
-  ASSERT_TRUE(iter != extensions.end());
-  EXPECT_FALSE(iter->second.critical);
-  EXPECT_EQ(23u, iter->second.value.Length());
+  ASSERT_TRUE(cert->has_subject_alt_names());
 }
 
 // Parses an Extensions that contains multiple extensions, sourced from a
 // real-world certificate.
-TEST(ParseExtensionsTest, Real) {
-  std::string data;
-  std::map<der::Input, ParsedExtension> extensions;
-  EnsureParsingExtensionsSucceeds("extensions_real.pem", &extensions, &data);
-
-  ASSERT_EQ(7u, extensions.size());
+TEST(ParseCertificateTest, ExtensionsReal) {
+  scoped_refptr<ParsedCertificate> cert =
+      ParseCertificateFromFile("extensions_real.pem");
+  ASSERT_TRUE(cert);
 
-  auto iter = extensions.find(KeyUsageOid());
-  ASSERT_TRUE(iter != extensions.end());
-  EXPECT_TRUE(iter->second.critical);
-  EXPECT_EQ(4u, iter->second.value.Length());
+  const auto& extensions = cert->unparsed_extensions();
+  ASSERT_EQ(4u, extensions.size());
 
-  iter = extensions.find(BasicConstraintsOid());
-  ASSERT_TRUE(iter != extensions.end());
-  EXPECT_TRUE(iter->second.critical);
-  EXPECT_EQ(8u, iter->second.value.Length());
+  EXPECT_TRUE(cert->has_key_usage());
+  EXPECT_TRUE(cert->has_basic_constraints());
+  EXPECT_TRUE(cert->has_basic_constraints());

[mattm, 2016/09/23 01:54:32]

repeated

[eroman, 2016/09/23 02:21:39]

Done.

 
-  iter = extensions.find(CertificatePoliciesOid());
+  auto iter = extensions.find(CertificatePoliciesOid());
   ASSERT_TRUE(iter != extensions.end());
   EXPECT_FALSE(iter->second.critical);
   EXPECT_EQ(16u, iter->second.value.Length());