wolfSSL/wolfssl
1
0
Fork 1
mirror of https://github.com/wolfSSL/wolfssl.git synced 2026-08-15 14:51:31 +02:00
Code Issues Packages Projects Releases Wiki Activity

Keep the record size probe clear of a suspended build

Browse Source 
Second review follow-up on the async record layer series.

Stop the probe reselecting the cipher side. BuildMessage()'s BUILD_MSG_BEGIN
case can call SetKeysSide() for DTLS with secure renegotiation, which swaps
the active encryption state and clears recordSzOverhead. That is not part of a
size calculation, and after the previous commit the suspended build survives
to resume against whatever side the probe last chose, so a DTLS 1.2 record
suspended for PREV_ORDER could resume against the renegotiation keys. Skip it
when sizeOnly is set; the sizes are the same either way.

The probe itself has to keep running. Not re-entering BuildMessage at all
while a build is suspended looks tidier, but wolfssl_local_GetMaxPlaintextSize()
derives the DTLS fragment size from this result, so falling back to the upper
bound there shrinks fragments inconsistently between calls and the MTU
reproducer fails its buffer comparison. Saving and restoring the two fields is
what keeps the answer exact.

Resume inside the record when handshake content is left. The previous commit
declined to skip the padding for a fragmented or coalesced
certificate_request, which was right, but left processReply at doProcessInit
with the index inside the record, so the resume still started a fresh record
parse in the middle of one. Mirror both halves of the end of record block
instead: set runProcessingOneMessage when content remains, advance past the
padding only at the boundary.

Note the shared state at the source. BuildMessage() and BuildTls13Message()
write ssl->options.buildMsgState even for a sizeOnly probe with asyncOkay
clear, where everything else goes to the caller's own arguments. Nothing said
so at those sites, so the next sizeOnly caller would reintroduce this.

Record why only one of the three wc_ecc_make_key_ex() calls in eccsi.c needs
a wait: the other two are preceded by wc_ecc_free(), which clears the marker
their pending path is gated on. Moving either free would make them pend.

Test changes. Force the overhead cache cold before probing, otherwise an AEAD
suite answers from the cache without ever calling BuildMessage and the
assertions hold no matter what the probe did. Compare against BuildMessage's
own figure rather than only checking the size is positive, and run the whole
thing for TLS 1.3 as well as TLS 1.2, since BuildTls13Message() clobbers the
state by a different route: its sizeOnly return bypasses exit_buildmsg
entirely. Checked by stubbing the restore out again, which fails the test.

