Bug 1029992, Improve AlgorithmIdentifier decoding in mozilla::pkix, r=cviecco
authorBrian Smith <brian@briansmith.org>
Tue, 24 Jun 2014 21:47:50 -0700
changeset 190894 93bc682527b4f3a0888d9877d6c79bef1d3be58c
parent 190893 11ce0e5823f49b19d847aaa760fd330fe0cc6172
child 190895 bfd435db3f9e084c038792100511691581ccdbc6
push id27018
push usercbook@mozilla.com
push dateThu, 26 Jun 2014 12:15:07 +0000
treeherdermozilla-central@bd03647cca20 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscviecco
bugs1029992
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1029992, Improve AlgorithmIdentifier decoding in mozilla::pkix, r=cviecco
security/pkix/lib/pkixder.cpp
security/pkix/lib/pkixder.h
security/pkix/lib/pkixocsp.cpp
security/pkix/test/gtest/pkixder_pki_types_tests.cpp
--- a/security/pkix/lib/pkixder.cpp
+++ b/security/pkix/lib/pkixder.cpp
@@ -100,19 +100,17 @@ SignedData(Input& input, /*out*/ Input& 
   if (ExpectTagAndGetValue(input, SEQUENCE, tbs) != Success) {
     return Failure;
   }
 
   if (input.GetSECItem(siBuffer, mark, signedData.data) != Success) {
     return Failure;
   }
 
-  if (Nested(input, SEQUENCE,
-             bind(AlgorithmIdentifier, _1, ref(signedData.signatureAlgorithm)))
-        != Success) {
+  if (AlgorithmIdentifier(input, signedData.signatureAlgorithm) != Success) {
     return Failure;
   }
 
   if (ExpectTagAndGetValue(input, BIT_STRING, signedData.signature)
         != Success) {
     return Failure;
   }
   if (signedData.signature.len == 0) {
--- a/security/pkix/lib/pkixder.h
+++ b/security/pkix/lib/pkixder.h
@@ -567,25 +567,31 @@ OID(Input& input, const uint8_t (&expect
 // PKI-specific types
 
 // AlgorithmIdentifier  ::=  SEQUENCE  {
 //         algorithm               OBJECT IDENTIFIER,
 //         parameters              ANY DEFINED BY algorithm OPTIONAL  }
 inline Result
 AlgorithmIdentifier(Input& input, SECAlgorithmID& algorithmID)
 {
-  if (ExpectTagAndGetValue(input, OIDTag, algorithmID.algorithm) != Success) {
+  Input value;
+  if (ExpectTagAndGetValue(input, der::SEQUENCE, value) != Success) {
+    return Failure;
+  }
+  if (ExpectTagAndGetValue(value, OIDTag, algorithmID.algorithm) != Success) {
     return Failure;
   }
   algorithmID.parameters.data = nullptr;
   algorithmID.parameters.len = 0;
-  if (input.AtEnd()) {
-    return Success;
+  if (!value.AtEnd()) {
+    if (Null(value) != Success) {
+      return Failure;
+    }
   }
-  return Null(input);
+  return End(value);
 }
 
 inline Result
 CertificateSerialNumber(Input& input, /*out*/ SECItem& value)
 {
   // http://tools.ietf.org/html/rfc5280#section-4.1.2.2:
   //
   // * "The serial number MUST be a positive integer assigned by the CA to
--- a/security/pkix/lib/pkixocsp.cpp
+++ b/security/pkix/lib/pkixocsp.cpp
@@ -679,19 +679,17 @@ SingleResponse(der::Input& input, Contex
 //        issuerKeyHash       OCTET STRING, -- Hash of issuer's public key
 //        serialNumber        CertificateSerialNumber }
 static inline der::Result
 CertID(der::Input& input, const Context& context, /*out*/ bool& match)
 {
   match = false;
 
   SECAlgorithmID hashAlgorithm;
-  if (der::Nested(input, der::SEQUENCE,
-                  bind(der::AlgorithmIdentifier, _1, ref(hashAlgorithm)))
-         != der::Success) {
+  if (der::AlgorithmIdentifier(input, hashAlgorithm) != der::Success) {
     return der::Failure;
   }
 
   SECItem issuerNameHash;
   if (der::ExpectTagAndGetValue(input, der::OCTET_STRING, issuerNameHash)
         != der::Success) {
     return der::Failure;
   }
--- a/security/pkix/test/gtest/pkixder_pki_types_tests.cpp
+++ b/security/pkix/test/gtest/pkixder_pki_types_tests.cpp
@@ -40,16 +40,17 @@ protected:
   {
     PR_SetError(0, 0);
   }
 };
 
 TEST_F(pkixder_pki_types_tests, AlgorithmIdentifierNoParams)
 {
   const uint8_t DER_ALGORITHM_IDENTIFIER_NO_PARAMS[] = {
+    0x30/*SEQUENCE*/, 0x06/*LENGTH*/,
     0x06, 0x04, 0xde, 0xad, 0xbe, 0xef   // OID
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_ALGORITHM_IDENTIFIER_NO_PARAMS,
                                 sizeof DER_ALGORITHM_IDENTIFIER_NO_PARAMS));
 
   const uint8_t expectedAlgorithmID[] = {
@@ -74,25 +75,22 @@ TEST_F(pkixder_pki_types_tests, Algorith
     0x06, 0x04, 0xde, 0xad, 0xbe, 0xef, // OID
     0x05, 0x00                          // NULL
   };
 
   Input input;
   ASSERT_EQ(Success, input.Init(DER_ALGORITHM_IDENTIFIER_NULL_PARAMS,
                                 sizeof DER_ALGORITHM_IDENTIFIER_NULL_PARAMS));
 
-  Input nested;
-  ASSERT_EQ(Success, ExpectTagAndGetValue(input, SEQUENCE, nested));
-
   const uint8_t expectedAlgorithmID[] = {
     0xde, 0xad, 0xbe, 0xef
   };
 
   SECAlgorithmID algorithmID;
-  ASSERT_EQ(Success, AlgorithmIdentifier(nested, algorithmID));
+  ASSERT_EQ(Success, AlgorithmIdentifier(input, algorithmID));
 
   ASSERT_EQ(sizeof expectedAlgorithmID, algorithmID.algorithm.len);
   ASSERT_TRUE(memcmp(algorithmID.algorithm.data, expectedAlgorithmID,
                      sizeof expectedAlgorithmID) == 0);
 
   ASSERT_EQ((size_t) 0, algorithmID.parameters.len);
   ASSERT_EQ(NULL, algorithmID.parameters.data);
 }