Compliance to new RFCs#10941
Open
Frauschi wants to merge 6 commits into
Open
Conversation
RFC 9846 section 6.2 defines a new generic alert, general_error(117), sent to indicate an error condition when no more specific alert is available or the sender wishes to conceal the specific error code. Define the alert in the AlertDescription enumeration and add its name to AlertTypeToString so it is recognized and logged. In TLS 1.3 it is handled as a fatal alert by the existing alert processing, which matches the specification.
RFC 9846 Section 4.7.3 upgrades the key update bound so that sending implementations MUST NOT allow the number of key updates to exceed 2^48-1, while receiving implementations MUST NOT enforce this on the peer. The DTLS 1.3 path already gated its sending epoch, but stream TLS 1.3 tracked no key update count. Add a keyUpdateCount field at the end of the Keys structure TLS 1.3 block, refuse to send a KeyUpdate in SendTls13KeyUpdate once the sender reaches the ceiling, and increment the count after deriving each new sending key. The receive path is left unchanged so it never enforces a limit on the peer. Add a test that exercises both sides of the boundary: seeded one below the ceiling, wolfSSL_update_keys succeeds and advances the sender count to exactly 2^48-1; at the ceiling the next KeyUpdate is refused with BAD_STATE_E and the count is left unchanged.
RFC 9846 Section 4.4.2 corrects the lower bound on CertificateRequest.extensions to 0, so a zero length extensions block is syntactically valid. The client rejected it early with INVALID_PARAMETER before parsing. Remove the early rejection so the extensions are parsed and an empty block is instead caught by the existing mandatory signature_algorithms check, which returns the correct missing_extension alert. TLSX_Parse handles a zero length input by parsing no extensions.
RFC 9973 "TLS 1.3 Extension for Using Certificates with an External Pre-Shared Key" is now published and obsoletes RFC 8773. The cert_with_extern_psk implementation was written against the 8773bis draft that became RFC 9973, so it is already compliant. Update the textual references in comments, the configure help comment, the extension codepoint comment, a test comment, and the Doxygen docs. This is a documentation only change. The WOLFSSL_CERT_WITH_EXTERN_PSK macro, the --enable-cert-with-extern-psk option, the public API names, and the extension codepoint 33 (0x0021) are all unchanged, and no logic is affected. Historical ChangeLog and README entries are left as they shipped.
The keylog reader used fscanf with three whitespace delimited tokens, which is not line aware. RFC 9850 Section 1 requires readers to ignore empty lines and lines whose first character is '#', and Section 2 recommends ignoring lines that do not conform to the format so secrets can still be recovered from corrupted files. A comment line such as "# note" was parsed as three fields spanning the line boundary, which desynchronized the rest of the file. Read one line at a time with fgets, skip empty and comment lines, and parse each line with sscanf, skipping any line that does not yield the three expected fields instead of aborting. Clear the field buffers each iteration so a short field cannot pick up stale bytes from a previous line, validate that the fixed length client random field is the exact expected length, and leave the secret length variable because it depends on the negotiated hash. Zero the new line buffer on every return path, matching the existing ForceZero handling of the secret buffers. Add a comment line, a blank line, and a malformed line to the TLS 1.3 keylog test data so the sniffer keylog test exercises the skip paths.
|
Contributor
Author
|
Jenkins retest this please. |
RFC 9850 notes that implementations informally use an environment
variable named SSLKEYLOGFILE to select the key log path, which lets tools
such as curl, NSS and Wireshark share one file. The key log writers only
used the compile-time WOLFSSL_SSLKEYLOGFILE_OUTPUT path.
Add an opt-in WOLFSSL_SSLKEYLOGFILE_USE_ENV macro (default off) that makes
both tls13ShowSecrets and tlsShowSecrets prefer XGETENV("SSLKEYLOGFILE")
when it is set and non-empty, falling back to the compile-time path
otherwise. Keeping it opt-in leaves the default behavior unchanged, so a
build that exports the variable for other applications is unaffected, and
no XGETENV reference is emitted unless the macro is defined (avoiding a
dependency on environment access, for example under NO_STDIO_FILESYSTEM).
XGETENV resolves to NULL on targets without environment support, so it
falls back automatically. This stays behind the existing SHOW_SECRETS and
WOLFSSL_SSLKEYLOGFILE compile-time guards, so production builds that do
not define them cannot log secrets regardless of the environment, as
required by the RFC security considerations.
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
2 participants
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.
RFC compliance updates for TLS 1.3, the sniffer key-log reader, and the external-PSK certificate extension. Six focused, independent changes, grouped by the RFC they address. No public API is changed and every change stays behind its existing compile-time guards.
RFC 9846 (RFC 8446 successor, TLS 1.3 clarifications & errata)
general_error(117)alert — define the new generic alert from §6.2 in theAlertDescriptionenum andAlertTypeToString. In TLS 1.3 it is processed as a fatal alert by the existing alert handling, matching the spec.keyUpdateCountfield is added at the end of theKeysTLS 1.3 block;SendTls13KeyUpdaterefuses once the ceiling is reached (BAD_STATE_E) and increments after deriving each new sending key. The receive path is untouched.CertificateRequestextensions — §4.4.2 sets the lower bound onCertificateRequest.extensionsto 0, so a zero-length block is valid. The client no longer rejects it early withINVALID_PARAMETER; the empty block is instead caught by the existing mandatorysignature_algorithmscheck, which returns the correctmissing_extensionalert.RFC 9850 (SSLKEYLOGFILE format)
fscanfwith three whitespace-delimited tokens, which is not line-aware, so a comment line was parsed as fields spanning the line boundary and desynchronized the rest of the file. It now reads one line at a time withfgets, skips empty and#comment lines (§1), and silently skips malformed lines that do not yield the three expected fields (§2) so secrets are still recoverable from a corrupted file. Field buffers are cleared each iteration, the fixed-length client-random field is length-validated, and the line buffer is zeroed on every return path.SSLKEYLOGFILEenvironment variable — RFC 9850 notes the informalSSLKEYLOGFILEenv var used by curl, NSS, Wireshark and various other applications to share one key-log path. Bothtls13ShowSecretsandtlsShowSecretscan now preferXGETENV("SSLKEYLOGFILE")(when set and non-empty), falling back to the compile-timeWOLFSSL_SSLKEYLOGFILE_OUTPUT. This is opt-in behind a newWOLFSSL_SSLKEYLOGFILE_USE_ENVmacro (default off), so a build that exports the variable for other applications is unaffected and noXGETENVreference is emitted unless requested. It remains under the existingSHOW_SECRETS/WOLFSSL_SSLKEYLOGFILEguards, so production builds cannot log secrets regardless of the environment, per the RFC security considerations.RFC 9973 (certificates with an external PSK)
cert_with_extern_pskimplementation was written against the 8773bis draft that became RFC 9973 and is already compliant. Only textual references (comments, configure help, extension-codepoint comment, a test comment, Doxygen) are updated. TheWOLFSSL_CERT_WITH_EXTERN_PSKmacro, the--enable-cert-with-extern-pskoption, the public API, and extension codepoint 33 (0x0021) are all unchanged.Testing
test_tls13_KeyUpdate_sender_limit(grouptls13) exercises both sides of the 2^48-1 boundary: seeded at 2^48-2,wolfSSL_update_keyssucceeds and advances the count to exactly 2^48-1; at the ceiling the next call is refused withBAD_STATE_Eand the count is unchanged. This pins the increment side-effect and the>=vs>boundary../configure --enable-tls13 --enable-debug && makeand ran./tests/unit.test --group tls13— the new test runs (not skipped) and passes; group result0 failed / 50 skipped / 30 passed, exit 0.INCOMPLETE_KEYLOG_LINE) line to the TLS 1.3 key-log test data so the sniffer keylog test exercises the new skip paths.make checkrecommended in CI across the standard build matrix.