Address the PR 10845 review findings:
* test_signature.c: the Falcon sign/verify test was guarded on
HAVE_PQC, which no build system ever defines (--enable-falcon sets
HAVE_FALCON only), so the test always compiled out to skipped. Gate
it on HAVE_FALCON && HAVE_LIBOQS like the library does, and drop the
now-unreferenced HAVE_PQC entry from .wolfssl_known_macro_extras.
Verified with --enable-opensslall --enable-experimental
--enable-falcon --with-liboqs: the test now executes and passes.
* api.c: revert the (OPENSSL_EXTRA || OPENSSL_ALL) widening of the
inner guards of test_wolfSSL_TXT_DB and the new
test_wolfSSL_NCONF_negative_paths. Both functions live inside the
enclosing OPENSSL_ALL block and their TEST_DECLs are OPENSSL_ALL-
gated (and TXT_DB/NCONF themselves are OPENSSL_ALL-only APIs), so
the widening could never take effect. Inner guards now match their
siblings (filesystem/BIO deps only).
* test_pkcs12.c: wrap the expected BAD_FUNC_ARG values of the new
parse guardrail asserts in WC_NO_ERR_TRACE() for consistency with
the rest of the PR's negative-path assertions.
All six affected tests pass in an opensslall+falcon+liboqs build:
falcon_sign_verify, TXT_DB, NCONF, NCONF_negative_paths,
PKCS12_parse_guardrails, PKCS12_create_guardrails.
The last two red PR jobs, both in the branch's new tests:
- intelasm (ASAN): test_wc_AesKeyExportArgMcdc calls wc_AesInit_Id() and
wc_AesInit_Label() which succeed (allocating the WC_DEBUG_CIPHER_LIFECYCLE
tag) but were never freed -> 8-byte LeakSanitizer leak. Add wc_AesFree()
to both blocks.
- no-client-no-client-auth (minimal server-only build):
test_wolfSSL_session_cache_api_direct's wolfSSL_new() returned NULL because
a certless server CTX has no usable cipher suite. Load the test server
cert/key (file, with a USE_CERT_BUFFERS_2048 fallback) before wolfSSL_new()
in the server-only path; the client path is cert-free as before.
Verified: --enable-all + ASAN run of the aes group is leak-free, and
CPPFLAGS="-DNO_WOLFSSL_CLIENT -DWOLFSSL_NO_CLIENT_AUTH" now passes.
Three more failures in the branch's added tests, found via the ASAN, C++
and no-client CI configs:
- test_wolfSSL_X509V3_EXT leaked 2296 bytes: the added
X509_get_ext_d2i(x509, NID, &critical, NULL) calls (used to exercise the
critical-flag output) discarded their allocated result. Free each per its
actual return type: BASIC_CONSTRAINTS, ASN1_STRING (key usage),
AUTHORITY_KEYID, AUTHORITY_INFO_ACCESS, and - for subject_key_identifier -
a STACK_OF(ASN1_OBJECT) (wolfSSL_X509_get_ext_d2i wraps a lone obj in a
stack). This was the real cause of the sanitize-asan / intelasm / krb-asan
job failures (the read_write_ex/ECH/dtls13 asserts printed there are
retry-masked and fail identically on master).
- C++ build (all-pq-cxx): void* from X509_get_ext_d2i does not implicitly
convert; add explicit WOLFSSL_X509_EXTENSION* casts.
- no-client link (all-no-client): wolfSSL[_CTX]_UseOCSPStapling[V2] (CSR/CSR2)
are client-side APIs; guard those blocks with !NO_WOLFSSL_CLIENT.
Verified: full --enable-all + ASAN run is leak-free and passes; --enable-all
-DNO_WOLFSSL_CLIENT builds and links.
The new MC/DC coverage tests broke many CI configs under -Werror (which is
auto-enabled for in-git-tree builds). Fixes, each verified with a real
-Werror build of the relevant config:
- test_aes.c: wrap the whole test_wc_AesSivArgMcdc definition in
WOLFSSL_AES_SIV && WOLFSSL_AES_128 (was body-only guarded while its
prototype is guarded) -> fixes -Wmissing-prototypes when SIV is off.
- test_aes.c: mark key/in/out (void) in test_wc_AesModesArgMcdc; they are
used only by the per-mode (CTR/CFB/OFB) blocks -> fixes -Wunused-variable
when no such mode is enabled.
- api.c: guard the test_CryptoCb_* callback helpers with
WOLF_CRYPTO_CB && WOLFSSL_TEST_STATIC_BUILD to match their only caller
-> fixes -Wunused-function in cryptocb non-static builds.
- api.c: register test_wc_CryptoCb_registry under its actual definition
condition (WOLF_CRYPTO_CB && HAVE_IO_TESTS_DEPENDENCIES && !ONLY_*) and
keep test_wc_CryptoCb registered unconditionally (as on master)
-> fixes undeclared / defined-but-unused across cryptocb configs.
- api.c: declare session-cache 'mode' under OPENSSL_EXTRA (its only uses)
-> fixes -Wunused-variable without opensslextra.
- api.c: guard the Enable/DisableOCSPStapling calls in
test_wolfSSL_crl_ocsp_object_api with HAVE_CERTIFICATE_STATUS_REQUEST[_V2]
-> fixes undefined references with OCSP but no stapling.
Verified clean under: --enable-ocsp --enable-ocspstapling, --enable-ocsp
(no stapling), and --enable-all; unit.test runs pass.
Several MC/DC coverage tests called WOLFSSL_LOCAL (hidden-visibility)
library functions directly from the in-tree unit.test:
- wc_AesCcmCheckTagSize() (test_aes.c)
- wc_CryptoCb_Init/Cleanup/GetDevIdAtIndex() (api.c)
These only link when the library is built with test-static visibility, so
normal (shared) builds failed at link with "undefined reference", breaking
essentially every CI build job. Gate the affected assertions on
WOLFSSL_TEST_STATIC_BUILD (in addition to the existing feature guards) so
they compile out where the symbols are hidden, matching the existing
wolfSSL convention for internal-symbol tests.
Verified: ./configure --enable-all (no WOLFSSL_TEST_STATIC_BUILD) now
builds tests/unit.test cleanly and the full suite passes.
Add a new option to require that an external Pre-Shared Key is negotiated
for a handshake to succeed, configured via the new APIs
wolfSSL_CTX_require_psk()/wolfSSL_require_psk(). When set, a handshake
that completes without negotiating an external PSK is aborted with
PSK_MISSING_ERROR instead of falling back to a certificate handshake, so
the PSK acts as an additional security factor.
This is a TLS 1.3 / DTLS 1.3 feature. In (D)TLS 1.2 the use of a PSK is
determined by the negotiated cipher suite, so a mandatory PSK is instead
configured there by restricting the cipher suite list to PSK suites; the
new APIs therefore reject non-TLS-1.3 contexts with BAD_FUNC_ARG.
To keep the requirement fail-closed, the APIs also disable version
downgrade on the object so a downgrade-capable context (e.g. one created
from a v23 method) cannot silently fall back to (D)TLS 1.2 and complete
without a PSK; a peer that does not support (D)TLS 1.3 fails to connect.
The requirement applies to external PSKs only (not session tickets):
session-ticket resumption is exempt. To preserve forward secrecy a
mandatory external PSK must also use an (EC)DHE key exchange; a pure
psk_ke handshake is rejected with PSK_KEY_ERROR. When used with
WOLFSSL_CERT_WITH_EXTERN_PSK, it also ensures that peers are properly
authenticated with both the PSK and via certificates.
The new APIs live alongside the existing wolfSSL_[CTX_]no_dhe_psk()/
only_dhe_psk() PSK options and do not depend on certificate support, so
the feature is usable in NO_CERTS (PSK-only) builds.
Added unit tests for the new APIs and enforcement.
tests/api.c: use WOLFSSL_FILETYPE_PEM, not SSL_FILETYPE_PEM;
tests/api/test_dtls.c and tests/api/test_dtls13.c: use WOLFSSL_ERROR_WANT_READ, not SSL_ERROR_WANT_READ.
ParseCipherList() only cleared the InitSuites mask for "!aNULL"/"!eNULL",
which governs generated defaults, so an explicitly listed ADH or NULL-cipher
suite survived (e.g. "ADH-AES128-SHA:!aNULL" still offered an unauthenticated
suite). Scrub the explicit suites after parsing; exclusions are order-
independent and sticky (a later "ALL" cannot re-enable them).
Add test_wolfSSL_set_cipher_list_exclusions.
Add wolfssl_local_MatchDnsNameConstraint() dispatching wildcard names
to the subtree matcher and literal names to plain base-name matching,
and use it for the ASN_DNS_TYPE branches of PermittedListOk() and
IsInExcludedList().
This also drops the outer name->len >= base len byte-length guard for
literal DNS names. That guard ran before MatchBaseName() could strip
the absolute-FQDN trailing dot, so a constraint base like
DNS:example.com. never matched the SAN example.com it denotes.
One trailing dot marks an absolute FQDN and is not part of the host:
"host.com." and "host.com" denote the same host. Strip it from the
URI host before classification (so "12.31.2.3." is still recognized
as an IPv4 address) and from the constraint base before the exact-match
comparison, mirroring what wolfssl_local_MatchBaseName() already does
for DNS name constraints. Only a single dot is the marker: an empty
last label ("host.com..") is rejected.
RFC 5280 4.2.1.10 defines URI name constraints in terms of a host that
is a fully qualified domain name; RFC 3986 IP-literal ([...]) and
IPv4address hosts are not DNS reg-names and cannot be meaningfully
matched against a DNS-style constraint base.
- Classify the host extracted by GetUriHost (IP-literal, IPv4address,
reg-name) and validate that a reg-name has no empty labels.
- wolfssl_local_MatchUriNameConstraint() no longer matches URIs whose
host is an IP address.
- ConfirmNameConstraints() fails closed: when URI constraints are
present, a URI SAN without a DNS host is rejected. A plain non-match
would otherwise let such names pass excluded-only constraints.
crl.c, internal.h: leak of sigParams requiring reorder the struct fields to that it is above memcpy part.
dtl13.c: free the DRLS fragments buffer in Dtls13FreeFsmResources in case fragment is never sent.
ocsp.c: only free cid if locally allocated.
tls.c: make sure ecc_kse is zeroized and can be freed.
tls13.c: set hsHashesEch after init so isn't lost on failure.
evp_pk.c: free key on the BIO error path
Fixed various tests to not leak or crash on memory allocation failure.
Address review on PR #10582:
- The client-side extended_master_secret consistency check skipped all
session-ticket resumptions, leaving a generic ticket resumption open to
an undetected EMS downgrade by a malicious server or MITM. The client
retains the EMS state for ticket sessions too (SetupSession), so the
check now applies to ticket resumption as well, mirroring the adjacent
cipher-suite check. Only EAP-FAST style resumption - where the
session-secret callback supplies the master secret for an opaque PAC
ticket - is exempt, matched precisely via ssl->sessionSecretCb just as
the callback invocation in DoServerHello does.
- Add test_tls_ems_resumption_server_downgrade, exercising the
client-direction downgrade (server resumes but omits EMS from its
ServerHello) for both session-ID and session-ticket resumption. This
client-side branch previously had no test coverage.
- Replace public wc_CryptoCb_GetDevice() with
wc_CryptoCb_IsDeviceRegistered() returns 1 or 0. keep the CryptoCb
struct and GetDevice private.
- Reject RegisterDevice(INVALID_DEVID) with BAD_FUNC_ARG instead of ALREADY_E.
- Document the new API and the ALREADY_E/BAD_FUNC_ARG returns.
- Fix table-full test to not leak when MAX_CRYPTO_DEVID_CALLBACKS >= 256.
David Garske
Daniele Lacamera
Eric Blankenhorn
Tobias Frauenschläger
Josh Holtrop
Sean Parkinson
philljj
Daniel Pouzzner
Emma Stensland
Lealem Amedie
aidan garske
Ruby Martin
Paul Adelsbach
Marco Oliverio
JacobBarthelmeh
Juliusz Sosinowicz
Alex Lanzano