Add CheckOCSPDateValid() to net/cert/internal

net/cert/internal/parse_ocsp_unittest.cc

 // Copyright 2016 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_ocsp.h"
 
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "net/cert/internal/test_helpers.h"
 #include "net/cert/x509_certificate.h"
+#include "net/der/encode_values.h"
 #include "net/test/test_data_directory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
 
 namespace {
 
+const base::TimeDelta kOCSPAgeOneWeek = base::TimeDelta::FromDays(7);
+
+const base::Time kWindowsEpoch =
+    base::Time::UnixEpoch() -
+    base::TimeDelta::FromSeconds(INT64_C(11644473600));

[Ryan Sleevi, 2016/07/08 02:21:02]

No need for the INT64_C now that we're C++11 (which added the support for int64
constants w/o the size specifier)

That said, base::Time(0) is equivalent to the Windows Epoch (it's part of
base::Time's API contract -
https://cs.chromium.org/chromium/src/base/time/time.h?rcl=0&l=7 ) so this should
be unnecessary

[dadrian, 2016/07/08 22:25:42]

Done.

+
 std::string GetFilePath(const std::string& file_name) {
   return std::string("net/data/parse_ocsp_unittest/") + file_name;
 }
 
 enum OCSPFailure {
   OCSP_SUCCESS,
   PARSE_CERT,
   PARSE_OCSP,
   OCSP_NOT_SUCCESSFUL,
   PARSE_OCSP_DATA,
@@ skipping 147 matching lines @@
 }
 
 TEST(ParseOCSPTest, OCSPOCSPSingleExtension) {
   ASSERT_EQ(OCSP_SUCCESS, ParseOCSP("has_single_extension.pem"));
 }
 
 TEST(ParseOCSPTest, OCSPMissingResponse) {
   ASSERT_EQ(PARSE_OCSP_SINGLE_RESPONSE, ParseOCSP("missing_response.pem"));
 }
 
+TEST(OCSPDateTest, Valid) {
+  OCSPSingleResponse response;
+
+  base::Time now = base::Time::Now();
+  base::Time this_update = now - base::TimeDelta::FromHours(1);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(this_update, &response.this_update));
+  response.has_next_update = false;
+  EXPECT_TRUE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+
+  base::Time next_update = this_update + base::TimeDelta::FromDays(7);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(next_update, &response.next_update));
+  response.has_next_update = true;
+  EXPECT_TRUE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+}
+
+TEST(OCSPDateTest, ThisUpdateInTheFuture) {
+  OCSPSingleResponse response;
+
+  base::Time now = base::Time::Now();
+  base::Time this_update = now + base::TimeDelta::FromHours(1);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(this_update, &response.this_update));
+  response.has_next_update = false;
+  EXPECT_FALSE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+
+  base::Time next_update = this_update + base::TimeDelta::FromDays(7);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(next_update, &response.next_update));
+  response.has_next_update = true;
+  EXPECT_FALSE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+}
+
+TEST(OCSPDateTest, NextUpdatePassed) {
+  OCSPSingleResponse response;
+
+  base::Time now = base::Time::Now();
+  base::Time this_update = now - base::TimeDelta::FromDays(6);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(this_update, &response.this_update));
+  response.has_next_update = false;
+  EXPECT_TRUE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+
+  base::Time next_update = now - base::TimeDelta::FromHours(1);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(next_update, &response.next_update));
+  response.has_next_update = true;
+  EXPECT_FALSE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+}
+
+TEST(OCSPDateTest, NextUpdateBeforeThisUpdate) {
+  OCSPSingleResponse response;
+
+  base::Time now = base::Time::Now();
+  base::Time this_update = now - base::TimeDelta::FromDays(1);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(this_update, &response.this_update));
+  response.has_next_update = false;
+  EXPECT_TRUE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+
+  base::Time next_update = this_update - base::TimeDelta::FromDays(1);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(next_update, &response.next_update));
+  response.has_next_update = true;
+  EXPECT_FALSE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+}
+
+TEST(OCSPDateTest, ThisUpdateOlderThanMaxAge) {
+  OCSPSingleResponse response;
+
+  base::Time now = base::Time::Now();
+  base::Time this_update = now - kOCSPAgeOneWeek;
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(this_update, &response.this_update));
+  response.has_next_update = false;
+  EXPECT_TRUE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+
+  base::Time next_update = now + base::TimeDelta::FromHours(1);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(next_update, &response.next_update));
+  response.has_next_update = true;
+  EXPECT_TRUE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+
+  ASSERT_TRUE(der::EncodeTimeAsGeneralizedTime(
+      this_update - base::TimeDelta::FromSeconds(1), &response.this_update));
+  response.has_next_update = false;
+  EXPECT_FALSE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+  response.has_next_update = true;
+  EXPECT_FALSE(CheckOCSPDateValid(response, now, kOCSPAgeOneWeek));
+}
+
+TEST(OCSPDateTest, VerifyTimeFromBeforeWindowsEpoch) {
+  OCSPSingleResponse response;
+  base::Time verify_time = kWindowsEpoch - base::TimeDelta::FromDays(1);

[Ryan Sleevi, 2016/07/08 02:21:02]

I think my concerns about Windows Epoch may have been confused. That is, I don't
ever expect we'll send a junk verify_time in to these routines (and if we do,
always failing is a perfectly reasonable response). My concern about Windows
Epochs was related to this_update and next_update fields, since those are the
'untrusted' data that comes from CAs.

That is, imagine a CA is compromised and issues an OCSP response that, for
whatever reason, says 1600 (or 2039 on 32-bit, but let's use Windows as the
example)

My hope and expectation is that we reject it.

|verify_time| I'm not worried about (because we should always be programatically
checking that, and so DCHECK'ing on junk is fine, and base::Time::Now()
shouldn't give us junk), and while the max age has the theoretical risk of
causing overflow, that's also a developer screw up.

[dadrian, 2016/07/08 18:04:46]

Since GeneralizedTime handles times from before the Windows epoch, and we
convert _to_ generalized time, this is the only possible error condition we can
force here. This test just checks that we don't accidentally fail open.

+
+  base::Time now = base::Time::Now();
+  base::Time this_update = now - base::TimeDelta::FromHours(1);
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(this_update, &response.this_update));
+  response.has_next_update = false;
+  EXPECT_FALSE(CheckOCSPDateValid(response, verify_time, kOCSPAgeOneWeek));
+
+  base::Time next_update = this_update + kOCSPAgeOneWeek;
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(next_update, &response.next_update));
+  response.has_next_update = true;
+  EXPECT_FALSE(CheckOCSPDateValid(response, verify_time, kOCSPAgeOneWeek));
+}
+
+TEST(OCSPDateTest, VerifyTimeMinusAgeFromBeforeWindowsEpoch) {
+  OCSPSingleResponse response;
+  base::Time verify_time = kWindowsEpoch + base::TimeDelta::FromDays(1);
+
+  base::Time this_update = kWindowsEpoch;
+  ASSERT_TRUE(
+      der::EncodeTimeAsGeneralizedTime(this_update, &response.this_update));
+  response.has_next_update = false;
+#ifdef OS_WIN
+  EXPECT_FALSE(CheckOCSPDateValid(response, verify_time, kOCSPAgeOneWeek));
+#else
+  EXPECT_TRUE(CheckOCSPDateValid(response, verify_time, kOCSPAgeOneWeek));
+#endif
+}
+
 }  // namespace net