Skip to content

Commit e5ab7fa

Browse files
FrauschiJacobBarthelmeh
authored andcommitted
x509: fix CA:FALSE bypass in wolfSSL_X509_verify_cert
When an untrusted issuer has CA:FALSE and no verify_cb is registered, the !isCa branch now fails closed (ret=WOLFSSL_FAILURE, goto exit) instead of falling through and skipping X509StoreVerifyCert for the leaf. SetupStoreCtxError_ex is also hardened to never overwrite a previously recorded error with success, preventing a later valid chain link from clobbering ctx->error back to X509_V_OK. Tests added for both the no-callback rejection and the error-preservation cases. Reported by: Nicholas Carlini (Anthropic) & Thai Duong (Calif.io)
1 parent a88dd07 commit e5ab7fa

3 files changed

Lines changed: 62 additions & 2 deletions

File tree

src/x509_str.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,11 @@ static void SetupStoreCtxError_ex(WOLFSSL_X509_STORE_CTX* ctx, int ret,
318318
{
319319
int error = GetX509Error(ret);
320320

321+
/* Do not overwrite a previously recorded error with success; preserve
322+
* the worst-seen error across the chain walk. */
323+
if (error == 0 && ctx->error != 0)
324+
return;
325+
321326
wolfSSL_X509_STORE_CTX_set_error(ctx, error);
322327
wolfSSL_X509_STORE_CTX_set_error_depth(ctx, depth);
323328
}
@@ -635,9 +640,14 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
635640
if (ctx->store->verify_cb) {
636641
ret = ctx->store->verify_cb(0, ctx);
637642
if (ret != WOLFSSL_SUCCESS) {
643+
ret = WOLFSSL_FAILURE;
638644
goto exit;
639645
}
640646
}
647+
else {
648+
ret = WOLFSSL_FAILURE;
649+
goto exit;
650+
}
641651
} else
642652
#endif
643653
{
@@ -2174,4 +2184,3 @@ int wolfSSL_X509_STORE_set1_param(WOLFSSL_X509_STORE *ctx,
21742184
#endif /* !WOLFCRYPT_ONLY */
21752185

21762186
#endif /* !WOLFSSL_X509_STORE_INCLUDED */
2177-

tests/api/test_ossl_x509_str.c

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,56 @@ int test_X509_STORE_InvalidCa(void)
10741074
ExpectIntEQ(X509_STORE_CTX_init(ctx, str, cert, untrusted), 1);
10751075
ExpectIntEQ(X509_verify_cert(ctx), 1);
10761076
ExpectIntEQ(last_errcode, X509_V_ERR_INVALID_CA);
1077+
/* Defense in depth: ctx->error must not be clobbered back to X509_V_OK
1078+
* by the later successful verification of the intermediate against the
1079+
* trusted root. The worst-seen error must persist. */
1080+
ExpectIntEQ(X509_STORE_CTX_get_error(ctx), X509_V_ERR_INVALID_CA);
1081+
1082+
X509_free(cert);
1083+
X509_STORE_free(str);
1084+
X509_STORE_CTX_free(ctx);
1085+
sk_X509_pop_free(untrusted, NULL);
1086+
#endif
1087+
return EXPECT_RESULT();
1088+
}
1089+
1090+
int test_X509_STORE_InvalidCa_NoCallback(void)
1091+
{
1092+
EXPECT_DECLS;
1093+
#if defined(OPENSSL_ALL) && !defined(NO_RSA) && !defined(NO_FILESYSTEM)
1094+
const char* filename = "./certs/intermediate/ca_false_intermediate/"
1095+
"test_int_not_cacert.pem";
1096+
const char* srvfile = "./certs/intermediate/ca_false_intermediate/"
1097+
"test_sign_bynoca_srv.pem";
1098+
X509_STORE_CTX* ctx = NULL;
1099+
X509_STORE* str = NULL;
1100+
XFILE fp = XBADFILE;
1101+
X509* cert = NULL;
1102+
STACK_OF(X509)* untrusted = NULL;
1103+
1104+
ExpectTrue((fp = XFOPEN(srvfile, "rb"))
1105+
!= XBADFILE);
1106+
ExpectNotNull(cert = PEM_read_X509(fp, 0, 0, 0 ));
1107+
if (fp != XBADFILE) {
1108+
XFCLOSE(fp);
1109+
fp = XBADFILE;
1110+
}
1111+
1112+
ExpectNotNull(str = X509_STORE_new());
1113+
ExpectNotNull(ctx = X509_STORE_CTX_new());
1114+
ExpectNotNull(untrusted = sk_X509_new_null());
1115+
1116+
/* Create cert chain stack with an intermediate that is CA:FALSE. */
1117+
ExpectIntEQ(test_X509_STORE_untrusted_load_cert_to_stack(filename,
1118+
untrusted), TEST_SUCCESS);
1119+
1120+
ExpectIntEQ(X509_STORE_load_locations(str,
1121+
"./certs/intermediate/ca_false_intermediate/test_ca.pem",
1122+
NULL), 1);
1123+
ExpectIntEQ(X509_STORE_CTX_init(ctx, str, cert, untrusted), 1);
1124+
/* No verify callback: verification must fail on CA:FALSE issuer. */
1125+
ExpectIntNE(X509_verify_cert(ctx), 1);
1126+
ExpectIntEQ(X509_STORE_CTX_get_error(ctx), X509_V_ERR_INVALID_CA);
10771127

10781128
X509_free(cert);
10791129
X509_STORE_free(str);
@@ -1793,4 +1843,3 @@ int test_X509_STORE_No_SSL_CTX(void)
17931843
#endif
17941844
return EXPECT_RESULT();
17951845
}
1796-

tests/api/test_ossl_x509_str.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ int test_wolfSSL_X509_STORE_CTX(void);
3131
int test_wolfSSL_X509_STORE_CTX_ex(void);
3232
int test_X509_STORE_untrusted(void);
3333
int test_X509_STORE_InvalidCa(void);
34+
int test_X509_STORE_InvalidCa_NoCallback(void);
3435
int test_wolfSSL_X509_STORE_CTX_trusted_stack_cleanup(void);
3536
int test_wolfSSL_X509_STORE_CTX_get_issuer(void);
3637
int test_wolfSSL_X509_STORE_set_flags(void);
@@ -51,6 +52,7 @@ int test_X509_STORE_No_SSL_CTX(void);
5152
TEST_DECL_GROUP("ossl_x509_store", test_wolfSSL_X509_STORE_CTX_ex), \
5253
TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_untrusted), \
5354
TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_InvalidCa), \
55+
TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_InvalidCa_NoCallback), \
5456
TEST_DECL_GROUP("ossl_x509_store", \
5557
test_wolfSSL_X509_STORE_CTX_trusted_stack_cleanup), \
5658
TEST_DECL_GROUP("ossl_x509_store", \

0 commit comments

Comments
 (0)