Bug 1097622 - Return ERROR_INVALID_TIME when decoding invalid time values. r=dkeeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Wed, 18 Feb 2015 15:56:00 -0500
changeset 256943 2163383b541a12f5cf39bb726432085d51eb2f6f
parent 256942 fd5016a0c130d53c4e178dab58a72099f9bf02c9
child 256944 13b2859e5c155a5993e86ffb914b0620cf6f4031
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdkeeler
bugs1097622
milestone38.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 1097622 - Return ERROR_INVALID_TIME when decoding invalid time values. r=dkeeler
security/manager/ssl/src/NSSErrorsService.cpp
security/manager/ssl/src/SSLServerCertVerification.cpp
security/pkix/lib/pkixcheck.cpp
security/pkix/test/gtest/pkixcheck_CheckValidity_tests.cpp
--- a/security/manager/ssl/src/NSSErrorsService.cpp
+++ b/security/manager/ssl/src/NSSErrorsService.cpp
@@ -141,26 +141,27 @@ NSSErrorsService::GetErrorClass(nsresult
 }
 
 bool
 ErrorIsOverridable(PRErrorCode code)
 {
   switch (code)
   {
     // Overridable errors.
-    case SEC_ERROR_UNKNOWN_ISSUER:
-    case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
-    case SSL_ERROR_BAD_CERT_DOMAIN:
-    case SEC_ERROR_EXPIRED_CERTIFICATE:
-    case SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED:
     case mozilla::pkix::MOZILLA_PKIX_ERROR_CA_CERT_USED_AS_END_ENTITY:
     case mozilla::pkix::MOZILLA_PKIX_ERROR_INADEQUATE_KEY_SIZE:
-    case mozilla::pkix::MOZILLA_PKIX_ERROR_V1_CERT_USED_AS_CA:
     case mozilla::pkix::MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE:
     case mozilla::pkix::MOZILLA_PKIX_ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE:
+    case mozilla::pkix::MOZILLA_PKIX_ERROR_V1_CERT_USED_AS_CA:
+    case SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED:
+    case SEC_ERROR_EXPIRED_CERTIFICATE:
+    case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
+    case SEC_ERROR_INVALID_TIME:
+    case SEC_ERROR_UNKNOWN_ISSUER:
+    case SSL_ERROR_BAD_CERT_DOMAIN:
       return true;
     // Non-overridable errors.
     default:
       return false;
   }
 }
 
 NS_IMETHODIMP
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -308,16 +308,17 @@ MapCertErrorToProbeValue(PRErrorCode err
     case SSL_ERROR_BAD_CERT_DOMAIN:                    return  9;
     case SEC_ERROR_EXPIRED_CERTIFICATE:                return 10;
     case mozilla::pkix::MOZILLA_PKIX_ERROR_CA_CERT_USED_AS_END_ENTITY: return 11;
     case mozilla::pkix::MOZILLA_PKIX_ERROR_V1_CERT_USED_AS_CA: return 12;
     case mozilla::pkix::MOZILLA_PKIX_ERROR_INADEQUATE_KEY_SIZE: return 13;
     case mozilla::pkix::MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE: return 14;
     case mozilla::pkix::MOZILLA_PKIX_ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE:
       return 15;
+    case SEC_ERROR_INVALID_TIME: return 16;
   }
   NS_WARNING("Unknown certificate error code. Does MapCertErrorToProbeValue "
              "handle everything in DetermineCertOverrideErrors?");
   return 0;
 }
 
 SECStatus
 DetermineCertOverrideErrors(CERTCertificate* cert, const char* hostName,
@@ -363,16 +364,17 @@ DetermineCertOverrideErrors(CERTCertific
       } else if (validity == secCertTimeNotValidYet) {
         collectedErrors |= nsICertOverrideService::ERROR_TIME;
         errorCodeTime =
           mozilla::pkix::MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE;
       }
       break;
     }
 
+    case SEC_ERROR_INVALID_TIME:
     case SEC_ERROR_EXPIRED_CERTIFICATE:
     case mozilla::pkix::MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE:
       collectedErrors = nsICertOverrideService::ERROR_TIME;
       errorCodeTime = defaultErrorCodeToReport;
       break;
 
     case SSL_ERROR_BAD_CERT_DOMAIN:
       collectedErrors = nsICertOverrideService::ERROR_MISMATCH;
