From 6426e9ea55818afb09905e66b8478cc4b127540e Mon Sep 17 00:00:00 2001
From: David Keeler <dkeeler@mozilla.com>
Date: Mon, 17 Mar 2014 14:34:34 -0700
Subject: [PATCH] bug 977870 - insanity::pkix: consume the rest of input when a
 CertID doesn't match in an OCSP response r=briansmith

---
 security/insanity/lib/pkixder.h                     |  5 +++++
 security/insanity/lib/pkixocsp.cpp                  | 13 +++++++++++++
 .../manager/ssl/tests/unit/test_ocsp_stapling.js    |  5 +----
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/security/insanity/lib/pkixder.h b/security/insanity/lib/pkixder.h
index 6b0e98257dac..99004b1a176e 100644
--- a/security/insanity/lib/pkixder.h
+++ b/security/insanity/lib/pkixder.h
@@ -163,6 +163,11 @@ public:
     return Success;
   }
 
+  void SkipToEnd()
+  {
+    input = end;
+  }
+
   bool AtEnd() const { return input == end; }
 
   class Mark
diff --git a/security/insanity/lib/pkixocsp.cpp b/security/insanity/lib/pkixocsp.cpp
index c9e3056178ee..ee23e3541811 100644
--- a/security/insanity/lib/pkixocsp.cpp
+++ b/security/insanity/lib/pkixocsp.cpp
@@ -603,6 +603,11 @@ SingleResponse(der::Input& input, Context& context)
   }
 
   if (!match) {
+    // This response does not reference the certificate we're interested in.
+    // By consuming the rest of our input and returning successfully, we can
+    // continue processing and examine another response that might have what
+    // we want.
+    input.SkipToEnd();
     return der::Success;
   }
 
@@ -745,6 +750,10 @@ CertID(der::Input& input, const Context& context, /*out*/ bool& match)
   const CERTCertificate& issuerCert = context.issuerCert;
 
   if (!SECITEM_ItemsAreEqual(&serialNumber, &cert.serialNumber)) {
+    // This does not reference the certificate we're interested in.
+    // Consume the rest of the input and return successfully to
+    // potentially continue processing other responses.
+    input.SkipToEnd();
     return der::Success;
   }
 
@@ -752,6 +761,8 @@ CertID(der::Input& input, const Context& context, /*out*/ bool& match)
 
   SECOidTag hashAlg = SECOID_GetAlgorithmTag(&hashAlgorithm);
   if (hashAlg != SEC_OID_SHA1) {
+    // Again, not interested in this response. Consume input, return success.
+    input.SkipToEnd();
     return der::Success;
   }
 
@@ -768,6 +779,8 @@ CertID(der::Input& input, const Context& context, /*out*/ bool& match)
     return der::Failure;
   }
   if (memcmp(hashBuf, issuerNameHash.data, issuerNameHash.len)) {
+    // Again, not interested in this response. Consume input, return success.
+    input.SkipToEnd();
     return der::Success;
   }
 
diff --git a/security/manager/ssl/tests/unit/test_ocsp_stapling.js b/security/manager/ssl/tests/unit/test_ocsp_stapling.js
index ae7d16d3e5d6..e4e45a8e9936 100644
--- a/security/manager/ssl/tests/unit/test_ocsp_stapling.js
+++ b/security/manager/ssl/tests/unit/test_ocsp_stapling.js
@@ -95,11 +95,8 @@ function add_tests_in_mode(useInsanity, certDB, otherTestCA) {
                 true);
   add_ocsp_test("ocsp-stapling-unknown.example.com",
                 getXPCOMStatusFromNSS(SEC_ERROR_OCSP_UNKNOWN_CERT), true);
-  // TODO(bug 977870): this should not result in SEC_ERROR_BAD_DER
   add_ocsp_test("ocsp-stapling-good-other.example.com",
-                getXPCOMStatusFromNSS(
-                  useInsanity ? SEC_ERROR_BAD_DER
-                              : SEC_ERROR_OCSP_UNKNOWN_CERT), true);
+                getXPCOMStatusFromNSS(SEC_ERROR_OCSP_UNKNOWN_CERT), true);
   // If the server doesn't staple an OCSP response, we continue as normal
   // (this means that even though stapling is enabled, we expect an OCSP
   // request).