diff --git a/src/internal.c b/src/internal.c index 95b71ddbfd1..d8b7a2633a8 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8268,6 +8268,49 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx) return ret; } + +#if defined(HAVE_SNI) || defined(WOLFSSL_CERT_SETUP_CB) +static int IsStaleCtxDer(const DerBuffer* der, byte weOwn, + const DerBuffer* ctxDer) +{ + return (!weOwn) && (der != NULL) && (der != ctxDer); +} + +/* Check the CTX still has the buffers the SSL points to but does not own. + * Returns BAD_STATE_E when the CTX replaced one. */ +int CheckSSL_CTX_CertsAndKeys(const WOLFSSL* ssl) +{ + const WOLFSSL_CTX* ctx = ssl->ctx; + const DerBuffer* ctxKey = ctx->privateKey; + int stale = 0; + +#ifdef WOLFSSL_DUAL_ALG_CERTS + /* Signing with the alternative key swaps it in as the key. */ + if (ssl->buffers.key == ctx->altPrivateKey) { + ctxKey = ctx->altPrivateKey; + } + /* After the swap, the buffer is owned by the key. */ + if (ssl->buffers.altKey != ssl->buffers.key) { + stale = IsStaleCtxDer(ssl->buffers.altKey, ssl->buffers.weOwnAltKey, + ctx->altPrivateKey); + } +#endif + if (stale || + IsStaleCtxDer(ssl->buffers.certificate, ssl->buffers.weOwnCert, + ctx->certificate) || + IsStaleCtxDer(ssl->buffers.certChain, ssl->buffers.weOwnCertChain, + ctx->certChain) || + /* A chain replaced at the same address can differ in count. */ + (!ssl->buffers.weOwnCertChain && (ssl->buffers.certChain != NULL) && + (ssl->buffers.certChainCnt != ctx->certChainCnt)) || + IsStaleCtxDer(ssl->buffers.key, ssl->buffers.weOwnKey, ctxKey)) { + WOLFSSL_MSG("CTX certificate or key replaced during handshake"); + WOLFSSL_ERROR_VERBOSE(BAD_STATE_E); + return BAD_STATE_E; + } + return 0; +} +#endif /* HAVE_SNI || WOLFSSL_CERT_SETUP_CB */ #endif /* NO_CERTS */ #ifndef NO_DH @@ -47416,6 +47459,13 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ], if(ssl && ssl->ctx && ssl->ctx->sniRecvCb) { WOLFSSL_MSG("Calling custom sni callback"); sniRet = ssl->ctx->sniRecvCb(ssl, &ad, ssl->ctx->sniRecvCbArg); + #ifndef NO_CERTS + ret = CheckSSL_CTX_CertsAndKeys(ssl); + if (ret != 0) { + SendAlert(ssl, alert_fatal, internal_error); + return ret; + } + #endif switch (sniRet) { case warning_return: /* TLS 1.3 error alerts are always fatal (RFC 8446 6.2) */ diff --git a/src/ssl_api_cert.c b/src/ssl_api_cert.c index 62e8f653d56..40ed4ee5f88 100644 --- a/src/ssl_api_cert.c +++ b/src/ssl_api_cert.c @@ -2949,6 +2949,8 @@ void wolfSSL_CTX_set_cert_cb(WOLFSSL_CTX* ctx, * unrecognized value. A fatal alert is sent when it failed. * @return WOLFSSL_ERROR_WANT_X509_LOOKUP when the callback returned a * negative value to ask to be called again. + * @return BAD_STATE_E when the callback replaced a certificate or key on the + * CTX that the SSL uses. A fatal alert is sent. */ int CertSetupCbWrapper(WOLFSSL* ssl) { @@ -2957,6 +2959,12 @@ int CertSetupCbWrapper(WOLFSSL* ssl) if (ssl->ctx->certSetupCb != NULL) { WOLFSSL_MSG("Calling user cert setup callback"); ret = ssl->ctx->certSetupCb(ssl, ssl->ctx->certSetupCbArg); + #ifndef NO_CERTS + if (CheckSSL_CTX_CertsAndKeys(ssl) != 0) { + SendAlert(ssl, alert_fatal, internal_error); + return BAD_STATE_E; + } + #endif if (ret == 1) { WOLFSSL_MSG("User cert callback returned success"); ret = 0; diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index 24792031363..d13c20e7cf7 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -4168,6 +4168,270 @@ int test_tls13_session_resumption_sni_mismatch(void) return EXPECT_RESULT(); } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + (defined(HAVE_SNI) || defined(WOLFSSL_CERT_SETUP_CB)) && \ + !defined(NO_RSA) && !defined(NO_FILESYSTEM) + #define TEST_CB_CTX_CERT_LOAD +#endif + +#ifdef TEST_CB_CTX_CERT_LOAD +enum { + CB_SNI, + CB_CERT_SETUP +}; + +enum { + CB_LOAD_CTX_CERT, + CB_LOAD_CTX_KEY, + CB_LOAD_SSL_CERT, + CB_SWITCH_CTX +}; + +typedef struct CbLoadCase { + WOLFSSL_CTX* ctx2; + int action; + int retry; + int loadRet; +} CbLoadCase; + +static void cb_load_run(WOLFSSL* ssl, CbLoadCase* c) +{ + WOLFSSL_CTX* ctx = wolfSSL_get_SSL_CTX(ssl); + + switch (c->action) { + case CB_LOAD_CTX_CERT: + c->loadRet = wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, + CERT_FILETYPE); + break; + case CB_LOAD_CTX_KEY: + c->loadRet = wolfSSL_CTX_use_PrivateKey_file(ctx, svrKeyFile, + CERT_FILETYPE); + break; + #ifdef OPENSSL_EXTRA + case CB_SWITCH_CTX: + c->loadRet = (wolfSSL_set_SSL_CTX(ssl, c->ctx2) == c->ctx2) ? + WOLFSSL_SUCCESS : WOLFSSL_FAILURE; + break; + #endif + default: + c->loadRet = wolfSSL_use_certificate_file(ssl, svrCertFile, + CERT_FILETYPE); + break; + } +} + +#ifdef HAVE_SNI +static int sni_load_cb(WOLFSSL* ssl, int* ad, void* arg) +{ + CbLoadCase* c = (CbLoadCase*)arg; + + (void)ad; + cb_load_run(ssl, c); + return c->retry ? warning_return : 0; +} +#endif + +#ifdef WOLFSSL_CERT_SETUP_CB +static int cert_setup_load_cb(WOLFSSL* ssl, void* arg) +{ + CbLoadCase* c = (CbLoadCase*)arg; + + cb_load_run(ssl, c); + return c->retry ? -1 : 1; +} +#endif + +static int test_cb_ctx_cert_load_one(int cb, method_provider client, + method_provider server, int action, int retry, int err) +{ + EXPECT_DECLS; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + CbLoadCase c; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + XMEMSET(&c, 0, sizeof(c)); + c.action = action; + c.retry = retry; + ExpectNotNull(c.ctx2 = wolfSSL_CTX_new(server())); + ExpectIntEQ(wolfSSL_CTX_use_certificate_file(c.ctx2, svrCertFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_use_PrivateKey_file(c.ctx2, svrKeyFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); +#ifndef NO_DH + /* Switching contexts takes the DH state from the new one. */ + SetDHCtx(c.ctx2); +#endif + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + client, server), 0); +#ifdef HAVE_SNI + if (cb == CB_SNI) { + const char* host = "example.com"; + + wolfSSL_CTX_set_servername_callback(ctx_s, sni_load_cb); + ExpectIntEQ(wolfSSL_CTX_set_servername_arg(ctx_s, &c), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, host, + (word16)XSTRLEN(host)), WOLFSSL_SUCCESS); + } +#endif +#ifdef WOLFSSL_CERT_SETUP_CB + if (cb == CB_CERT_SETUP) { + wolfSSL_CTX_set_cert_cb(ctx_s, cert_setup_load_cb, &c); + } +#endif + if (err == 0) { + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + } + else { + ExpectIntNE(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR), err); + } + ExpectIntEQ(c.loadRet, WOLFSSL_SUCCESS); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + wolfSSL_CTX_free(c.ctx2); + return EXPECT_RESULT(); +} +#endif /* TEST_CB_CTX_CERT_LOAD */ + +/* Replacing the certificate or key on the context from the SNI or certificate + * setup callback fails the handshake. Loading one on the session is allowed. */ +int test_tls_cb_ctx_cert_load(void) +{ + EXPECT_DECLS; +#ifdef TEST_CB_CTX_CERT_LOAD + static const int cbs[] = { + #ifdef HAVE_SNI + CB_SNI, + #endif + #ifdef WOLFSSL_CERT_SETUP_CB + CB_CERT_SETUP, + #endif + }; + static const struct { + method_provider client; + method_provider server; + } methods[] = { + #ifndef WOLFSSL_NO_TLS12 + { wolfTLSv1_2_client_method, wolfTLSv1_2_server_method }, + #endif + #ifdef WOLFSSL_TLS13 + { wolfTLSv1_3_client_method, wolfTLSv1_3_server_method }, + #endif + }; + static const struct { + int action; + int retry; /* callback returns its warning or retry value */ + int err; + } cases[] = { + #ifdef WOLFSSL_COPY_CERT + { CB_LOAD_CTX_CERT, 0, 0 }, + #else + { CB_LOAD_CTX_CERT, 0, WC_NO_ERR_TRACE(BAD_STATE_E) }, + { CB_LOAD_CTX_CERT, 1, WC_NO_ERR_TRACE(BAD_STATE_E) }, + #endif + #if defined(WOLFSSL_COPY_KEY) || defined(WOLFSSL_BLIND_PRIVATE_KEY) + { CB_LOAD_CTX_KEY, 0, 0 }, + #else + { CB_LOAD_CTX_KEY, 0, WC_NO_ERR_TRACE(BAD_STATE_E) }, + #endif + { CB_LOAD_SSL_CERT, 0, 0 }, + #ifdef OPENSSL_EXTRA + { CB_SWITCH_CTX, 0, 0 }, + #endif + }; + size_t i; + size_t j; + size_t k; + + for (k = 0; k < XELEM_CNT(cbs); k++) { + for (i = 0; i < XELEM_CNT(methods); i++) { + for (j = 0; j < XELEM_CNT(cases); j++) { + ExpectIntEQ(test_cb_ctx_cert_load_one(cbs[k], + methods[i].client, methods[i].server, cases[j].action, + cases[j].retry, cases[j].err), TEST_SUCCESS); + } + } + } +#endif + return EXPECT_RESULT(); +} + +#if defined(WOLFSSL_DUAL_ALG_CERTS) && defined(HAVE_ECC) && \ + !defined(WOLFSSL_COPY_KEY) && !defined(WOLFSSL_BLIND_PRIVATE_KEY) + #define TEST_SSL_CTX_ALT_KEY +#endif + +/* The buffers the SSL shares with its context are checked by pointer and by + * chain count. The alternative key swapped in to sign is still the context's. + */ +int test_tls_ssl_ctx_certs_check(void) +{ + EXPECT_DECLS; +#if (defined(HAVE_SNI) || defined(WOLFSSL_CERT_SETUP_CB)) && \ + !defined(NO_CERTS) && !defined(NO_TLS) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_FILESYSTEM) && defined(WOLFSSL_PEM_TO_DER) && \ + !defined(NO_RSA) && !defined(WOLFSSL_COPY_CERT) + WOLFSSL_CTX* ctx = NULL; + WOLFSSL* ssl = NULL; +#ifdef TEST_SSL_CTX_ALT_KEY + byte* altKey = NULL; + size_t altKeySz = 0; +#endif + + ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_server_method())); + ExpectIntEQ(wolfSSL_CTX_use_certificate_chain_file(ctx, svrCertFile), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_use_PrivateKey_file(ctx, svrKeyFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); +#ifdef TEST_SSL_CTX_ALT_KEY + ExpectIntEQ(wolfSSL_CTX_use_AltPrivateKey_file(ctx, eccKeyFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); +#endif + ExpectNotNull(ssl = wolfSSL_new(ctx)); + ExpectIntEQ(CheckSSL_CTX_CertsAndKeys(ssl), 0); + + /* Chain replaced at the same address with a different count. */ + if (ctx != NULL) { + ctx->certChainCnt++; + } + ExpectIntEQ(CheckSSL_CTX_CertsAndKeys(ssl), WC_NO_ERR_TRACE(BAD_STATE_E)); + if (ctx != NULL) { + ctx->certChainCnt--; + } + +#ifdef TEST_SSL_CTX_ALT_KEY + /* Same swap as SendTls13CertificateVerify with the alternative key. */ + if (ssl != NULL) { + ssl->buffers.key = ssl->buffers.altKey; + ssl->buffers.weOwnKey = ssl->buffers.weOwnAltKey; + ssl->buffers.weOwnAltKey = 0; + } + ExpectIntEQ(CheckSSL_CTX_CertsAndKeys(ssl), 0); + + /* A new alternative key on the SSL leaves the swapped key with the CTX. */ + ExpectIntEQ(load_file("./certs/ecc-key.der", &altKey, &altKeySz), 0); + ExpectIntEQ(wolfSSL_use_AltPrivateKey_buffer(ssl, altKey, (long)altKeySz, + WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS); + ExpectIntEQ(CheckSSL_CTX_CertsAndKeys(ssl), 0); + + ExpectIntEQ(wolfSSL_CTX_use_AltPrivateKey_file(ctx, eccKeyFile, + CERT_FILETYPE), WOLFSSL_SUCCESS); + ExpectIntEQ(CheckSSL_CTX_CertsAndKeys(ssl), WC_NO_ERR_TRACE(BAD_STATE_E)); + XFREE(altKey, NULL, DYNAMIC_TYPE_TMP_BUFFER); +#endif + + wolfSSL_free(ssl); + wolfSSL_CTX_free(ctx); +#endif + return EXPECT_RESULT(); +} + /* Regression test for the post-ALPN_Select PSK-head check. * When ALPN_Select runs before CheckPreSharedKeys (so the per-PSK * binding check has the negotiated ALPN available), TLSX_SetALPN diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index 4824c5ecc88..a3f24d080c2 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -71,6 +71,8 @@ int test_tls12_client_hello_no_tls13_suites(void); int test_tls_set_session_min_downgrade(void); int test_tls12_session_id_resumption_sni_mismatch(void); int test_tls13_session_resumption_sni_mismatch(void); +int test_tls_cb_ctx_cert_load(void); +int test_tls_ssl_ctx_certs_check(void); int test_tls13_resumption_with_alpn(void); int test_tls12_session_id_resumption_alpn_mismatch(void); int test_tls13_session_resumption_alpn_mismatch(void); @@ -152,6 +154,8 @@ int test_tls_param_flags_crl_check(void); TEST_DECL_GROUP("tls", test_tls_set_session_min_downgrade), \ TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_sni_mismatch), \ TEST_DECL_GROUP("tls", test_tls13_session_resumption_sni_mismatch), \ + TEST_DECL_GROUP("tls", test_tls_cb_ctx_cert_load), \ + TEST_DECL_GROUP("tls", test_tls_ssl_ctx_certs_check), \ TEST_DECL_GROUP("tls", test_tls13_resumption_with_alpn), \ TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_alpn_mismatch),\ TEST_DECL_GROUP("tls", test_tls13_session_resumption_alpn_mismatch), \ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 6f9c8cddb6e..f6111c7d634 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -5684,6 +5684,13 @@ typedef struct Buffers { * not reference counted, so a session must not point at the context's. */ WOLFSSL_LOCAL int CopySSL_CTX_DhParams(WOLFSSL* ssl, WOLFSSL_CTX* ctx); #endif +#if !defined(NO_CERTS) && \ + (defined(HAVE_SNI) || defined(WOLFSSL_CERT_SETUP_CB)) +#ifdef WOLFSSL_API_PREFIX_MAP + #define CheckSSL_CTX_CertsAndKeys wolfSSL_CheckSSL_CTX_CertsAndKeys +#endif +WOLFSSL_TEST_VIS int CheckSSL_CTX_CertsAndKeys(const WOLFSSL* ssl); +#endif /* sub-states for send/do key share (key exchange) */ enum asyncState {