Also spell the new guard in cryptocb_test() as #if defined(WOLFSSL_ASYNC_CRYPT)
to match the rest of that file, which uses that form 170 times against 4.
This commit is contained in:
Tobias Frauenschläger
2026-08-05 20:39:57 +02:00
parent ac7bff1147
commit ff730fec9e
6 changed files with 94 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/async.yml
+1 -1
View File
@@ -71,7 +71,7 @@ jobs:
run: |
cat > "$RUNNER_TEMP/async-configs.json" <<'EOF'
[
{"comment": "The only entry that pairs the software async simulator with --enable-all. --enable-all turns on cryptocb, which stops configure.ac from auto-enabling the simulator, so the asynccrypt-all entries below define WOLFSSL_ASYNC_CRYPT but never actually return WC_PENDING_E. Without this one nothing exercises TLS 1.3 post-handshake auth or DTLS writes against a pending crypto op.",
{"comment": "The only entry that pairs the software async simulator with --enable-all. --enable-all turns on cryptocb, which stops configure.ac from auto-enabling the simulator, so the asynccrypt-all entries below define WOLFSSL_ASYNC_CRYPT but never actually return WC_PENDING_E. Without this one nothing exercises TLS 1.3 post-handshake auth or DTLS writes against a pending crypto op. The minutes value is a projection, not a CI measurement: this config takes 1.6 min locally where the asynccrypt-all entries below take 1.4 against their declared 3. Refresh it from the first real run.",
"name": "asynccrypt-sw-all-dtls13", "minutes": 3,
"configure": ["--enable-asynccrypt-sw", "--enable-all",
"--enable-dtls13",
src/internal.c
+33 -29
View File
@@ -25481,26 +25481,26 @@ static int DoProcessReplyEx(WOLFSSL* ssl, int allowSocketErr)
&ssl->buffers.inputBuffer.idx,
ssl->curStartIdx + ssl->curSize);
#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WOLFSSL_POST_HANDSHAKE_AUTH)
/* Post-handshake authentication runs the connect state
* machine from inside this handler and resumes through
* wolfSSL_negotiate() rather than by reprocessing this
* record, which is why it leaves processReply at
* doProcessInit. Finish the record's accounting here,
* or the trailing MAC is parsed as the next record
* header and the read fails with VERSION_ERROR. An
* ordinary pending message leaves processReply at
* runProcessingOneMessage and must not be advanced.
* The content check matches the end of record test
* below: a fragmented certificate_request rewinds
* inOutIdx to reprocess the fragment, and coalesced
* handshake messages leave idx inside the record, so
* in both cases the padding must not be skipped. */
/* Post-handshake auth resumes through
* wolfSSL_negotiate() instead of reprocessing this
* record, so it leaves processReply at doProcessInit
* (an ordinary pending message leaves it at
* runProcessingOneMessage). Finish the record here or
* the trailing MAC is read as the next record header
* and fails with VERSION_ERROR. Mirrors the end of
* record block below: resume inside the record when
* content is left, else skip the padding. */
if (ret == WC_NO_ERR_TRACE(WC_PENDING_E) &&
ssl->options.processReply == doProcessInit &&
(ssl->buffers.inputBuffer.idx -
ssl->curStartIdx) >= ssl->curSize &&
IsEncryptionOn(ssl, 0)) {
ssl->buffers.inputBuffer.idx += ssl->keys.padSz;
ssl->options.processReply == doProcessInit) {
if ((ssl->buffers.inputBuffer.idx -
ssl->curStartIdx) < ssl->curSize) {
ssl->options.processReply =
runProcessingOneMessage;
}
else if (IsEncryptionOn(ssl, 0)) {
ssl->buffers.inputBuffer.idx +=
ssl->keys.padSz;
}
}
#endif
#ifdef WOLFSSL_EARLY_DATA
@@ -26277,6 +26277,10 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
if (ret == WC_NO_ERR_TRACE(WC_NO_PENDING_E))
#endif
{
/* Note: these hit ssl->options even for a sizeOnly probe, where every
* other result goes to lcl_args, so a probe destroys the resume point
* of a suspended asynchronous build. wolfssl_local_GetRecordSize() is
* the only sizeOnly caller and restores them; a new one must too. */
ret = 0;
#ifdef WOLFSSL_ASYNC_CRYPT
ssl->options.buildArgsSet = 1;
@@ -26294,7 +26298,10 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
case BUILD_MSG_BEGIN:
{
#if defined(WOLFSSL_DTLS) && defined(HAVE_SECURE_RENEGOTIATION)
if (ssl->options.dtls && DtlsSCRKeysSet(ssl)) {
/* Skipped for a size probe: the size is the same either way, and
* SetKeysSide() would swap the active encryption state and clear
* recordSzOverhead under a suspended asynchronous build. */
if (!sizeOnly && ssl->options.dtls && DtlsSCRKeysSet(ssl)) {
/* For epochs >1 the current cipher parameters are located in
* ssl->secure_renegotiation->tmp_keys. Previous cipher
* parameters and for epoch 1 use ssl->keys */
@@ -45181,18 +45188,9 @@ int wolfssl_local_GetRecordSize(WOLFSSL *ssl, int payloadSz, int isEncrypted)
int isDtls13 = ssl->options.dtls && ssl->options.tls1_3;
#endif
#ifdef WOLFSSL_ASYNC_CRYPT
/* The size probe below borrows two fields that also describe an
* asynchronous BuildMessage still in flight: buildMsgState is its
* resume point, and buildArgsSet says its arguments are live.
* SendData() calls us again on every retry, so without saving them the
* probe rewinds the suspended record to BUILD_MSG_BEGIN and clears the
* arguments flag. The resumed call then re-applies the header and
* cipher overhead to arguments that already carry it, and the record
* is rejected with BUFFER_E. */
byte savedBuildMsgState = ssl->options.buildMsgState;
byte savedBuildArgsSet = ssl->options.buildArgsSet;
#endif
if (ssl->specs.cipher_type == aead && ssl->recordSzOverhead != 0
#ifdef WOLFSSL_DTLS13
&& (!isDtls13 || payloadSz + (int)ssl->recordSzOverhead
@@ -45205,6 +45203,12 @@ int wolfssl_local_GetRecordSize(WOLFSSL *ssl, int payloadSz, int isEncrypted)
recordSz = BuildMessage(ssl, NULL, 0, NULL, payloadSz, application_data,
0, 1, 0, CUR_ORDER);
#ifdef WOLFSSL_ASYNC_CRYPT
/* Sizing shares the build state machine with an asynchronous
* BuildMessage that SendData() re-sizes on every retry, so the probe
* runs while that record is suspended. Restore its resume point, or
* the record is sized twice and rejected with BUFFER_E. Skipping the
* probe is not an option: wolfssl_local_GetMaxPlaintextSize() derives
* the DTLS fragment size from this result, so it must stay exact. */
ssl->options.buildMsgState = savedBuildMsgState;
ssl->options.buildArgsSet = savedBuildArgsSet;
#endif
src/tls13.c
+4
View File
@@ -3366,6 +3366,10 @@ int BuildTls13Message(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
if (ret == WC_NO_ERR_TRACE(WC_NO_PENDING_E))
#endif
{
/* Note: these hit ssl->options even for a sizeOnly probe, where every
* other result goes to lcl_args, so a probe destroys the resume point
* of a suspended asynchronous build. wolfssl_local_GetRecordSize() is
* the only sizeOnly caller and restores them; a new one must too. */
ret = 0;
ssl->options.buildMsgState = BUILD_MSG_BEGIN;
XMEMSET(args, 0, sizeof(BuildMsg13Args));
tests/api/test_tls.c
+50 -13
View File
@@ -2927,37 +2927,55 @@ int test_record_size_matches_build_message(void)
return EXPECT_RESULT();
}
int test_record_size_preserves_build_msg_state(void)
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
defined(WOLFSSL_ASYNC_CRYPT)
/* SendData() sizes the output buffer on every retry, including while an
* asynchronous BuildMessage is suspended part way through a record. Sizing
* runs the same build state machine, so the probe must not re-enter it: it
* would rewind buildMsgState, clear buildArgsSet, and the resumed record would
* be sized a second time. Check that a suspended build survives a probe, and
* that a probe from a clean state still returns the exact record size. */
static int record_size_state_check(method_provider client_method,
method_provider server_method)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
defined(WOLFSSL_ASYNC_CRYPT) && !defined(WOLFSSL_NO_TLS12)
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
struct test_memio_ctx test_ctx;
int sz;
int expectedSz = 0, cleanSz = 0, busySz = 0;
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);
client_method, server_method), 0);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
/* SendData() re-probes the record size on every retry, which happens while
* an asynchronous BuildMessage is suspended part way through a record. The
* probe shares buildMsgState and buildArgsSet with that build, so it must
* put both back or the resumed record is sized twice. */
if (ssl_c != NULL) {
expectedSz = BuildMessage(ssl_c, NULL, 0, NULL, 256,
application_data, 0, 1, 0, CUR_ORDER);
ssl_c->options.buildMsgState = BUILD_MSG_BEGIN;
ssl_c->options.buildArgsSet = 0;
ExpectIntGT(expectedSz, 256);
/* Clearing the cache is what forces the BuildMessage path; an AEAD
* suite would otherwise answer from ssl->recordSzOverhead and the
* assertions below would hold no matter what the probe did. */
ssl_c->recordSzOverhead = 0;
cleanSz = wolfssl_local_GetRecordSize(ssl_c, 256, 1);
ExpectIntEQ(cleanSz, expectedSz);
/* Same probe with a build suspended mid-record. */
ssl_c->recordSzOverhead = 0;
ssl_c->options.buildMsgState = BUILD_MSG_ENCRYPT;
ssl_c->options.buildArgsSet = 1;
sz = wolfssl_local_GetRecordSize(ssl_c, 256, 1);
busySz = wolfssl_local_GetRecordSize(ssl_c, 256, 1);
ExpectIntGT(sz, 256);
ExpectIntEQ(ssl_c->options.buildMsgState, BUILD_MSG_ENCRYPT);
ExpectIntEQ(ssl_c->options.buildArgsSet, 1);
/* Still exact: wolfssl_local_GetMaxPlaintextSize() sizes DTLS
* fragments from this, so it may not degrade to an upper bound. */
ExpectIntEQ(busySz, expectedSz);
/* Nothing was really built, so do not leave the flag claiming the
* async arguments are live. */
ssl_c->options.buildMsgState = BUILD_MSG_BEGIN;
ssl_c->options.buildArgsSet = 0;
}
@@ -2966,6 +2984,25 @@ int test_record_size_preserves_build_msg_state(void)
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_c);
wolfSSL_CTX_free(ctx_s);
return EXPECT_RESULT();
}
#endif /* HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES && WOLFSSL_ASYNC_CRYPT */
int test_record_size_preserves_build_msg_state(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
defined(WOLFSSL_ASYNC_CRYPT)
#ifndef WOLFSSL_NO_TLS12
ExpectIntEQ(record_size_state_check(wolfTLSv1_2_client_method,
wolfTLSv1_2_server_method), TEST_SUCCESS);
#endif
#ifdef WOLFSSL_TLS13
/* BuildTls13Message() clobbers buildMsgState by a different route: its
* sizeOnly return bypasses exit_buildmsg entirely. */
ExpectIntEQ(record_size_state_check(wolfTLSv1_3_client_method,
wolfTLSv1_3_server_method), TEST_SUCCESS);
#endif
#endif
return EXPECT_RESULT();
}
wolfcrypt/src/eccsi.c
+5 -2
View File
@@ -465,8 +465,11 @@ int wc_MakeEccsiKey(EccsiKey* key, WC_RNG* rng)
err = wc_ecc_make_key_ex(rng, key->ecc.dp->size, &key->ecc,
key->ecc.dp->id);
#ifdef WOLFSSL_ASYNC_CRYPT
/* ECCSI has no asynchronous API, so the caller cannot resume a
* pending key generation. Complete it here. */
/* ECCSI has no asynchronous API, so the caller cannot resume a pending
* key generation - complete it here. The key->pubkey sites in
* eccsi_make_pair() and eccsi_gen_sig() need no wait: each is preceded
* by wc_ecc_free(&key->pubkey), which clears the marker that
* _ecc_make_key_ex() gates its pending path on. */
err = wc_AsyncWait(err, &key->ecc.asyncDev, WC_ASYNC_FLAG_NONE);
#endif
}
wolfcrypt/test/test.c
+1 -1
View File
@@ -81235,7 +81235,7 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t cryptocb_test(void)
if (ret == 0) {
haveSrc = 1;
ret = wc_ecc_make_key(eccRng, 32, srcKey);
#ifdef WOLFSSL_ASYNC_CRYPT
#if defined(WOLFSSL_ASYNC_CRYPT)
ret = wc_AsyncWait(ret, &srcKey->asyncDev, WC_ASYNC_FLAG_NONE);
#endif
}