Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -12053,17 +12053,27 @@ static int wolfSSLReceive(WOLFSSL* ssl, byte* buf, word32 sz)
}


/* Switch dynamic output buffer back to static, buffer is assumed clear */
/* Switch dynamic output buffer back to static, discarding any pending output.
* Safe to call whether or not the buffer is dynamic; callers need not check
* dynamicFlag first. */
void ShrinkOutputBuffer(WOLFSSL* ssl)
{
WOLFSSL_MSG("Shrinking output buffer");
XFREE(ssl->buffers.outputBuffer.buffer - ssl->buffers.outputBuffer.offset,
ssl->heap, DYNAMIC_TYPE_OUT_BUFFER);
if (ssl->buffers.outputBuffer.dynamicFlag) {
Comment thread
kareem-wolfssl marked this conversation as resolved.
/* Discarded records may hold plaintext, so wipe before releasing,
* as ShrinkInputBuffer does. */
ForceZero(ssl->buffers.outputBuffer.buffer,
ssl->buffers.outputBuffer.bufferSize);
XFREE(ssl->buffers.outputBuffer.buffer -
Comment thread
kareem-wolfssl marked this conversation as resolved.
ssl->buffers.outputBuffer.offset,
ssl->heap, DYNAMIC_TYPE_OUT_BUFFER);
}
ssl->buffers.outputBuffer.buffer = ssl->buffers.outputBuffer.staticBuffer;
ssl->buffers.outputBuffer.bufferSize = STATIC_BUFFER_LEN;
ssl->buffers.outputBuffer.dynamicFlag = 0;
ssl->buffers.outputBuffer.offset = 0;
/* idx and length are assumed to be 0. */
ssl->buffers.outputBuffer.idx = 0;
ssl->buffers.outputBuffer.length = 0;
}


Expand Down
9 changes: 9 additions & 0 deletions src/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -5705,6 +5705,15 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out,
ssl->earlyData = no_early_data;
ssl->earlyDataSz = 0;
#endif
#ifdef HAVE_RPK
/* Drop the negotiated cert types so one peer's choice cannot carry into
* the next handshake. Clearing the counts is enough: every read of the
* type arrays is gated on its count. */
ssl->options.rpkState.sending_ClientCertTypeCnt = 0;
ssl->options.rpkState.sending_ServerCertTypeCnt = 0;
ssl->options.rpkState.received_ClientCertTypeCnt = 0;
ssl->options.rpkState.received_ServerCertTypeCnt = 0;
#endif

