Repository navigation
Conversation
Usage() printed each message with printf("%s", msg[++msgid]) in a
sequence that had to match the conditional entries of the usage tables,
and the two had drifted apart: the client printed the -H message twice
and never printed the TLS 1.3 OCSP multi line or --crypto-policy, and
the server printed --pqc four times for a single entry. A default build
printed "(null)" once at the end of "client --help" and three times at
the end of "server --help", and the Japanese tables were off as well.
Keep printing the messages that take an argument one by one, then print
the rest of the table in order up to its NULL terminator, so Usage()
cannot run past the end of a table whatever options are built in. The
server's -M, -m, -2 and -4 lines, which were printed with hard-coded
printf() calls, move into both tables so they keep their place.
In the Japanese tables, guard the server -T entry with
HAVE_SESSION_TICKET as the English one is, add the missing client --ech
entry (untranslated for now), add missing commas, and fix the server's
ML-KEM line break, a blank line after -E and the closing hint, which
said "TSL" and "client". Drop the duplicate SecP521r1MLKEM1024 line
from the --pqc lists.
The group name printed with the results was read from theadInfo[0].group before the threads for the run were set up, so it reflected the previous run: the first group was shown as "N/A", and a TLS 1.2 cipher run after a TLS 1.3 one was shown with a group it did not use. Use the same condition that sets info->group.
SecP521r1MLKEM1024 is one of the extra hybrid groups, which need --enable-experimental --enable-extra-pqc-hybrids, so the commands in item 15 failed with the configure line given there. Use SecP384r1MLKEM1024, which is enabled by default.
Make the help text match what configure does: - --enable-aesecb is disabled by default, not enabled. - --enable-error-queue-per-thread is enabled by default when thread local storage is available and the build is not single threaded. - --enable-tls13-draft18 has done nothing for a long time. Say so, and warn when it is given. - --enable-64bit only leaves sp_c32.c out of the SP C build, and --enable-32bit selects 32-bit machine words and leaves out 64-bit only code without adding -m32. - --enable-asn-print is only on by default for Linux, macOS and FreeBSD hosts. - --enable-ticket-nonce-malloc is turned on by --enable-curl. - Declare aescbc-length-checks with hyphens, as its help string and the other options are spelled. configure accepts both spellings either way. --with-intelqa-sync added -DO_CRYPTO where the async path adds -DDO_CRYPTO; fix the typo.
Several entries in the build option comments at the top of the source files described the macro wrongly: - WOLFSSL_REQUIRE_TCA makes the client fail the handshake when the server does not acknowledge the trusted_ca_keys extension. - WOLFSSL_NO_CLIENT_CERT_ERROR makes a TLS 1.3 client abort when asked for a certificate it cannot provide. - WOLFSSL_ALWAYS_KEEP_SNI makes the server keep the client's SNI when it has not set one itself. - WOLFSSL_CURVE25519_BLINDING applies to wolfCrypt generally and is on by default for the C implementation. - WOLFSSL_DTLS_DISALLOW_FUTURE rejects records whose sequence number is too far ahead, not records from a future epoch. - WOLFSSL_SESSION_EXPORT_NOPEER leaves out the peer IP address and port, not peer certificate information. - HAVE_AESGCM_DECRYPT only matters together with NO_AES_DECRYPT. - GCM_TABLE is the 8-bit (256 entry) table, and GCM_TABLE_4BIT is what configure selects by default.
Several \ingroup names were not defined in doxygen_groups.h, so Doxygen made no group page for those functions: - Move the nine "SSL" functions into CertsKeys, openSSL and Setup. - Point DH, Ed25519, Ed448, CryptoCb and RNGBank at the groups they were meant for (Diffie-Hellman, ED25519, ED448, Crypto and Random). - Define QUIC, ASCON, OCSP, Atomic, Mutex, RwLock, Threading, String, File and DSP (the Hexagon DSP handle callback, previously "Callback"). - Move wc_BufferKeyEncrypt() and wc_BufferKeyDecrypt() from the crypto callback group to Password. Also: - Add \ingroup Setup to wolfDTLSv1_2_server_method() and wolfDTLSv1_2_client_method_ex(), and fix their examples. - Document wolfDTLSv1_3_client_method_ex() and wolfDTLSv1_3_server_method_ex(). - wolfSSL_CTX_set1_groups_list() matches group names case-insensitively and needs OPENSSL_EXTRA rather than HAVE_ECC; fix the wrong function called in the wolfSSL_set1_groups_list() and wolfSSL_preferred_group() examples. - Rewrite the wolfSSL_StaticBufferSz_ex() comment from the code: the parameters it listed did not exist, and its example passed a NULL buffer, which returns BAD_FUNC_ARG. - Replace an ideographic space in an example.
None of the ECH functions had API documentation. Add Doxygen comments for the configuration generate, set and get functions, the enable and trial decryption switches, wolfSSL_GetEchRetryConfigs() and wolfSSL_GetEchStatus(), describing the return values, the defaults used for zero algorithm IDs and which side each function is meant for.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the documentation, build-option, and example updates.
No open issues 🎉
What changed in this PR
This PR aligns wolfSSL’s API documentation, build-option descriptions, and examples with current behavior, addressing issues found during the manual review.
Changes:
- Fixes client/server help-table printing and benchmark group labels.
- Corrects configure help, the Intel QA sync define, and post-quantum installation examples.
- Adds ECH and DTLS API documentation, repairs Doxygen groups, and clarifies build-option comments.
| File | Description |
|---|---|
wolfcrypt/src/aes.c |
Clarifies AES-GCM decryption and lookup-table comments. |
src/tls13.c |
Corrects the client-certificate error option description. |
src/tls.c |
Clarifies SNI, trusted-CA, and Curve25519 option comments. |
src/internal.c |
Corrects DTLS sequence and session-export option comments. |
INSTALL |
Uses a post-quantum group supported by the documented configuration. |
examples/server/server.c |
Prints help through the table terminator and repairs bilingual entries. |
examples/client/client.c |
Simplifies help printing and fixes Japanese table entries. |
examples/benchmark/tls_bench.c |
Derives group labels from the current benchmark run. |
doc/dox_comments/header_files/wc_port.h |
Assigns DSP callbacks to the DSP documentation group. |
doc/dox_comments/header_files/wc_keystore.h |
Assigns key-store APIs to the defined Crypto group. |
doc/dox_comments/header_files/wc_encrypt.h |
Groups password-based key encryption APIs under Password. |
doc/dox_comments/header_files/ssl.h |
Documents ECH and DTLS methods; corrects API examples and groups. |
doc/dox_comments/header_files/rng_bank.h |
Assigns root RNG APIs to the Random group. |
doc/dox_comments/header_files/memory.h |
Corrects static-buffer sizing documentation and its example. |
doc/dox_comments/header_files/doxygen_groups.h |
Defines missing algorithm, platform, threading, and QUIC groups. |
doc/dox_comments/header_files/cryptocb.h |
Assigns crypto callback APIs to the defined Crypto group. |
doc/dox_comments/header_files/asn_public.h |
Corrects Diffie-Hellman and EdDSA group identifiers. |
configure.ac |
Corrects option help, warns about obsolete draft support, and fixes the QAT define. |
Review effort: Balanced
|
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #12038
Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): INSTALL, configure.ac, doc/dox_comments/header_files/asn_public.h, doc/dox_comments/header_files/cryptocb.h, doc/dox_comments/header_files/doxygen_groups.h, doc/dox_comments/header_files/memory.h, doc/dox_comments/header_files/rng_bank.h, doc/dox_comments/header_files/ssl.h, doc/dox_comments/header_files/wc_encrypt.h, doc/dox_comments/header_files/wc_keystore.h, doc/dox_comments/header_files/wc_port.h, examples/benchmark/tls_bench.c, examples/client/client.c, examples/server/server.c
Coverage: 4 of 4 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Max
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes for problems in this repository found while reviewing the wolfSSL manual against current master (wolfSSL/documentation#296). One commit per topic:
client --help/server --help.Usage()printed the usage tables with a hand-maintainedprintf("%s", msg[++msgid])sequence that had drifted from the tables' conditional entries, so a default build printed "(null)" once (client) and three times (server).Usage()now prints the messages that take an argument one by one and then the rest of the table up to its NULL terminator. The server's hard-coded-M,-m,-2and-4lines move into both language tables. The Japanese tables get a missingHAVE_SESSION_TICKETguard, a missing--echentry, missing commas and some text fixes, and the duplicateSecP521r1MLKEM1024line is dropped from the--pqclists.SecP521r1MLKEM1024needs--enable-experimental --enable-extra-pqc-hybrids, so item 15 failed with its own configure line. It now usesSecP384r1MLKEM1024, which is enabled by default.aesecb,error-queue-per-thread,tls13-draft18(which now also warns that it is ignored),64bit,32bit,asn-printandticket-nonce-malloc.aescbc-length-checksis now declared with hyphens; both spellings still work. Also fixes the-DO_CRYPTOtypo in--with-intelqa-sync(the async path uses-DDO_CRYPTO).WOLFSSL_REQUIRE_TCA,WOLFSSL_NO_CLIENT_CERT_ERROR,WOLFSSL_ALWAYS_KEEP_SNI,WOLFSSL_CURVE25519_BLINDING,WOLFSSL_DTLS_DISALLOW_FUTURE,WOLFSSL_SESSION_EXPORT_NOPEER,HAVE_AESGCM_DECRYPT,GCM_TABLEandGCM_TABLE_4BITinsrc/internal.c,src/tls.c,src/tls13.candwolfcrypt/src/aes.c.\ingroupnow names a group defined indoxygen_groups.h. Before this, functions inSSL,QUICand 14 other undefined groups got no group page. Also: missing\ingroupon two DTLS 1.2 methods, new docs for the DTLS 1.3_exmethods, theset1_groups_listcase-sensitivity and build-guard text, two wrong example calls, a rewrittenwolfSSL_StaticBufferSz_ex()comment (its example passed a NULL buffer, which returnsBAD_FUNC_ARG), and an ideographic space.The new Doxygen groups (QUIC, ASCON, OCSP, Atomic, Mutex, RwLock, Threading, String, File, DSP) also need
mkdocs.ymlnav entries in the documentation repository before they appear in the manual.Testing
./configure && make check: 6 pass, 5 skip, 0 fail.--helpand-? 1under a maximal config (--enable-all --enable-experimental --enable-ech --enable-dual-alg-certs --enable-rpk --enable-ocspstapling --enable-truncatedhmac --enable-anon --enable-atomicuser --enable-mcast --enable-sys-ca-certs --enable-brainpool --enable-dtls13 --with-sys-crypto-policy=yes, which checks the table sizes) and a small one (--disable-tls13 --disable-dh --enable-dtls --enable-psk --disable-session-ticket). No "(null)" in any table.--enable-kyber --enable-dilithium: the quick-start commands and the ML-DSA certificate commands both connect withSecP384r1MLKEM1024;SecP521r1MLKEM1024fails on that build as described.-g -m -t 1over a TLS 1.3 and a TLS 1.2 cipher shows the correct group for every run, and "N/A" for TLS 1.2.--helptext;--enable-aescbc-length-checksand--enable-aescbc_length_checksboth defineWOLFSSL_AES_CBC_LENGTH_CHECKS;--enable-tls13-draft18prints the warning;--enable-curldefinesWOLFSSL_TICKET_NONCE_MALLOC. The Intel QA sync change could not be built without a QAT driver.wolfSSL_StaticBufferSz_ex()example compiled and run against an--enable-staticmemorybuild.\ingrouphas a matching\defgroup); Doxygen itself was not run.Checklist