Remove X509_STORE_set0_additional_untrusted.

This was added in
https://boringssl-review.googlesource.com/c/boringssl/+/12980/, but does
not appear to be used anymore. The corresponding function does not exist
in OpenSSL.

This simplifies the tests slightly, some of which were inadvertently
specifying the boolean and some weren't.

Change-Id: I9b956dcd9f7151910f93f377d207c88273bd9ccf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49747
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
grpc-202302
David Benjamin 3 years ago committed by Boringssl LUCI CQ
parent 8f5eb80b81
commit 523d6c74c0
  1. 1
      crypto/x509/internal.h
  2. 5
      crypto/x509/x509_lu.c
  3. 68
      crypto/x509/x509_test.cc
  4. 36
      crypto/x509/x509_vfy.c
  5. 5
      include/openssl/x509_vfy.h

@ -268,7 +268,6 @@ struct x509_store_st {
int cache; // if true, stash any hits int cache; // if true, stash any hits
STACK_OF(X509_OBJECT) *objs; // Cache of all objects STACK_OF(X509_OBJECT) *objs; // Cache of all objects
CRYPTO_MUTEX objs_lock; CRYPTO_MUTEX objs_lock;
STACK_OF(X509) *additional_untrusted;
// These are external lookup methods // These are external lookup methods
STACK_OF(X509_LOOKUP) *get_cert_methods; STACK_OF(X509_LOOKUP) *get_cert_methods;

@ -404,11 +404,6 @@ int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x)
return ret; return ret;
} }
void X509_STORE_set0_additional_untrusted(X509_STORE *ctx,
STACK_OF(X509) *untrusted) {
ctx->additional_untrusted = untrusted;
}
int X509_OBJECT_up_ref_count(X509_OBJECT *a) int X509_OBJECT_up_ref_count(X509_OBJECT *a)
{ {
switch (a->type) { switch (a->type) {

@ -1111,9 +1111,8 @@ static const time_t kReferenceTime = 1474934400 /* Sep 27th, 2016 */;
static int Verify( static int Verify(
X509 *leaf, const std::vector<X509 *> &roots, X509 *leaf, const std::vector<X509 *> &roots,
const std::vector<X509 *> &intermediates, const std::vector<X509 *> &intermediates,
const std::vector<X509_CRL *> &crls, unsigned long flags, const std::vector<X509_CRL *> &crls, unsigned long flags = 0,
bool use_additional_untrusted, std::function<void(X509_VERIFY_PARAM *)> configure_callback = nullptr,
std::function<void(X509_VERIFY_PARAM *)> configure_callback,
int (*verify_callback)(int, X509_STORE_CTX *) = nullptr) { int (*verify_callback)(int, X509_STORE_CTX *) = nullptr) {
bssl::UniquePtr<STACK_OF(X509)> roots_stack(CertsToStack(roots)); bssl::UniquePtr<STACK_OF(X509)> roots_stack(CertsToStack(roots));
bssl::UniquePtr<STACK_OF(X509)> intermediates_stack( bssl::UniquePtr<STACK_OF(X509)> intermediates_stack(
@ -1133,14 +1132,8 @@ static int Verify(
return X509_V_ERR_UNSPECIFIED; return X509_V_ERR_UNSPECIFIED;
} }
if (use_additional_untrusted) { if (!X509_STORE_CTX_init(ctx.get(), store.get(), leaf,
X509_STORE_set0_additional_untrusted(store.get(), intermediates_stack.get())) {
intermediates_stack.get());
}
if (!X509_STORE_CTX_init(
ctx.get(), store.get(), leaf,
use_additional_untrusted ? nullptr : intermediates_stack.get())) {
return X509_V_ERR_UNSPECIFIED; return X509_V_ERR_UNSPECIFIED;
} }
@ -1164,27 +1157,6 @@ static int Verify(
return X509_V_OK; return X509_V_OK;
} }
static int Verify(
X509 *leaf, const std::vector<X509 *> &roots,
const std::vector<X509 *> &intermediates,
const std::vector<X509_CRL *> &crls, unsigned long flags = 0,
std::function<void(X509_VERIFY_PARAM *)> configure_callback = nullptr) {
const int r1 = Verify(leaf, roots, intermediates, crls, flags, false,
configure_callback);
const int r2 =
Verify(leaf, roots, intermediates, crls, flags, true, configure_callback);
if (r1 != r2) {
fprintf(stderr,
"Verify with, and without, use_additional_untrusted gave different "
"results: %d vs %d.\n",
r1, r2);
return false;
}
return r1;
}
TEST(X509Test, TestVerify) { TEST(X509Test, TestVerify) {
// cross_signing_root // cross_signing_root
// | // |
@ -1325,7 +1297,7 @@ TEST(X509Test, ZeroLengthsWithX509PARAM) {
// The correct value should work. // The correct value should work.
ASSERT_EQ(X509_V_OK, ASSERT_EQ(X509_V_OK,
Verify(leaf.get(), {root.get()}, {}, empty_crls, 0, false, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0,
[&test](X509_VERIFY_PARAM *param) { [&test](X509_VERIFY_PARAM *param) {
ASSERT_TRUE(test.func(param, test.correct_value, ASSERT_TRUE(test.func(param, test.correct_value,
test.correct_value_len)); test.correct_value_len));
@ -1333,7 +1305,7 @@ TEST(X509Test, ZeroLengthsWithX509PARAM) {
// The wrong value should trigger a verification error. // The wrong value should trigger a verification error.
ASSERT_EQ(test.mismatch_error, ASSERT_EQ(test.mismatch_error,
Verify(leaf.get(), {root.get()}, {}, empty_crls, 0, false, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0,
[&test](X509_VERIFY_PARAM *param) { [&test](X509_VERIFY_PARAM *param) {
ASSERT_TRUE(test.func(param, test.incorrect_value, ASSERT_TRUE(test.func(param, test.incorrect_value,
test.incorrect_value_len)); test.incorrect_value_len));
@ -1342,7 +1314,7 @@ TEST(X509Test, ZeroLengthsWithX509PARAM) {
// Passing zero as the length, unlike OpenSSL, should trigger an error and // Passing zero as the length, unlike OpenSSL, should trigger an error and
// should cause verification to fail. // should cause verification to fail.
ASSERT_EQ(X509_V_ERR_INVALID_CALL, ASSERT_EQ(X509_V_ERR_INVALID_CALL,
Verify(leaf.get(), {root.get()}, {}, empty_crls, 0, false, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0,
[&test](X509_VERIFY_PARAM *param) { [&test](X509_VERIFY_PARAM *param) {
ASSERT_FALSE(test.func(param, test.correct_value, 0)); ASSERT_FALSE(test.func(param, test.correct_value, 0));
})); }));
@ -1350,7 +1322,7 @@ TEST(X509Test, ZeroLengthsWithX509PARAM) {
// Passing an empty value should be an error when setting and should cause // Passing an empty value should be an error when setting and should cause
// verification to fail. // verification to fail.
ASSERT_EQ(X509_V_ERR_INVALID_CALL, ASSERT_EQ(X509_V_ERR_INVALID_CALL,
Verify(leaf.get(), {root.get()}, {}, empty_crls, 0, false, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0,
[&test](X509_VERIFY_PARAM *param) { [&test](X509_VERIFY_PARAM *param) {
ASSERT_FALSE(test.func(param, nullptr, 0)); ASSERT_FALSE(test.func(param, nullptr, 0));
})); }));
@ -1358,7 +1330,7 @@ TEST(X509Test, ZeroLengthsWithX509PARAM) {
// Passing a value with embedded NULs should also be an error and should // Passing a value with embedded NULs should also be an error and should
// also cause verification to fail. // also cause verification to fail.
ASSERT_EQ(X509_V_ERR_INVALID_CALL, ASSERT_EQ(X509_V_ERR_INVALID_CALL,
Verify(leaf.get(), {root.get()}, {}, empty_crls, 0, false, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0,
[&test](X509_VERIFY_PARAM *param) { [&test](X509_VERIFY_PARAM *param) {
ASSERT_FALSE(test.func(param, "a", 2)); ASSERT_FALSE(test.func(param, "a", 2));
})); }));
@ -1368,14 +1340,14 @@ TEST(X509Test, ZeroLengthsWithX509PARAM) {
// The correct value should still work. // The correct value should still work.
ASSERT_EQ(X509_V_OK, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0, ASSERT_EQ(X509_V_OK, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0,
false, [](X509_VERIFY_PARAM *param) { [](X509_VERIFY_PARAM *param) {
ASSERT_TRUE(X509_VERIFY_PARAM_set1_ip( ASSERT_TRUE(X509_VERIFY_PARAM_set1_ip(
param, kIP, sizeof(kIP))); param, kIP, sizeof(kIP)));
})); }));
// Incorrect values should still fail. // Incorrect values should still fail.
ASSERT_EQ(X509_V_ERR_IP_ADDRESS_MISMATCH, ASSERT_EQ(X509_V_ERR_IP_ADDRESS_MISMATCH,
Verify(leaf.get(), {root.get()}, {}, empty_crls, 0, false, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0,
[](X509_VERIFY_PARAM *param) { [](X509_VERIFY_PARAM *param) {
ASSERT_TRUE(X509_VERIFY_PARAM_set1_ip(param, kWrongIP, ASSERT_TRUE(X509_VERIFY_PARAM_set1_ip(param, kWrongIP,
sizeof(kWrongIP))); sizeof(kWrongIP)));
@ -1384,14 +1356,14 @@ TEST(X509Test, ZeroLengthsWithX509PARAM) {
// Zero length values should trigger an error when setting and cause // Zero length values should trigger an error when setting and cause
// verification to always fail. // verification to always fail.
ASSERT_EQ(X509_V_ERR_INVALID_CALL, ASSERT_EQ(X509_V_ERR_INVALID_CALL,
Verify(leaf.get(), {root.get()}, {}, empty_crls, 0, false, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0,
[](X509_VERIFY_PARAM *param) { [](X509_VERIFY_PARAM *param) {
ASSERT_FALSE(X509_VERIFY_PARAM_set1_ip(param, kIP, 0)); ASSERT_FALSE(X509_VERIFY_PARAM_set1_ip(param, kIP, 0));
})); }));
// ... and so should NULL values. // ... and so should NULL values.
ASSERT_EQ(X509_V_ERR_INVALID_CALL, ASSERT_EQ(X509_V_ERR_INVALID_CALL,
Verify(leaf.get(), {root.get()}, {}, empty_crls, 0, false, Verify(leaf.get(), {root.get()}, {}, empty_crls, 0,
[](X509_VERIFY_PARAM *param) { [](X509_VERIFY_PARAM *param) {
ASSERT_FALSE(X509_VERIFY_PARAM_set1_ip(param, nullptr, 0)); ASSERT_FALSE(X509_VERIFY_PARAM_set1_ip(param, nullptr, 0));
})); }));
@ -2500,8 +2472,7 @@ TEST(X509Test, CommonNameFallback) {
ASSERT_TRUE(with_ip); ASSERT_TRUE(with_ip);
auto verify_cert = [&](X509 *leaf, unsigned flags, const char *host) { auto verify_cert = [&](X509 *leaf, unsigned flags, const char *host) {
return Verify( return Verify(leaf, {root.get()}, {}, {}, 0, [&](X509_VERIFY_PARAM *param) {
leaf, {root.get()}, {}, {}, 0, false, [&](X509_VERIFY_PARAM *param) {
ASSERT_TRUE(X509_VERIFY_PARAM_set1_host(param, host, strlen(host))); ASSERT_TRUE(X509_VERIFY_PARAM_set1_host(param, host, strlen(host)));
X509_VERIFY_PARAM_set_hostflags(param, flags); X509_VERIFY_PARAM_set_hostflags(param, flags);
}); });
@ -2617,7 +2588,7 @@ TEST(X509Test, CommonNameAndNameConstraints) {
auto verify_cert = [&](X509 *leaf, unsigned flags, const char *host) { auto verify_cert = [&](X509 *leaf, unsigned flags, const char *host) {
return Verify( return Verify(
leaf, {root.get()}, {intermediate.get()}, {}, 0, false, leaf, {root.get()}, {intermediate.get()}, {}, 0,
[&](X509_VERIFY_PARAM *param) { [&](X509_VERIFY_PARAM *param) {
ASSERT_TRUE(X509_VERIFY_PARAM_set1_host(param, host, strlen(host))); ASSERT_TRUE(X509_VERIFY_PARAM_set1_host(param, host, strlen(host)));
X509_VERIFY_PARAM_set_hostflags(param, flags); X509_VERIFY_PARAM_set_hostflags(param, flags);
@ -2638,12 +2609,12 @@ TEST(X509Test, CommonNameAndNameConstraints) {
// separately call |X509_check_host|. // separately call |X509_check_host|.
EXPECT_EQ(X509_V_ERR_NAME_CONSTRAINTS_WITHOUT_SANS, EXPECT_EQ(X509_V_ERR_NAME_CONSTRAINTS_WITHOUT_SANS,
Verify(not_permitted.get(), {root.get()}, {intermediate.get()}, {}, Verify(not_permitted.get(), {root.get()}, {intermediate.get()}, {},
0 /* no flags */, false, nullptr)); 0 /* no flags */, nullptr));
// If the leaf certificate has SANs, the common name fallback is always // If the leaf certificate has SANs, the common name fallback is always
// disabled, so the name constraints do not apply. // disabled, so the name constraints do not apply.
EXPECT_EQ(X509_V_OK, Verify(not_permitted_with_sans.get(), {root.get()}, EXPECT_EQ(X509_V_OK, Verify(not_permitted_with_sans.get(), {root.get()},
{intermediate.get()}, {}, 0, false, nullptr)); {intermediate.get()}, {}, 0, nullptr));
EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH, EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
verify_cert(not_permitted_with_sans.get(), 0 /* no flags */, verify_cert(not_permitted_with_sans.get(), 0 /* no flags */,
kCommonNameNotPermittedWithSANs)); kCommonNameNotPermittedWithSANs));
@ -2651,7 +2622,7 @@ TEST(X509Test, CommonNameAndNameConstraints) {
// If the common name does not look like a DNS name, we apply neither name // If the common name does not look like a DNS name, we apply neither name
// constraints nor common name fallback. // constraints nor common name fallback.
EXPECT_EQ(X509_V_OK, Verify(not_dns.get(), {root.get()}, {intermediate.get()}, EXPECT_EQ(X509_V_OK, Verify(not_dns.get(), {root.get()}, {intermediate.get()},
{}, 0, false, nullptr)); {}, 0, nullptr));
EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH, EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
verify_cert(not_dns.get(), 0 /* no flags */, kCommonNameNotDNS)); verify_cert(not_dns.get(), 0 /* no flags */, kCommonNameNotDNS));
} }
@ -2675,8 +2646,7 @@ TEST(X509Test, ServerGatedCryptoEKUs) {
auto verify_cert = [&root](X509 *leaf) { auto verify_cert = [&root](X509 *leaf) {
return Verify(leaf, {root.get()}, /*intermediates=*/{}, /*crls=*/{}, return Verify(leaf, {root.get()}, /*intermediates=*/{}, /*crls=*/{},
/*flags=*/0, /*use_additional_untrusted=*/false, /*flags=*/0, [&](X509_VERIFY_PARAM *param) {
[&](X509_VERIFY_PARAM *param) {
ASSERT_TRUE(X509_VERIFY_PARAM_set_purpose( ASSERT_TRUE(X509_VERIFY_PARAM_set_purpose(
param, X509_PURPOSE_SSL_SERVER)); param, X509_PURPOSE_SSL_SERVER));
}); });

@ -222,8 +222,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
X509_up_ref(ctx->cert); X509_up_ref(ctx->cert);
ctx->last_untrusted = 1; ctx->last_untrusted = 1;
/* We use a temporary STACK so we can chop and hack at it. /* We use a temporary STACK so we can chop and hack at it. */
* sktmp = ctx->untrusted ++ ctx->ctx->additional_untrusted */
if (ctx->untrusted != NULL if (ctx->untrusted != NULL
&& (sktmp = sk_X509_dup(ctx->untrusted)) == NULL) { && (sktmp = sk_X509_dup(ctx->untrusted)) == NULL) {
OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE); OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
@ -231,28 +230,6 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
goto end; goto end;
} }
if (ctx->ctx->additional_untrusted != NULL) {
if (sktmp == NULL) {
sktmp = sk_X509_new_null();
if (sktmp == NULL) {
OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
ctx->error = X509_V_ERR_OUT_OF_MEM;
goto end;
}
}
for (size_t k = 0; k < sk_X509_num(ctx->ctx->additional_untrusted);
k++) {
if (!sk_X509_push(sktmp,
sk_X509_value(ctx->ctx->additional_untrusted,
k))) {
OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
ctx->error = X509_V_ERR_OUT_OF_MEM;
goto end;
}
}
}
num = sk_X509_num(ctx->chain); num = sk_X509_num(ctx->chain);
x = sk_X509_value(ctx->chain, num - 1); x = sk_X509_value(ctx->chain, num - 1);
depth = param->depth; depth = param->depth;
@ -1351,17 +1328,6 @@ static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl,
return; return;
} }
} }
for (i = 0; i < sk_X509_num(ctx->ctx->additional_untrusted); i++) {
crl_issuer = sk_X509_value(ctx->ctx->additional_untrusted, i);
if (X509_NAME_cmp(X509_get_subject_name(crl_issuer), cnm))
continue;
if (X509_check_akid(crl_issuer, crl->akid) == X509_V_OK) {
*pissuer = crl_issuer;
*pcrl_score |= CRL_SCORE_AKID;
return;
}
}
} }
/* /*

@ -306,11 +306,6 @@ OPENSSL_EXPORT int X509_STORE_set_trust(X509_STORE *ctx, int trust);
OPENSSL_EXPORT int X509_STORE_set1_param(X509_STORE *ctx, OPENSSL_EXPORT int X509_STORE_set1_param(X509_STORE *ctx,
X509_VERIFY_PARAM *pm); X509_VERIFY_PARAM *pm);
OPENSSL_EXPORT X509_VERIFY_PARAM *X509_STORE_get0_param(X509_STORE *ctx); OPENSSL_EXPORT X509_VERIFY_PARAM *X509_STORE_get0_param(X509_STORE *ctx);
// X509_STORE_set0_additional_untrusted sets a stack of additional, untrusted
// certificates that are available for chain building. This function does not
// take ownership of the stack.
OPENSSL_EXPORT void X509_STORE_set0_additional_untrusted(
X509_STORE *ctx, STACK_OF(X509) *untrusted);
OPENSSL_EXPORT void X509_STORE_set_verify(X509_STORE *ctx, OPENSSL_EXPORT void X509_STORE_set_verify(X509_STORE *ctx,
X509_STORE_CTX_verify_fn verify); X509_STORE_CTX_verify_fn verify);

Loading…
Cancel
Save