#if defined(HAVE_TLS_EXTENSIONS) && !defined(NO_TLS)
TLSX_FreeAll(ssl->extensions, ssl->heap);
Expand Down
33 changes: 33 additions & 0 deletions src/tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -13540,6 +13540,22 @@ static int TLSX_ClientCertificateType_Parse(WOLFSSL* ssl, const byte* input,
else if (msgType == server_hello || msgType == encrypted_extensions) {
/* parse it in client side */
if (length == 1) {
/* Same offered-vs-received binding as server_cert_type: an
* unsolicited value lets the peer pick the form this client
* presents its own credential in. */
if (ssl->options.rpkState.sending_ClientCertTypeCnt == 0) {
WOLFSSL_MSG("client_cert_type received but never offered");
WOLFSSL_ERROR_VERBOSE(UNSUPPORTED_EXTENSION);
return UNSUPPORTED_EXTENSION;
}
if (!IsCertTypeListed(*input,
ssl->options.rpkState.sending_ClientCertTypeCnt,
ssl->options.rpkState.sending_ClientCertTypes)) {
WOLFSSL_MSG("client_cert_type value was not offered");
WOLFSSL_ERROR_VERBOSE(UNSUPPORTED_EXTENSION);
return UNSUPPORTED_EXTENSION;
}

ssl->options.rpkState.received_ClientCertTypeCnt = 1;
ssl->options.rpkState.received_ClientCertTypes[0] = *input;
}
Expand Down Expand Up @@ -13740,6 +13756,23 @@ static int TLSX_ServerCertificateType_Parse(WOLFSSL* ssl, const byte* input,
if (length != 1) /* length slould be 1 */
return BUFFER_E;

/* RFC 7250 4.1, RFC 8446 4.2: the server may only answer with a type
* the client offered. ProcessPeerCertParse() treats the stored value as
* negotiated, so an unsolicited one lets the peer select RawPublicKey
* and skip chain verification. */
if (ssl->options.rpkState.sending_ServerCertTypeCnt == 0) {
WOLFSSL_MSG("server_cert_type received but never offered");
WOLFSSL_ERROR_VERBOSE(UNSUPPORTED_EXTENSION);
return UNSUPPORTED_EXTENSION;
}
if (!IsCertTypeListed(*input,
ssl->options.rpkState.sending_ServerCertTypeCnt,
ssl->options.rpkState.sending_ServerCertTypes)) {
WOLFSSL_MSG("server_cert_type value was not offered");
WOLFSSL_ERROR_VERBOSE(UNSUPPORTED_EXTENSION);
return UNSUPPORTED_EXTENSION;
}

ssl->options.rpkState.received_ServerCertTypeCnt = 1;
ssl->options.rpkState.received_ServerCertTypes[0] = *input;
}
Expand Down
63 changes: 42 additions & 21 deletions src/x509_str.c
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,6 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
{
int ret = WC_NO_ERR_TRACE(WOLFSSL_FAILURE);
int done = 0;
int added = 0;
int i = 0;
int numFailedCerts = 0;
int depth = 0;
Expand Down Expand Up @@ -997,11 +996,8 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
&depth, origDepth);
continue;
}
added = 1;
ret = X509StoreVerifyCert(ctx);
if (ret != WOLFSSL_SUCCESS) {
if ((origDepth - depth) <= 1)
added = 0;
X509VerifyCertSetupRetry(ctx, certs, failedCerts,
&depth, origDepth);
continue;
Expand All @@ -1023,21 +1019,14 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
!= WOLFSSL_SUCCESS) {
/* Could not guarantee the temporary intermediates were
* dropped; fail closed rather than risk verifying the current
* certificate against one. Leave `added` set: they are still
* loaded, so the exit cleanup makes a final attempt to drop
* them. */
* certificate against one. */
Comment thread
kareem-wolfssl marked this conversation as resolved.
ret = WOLFSSL_FATAL_ERROR;
goto exit;
}
added = 0;
ret = X509StoreVerifyCert(ctx);
if (ret != WOLFSSL_SUCCESS) {
/* WOLFSSL_PARTIAL_CHAIN may only terminate the chain at a
* certificate the caller actually trusts. The previous
* "added == 1" guard merely confirmed that some untrusted
* intermediate had been temporarily loaded into the
* CertManager during chain building, which would accept
* chains that never reach a trust anchor. Verify that
* certificate the caller actually trusts, so verify that
* ctx->current_cert is itself in the original trust set. */
if (((ctx->flags & WOLFSSL_PARTIAL_CHAIN) ||
(ctx->store->param != NULL &&
Expand Down Expand Up @@ -1149,11 +1138,17 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
}
}
}
/* Remove intermediates that were added to CM */
/* Remove intermediates that were added to CM. Unconditional: anything left
* resident anchors verification for every other user of this CM. */
if (ctx != NULL) {
if (ctx->store != NULL) {
if (added == 1) {
wolfSSL_CertManagerUnloadTempIntermediateCerts(ctx->store->cm);
if (wolfSSL_CertManagerUnloadTempIntermediateCerts(ctx->store->cm)
!= WOLFSSL_SUCCESS) {
WOLFSSL_MSG("Failed to unload temporary intermediates");
/* Any left resident would anchor later verifications, so do
* not report success. */
if (ret == WOLFSSL_SUCCESS)
ret = WOLFSSL_FAILURE;
}
}
if (orig != NULL) {
Expand Down Expand Up @@ -1777,13 +1772,15 @@ static void X509StoreFreeObjList(WOLFSSL_X509_STORE* store,
* the numAdded to the store >= is used when comparing to 0. */
i = wolfSSL_sk_X509_OBJECT_num(objs) - 1;
while (cnt > 0 && i >= 0) {
/* The inner X509 is owned by somebody else, NULL out the reference */
obj = (WOLFSSL_X509_OBJECT *)wolfSSL_sk_X509_OBJECT_value(objs, i);
if (obj != NULL) {
/* Only certificates are borrowed, so only they consume numAdded. The
* CRL object appended after them must not shift this window. */
if (obj != NULL && obj->type == WOLFSSL_X509_LU_X509) {
/* The inner X509 is owned by somebody else, NULL out the ref */
obj->type = (WOLFSSL_X509_LOOKUP_TYPE)0;
obj->data.ptr = NULL;
cnt--;
}
cnt--;
i--;
}
Comment on lines 1773 to 1785

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

X509StoreFreeObjList is a mess. Let's instead increase the reference counter of the objects placed inside of a WOLFSSL_X509_OBJECT (both x509 and x509_crl have ref counters) and just always call the correct _free function.


Expand Down Expand Up @@ -2522,6 +2519,9 @@ WOLF_STACK_OF(WOLFSSL_X509_OBJECT)* wolfSSL_X509_STORE_get0_objects(
{
WOLFSSL_STACK* ret = NULL;
WOLFSSL_STACK* cert_stack = NULL;
/* Set once the certificates have been handed over to "ret". Until then
* cert_stack still owns them and the error path must not free them. */
byte certsOwned = 0;
#if ((defined(WOLFSSL_SIGNER_DER_CERT) && !defined(NO_FILESYSTEM)) || \
(defined(HAVE_CRL)))
WOLFSSL_X509_OBJECT* obj = NULL;
Expand Down Expand Up @@ -2595,6 +2595,7 @@ WOLF_STACK_OF(WOLFSSL_X509_OBJECT)* wolfSSL_X509_STORE_get0_objects(
while (wolfSSL_sk_X509_num(cert_stack) > 0) {
wolfSSL_sk_X509_pop(cert_stack);
}
certsOwned = 1;
#endif

#ifdef HAVE_CRL
Expand Down Expand Up @@ -2625,8 +2626,28 @@ WOLF_STACK_OF(WOLFSSL_X509_OBJECT)* wolfSSL_X509_STORE_get0_objects(
store->objs = ret;
return ret;
err_cleanup:
if (ret != NULL)
X509StoreFreeObjList(store, ret);
if (ret != NULL) {
if (certsOwned) {
X509StoreFreeObjList(store, ret);
}
else {
/* cert_stack still owns these certificates. pop_free() below runs
* wolfSSL_X509_OBJECT_free() on each entry, which frees the inner
* X509, so clear the references or they are freed twice. */
int j;
WOLFSSL_X509_OBJECT* cur;

for (j = 0; j < wolfSSL_sk_X509_OBJECT_num(ret); j++) {
cur = (WOLFSSL_X509_OBJECT*)wolfSSL_sk_X509_OBJECT_value(ret,
j);
if (cur != NULL) {
cur->type = (WOLFSSL_X509_LOOKUP_TYPE)0;
cur->data.ptr = NULL;
}
}
wolfSSL_sk_X509_OBJECT_pop_free(ret, NULL);
}
}
Comment on lines 2628 to +2650

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, just increase the reference counter when placing objects inside the stack and simplify the cleanup logic to always call the free.

if (cert_stack != NULL) {
while (store->numAdded > 0) {
wolfSSL_sk_X509_pop(cert_stack);
Expand Down
39 changes: 39 additions & 0 deletions tests/api/test_dtls13.c
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,45 @@ int test_dtls13_epochs(void) {
return EXPECT_RESULT();
}

int test_dtls13_alert_with_pending_output(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
WOLFSSL_CTX *ctx_c = NULL;
WOLFSSL_CTX *ctx_s = NULL;
WOLFSSL *ssl_c = NULL;
WOLFSSL *ssl_s = NULL;
struct test_memio_ctx test_ctx;
char msg[1300];

XMEMSET(&test_ctx, 0, sizeof(test_ctx));
XMEMSET(msg, 'A', sizeof(msg));

ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

/* Stall the transport, then queue a record big enough that adding the
* alert would exceed the MTU. */
test_ctx.s_force_want_write = 1;
ExpectIntLT(wolfSSL_write(ssl_s, msg, (int)sizeof(msg)), 0);
ExpectIntGT((int)ssl_s->buffers.outputBuffer.length, 1288);

/* EndOfEarlyData is not valid in DTLS 1.3 and raises a fatal alert. */
ExpectIntEQ(Dtls13CheckEpoch(ssl_s, end_of_early_data), SANITY_MSG_E);

ExpectIntLE((int)(ssl_s->buffers.outputBuffer.idx +
ssl_s->buffers.outputBuffer.length),
(int)ssl_s->buffers.outputBuffer.bufferSize);

wolfSSL_free(ssl_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_c);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}

/*-- ack_order (test_dtls.c lines 873,951) ---*/
int test_dtls13_ack_order(void)
{
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_dtls13.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ int test_dtls13_basic_connection_id(void);
int test_dtls13_hrr_want_write(void);
int test_dtls13_every_write_want_write(void);
int test_dtls13_epochs(void);
int test_dtls13_alert_with_pending_output(void);
int test_dtls13_ack_order(void);
int test_dtls13_ack_overflow(void);
int test_dtls13_ack_dup_write_counter(void);
Expand Down Expand Up @@ -71,6 +72,7 @@ int test_dtls13_reuse_after_clear(void);
TEST_DECL_GROUP("dtls13", test_dtls13_hrr_want_write), \
TEST_DECL_GROUP("dtls13", test_dtls13_every_write_want_write), \
TEST_DECL_GROUP("dtls13", test_dtls13_epochs), \
TEST_DECL_GROUP("dtls13", test_dtls13_alert_with_pending_output), \
TEST_DECL_GROUP("dtls13", test_dtls13_ack_order), \
TEST_DECL_GROUP("dtls13", test_dtls13_ack_overflow), \
TEST_DECL_GROUP("dtls13", test_dtls13_ack_dup_write_counter), \
Expand Down
Loading
Loading