@github-actions github-actions Bot left a comment

@github-actions github-actions Bot left a comment

Wire up the TLS-v1.3 (RFC 8446) KDF as an ACVP-testable algorithm in
the FIPS modulewrapper. The acvptool subprocess handler (tls13.go)
already existed but had no corresponding modulewrapper implementation.
Add HKDFExtract and HKDFExpandLabel commands to the modulewrapper,
register the TLS-v1.3 algorithm in the ACVP config (SHA2-256/384,
DHE/PSK/PSK-DHE modes), and include NIST-published test vectors from
the ACVP-Server repository.
All 250 NIST test cases pass.

Introduces CRYPTO_tls13_hkdf_expand_label in crypto/fipsmodule/tls/kdf.c,
mirroring the CRYPTO_tls1_prf pattern already in that file. The function
builds the HkdfLabel structure (RFC 8446 §7.1) via CBB and calls
HKDF_expand, bracketed by FIPS_service_indicator_lock_state /
unlock_state and a post-success TLS13_KDF_verify_service_indicator call
that approves SHA2-256 and SHA2-384 (matching what the ACVP config
registers).
Consumers now delegate:
 * ssl/tls13_enc.cc's hkdf_expand_label collapses to a thin wrapper.
 * The ACVP modulewrapper's HKDFExpandLabel delegates to the same
 function, so ACVP exercises the actual FIPS-module code path
 instead of a parallel label-construction reimplementation.
A TLS13-KDF KAT (HKDF_extract -> CRYPTO_tls13_hkdf_expand_label, using
the RFC 8448 early-secret / 'c e traffic' derivation) is added to the
FIPS self-test, along with a matching break-kat.go entry. A new
parameterised TLS13KDF_ServiceIndicatorTest covers the approval policy
across SHA-1 / SHA-224 / SHA-256 / SHA-384 / SHA-512.

The RFC 8446 Section 7.1 "opaque label<7..255>" lower bound on the HkdfLabel.label field does not match real caller behavior: SSL_export_keying_material permits a zero-length caller label, which the ssl/test/runner exerciser relies on for TLS 1.3 exporter interop tests. Neither the pre-refactor ssl/tls13_enc.cc hkdf_expand_label nor BoringSSL's CRYPTO_tls13_hkdf_expand_label enforce the lower bound, so rejecting label_len == 0 here regressed every TLS-TLS13-* and QUIC-TLS13-* runner test that uses exportKeyingMaterial.
The upper bound on out_len is already enforced implicitly by CBB_add_u16, so the explicit check is redundant. Drop the whole block and document what CBB enforces for us.

tests/ci/run_fips_callback_tests.sh::run_all_break_tests enumerates every KAT known to util/fipstools/break-kat.go, tampers the crypto_test binary for each, and runs FIPSCallback.PowerOnSelfTests expecting the AWS_LC_fips_failure_callback to be invoked with the exact failure-message sequence listed in crypto/fips_callback_test.cc::kat_failure_messages.
This PR added TLS13-KDF to break-kat.go and to boringssl_self_test_fast but forgot to teach the callback test about it, so when the broken binary runs with FIPS_CALLBACK_TEST_EXPECTED_FAILURE=TLS13-KDF the lookup misses and the test body hits FAIL("Failed to find expected message for ... TLS13-KDF"). That's what's blocking the fips-callback-amazonlinux:2023-aarch64 job.
The failure-message sequence mirrors TLS-KDF (same enclosing boringssl_self_test_fast path, so the outer "Power on self test failed" string is produced from bcm.c::BORINGSSL_bcm_power_on_self_test). The Expected/Calculated hex pair is derived by running the HKDF-Extract(SHA256, IKM=0^32, salt=kTLS13Salt) then CRYPTO_tls13_hkdf_expand_label(label="c e traffic", context=kTLS13ClientHelloHash) chain that self_check.c uses; break-kat zeroes the 32-byte kTLS13Secret in the binary image, producing the given Calculated bytes.

Address review feedback:
- Rename the new modulewrapper ACVP handlers HKDFExtract/HKDFExpandLabel
 to TLS13_HKDFExtract/TLS13_HKDFExpandLabel. The ACVP command strings
 on the wire are unchanged; only the C++ identifiers move. This matches
 the existing TLSKDF / "TLSKDF/1.2/..." convention in the same file
 and disambiguates the new helpers from the generic HKDF / HKDF_expand
 helpers (KDA/HKDF and KDF/Feedback) already defined in this TU.
- Add in-code attribution for the parts of this change that are ported
 from BoringSSL: the pair of ACVP handlers in modulewrapper.cc and the
 HkdfLabel/CBB construction in CRYPTO_tls13_hkdf_expand_label. The
 comment on the latter also calls out that the FIPS service-indicator
 lock/unlock and TLS13_KDF_verify_service_indicator call are
 AWS-LC-specific, to make the port vs. novel split explicit for future
 reviewers and for resyncs with BoringSSL upstream.

Address review comments from Nevine on PR aws#3247:
* include/openssl/kdf.h: correct the documented contract for
 CRYPTO_tls13_hkdf_expand_label to match the actual implementation
 behavior. The previous text required |label_len| in [1, 249] but
 commit 12e6a75 intentionally stopped enforcing the lower bound to
 support SSL_export_keying_material callers that pass an empty label.
* crypto/fipsmodule/tls/kdf.c: restore the explicit
 |out_len > UINT16_MAX| check that was removed in 12e6a75. CBB_add_u16
 cannot enforce the bound on its own because the size_t-to-uint16_t
 narrowing happens in the implicit conversion at the call site, before
 CBB_add_u16 ever sees the value, silently producing a spec-violating
 HkdfLabel.length on oversized requests. The empty-label allowance from
 12e6a75 stays; only the upper-bound check comes back. Comment
 clarified to explain which bounds CBB does and does not enforce.