update master-with-bazel from master branch

BUILD.generated_tests.bzl

@@ -277,6 +277,7 @@ crypto_test_data = [
     "src/crypto/x509/test/policy_leaf_any.pem",
     "src/crypto/x509/test/policy_leaf_duplicate.pem",
     "src/crypto/x509/test/policy_leaf_invalid.pem",
+    "src/crypto/x509/test/policy_leaf_none.pem",
     "src/crypto/x509/test/policy_leaf_oid1.pem",
     "src/crypto/x509/test/policy_leaf_oid2.pem",
     "src/crypto/x509/test/policy_leaf_oid3.pem",

sources.json

@@ -691,6 +691,7 @@
     "src/crypto/x509/test/policy_leaf_any.pem", 
     "src/crypto/x509/test/policy_leaf_duplicate.pem", 
     "src/crypto/x509/test/policy_leaf_invalid.pem", 
+    "src/crypto/x509/test/policy_leaf_none.pem", 
     "src/crypto/x509/test/policy_leaf_oid1.pem", 
     "src/crypto/x509/test/policy_leaf_oid2.pem", 
     "src/crypto/x509/test/policy_leaf_oid3.pem", 

src/crypto/x509/test/make_policy_certs.go

@@ -302,6 +302,10 @@ func main() {
 	leafSingle.template.PolicyIdentifiers = []asn1.ObjectIdentifier{testOID5}
 	mustGenerateCertificate("policy_leaf_oid5.pem", &leafSingle, &intermediate)
 
+	leafNone := leaf
+	leafNone.template.PolicyIdentifiers = nil
+	mustGenerateCertificate("policy_leaf_none.pem", &leafNone, &intermediate)
+
 	// Make version of Root, signed by Root 2, with policy mapping inhibited.
 	// This can be combined with intermediateMapped to test the combination.
 	b = cryptobyte.NewBuilder(nil)

src/crypto/x509/test/policy_leaf_none.pem

@@ -0,0 +1,10 @@
+-----BEGIN CERTIFICATE-----
+MIIBezCCASCgAwIBAgIBAzAKBggqhkjOPQQDAjAeMRwwGgYDVQQDExNQb2xpY3kg
+SW50ZXJtZWRpYXRlMCAXDTAwMDEwMTAwMDAwMFoYDzIxMDAwMTAxMDAwMDAwWjAa
+MRgwFgYDVQQDEw93d3cuZXhhbXBsZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMB
+BwNCAASRKti8VW2Rkma+Kt9jQkMNitlCs0l5w8u3SSwm7HZREvmcBCJBjVIREacR
+qI0umhzR2V5NLzBBP9yPD/A+Ch5Xo1EwTzAOBgNVHQ8BAf8EBAMCAgQwEwYDVR0l
+BAwwCgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADAaBgNVHREEEzARgg93d3cuZXhh
+bXBsZS5jb20wCgYIKoZIzj0EAwIDSQAwRgIhAIDFeeYJ8nmYo09OnJFpNS3A6fYO
+ZliHkAqOsg193DTnAiEA3OSHLCczcvRjMG+qd/FI61u2sKU1hhHh7uHtD/YO/dA=
+-----END CERTIFICATE-----

src/crypto/x509/x509_test.cc

@@ -5313,6 +5313,9 @@ TEST(X509Test, Policy) {
   bssl::UniquePtr<X509> leaf_invalid(CertFromPEM(
       GetTestData("crypto/x509/test/policy_leaf_invalid.pem").c_str()));
   ASSERT_TRUE(leaf_invalid);
+  bssl::UniquePtr<X509> leaf_none(CertFromPEM(
+      GetTestData("crypto/x509/test/policy_leaf_none.pem").c_str()));
+  ASSERT_TRUE(leaf_none);
   bssl::UniquePtr<X509> leaf_oid1(CertFromPEM(
       GetTestData("crypto/x509/test/policy_leaf_oid1.pem").c_str()));
   ASSERT_TRUE(leaf_oid1);
@@ -5335,14 +5338,6 @@ TEST(X509Test, Policy) {
       GetTestData("crypto/x509/test/policy_leaf_require1.pem").c_str()));
   ASSERT_TRUE(leaf_require1);
 
-  // By default, OpenSSL does not check policies, so even syntax errors in the
-  // certificatePolicies extension go unnoticed. (This is probably not
-  // important.)
-  EXPECT_EQ(X509_V_OK, Verify(leaf.get(), {root.get()},
-                              {intermediate.get()}, /*crls=*/{}));
-  EXPECT_EQ(X509_V_OK, Verify(leaf_invalid.get(), {root.get()},
-                              {intermediate.get()}, /*crls=*/{}));
-
   auto set_policies = [](X509_VERIFY_PARAM *param,
                          std::vector<const ASN1_OBJECT *> oids) {
     for (const ASN1_OBJECT *oid : oids) {
@@ -5351,9 +5346,6 @@ TEST(X509Test, Policy) {
       ASSERT_TRUE(X509_VERIFY_PARAM_add0_policy(param, copy.get()));
       copy.release();  // |X509_VERIFY_PARAM_add0_policy| takes ownership on
                        // success.
-      // TODO(davidben): |X509_VERIFY_PARAM_add0_policy| does not set this flag,
-      // while |X509_VERIFY_PARAM_set1_policies| does. Is this a bug?
-      X509_VERIFY_PARAM_set_flags(param, X509_V_FLAG_POLICY_CHECK);
     }
   };
 
@@ -5399,6 +5391,9 @@ TEST(X509Test, Policy) {
                    [&](X509_VERIFY_PARAM *param) {
                      set_policies(param, {oid1.get()});
                    }));
+  EXPECT_EQ(X509_V_ERR_INVALID_POLICY_EXTENSION,
+            Verify(leaf_invalid.get(), {root.get()}, {intermediate.get()},
+                   /*crls=*/{}));
 
   // There is a duplicate policy in the policy extension.
   EXPECT_EQ(X509_V_ERR_INVALID_POLICY_EXTENSION,
@@ -5443,10 +5438,17 @@ TEST(X509Test, Policy) {
                      set_policies(param, {oid3.get()});
                    }));
 
+  // requireExplicitPolicy applies even if the application does not configure a
+  // user-initial-policy-set. If the validation results in no policies, the
+  // chain is invalid.
+  EXPECT_EQ(X509_V_ERR_NO_EXPLICIT_POLICY,
+            Verify(leaf_none.get(), {root.get()}, {intermediate_require.get()},
+                   /*crls=*/{}));
+
   // A leaf can also set requireExplicitPolicy.
   EXPECT_EQ(X509_V_OK,
             Verify(leaf_require.get(), {root.get()}, {intermediate.get()},
-                   /*crls=*/{}, X509_V_FLAG_POLICY_CHECK));
+                   /*crls=*/{}, /*flags=*/0));
   EXPECT_EQ(X509_V_OK, Verify(leaf_require.get(), {root.get()},
                               {intermediate.get()}, /*crls=*/{},
                               /*flags=*/0, [&](X509_VERIFY_PARAM *param) {

src/crypto/x509/x509_vfy.c

@@ -447,7 +447,6 @@ int X509_verify_cert(X509_STORE_CTX *ctx) {
 
   // Check revocation status: we do this after copying parameters because
   // they may be needed for CRL signature verification.
-
   ok = ctx->check_revocation(ctx);
   if (!ok) {
     goto end;
@@ -464,14 +463,13 @@ int X509_verify_cert(X509_STORE_CTX *ctx) {
   }
 
   // Check name constraints
-
   ok = check_name_constraints(ctx);
   if (!ok) {
     goto end;
   }
 
-  // If we get this far evaluate policies
-  if (!bad_chain && (ctx->param->flags & X509_V_FLAG_POLICY_CHECK)) {
+  // If we get this far, evaluate policies.
+  if (!bad_chain) {
     ok = ctx->check_policy(ctx);
   }
 
@@ -1612,6 +1610,7 @@ static int cert_crl(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x) {
 }
 
 static int check_policy(X509_STORE_CTX *ctx) {
+  // TODO(davidben): Why do we disable policy validation for CRL paths?
   if (ctx->parent) {
     return 1;
   }

src/crypto/x509/x509_vpm.c

@@ -350,9 +350,6 @@ int X509_VERIFY_PARAM_set1_name(X509_VERIFY_PARAM *param, const char *name) {
 
 int X509_VERIFY_PARAM_set_flags(X509_VERIFY_PARAM *param, unsigned long flags) {
   param->flags |= flags;
-  if (flags & X509_V_FLAG_POLICY_MASK) {
-    param->flags |= X509_V_FLAG_POLICY_CHECK;
-  }
   return 1;
 }
 
@@ -398,8 +395,6 @@ int X509_VERIFY_PARAM_add0_policy(X509_VERIFY_PARAM *param,
   if (!sk_ASN1_OBJECT_push(param->policies, policy)) {
     return 0;
   }
-  // TODO(davidben): This does not set |X509_V_FLAG_POLICY_CHECK|, while
-  // |X509_VERIFY_PARAM_set1_policies| does. Is this a bug?
   return 1;
 }
 
@@ -421,7 +416,6 @@ int X509_VERIFY_PARAM_set1_policies(X509_VERIFY_PARAM *param,
     return 0;
   }
 
-  param->flags |= X509_V_FLAG_POLICY_CHECK;
   return 1;
 }
 

src/include/openssl/x509.h

@@ -2565,7 +2565,7 @@ OPENSSL_EXPORT void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth);
 #define X509_V_FLAG_X509_STRICT 0x00
 // This flag does nothing as proxy certificate support has been removed.
 #define X509_V_FLAG_ALLOW_PROXY_CERTS 0x40
-// Enable policy checking
+// Does nothing as its functionality has been enabled by default.
 #define X509_V_FLAG_POLICY_CHECK 0x80
 // Policy variable require-explicit-policy
 #define X509_V_FLAG_EXPLICIT_POLICY 0x100
@@ -2602,11 +2602,6 @@ OPENSSL_EXPORT void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth);
 #define X509_VP_FLAG_LOCKED 0x8
 #define X509_VP_FLAG_ONCE 0x10
 
-// Internal use: mask of policy related options
-#define X509_V_FLAG_POLICY_MASK                             \
-  (X509_V_FLAG_POLICY_CHECK | X509_V_FLAG_EXPLICIT_POLICY | \
-   X509_V_FLAG_INHIBIT_ANY | X509_V_FLAG_INHIBIT_MAP)
-
 OPENSSL_EXPORT int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h,
                                               int type, X509_NAME *name);
 OPENSSL_EXPORT X509_OBJECT *X509_OBJECT_retrieve_by_subject(

src/sources.cmake

@@ -127,6 +127,7 @@ set(
   crypto/x509/test/policy_leaf_any.pem
   crypto/x509/test/policy_leaf_duplicate.pem
   crypto/x509/test/policy_leaf_invalid.pem
+  crypto/x509/test/policy_leaf_none.pem
   crypto/x509/test/policy_leaf_oid1.pem
   crypto/x509/test/policy_leaf_oid2.pem
   crypto/x509/test/policy_leaf_oid3.pem