Convert tests that parse an Extensions sequence to instead

net/cert/internal/parse_certificate_unittest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/internal/parse_certificate.h"
 
 #include "base/strings/stringprintf.h"
 #include "net/cert/internal/cert_errors.h"
 // TODO(eroman): These tests should be moved into
 //               parsed_certificate_unittest.cc; this include dependency should
@@ skipping 400 matching lines @@
   ASSERT_FALSE(ParseCertificateFromFile("extension_critical_0.pem"));
 }
 
 // Parses an Extension whose critical field is 3. Under DER-encoding BOOLEAN
 // values must an octet of either all zero bits, or all 1 bits, so this is not
 // valid.
 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());
   EXPECT_FALSE(iter->second.critical);
   EXPECT_EQ(45u, iter->second.value.Length());
 }
 
 // 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);
+TEST(ParseCertificateTest, KeyUsage) {
+  scoped_refptr<ParsedCertificate> cert =
+      ParseCertificateFromFile("key_usage.pem");
+  ASSERT_TRUE(cert);
 
-  ASSERT_EQ(1u, extensions.size());
-
-  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());
   EXPECT_FALSE(iter->second.critical);
   EXPECT_EQ(95u, iter->second.value.Length());
 }
 
 // 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);
+TEST(ParseCertificateTest, SubjectAltName) {
+  scoped_refptr<ParsedCertificate> cert =
+      ParseCertificateFromFile("subject_alt_name.pem");
+  ASSERT_TRUE(cert);
 
-  ASSERT_EQ(1u, extensions.size());
-
-  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);
+TEST(ParseCertificateTest, ExtensionsReal) {
+  scoped_refptr<ParsedCertificate> cert =
+      ParseCertificateFromFile("extensions_real.pem");
+  ASSERT_TRUE(cert);
 
-  ASSERT_EQ(7u, extensions.size());
+  const auto& extensions = cert->unparsed_extensions();
+  ASSERT_EQ(4u, extensions.size());
 
-  auto iter = extensions.find(KeyUsageOid());
-  ASSERT_TRUE(iter != extensions.end());
-  EXPECT_TRUE(iter->second.critical);
-  EXPECT_EQ(4u, 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(BasicConstraintsOid());
-  ASSERT_TRUE(iter != extensions.end());
-  EXPECT_TRUE(iter->second.critical);
-  EXPECT_EQ(8u, iter->second.value.Length());
-
-  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());
 
   // TODO(eroman): Verify the other 4 extensions' values.
 }
 
 // Parses a BasicConstraints with no CA or pathlen.
 TEST(ParseCertificateTest, BasicConstraintsNotCa) {
   scoped_refptr<ParsedCertificate> cert =
@@ skipping 189 matching lines @@
       0x00,        // Number of unused bits
   };
 
   der::BitString key_usage;
   ASSERT_FALSE(ParseKeyUsage(der::Input(der), &key_usage));
 }
 
 }  // namespace
 
 }  // namespace net