Fix policy validation when the user policy set is NULL

OpenSSL interprets NULL and empty lists as {anyPolicy}. I intended to
implement this, but didn't quite get the NULL case right. Fix this and
add a test.

Change-Id: I50dbf02695f424697e28a6e0ec4fd50b2822f44f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58425
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
This commit is contained in:
David Benjamin 2023-03-29 14:00:20 +09:00 committed by Boringssl LUCI CQ
parent 678bae4083
commit fca688f26b
3 changed files with 15 additions and 7 deletions

View File

@ -404,7 +404,8 @@ int x509_digest_verify_init(EVP_MD_CTX *ctx, const X509_ALGOR *sigalg,
// Path-building functions.
// X509_policy_check checks certificate policies in |certs|. |user_policies| is
// the user-initial-policy-set. |flags| is a set of |X509_V_FLAG_*| values to
// the user-initial-policy-set. If |user_policies| is NULL or empty, it is
// interpreted as anyPolicy. |flags| is a set of |X509_V_FLAG_*| values to
// apply. It returns |X509_V_OK| on success and |X509_V_ERR_*| on error. It
// additionally sets |*out_current_cert| to the certificate where the error
// occurred. If the function succeeded, or the error applies to the entire

View File

@ -579,7 +579,7 @@ static int process_policy_constraints(const X509 *x509, size_t *explicit_policy,
// evaluation.
static int has_explicit_policy(STACK_OF(X509_POLICY_LEVEL) *levels,
const STACK_OF(ASN1_OBJECT) *user_policies) {
assert(sk_ASN1_OBJECT_is_sorted(user_policies));
assert(user_policies == NULL || sk_ASN1_OBJECT_is_sorted(user_policies));
// Step (g.i). If the policy graph is empty, the intersection is empty.
size_t num_levels = sk_X509_POLICY_LEVEL_num(levels);
@ -763,12 +763,14 @@ int X509_policy_check(const STACK_OF(X509) *certs,
// is only necessary to check if the user-constrained-policy-set is not empty.
if (explicit_policy == 0) {
// Build a sorted copy of |user_policies| for more efficient lookup.
user_policies_sorted = sk_ASN1_OBJECT_dup(user_policies);
if (user_policies_sorted == NULL) {
goto err;
if (user_policies != NULL) {
user_policies_sorted = sk_ASN1_OBJECT_dup(user_policies);
if (user_policies_sorted == NULL) {
goto err;
}
sk_ASN1_OBJECT_set_cmp_func(user_policies_sorted, asn1_object_cmp);
sk_ASN1_OBJECT_sort(user_policies_sorted);
}
sk_ASN1_OBJECT_set_cmp_func(user_policies_sorted, asn1_object_cmp);
sk_ASN1_OBJECT_sort(user_policies_sorted);
if (!has_explicit_policy(levels, user_policies_sorted)) {
ret = X509_V_ERR_NO_EXPLICIT_POLICY;

View File

@ -5358,6 +5358,8 @@ TEST(X509Test, Policy) {
};
// The chain is good for |oid1| and |oid2|, but not |oid3|.
EXPECT_EQ(X509_V_OK, Verify(leaf.get(), {root.get()}, {intermediate.get()},
/*crls=*/{}, X509_V_FLAG_EXPLICIT_POLICY));
EXPECT_EQ(X509_V_OK,
Verify(leaf.get(), {root.get()}, {intermediate.get()}, /*crls=*/{},
X509_V_FLAG_EXPLICIT_POLICY, [&](X509_VERIFY_PARAM *param) {
@ -5442,6 +5444,9 @@ TEST(X509Test, Policy) {
}));
// 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));
EXPECT_EQ(X509_V_OK, Verify(leaf_require.get(), {root.get()},
{intermediate.get()}, /*crls=*/{},
/*flags=*/0, [&](X509_VERIFY_PARAM *param) {