--- a/security/pkix/lib/pkixcheck.cpp
+++ b/security/pkix/lib/pkixcheck.cpp
@@ -32,31 +32,41 @@ namespace mozilla { namespace pkix {
 // 4.1.2.5 Validity
 
 Result
 CheckValidity(Input encodedValidity, Time time)
 {
   Reader validity(encodedValidity);
   Time notBefore(Time::uninitialized);
   if (der::TimeChoice(validity, notBefore) != Success) {
-    return Result::ERROR_EXPIRED_CERTIFICATE;
-  }
-  if (time < notBefore) {
-    return Result::ERROR_NOT_YET_VALID_CERTIFICATE;
+    return Result::ERROR_INVALID_DER_TIME;
   }
 
   Time notAfter(Time::uninitialized);
   if (der::TimeChoice(validity, notAfter) != Success) {
-    return Result::ERROR_EXPIRED_CERTIFICATE;
+    return Result::ERROR_INVALID_DER_TIME;
+  }
+
+  if (der::End(validity) != Success) {
+    return Result::ERROR_INVALID_DER_TIME;
   }
+
+  if (notBefore > notAfter) {
+    return Result::ERROR_INVALID_DER_TIME;
+  }
+
+  if (time < notBefore) {
+    return Result::ERROR_NOT_YET_VALID_CERTIFICATE;
+  }
+
   if (time > notAfter) {
     return Result::ERROR_EXPIRED_CERTIFICATE;
   }
 
-  return der::End(validity);
+  return Success;
 }
 
 // 4.1.2.7 Subject Public Key Info
 
 Result
 CheckSubjectPublicKeyInfo(Reader& input, TrustDomain& trustDomain,
                           EndEntityOrCA endEntityOrCA)
 {
--- a/security/pkix/test/gtest/pkixcheck_CheckValidity_tests.cpp
+++ b/security/pkix/test/gtest/pkixcheck_CheckValidity_tests.cpp
@@ -65,38 +65,37 @@ class pkixcheck_CheckValidity : public :
 
 TEST_F(pkixcheck_CheckValidity, BothEmptyNull)
 {
   static const uint8_t DER[] = {
     0x17/*UTCTime*/, 0/*length*/,
     0x17/*UTCTime*/, 0/*length*/,
   };
   static const Input validity(DER);
-  ASSERT_EQ(Result::ERROR_EXPIRED_CERTIFICATE, CheckValidity(validity, NOW));
+  ASSERT_EQ(Result::ERROR_INVALID_DER_TIME, CheckValidity(validity, NOW));
 }
 
 TEST_F(pkixcheck_CheckValidity, NotBeforeEmptyNull)
 {
   static const uint8_t DER[] = {
     0x17/*UTCTime*/, 0x00/*length*/,
     NEWER_UTCTIME
   };
   static const Input validity(DER);
-  ASSERT_EQ(Result::ERROR_EXPIRED_CERTIFICATE, CheckValidity(validity, NOW));
+  ASSERT_EQ(Result::ERROR_INVALID_DER_TIME, CheckValidity(validity, NOW));
 }
 
 TEST_F(pkixcheck_CheckValidity, NotAfterEmptyNull)
 {
   static const uint8_t DER[] = {
     NEWER_UTCTIME,
     0x17/*UTCTime*/, 0x00/*length*/,
   };
   static const Input validity(DER);
-  ASSERT_EQ(Result::ERROR_NOT_YET_VALID_CERTIFICATE,
-            CheckValidity(validity, NOW));
+  ASSERT_EQ(Result::ERROR_INVALID_DER_TIME, CheckValidity(validity, NOW));
 }
 
 static const uint8_t OLDER_UTCTIME_NEWER_UTCTIME_DATA[] = {
   OLDER_UTCTIME,
   NEWER_UTCTIME,
 };
 static const Input
 OLDER_UTCTIME_NEWER_UTCTIME(OLDER_UTCTIME_NEWER_UTCTIME_DATA);
@@ -150,11 +149,10 @@ TEST_F(pkixcheck_CheckValidity, InvalidA
 
 TEST_F(pkixcheck_CheckValidity, InvalidNotAfterBeforeNotBefore)
 {
   static const uint8_t DER[] = {
     NEWER_UTCTIME,
     OLDER_UTCTIME,
   };
   static const Input validity(DER);
-  ASSERT_EQ(Result::ERROR_NOT_YET_VALID_CERTIFICATE,
-            CheckValidity(validity, NOW));
+  ASSERT_EQ(Result::ERROR_INVALID_DER_TIME, CheckValidity(validity, NOW));
 }