Do a better job testing expiration checks

Instead of staggering the ranges, we can just generate three variants of
each certificate. Also internal_verify is written in a remarkably goofy
way, so the partial chain case is a little interesting and worth
testing.

Change-Id: I94bfe58f54c5da2b88581af204c4e9d5b919f50b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65047
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index ab8ef7f..8b9cd64 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -4061,87 +4061,117 @@
   bssl::UniquePtr<EVP_PKEY> key = PrivateKeyFromPEM(kP256Key);
   ASSERT_TRUE(key);
 
-  // The following are measured in seconds relative to kReferenceTime. The
-  // validity periods are staggered so we can independently test both leaf and
-  // root time checks.
-  const int64_t kSecondsInDay = 24 * 3600;
-  const int64_t kRootStart = -30 * kSecondsInDay;
-  const int64_t kIntermediateStart = -20 * kSecondsInDay;
-  const int64_t kLeafStart = -10 * kSecondsInDay;
-  const int64_t kIntermediateEnd = 10 * kSecondsInDay;
-  const int64_t kLeafEnd = 20 * kSecondsInDay;
-  const int64_t kRootEnd = 30 * kSecondsInDay;
-
-  bssl::UniquePtr<X509> root =
-      MakeTestCert("Root", "Root", key.get(), /*is_ca=*/true);
-  ASSERT_TRUE(root);
-  ASSERT_TRUE(ASN1_TIME_adj(X509_getm_notBefore(root.get()), kReferenceTime,
-                            /*offset_day=*/0,
-                            /*offset_sec=*/kRootStart));
-  ASSERT_TRUE(ASN1_TIME_adj(X509_getm_notAfter(root.get()), kReferenceTime,
-                            /*offset_day=*/0,
-                            /*offset_sec=*/kRootEnd));
-  ASSERT_TRUE(X509_sign(root.get(), key.get(), EVP_sha256()));
-
-  bssl::UniquePtr<X509> intermediate =
-      MakeTestCert("Root", "Intermediate", key.get(), /*is_ca=*/true);
-  ASSERT_TRUE(intermediate);
-  ASSERT_TRUE(ASN1_TIME_adj(X509_getm_notBefore(intermediate.get()),
-                            kReferenceTime,
-                            /*offset_day=*/0,
-                            /*offset_sec=*/kIntermediateStart));
-  ASSERT_TRUE(ASN1_TIME_adj(X509_getm_notAfter(intermediate.get()),
-                            kReferenceTime,
-                            /*offset_day=*/0,
-                            /*offset_sec=*/kIntermediateEnd));
-  ASSERT_TRUE(X509_sign(intermediate.get(), key.get(), EVP_sha256()));
-
-  bssl::UniquePtr<X509> leaf =
-      MakeTestCert("Intermediate", "Leaf", key.get(), /*is_ca=*/false);
-  ASSERT_TRUE(leaf);
-  ASSERT_TRUE(ASN1_TIME_adj(X509_getm_notBefore(leaf.get()), kReferenceTime,
-                            /*offset_day=*/0,
-                            /*offset_sec=*/kLeafStart));
-  ASSERT_TRUE(ASN1_TIME_adj(X509_getm_notAfter(leaf.get()), kReferenceTime,
-                            /*offset_day=*/0,
-                            /*offset_sec=*/kLeafEnd));
-  ASSERT_TRUE(X509_sign(leaf.get(), key.get(), EVP_sha256()));
-
-  struct VerifyAt {
-    int64_t time;
-    void operator()(X509_VERIFY_PARAM *param) const {
-      X509_VERIFY_PARAM_set_time_posix(param, time);
+  auto make_cert = [&](const char *issuer, const char *subject, bool is_ca,
+                       int not_before_offset,
+                       int not_after_offset) -> bssl::UniquePtr<X509> {
+    bssl::UniquePtr<X509> cert =
+        MakeTestCert(issuer, subject, key.get(), is_ca);
+    if (cert == nullptr ||
+        !ASN1_TIME_adj(X509_getm_notBefore(cert.get()), kReferenceTime,
+                       /*offset_day=*/not_before_offset,
+                       /*offset_sec=*/0) ||
+        !ASN1_TIME_adj(X509_getm_notAfter(cert.get()), kReferenceTime,
+                       /*offset_day=*/not_after_offset,
+                       /*offset_sec=*/0) ||
+        !X509_sign(cert.get(), key.get(), EVP_sha256())) {
+      return nullptr;
     }
+    return cert;
   };
 
+  struct Certs {
+    bssl::UniquePtr<X509> not_yet_valid, valid, expired;
+  };
+  auto make_certs = [&](const char *issuer, const char *subject,
+                        bool is_ca) -> Certs {
+    Certs certs;
+    certs.not_yet_valid =
+        make_cert(issuer, subject, is_ca, /*not_before_offset=*/1,
+                  /*not_after_offset=*/2);
+    certs.valid = make_cert(issuer, subject, is_ca, /*not_before_offset=*/-1,
+                            /*not_after_offset=*/1);
+    certs.expired = make_cert(issuer, subject, is_ca, /*not_before_offset=*/-2,
+                              /*not_after_offset=*/-1);
+    if (certs.not_yet_valid == nullptr || certs.valid == nullptr ||
+        certs.expired == nullptr) {
+      return Certs{};
+    }
+    return certs;
+  };
+
+  Certs root = make_certs("Root", "Root", /*is_ca=*/true);
+  ASSERT_TRUE(root.valid);
+  Certs root_cross = make_certs("Root 2", "Root", /*is_ca=*/true);
+  ASSERT_TRUE(root_cross.valid);
+  Certs intermediate = make_certs("Root", "Intermediate", /*is_ca=*/true);
+  ASSERT_TRUE(intermediate.valid);
+  Certs leaf = make_certs("Intermediate", "Leaf", /*is_ca=*/false);
+  ASSERT_TRUE(leaf.valid);
+
   for (bool check_time : {true, false}) {
     SCOPED_TRACE(check_time);
-    unsigned long flags = check_time ? 0 : X509_V_FLAG_NO_CHECK_TIME;
-    int not_yet_valid = check_time ? X509_V_ERR_CERT_NOT_YET_VALID : X509_V_OK;
-    int has_expired = check_time ? X509_V_ERR_CERT_HAS_EXPIRED : X509_V_OK;
+    for (bool partial_chain : {true, false}) {
+      SCOPED_TRACE(partial_chain);
+      unsigned long flags = 0;
+      if (!check_time) {
+        flags |= X509_V_FLAG_NO_CHECK_TIME;
+      }
+      if (partial_chain) {
+        flags |= X509_V_FLAG_PARTIAL_CHAIN;
+      }
 
-    EXPECT_EQ(not_yet_valid,
-              Verify(leaf.get(), {root.get()}, {intermediate.get()}, {}, flags,
-                     VerifyAt{kReferenceTime + kRootStart - 1}));
-    EXPECT_EQ(not_yet_valid,
-              Verify(leaf.get(), {root.get()}, {intermediate.get()}, {}, flags,
-                     VerifyAt{kReferenceTime + kIntermediateStart - 1}));
-    EXPECT_EQ(not_yet_valid,
-              Verify(leaf.get(), {root.get()}, {intermediate.get()}, {}, flags,
-                     VerifyAt{kReferenceTime + kLeafStart - 1}));
+      int not_yet_valid =
+          check_time ? X509_V_ERR_CERT_NOT_YET_VALID : X509_V_OK;
+      int has_expired = check_time ? X509_V_ERR_CERT_HAS_EXPIRED : X509_V_OK;
 
-    EXPECT_EQ(X509_V_OK, Verify(leaf.get(), {root.get()}, {intermediate.get()},
-                                {}, flags, VerifyAt{kReferenceTime}));
+      EXPECT_EQ(not_yet_valid,
+                Verify(leaf.not_yet_valid.get(), {root.valid.get()},
+                       {intermediate.valid.get()}, {}, flags));
+      EXPECT_EQ(not_yet_valid,
+                Verify(leaf.valid.get(), {root.valid.get()},
+                       {intermediate.not_yet_valid.get()}, {}, flags));
+      EXPECT_EQ(not_yet_valid,
+                Verify(leaf.valid.get(), {root.not_yet_valid.get()},
+                       {intermediate.valid.get()}, {}, flags));
 
-    EXPECT_EQ(has_expired,
-              Verify(leaf.get(), {root.get()}, {intermediate.get()}, {}, flags,
-                     VerifyAt{kReferenceTime + kRootEnd + 1}));
-    EXPECT_EQ(has_expired,
-              Verify(leaf.get(), {root.get()}, {intermediate.get()}, {}, flags,
-                     VerifyAt{kReferenceTime + kIntermediateEnd + 1}));
-    EXPECT_EQ(has_expired,
-              Verify(leaf.get(), {root.get()}, {intermediate.get()}, {}, flags,
-                     VerifyAt{kReferenceTime + kLeafEnd + 1}));
+      EXPECT_EQ(X509_V_OK, Verify(leaf.valid.get(), {root.valid.get()},
+                                  {intermediate.valid.get()}, {}, flags));
+
+      EXPECT_EQ(has_expired, Verify(leaf.expired.get(), {root.valid.get()},
+                                    {intermediate.valid.get()}, {}, flags));
+      EXPECT_EQ(has_expired, Verify(leaf.valid.get(), {root.valid.get()},
+                                    {intermediate.expired.get()}, {}, flags));
+      EXPECT_EQ(has_expired, Verify(leaf.valid.get(), {root.expired.get()},
+                                    {intermediate.valid.get()}, {}, flags));
+
+      if (!partial_chain) {
+        // By default, non-self-signed certificates are not valid trust anchors.
+        EXPECT_EQ(X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT,
+                  Verify(leaf.valid.get(), {root_cross.valid.get()},
+                         {intermediate.valid.get()}, {}, flags));
+      } else {
+        // |X509_V_FLAG_PARTIAL_CHAIN| allows non-self-signed trust anchors.
+        EXPECT_EQ(X509_V_OK, Verify(leaf.valid.get(), {root_cross.valid.get()},
+                                    {intermediate.valid.get()}, {}, flags));
+        // Expiry of the trust anchor must still be checked.
+        EXPECT_EQ(not_yet_valid,
+                  Verify(leaf.valid.get(), {root_cross.not_yet_valid.get()},
+                         {intermediate.valid.get()}, {}, flags));
+        EXPECT_EQ(has_expired,
+                  Verify(leaf.valid.get(), {root_cross.expired.get()},
+                         {intermediate.valid.get()}, {}, flags));
+      }
+
+      // When the trust anchor is the target certificate, expiry should also be
+      // checked.
+      EXPECT_EQ(X509_V_OK,
+                Verify(root.valid.get(), {root.valid.get()}, {}, {}, flags));
+      EXPECT_EQ(not_yet_valid,
+                Verify(root.not_yet_valid.get(), {root.not_yet_valid.get()}, {},
+                       {}, flags));
+      EXPECT_EQ(has_expired, Verify(root.expired.get(), {root.expired.get()},
+                                    {}, {}, flags));
+    }
   }
 }