Skip to content

Add compile-time crypto callback async poll for record ciphers#10990

Draft
julek-wolfssl wants to merge 2 commits into
wolfSSL:masterfrom
julek-wolfssl:cryptocb-async-poll-compile-time
Draft

Add compile-time crypto callback async poll for record ciphers#10990
julek-wolfssl wants to merge 2 commits into
wolfSSL:masterfrom
julek-wolfssl:cryptocb-async-poll-compile-time

Conversation

@julek-wolfssl

Copy link
Copy Markdown
Member

Summary

  • Adds WOLF_CRYPTO_CB_ASYNC_POLL, giving crypto callback devices a QAT/Nitrox-style "poll to fill output" completion model for TLS record ciphers (AES and 3DES) instead of the re-invoke model, fixing silent record corruption when a callback returns WC_PENDING_E.
  • Without the feature (and without a software/QAT/Cavium backend), a pending bulk cipher op now errors out with ASYNC_OP_E instead of corrupting the record.

Test plan

  • tests/api/test_async.c covers direct AES-GCM/CBC/CCM and 3DES poll completion at multiple pend depths
  • Negative cases for cipher types defined but not dispatched (ChaCha, single DES)
  • Full TLS 1.3 handshake+echo including the no-poll failure path

Copilot AI review requested due to automatic review settings July 24, 2026 15:01
@julek-wolfssl julek-wolfssl self-assigned this Jul 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a compile-time “poll to fill output” completion model for crypto-callback–backed async TLS record ciphers (AES + 3DES) via WOLF_CRYPTO_CB_ASYNC_POLL, aiming to prevent silent record corruption when a callback returns WC_PENDING_E by ensuring completion happens during async polling (or fails loudly when no completion path exists).

Changes:

  • Adds a new crypto-callback algo type (WC_ALGO_TYPE_ASYNC_POLL) and wc_CryptoCb_Poll() entry point to support poll-based completion.
  • Routes selected async operations through callback polling during wolfSSL_AsyncPoll() / async event polling, and converts “pending but no completion path” into ASYNC_OP_E for TLS record ciphers.
  • Adds a dedicated async test suite (tests/api/test_async.c) plus CI coverage to exercise poll completion and negative paths.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
wolfssl/wolfcrypt/types.h Adds WC_ALGO_TYPE_ASYNC_POLL to identify callback poll re-entry.
wolfssl/wolfcrypt/settings.h Adds a config-time guard for WOLF_CRYPTO_CB_ASYNC_POLL vs. WC_ASYNC_NO_CRYPT.
wolfssl/wolfcrypt/cryptocb.h Declares wc_CryptoCb_Poll() for callback poll completion.
wolfssl/wolfcrypt/async.h Extends WC_ASYNC_DEV to track a callback device ID for polling.
wolfcrypt/src/cryptocb.c Implements wc_CryptoCb_Poll() to re-enter the callback with WC_ALGO_TYPE_ASYNC_POLL.
wolfcrypt/src/async.c Integrates callback polling into async event polling/queue polling and sets routing state.
tests/api/test_async.h Declares new async poll-completion unit tests and registration macro.
tests/api/test_async.c Adds unit + TLS 1.3 integration tests for poll completion and negative cases.
tests/api/include.am Adds new async test sources to the unit test build and dist list.
tests/api.c Registers the new “async” test group in the test runner.
src/tls13.c Fails fast with ASYNC_OP_E when a bulk cipher pends but no completion path exists.
src/internal.c Applies the same “no completion path” failure for TLS 1.2 record ciphers; adjusts async pop logic for poll-completed events; preserves ssl->error on send failures.
examples/async/user_settings.h Silences the new async-crypt + cryptocb TLS warning for the async example config.
configure.ac Adds a configure-time warning about cryptocb+async TLS limitations and silences source-level warnings.
.wolfssl_known_macro_extras Registers WOLF_CRYPTO_CB_ASYNC_POLL as a recognized macro.
.github/workflows/async-examples.yml Adds CI job to build with WOLF_CRYPTO_CB_ASYNC_POLL and run the async test group.
Comments suppressed due to low confidence (1)

