diff --git a/CHANGELOG.md b/CHANGELOG.md index 086b8468f..591b5911b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Add new entries under `Unreleased`; published release sections are history. ## Unreleased +- HKDF-Extract rejects invalid buffer arguments (N-160). - ECDHE key exchange and ECDSA signing fail when key generation fails (N-156). - RSA keys over 4096 bits are refused (N-146). - TLS rejects certificates with empty version, public-key or signature fields, diff --git a/tests/x509/CMakeLists.txt b/tests/x509/CMakeLists.txt index c1d1e1a7d..15e7608bc 100644 --- a/tests/x509/CMakeLists.txt +++ b/tests/x509/CMakeLists.txt @@ -193,3 +193,17 @@ target_compile_options(test_ec_keygen_status PRIVATE -Wall -Wextra -Wno-pointer-to-int-cast -UAMINETXDUO_IPV6) add_test(NAME tls_ec_keygen_status COMMAND test_ec_keygen_status) + +# N-160: HKDF EXTRACT refuses a NULL output, and a NULL IKM with a length, +# before it stores anything in its context; RFC 5869 and empty-IKM controls +# through the vendor HKDF, HMAC and SHA-256. +add_executable(test_hkdf_extract_ptr + test_hkdf_extract_ptr.c + "${CMAKE_SOURCE_DIR}/tests/fuzz/fuzz_nxstub.c" + "${CMAKE_SOURCE_DIR}/tests/fuzz/fuzz_txstub.c") + +target_link_libraries(test_hkdf_extract_ptr PRIVATE test_x509_nx_crypto) + +target_compile_options(test_hkdf_extract_ptr PRIVATE -Wall -Wextra -UAMINETXDUO_IPV6) + +add_test(NAME tls_hkdf_extract_ptr COMMAND test_hkdf_extract_ptr) diff --git a/tests/x509/test_hkdf_extract_ptr.c b/tests/x509/test_hkdf_extract_ptr.c new file mode 100644 index 000000000..3a785615b --- /dev/null +++ b/tests/x509/test_hkdf_extract_ptr.c @@ -0,0 +1,207 @@ +/* + * N-160: NX_CRYPTO_HKDF_EXTRACT copies the PRK to `output` and hands `input` + * to the HMAC as the IKM. A NULL output, or a NULL input with a length, is + * refused with NX_CRYPTO_POINTER_ERROR before the arm stores anything in the + * context: the HKDF control block is compared byte for byte across the call. + * + * Nothing invalid is ever dereferenced here. The refused calls never reach + * a copy or the HMAC, and H_CONTROLS_ONLY builds the valid half alone, which + * is what runs against a tree without the guard. + * + * The controls run the same sequence as _nx_secure_tls_hkdf_extract() + * (nx_secure_tls_1_3_generate_keys.c): init with the IKM, SET_HMAC, SET_HASH, + * EXTRACT. RFC 5869 A.1 and A.3 (an empty salt), an empty IKM both as NULL + * and as a pointer, and an output that is also the salt, as the handshake and + * master secret derivations pass it. The empty-IKM PRK is HMAC-SHA-256 with + * the A.1 salt over no bytes, computed outside this tree (Python hmac). + * + * SPDX-License-Identifier: MIT + */ + +#include +#include + +#include "nx_crypto_hkdf.h" +#include "nx_crypto_hmac.h" + +extern NX_CRYPTO_METHOD crypto_method_hkdf; +extern NX_CRYPTO_METHOD crypto_method_hmac; +extern NX_CRYPTO_METHOD crypto_method_sha256; + +static int failures; +static int checks; + +static void expect(int ok, const char *what) +{ + checks++; + if (!ok) + { + printf("FAIL %s\n", what); + failures++; + } +} + +static union +{ + ULONG align; + UCHAR bytes[sizeof(NX_CRYPTO_HKDF) + sizeof(NX_CRYPTO_HMAC)]; +} metadata; + +/* RFC 5869 A.1 and A.3. */ +static UCHAR a1_ikm[22]; +static UCHAR a1_salt[13]; +static const UCHAR a1_prk[32] = +{ + 0x07, 0x77, 0x09, 0x36, 0x2c, 0x2e, 0x32, 0xdf, 0x0d, 0xdc, 0x3f, 0x0d, 0xc4, 0x7b, 0xba, 0x63, + 0x90, 0xb6, 0xc7, 0x3b, 0xb5, 0x0f, 0x9c, 0x31, 0x22, 0xec, 0x84, 0x4a, 0xd7, 0xc2, 0xb3, 0xe5 +}; +static const UCHAR a3_prk[32] = +{ + 0x19, 0xef, 0x24, 0xa3, 0x2c, 0x71, 0x7b, 0x16, 0x7f, 0x33, 0xa9, 0x1d, 0x6f, 0x64, 0x8b, 0xdf, + 0x96, 0x59, 0x67, 0x76, 0xaf, 0xdb, 0x63, 0x77, 0xac, 0x43, 0x4c, 0x1c, 0x29, 0x3c, 0xcb, 0x04 +}; +/* HMAC-SHA-256(key = A.1 salt, message = empty). */ +static const UCHAR empty_ikm_prk[32] = +{ + 0x90, 0xa3, 0x3d, 0x18, 0x6b, 0x94, 0x0b, 0xac, 0x8a, 0x4e, 0x69, 0xef, 0xce, 0x8b, 0x74, 0xba, + 0x4c, 0x71, 0x86, 0x40, 0x14, 0x0d, 0x54, 0xd8, 0x53, 0xb5, 0xe3, 0x0e, 0xd8, 0xd7, 0x70, 0x6b +}; + +/* init, SET_HMAC, SET_HASH: the context EXTRACT runs in. */ +static UINT setup(UCHAR *ikm, UINT ikm_len) +{ + UINT status; + + memset(&metadata, 0, sizeof(metadata)); + status = crypto_method_hkdf.nx_crypto_init(&crypto_method_hkdf, ikm, ikm_len << 3, NX_CRYPTO_NULL, + metadata.bytes, sizeof(metadata.bytes)); + if (status == NX_CRYPTO_SUCCESS) + status = crypto_method_hkdf.nx_crypto_operation(NX_CRYPTO_HKDF_SET_HMAC, NX_CRYPTO_NULL, + &crypto_method_hmac, NX_CRYPTO_NULL, 0, + NX_CRYPTO_NULL, 0, NX_CRYPTO_NULL, + NX_CRYPTO_NULL, 0, metadata.bytes, + sizeof(metadata.bytes), NX_CRYPTO_NULL, + NX_CRYPTO_NULL); + if (status == NX_CRYPTO_SUCCESS) + status = crypto_method_hkdf.nx_crypto_operation(NX_CRYPTO_HKDF_SET_HASH, NX_CRYPTO_NULL, + &crypto_method_sha256, NX_CRYPTO_NULL, 0, + NX_CRYPTO_NULL, 0, NX_CRYPTO_NULL, + NX_CRYPTO_NULL, 0, metadata.bytes, + sizeof(metadata.bytes), NX_CRYPTO_NULL, + NX_CRYPTO_NULL); + return status; +} + +static UINT extract(UCHAR *salt, UINT salt_len, UCHAR *ikm, UINT ikm_len, UCHAR *out, UINT out_len) +{ + return crypto_method_hkdf.nx_crypto_operation(NX_CRYPTO_HKDF_EXTRACT, NX_CRYPTO_NULL, + &crypto_method_hkdf, salt, salt_len << 3, + ikm, ikm_len, NX_CRYPTO_NULL, out, out_len, + metadata.bytes, sizeof(metadata.bytes), + NX_CRYPTO_NULL, NX_CRYPTO_NULL); +} + +static void control(const char *name, UCHAR *salt, UINT salt_len, UCHAR *ikm, UINT ikm_len, + const UCHAR *want) +{ + UCHAR out[32 + 8]; + UINT status; + char what[160]; + + memset(out, 0x5A, sizeof(out)); + status = setup(ikm, ikm_len); + if (status == NX_CRYPTO_SUCCESS) + status = extract(salt, salt_len, ikm, ikm_len, out, 32); + + snprintf(what, sizeof(what), "%s: status 0x%x", name, status); + expect(status == NX_CRYPTO_SUCCESS, what); + snprintf(what, sizeof(what), "%s: the PRK", name); + expect(memcmp(out, want, 32) == 0, what); + snprintf(what, sizeof(what), "%s: nothing past the PRK", name); + expect(out[32] == 0x5A && out[39] == 0x5A, what); +} + +/* The handshake and master secrets are derived in place: output is salt. */ +static void control_aliased(void) +{ + UCHAR buf[32]; + UINT status; + + memset(buf, 0, sizeof(buf)); + memcpy(buf, a1_salt, sizeof(a1_salt)); + status = setup(a1_ikm, sizeof(a1_ikm)); + if (status == NX_CRYPTO_SUCCESS) + status = extract(buf, sizeof(a1_salt), a1_ikm, sizeof(a1_ikm), buf, sizeof(buf)); + + expect(status == NX_CRYPTO_SUCCESS, "output aliasing the salt: status"); + expect(memcmp(buf, a1_prk, 32) == 0, "output aliasing the salt: the A.1 PRK"); +} + +/* What was refused before the guard, and still is. */ +static void unchanged_contracts(void) +{ + UCHAR out[32]; + + setup(a1_ikm, sizeof(a1_ikm)); + expect(extract(NX_CRYPTO_NULL, 0, a1_ikm, sizeof(a1_ikm), out, sizeof(out)) == NX_CRYPTO_POINTER_ERROR, + "a NULL salt is still NX_CRYPTO_POINTER_ERROR"); + + setup(a1_ikm, sizeof(a1_ikm)); + expect(extract(a1_salt, sizeof(a1_salt), a1_ikm, sizeof(a1_ikm), out, 31) == NX_CRYPTO_SIZE_ERROR, + "an output one byte short is still NX_CRYPTO_SIZE_ERROR"); +} + +#ifndef H_CONTROLS_ONLY +static void refused(const char *name, UCHAR *ikm, UINT ikm_len, UCHAR *out) +{ + static UCHAR before[sizeof(metadata.bytes)]; + UINT status; + char what[160]; + + setup(a1_ikm, sizeof(a1_ikm)); + memcpy(before, metadata.bytes, sizeof(before)); + + status = extract(a1_salt, sizeof(a1_salt), ikm, ikm_len, out, 32); + + snprintf(what, sizeof(what), "N-160 %s: status 0x%x, want NX_CRYPTO_POINTER_ERROR", name, status); + expect(status == NX_CRYPTO_POINTER_ERROR, what); + snprintf(what, sizeof(what), "N-160 %s: the HKDF context is untouched", name); + expect(memcmp(before, metadata.bytes, sizeof(before)) == 0, what); +} +#endif + +int main(void) +{ + UCHAR empty[1]; + + memset(a1_ikm, 0x0b, sizeof(a1_ikm)); + { + UINT i; + for (i = 0; i < sizeof(a1_salt); i++) + a1_salt[i] = (UCHAR)i; + } + + control("RFC 5869 A.1", a1_salt, sizeof(a1_salt), a1_ikm, sizeof(a1_ikm), a1_prk); + control("RFC 5869 A.3 (empty salt)", empty, 0, a1_ikm, sizeof(a1_ikm), a3_prk); + control("empty IKM as NULL", a1_salt, sizeof(a1_salt), NX_CRYPTO_NULL, 0, empty_ikm_prk); + control("empty IKM as a pointer", a1_salt, sizeof(a1_salt), empty, 0, empty_ikm_prk); + control_aliased(); + unchanged_contracts(); + +#ifndef H_CONTROLS_ONLY + { + UCHAR out[32]; + UCHAR canary[32]; + + memset(out, 0x5A, sizeof(out)); + memset(canary, 0x5A, sizeof(canary)); + refused("NULL output", a1_ikm, sizeof(a1_ikm), NX_CRYPTO_NULL); + refused("NULL IKM with a length", NX_CRYPTO_NULL, sizeof(a1_ikm), out); + expect(memcmp(out, canary, sizeof(out)) == 0, "N-160 NULL IKM with a length: no PRK written"); + } +#endif + + printf("hkdf_extract_ptr: %d checks, %d failures\n", checks, failures); + + return failures == 0 ? 0 : 1; +} diff --git a/third_party/netxduo b/third_party/netxduo index 12d4f5f20..9663fe4bd 160000 --- a/third_party/netxduo +++ b/third_party/netxduo @@ -1 +1 @@ -Subproject commit 12d4f5f2007b7cad1bdcbca7934e98e287e2150a +Subproject commit 9663fe4bd0baeae72183c0968efe927f5bdaa647 diff --git a/tools/ci.sh b/tools/ci.sh index 2cd8af01c..9aeced474 100755 --- a/tools/ci.sh +++ b/tools/ci.sh @@ -464,7 +464,10 @@ host_test_targets() { # builddir # 549 with tls_ec_keygen_status (N-156): ECDH setup and ECDSA sign # return a failed key-pair generation's status and call nothing after # it (all hosts). -HOST_TESTS_EXPECTED=549 +# 550 with tls_hkdf_extract_ptr (N-160): HKDF EXTRACT refuses a NULL +# output or a NULL IKM with a length before touching its context +# (all hosts). +HOST_TESTS_EXPECTED=550 case "$(uname -m)" in x86_64|amd64) ;; # test_inet, test_route, test_expunge, test_expunge_cork, test_select,