wolfcrypt/src/async.c:772

  • In wolfAsync_EventQueuePoll(), the callback poll is performed whenever cryptocb.devId is set, regardless of the event’s current ret value. Polling a non-pending event can incorrectly overwrite a successful completion with WC_NO_PENDING_E (or another error) depending on the device’s behavior. This should only poll when event->ret is still WC_PENDING_E.
                    if (asyncDev->cryptocb.devId != INVALID_DEVID) {
                        /* Re-enter the crypto callback to complete the job
                         * and fill the output buffer. */
                        event->ret =
                            wc_CryptoCb_Poll(asyncDev->cryptocb.devId);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread wolfcrypt/src/cryptocb.c Outdated
Comment thread wolfcrypt/src/async.c Outdated
Comment thread wolfcrypt/src/async.c Outdated
Comment thread wolfcrypt/src/async.c

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

wolfcrypt/src/cryptocb.c:481

  • wc_CryptoCb_Poll() currently lets WC_NO_PENDING_E propagate from the device callback. That contradicts the intent to never report “no pending” for an in-flight job and can also leave a DONE event with ret==WC_NO_PENDING_E, which wolfSSL_AsyncPop() treats specially (doesn’t clear ssl->asyncDev), potentially leaving stale async state. Consider mapping WC_NO_PENDING_E to a hard failure (WC_HW_E) like other unsupported polling cases.
        ret = dev->cb(devId, &info, dev->ctx);
        if (ret == WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE) ||
            ret == WC_NO_ERR_TRACE(NOT_COMPILED_IN)) {
            /* Device does not implement polling: job can never complete. */
            ret = WC_NO_ERR_TRACE(WC_HW_E);
        }

wolfcrypt/src/async.c:433

  • wolfAsync_DevCtxInit() stamps asyncDev->cryptocb.devId for AES/3DES whenever WOLF_CRYPTO_CB_ASYNC_POLL is enabled. In builds that also enable a hardware async backend (Intel QAT / Cavium / async SW), this can cause wolfAsync_EventPoll() to attempt wc_CryptoCb_Poll() for a non-crypto-callback devId, overriding the real backend’s pending state and potentially forcing WC_HW_E. Consider only enabling the poll-routing stamp when crypto callbacks are the active async backend for bulk ciphers (i.e., not HAVE_INTEL_QA/HAVE_CAVIUM/WOLFSSL_ASYNC_CRYPT_SW).
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_ASYNC_POLL)

Comment thread tests/api/test_async.c Outdated
A crypto callback that returns WC_PENDING_E for a TLS record cipher
silently corrupted records: Encrypt()/Decrypt() advance the cipher
state to CIPHER_STATE_END before the pending check, so on resume the
record is shipped without re-running the cipher.

WOLF_CRYPTO_CB_ASYNC_POLL gives crypto callback devices the QAT/Nitrox
"poll to fill output" completion model. On WC_PENDING_E the async event
stays queued; wolfSSL_AsyncPoll() re-enters the device with the new
WC_ALGO_TYPE_ASYNC_POLL (wc_CryptoCb_Poll) to finish the job and fill
the output buffer. The re-entry only polls while the event is still
pending, and a device that cannot complete the job (no poll support, or
it reports nothing pending) hard-fails with WC_HW_E rather than
reporting the op done with an unfilled buffer. Only the two async record
ciphers (AES and 3DES markers) are routed to poll completion, and only
when crypto callbacks are the async backend (not QAT/Cavium/SW);
handshake PK keeps the re-invoke model. The wolfSSL_AsyncPop eviction is
gated for poll-capable devices so the existing resume-at-CIPHER_STATE_END
path becomes correct with no record state-machine changes.

Without the feature (and without a software/QAT/Cavium backend) a
pending bulk cipher op now errors out with ASYNC_OP_E instead of
corrupting the record, and configure/cryptocb.c warn about the
unsupported combination.

Tests in tests/api/test_async.c cover direct AES-GCM/CBC/CCM and 3DES
poll completion in both directions at multiple pend depths, negative
cases for cipher types defined but not dispatched (ChaCha, single DES),
and full TLS 1.3 handshake+echo: an encrypt-offload run, a both-
directions run that offloads encrypt and decrypt on both peers using a
per-peer device, and the no-poll failure path.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comment thread wolfcrypt/src/cryptocb.c Outdated
wc_CryptoCb_Poll resolved the poll device with wc_CryptoCb_GetDevice,
bypassing the WOLF_CRYPTO_CB_FIND remapping the cipher submit uses. With a
device-find callback, submit could route to a mapped device while poll
looked up the unmapped id and hard-failed with WC_HW_E. Resolve via
wc_CryptoCb_FindDevice under WC_ALGO_TYPE_CIPHER so poll lands on the same
device as submit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comment thread wolfcrypt/src/cryptocb.c
Comment on lines +471 to +479
/* Resolve with WC_ALGO_TYPE_CIPHER, the algo the op was submitted under, so
* a WOLF_CRYPTO_CB_FIND remap lands on the same device as the submit. */
CryptoCb* dev = wc_CryptoCb_FindDevice(devId, WC_ALGO_TYPE_CIPHER);
if (dev != NULL && dev->cb != NULL) {
wc_CryptoInfo info;
XMEMSET(&info, 0, sizeof(info));
info.algo_type = WC_ALGO_TYPE_ASYNC_POLL;
ret = dev->cb(devId, &info, dev->ctx);
if (ret == WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE) ||
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants