diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c2d51b3e..04f9e0c84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Add new entries under `Unreleased`; published release sections are history. ## Unreleased +- TLS refuses RSA keys with an even modulus (N-171). - TLS no longer clears a record's packets after TCP has taken them (GHSA-8w5x-ff58-2fr2). - HKDF-Extract rejects invalid buffer arguments (N-160). - ECDHE key exchange and ECDSA signing fail when key generation fails (N-156). diff --git a/src/tls/ami_tls_crypto.c b/src/tls/ami_tls_crypto.c index 30c5cc993..266f989c0 100644 --- a/src/tls/ami_tls_crypto.c +++ b/src/tls/ami_tls_crypto.c @@ -502,6 +502,15 @@ NX_CRYPTO_HUGE_NUMBER modulus_hn, exponent_hn, input_hn, output_hn, p_hn, q_hn { return(NX_CRYPTO_SIZE_ERROR); } + /* An RSA modulus is odd. The certificate parser takes any bytes, and an + even one -- zero above all -- is refused by crypto68k and handed to the + vendored arithmetic, whose modulus scan walks below its buffer for zero + (N-171). Refused here, before any number is set up or computed, on + every path: public and CRT, accelerated and reference. */ + if ((modulus[modulus_length - 1u] & 1u) == 0u) + { + return(NX_CRYPTO_INVALID_KEY); + } /* Rounded to whole limbs as the carve is; the vendored operation's comment has the arithmetic. */ { diff --git a/tests/fuzz/CMakeLists.txt b/tests/fuzz/CMakeLists.txt index 0a8bd5051..cb0735be9 100644 --- a/tests/fuzz/CMakeLists.txt +++ b/tests/fuzz/CMakeLists.txt @@ -606,6 +606,24 @@ if(CMAKE_SIZEOF_VOID_P EQUAL 4) target_link_libraries(tls_rsa_bounds PRIVATE fuzz_tls_ami fuzz_tls_nx_secure fuzz_tls_nx_crypto fuzz_tls_netx fuzz_tls_ami fuzz_tls_nx_secure fuzz_tls_nx_crypto fuzz_tls_netx) + # N-171: ours refuses an even (above all a zero) modulus before any + # exponentiation. The four exponentiation entry points are wrapped so that + # an even modulus never reaches real arithmetic, with or without the check. + add_executable(tls_rsa_modulus_parity + tls_rsa_modulus_parity.c + fuzz_nxstub.c + fuzz_txstub.c) + target_include_directories(tls_rsa_modulus_parity PRIVATE ${_fuzz_tls_crypto_inc}) + target_compile_definitions(tls_rsa_modulus_parity PRIVATE ${_fuzz_tls_def}) + target_compile_options(tls_rsa_modulus_parity PRIVATE -UAMINETXDUO_IPV6 ${_fuzz_san} -Wall -Wextra) + target_link_options(tls_rsa_modulus_parity PRIVATE ${_fuzz_san} + -Wl,--wrap=c68k_huge_number_mont_power_modulus + -Wl,--wrap=c68k_crt_power_modulus + -Wl,--wrap=_nx_crypto_huge_number_mont_power_modulus + -Wl,--wrap=_nx_crypto_huge_number_crt_power_modulus) + target_link_libraries(tls_rsa_modulus_parity PRIVATE + fuzz_tls_ami fuzz_tls_nx_secure fuzz_tls_nx_crypto fuzz_tls_netx + fuzz_tls_ami fuzz_tls_nx_secure fuzz_tls_nx_crypto fuzz_tls_netx) endif() @@ -730,6 +748,7 @@ if(CMAKE_SIZEOF_VOID_P EQUAL 4) add_test(NAME tls_rsa_key_regression COMMAND tls_rsa_key_regression) add_test(NAME tls_rsa_bounds COMMAND tls_rsa_bounds) + add_test(NAME tls_rsa_modulus_parity COMMAND tls_rsa_modulus_parity) # EIGHT STREAMS, NOT ONE, and the same number of mutations between them. # Four was the first attempt and left this at 8.14 s of the ten-second diff --git a/tests/fuzz/tls_rsa_modulus_parity.c b/tests/fuzz/tls_rsa_modulus_parity.c new file mode 100644 index 000000000..348ae994a --- /dev/null +++ b/tests/fuzz/tls_rsa_modulus_parity.c @@ -0,0 +1,345 @@ +/* + * N-171: ami_crypto_method_rsa refuses an even modulus -- zero above all -- + * before any number is set up or any exponentiation runs. The certificate + * parser accepts any modulus bytes; crypto68k refuses an even one and hands + * it to the vendored arithmetic, whose modulus scan walks below its buffer + * for zero. + * + * The four exponentiation entry points the method can reach are wrapped at + * link time (-Wl,--wrap): accelerated and reference, public and CRT. Each + * wrapper counts its calls and runs the real routine only for an odd + * modulus; for an even one it returns a defined zero instead. So no even + * modulus ever reaches real arithmetic, and the same program runs safely + * against the method without the check -- there the wrappers are reached and + * the calls report success, which is what fails. + * + * Every refused case is checked for the status, for no exponentiation call + * and for an output buffer left exactly as it was. The controls are real + * keys from tests/tls in both arithmetic modes: the leaf's signature + * verified with the CA's public key, and a private-key operation through the + * CRT with the device key's primes that its public key inverts. + * + * 32-BIT ONLY, for tls_rsa_key_regression's reason (tests/fuzz/CMakeLists.txt). + * + * SPDX-License-Identifier: MIT + */ + +#include +#include + +#include "nx_secure_tls.h" +#include "nx_secure_x509.h" +#include "nx_crypto_rsa.h" +#include "ami_tls_crypto.h" +#include "crypto68k.h" +#include "tls.h" + +#include "tls_test_certs.h" + +/* tls.library's timer, which the crypto module reads for its counters. */ +BOOL ami_tls_timer_is_open(VOID) { return 0; } +ULONG ami_tls_eclock(VOID) { return 0; } +ULONG ami_tls_eclock_micros(ULONG ticks) { return ticks; } + +static int checks; +static int failures; +static const char *mode_name = ""; + +#define CHECK(c, what) do { checks++; if (!(c)) { failures++; \ + printf("FAIL %s: %s\n", mode_name, what); } } while (0) + +/* ------------------------------------------------- the wrapped routines -- */ + +static int h_calls; /* exponentiations reached */ + +static int h_even(const NX_CRYPTO_HUGE_NUMBER *m) +{ + return (m -> nx_crypto_huge_number_size == 0) || + ((m -> nx_crypto_huge_number_data[0] & 1u) == 0); +} + +/* A defined zero in place of the result real arithmetic would have made. */ +static void h_zero(NX_CRYPTO_HUGE_NUMBER *result) +{ + result -> nx_crypto_huge_number_data[0] = 0; + result -> nx_crypto_huge_number_size = 1; + result -> nx_crypto_huge_number_is_negative = NX_CRYPTO_FALSE; +} + +VOID __real_c68k_huge_number_mont_power_modulus(NX_CRYPTO_HUGE_NUMBER *x, NX_CRYPTO_HUGE_NUMBER *e, + NX_CRYPTO_HUGE_NUMBER *m, NX_CRYPTO_HUGE_NUMBER *result, + HN_UBASE *scratch, UINT scratch_limbs); +VOID __wrap_c68k_huge_number_mont_power_modulus(NX_CRYPTO_HUGE_NUMBER *x, NX_CRYPTO_HUGE_NUMBER *e, + NX_CRYPTO_HUGE_NUMBER *m, NX_CRYPTO_HUGE_NUMBER *result, + HN_UBASE *scratch, UINT scratch_limbs) +{ + h_calls++; + if (h_even(m)) + h_zero(result); + else + __real_c68k_huge_number_mont_power_modulus(x, e, m, result, scratch, scratch_limbs); +} + +VOID __real_c68k_crt_power_modulus(NX_CRYPTO_HUGE_NUMBER *x, NX_CRYPTO_HUGE_NUMBER *e, + NX_CRYPTO_HUGE_NUMBER *p, NX_CRYPTO_HUGE_NUMBER *q, + NX_CRYPTO_HUGE_NUMBER *m, NX_CRYPTO_HUGE_NUMBER *result, + HN_UBASE *scratch, HN_UBASE *powm_scratch, UINT powm_scratch_limbs); +VOID __wrap_c68k_crt_power_modulus(NX_CRYPTO_HUGE_NUMBER *x, NX_CRYPTO_HUGE_NUMBER *e, + NX_CRYPTO_HUGE_NUMBER *p, NX_CRYPTO_HUGE_NUMBER *q, + NX_CRYPTO_HUGE_NUMBER *m, NX_CRYPTO_HUGE_NUMBER *result, + HN_UBASE *scratch, HN_UBASE *powm_scratch, UINT powm_scratch_limbs) +{ + h_calls++; + if (h_even(m)) + h_zero(result); + else + __real_c68k_crt_power_modulus(x, e, p, q, m, result, scratch, powm_scratch, powm_scratch_limbs); +} + +VOID __real__nx_crypto_huge_number_mont_power_modulus(NX_CRYPTO_HUGE_NUMBER *x, NX_CRYPTO_HUGE_NUMBER *e, + NX_CRYPTO_HUGE_NUMBER *m, + NX_CRYPTO_HUGE_NUMBER *result, HN_UBASE *scratch); +VOID __wrap__nx_crypto_huge_number_mont_power_modulus(NX_CRYPTO_HUGE_NUMBER *x, NX_CRYPTO_HUGE_NUMBER *e, + NX_CRYPTO_HUGE_NUMBER *m, + NX_CRYPTO_HUGE_NUMBER *result, HN_UBASE *scratch) +{ + h_calls++; + if (h_even(m)) + h_zero(result); + else + __real__nx_crypto_huge_number_mont_power_modulus(x, e, m, result, scratch); +} + +VOID __real__nx_crypto_huge_number_crt_power_modulus(NX_CRYPTO_HUGE_NUMBER *x, NX_CRYPTO_HUGE_NUMBER *e, + NX_CRYPTO_HUGE_NUMBER *p, NX_CRYPTO_HUGE_NUMBER *q, + NX_CRYPTO_HUGE_NUMBER *m, + NX_CRYPTO_HUGE_NUMBER *result, HN_UBASE *scratch); +VOID __wrap__nx_crypto_huge_number_crt_power_modulus(NX_CRYPTO_HUGE_NUMBER *x, NX_CRYPTO_HUGE_NUMBER *e, + NX_CRYPTO_HUGE_NUMBER *p, NX_CRYPTO_HUGE_NUMBER *q, + NX_CRYPTO_HUGE_NUMBER *m, + NX_CRYPTO_HUGE_NUMBER *result, HN_UBASE *scratch) +{ + h_calls++; + if (h_even(m)) + h_zero(result); + else + __real__nx_crypto_huge_number_crt_power_modulus(x, e, p, q, m, result, scratch); +} + +/* ------------------------------------------------------------ fixture -- */ + +static ULONG metadata_words[32768 / sizeof(ULONG)]; +#define METADATA ((UCHAR *)metadata_words) +#define METADATA_SIZE ((ULONG)sizeof(metadata_words)) + +static const NX_CRYPTO_METHOD *rsa; + +static const NX_CRYPTO_METHOD *ami_rsa(void) +{ + const NX_SECURE_TLS_CIPHERSUITE_INFO *table = + ami_crypto_tls_ciphers_ecc.nx_secure_tls_ciphersuite_lookup_table; + USHORT n = ami_crypto_tls_ciphers_ecc.nx_secure_tls_ciphersuite_lookup_table_size; + USHORT i; + + for (i = 0; i < n; i++) + if (table[i].nx_secure_tls_ciphersuite == TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) + return table[i].nx_secure_tls_public_auth; + return NX_NULL; +} + +static NX_SECURE_X509_CERT ca, leaf, device; +static UCHAR ca_der[2048], leaf_der[2048], device_der[2048], device_key[4096]; + +static int load_certs(void) +{ + memcpy(ca_der, test_ca_cert_der, test_ca_cert_der_len); + memcpy(leaf_der, test_device_cert_der, test_device_cert_der_len); + memcpy(device_der, test_device_cert_der, test_device_cert_der_len); + memcpy(device_key, test_device_cert_key_der, test_device_cert_key_der_len); + return _nx_secure_x509_certificate_initialize(&ca, ca_der, (USHORT)test_ca_cert_der_len, + NX_NULL, 0, NX_NULL, 0, + NX_SECURE_X509_KEY_TYPE_NONE) == NX_SUCCESS && + _nx_secure_x509_certificate_initialize(&leaf, leaf_der, (USHORT)test_device_cert_der_len, + NX_NULL, 0, NX_NULL, 0, + NX_SECURE_X509_KEY_TYPE_NONE) == NX_SUCCESS && + _nx_secure_x509_certificate_initialize(&device, device_der, (USHORT)test_device_cert_der_len, + NX_NULL, 0, device_key, + (USHORT)test_device_cert_key_der_len, + NX_SECURE_X509_KEY_TYPE_RSA_PKCS1_DER) == NX_SUCCESS; +} + +/* One operation: init with the modulus, the primes if given, then the + exponentiation (NX_CRYPTO_DECRYPT is the raw x^e mod m either way). */ +static UINT rsa_op(const UCHAR *modulus, UINT modulus_length, + const UCHAR *exponent, UINT exponent_length, + const UCHAR *p, UINT p_length, const UCHAR *q, UINT q_length, + const UCHAR *in, UINT in_length, UCHAR *out, ULONG out_length) +{ + VOID *handler = NX_NULL; + UINT status; + + status = rsa -> nx_crypto_init((NX_CRYPTO_METHOD *)rsa, (UCHAR *)modulus, + (NX_CRYPTO_KEY_SIZE)(modulus_length << 3), + &handler, METADATA, METADATA_SIZE); + if ((status == NX_CRYPTO_SUCCESS) && (p != NX_NULL)) + status = rsa -> nx_crypto_operation(NX_CRYPTO_SET_PRIME_P, handler, (NX_CRYPTO_METHOD *)rsa, + NX_NULL, 0, (UCHAR *)p, p_length, NX_NULL, NX_NULL, 0, + METADATA, METADATA_SIZE, NX_NULL, NX_NULL); + if ((status == NX_CRYPTO_SUCCESS) && (q != NX_NULL)) + status = rsa -> nx_crypto_operation(NX_CRYPTO_SET_PRIME_Q, handler, (NX_CRYPTO_METHOD *)rsa, + NX_NULL, 0, (UCHAR *)q, q_length, NX_NULL, NX_NULL, 0, + METADATA, METADATA_SIZE, NX_NULL, NX_NULL); + if (status == NX_CRYPTO_SUCCESS) + status = rsa -> nx_crypto_operation(NX_CRYPTO_DECRYPT, handler, (NX_CRYPTO_METHOD *)rsa, + (UCHAR *)exponent, (NX_CRYPTO_KEY_SIZE)(exponent_length << 3), + (UCHAR *)in, in_length, NX_NULL, out, out_length, + METADATA, METADATA_SIZE, NX_NULL, NX_NULL); + if (rsa -> nx_crypto_cleanup) + (void)rsa -> nx_crypto_cleanup(METADATA); + return status; +} + +#define H_CANARY 0xA7 +#define H_MAX 512 + +static int all_canary(const UCHAR *b, size_t n) +{ + size_t i; + + for (i = 0; i < n; i++) + if (b[i] != H_CANARY) + return 0; + return 1; +} + +/* --------------------------------------------------------------- cases -- */ + +static void refused(const char *name, const UCHAR *modulus, UINT modulus_length, int crt) +{ + const NX_SECURE_RSA_PRIVATE_KEY *priv = &device.nx_secure_x509_private_key.rsa_private_key; + const NX_SECURE_RSA_PUBLIC_KEY *pub = &ca.nx_secure_x509_public_key.rsa_public_key; + static UCHAR out[H_MAX + 16]; + static UCHAR in[H_MAX]; + UINT status; + char what[160]; + + memset(out, H_CANARY, sizeof(out)); + memset(in, 0x11, sizeof(in)); + in[0] = 0x00; + h_calls = 0; + + if (crt) + status = rsa_op(modulus, modulus_length, + priv -> nx_secure_rsa_private_exponent, priv -> nx_secure_rsa_private_exponent_length, + priv -> nx_secure_rsa_private_prime_p, priv -> nx_secure_rsa_private_prime_p_length, + priv -> nx_secure_rsa_private_prime_q, priv -> nx_secure_rsa_private_prime_q_length, + in, modulus_length, out, modulus_length); + else + status = rsa_op(modulus, modulus_length, + pub -> nx_secure_rsa_public_exponent, pub -> nx_secure_rsa_public_exponent_length, + NX_NULL, 0, NX_NULL, 0, in, modulus_length, out, modulus_length); + + snprintf(what, sizeof(what), "N-171 %s: status 0x%x, want NX_CRYPTO_INVALID_KEY", name, status); + CHECK(status == NX_CRYPTO_INVALID_KEY, what); + snprintf(what, sizeof(what), "N-171 %s: %d exponentiations reached, want 0", name, h_calls); + CHECK(h_calls == 0, what); + snprintf(what, sizeof(what), "N-171 %s: the output is untouched", name); + CHECK(all_canary(out, sizeof(out)), what); +} + +/* The leaf's signature, decrypted with the CA's public key: PKCS#1 type 1. */ +static void control_public(void) +{ + const NX_SECURE_RSA_PUBLIC_KEY *pub = &ca.nx_secure_x509_public_key.rsa_public_key; + static UCHAR out[H_MAX]; + UINT status; + + h_calls = 0; + status = rsa_op(pub -> nx_secure_rsa_public_modulus, pub -> nx_secure_rsa_public_modulus_length, + pub -> nx_secure_rsa_public_exponent, pub -> nx_secure_rsa_public_exponent_length, + NX_NULL, 0, NX_NULL, 0, + leaf.nx_secure_x509_signature_data, leaf.nx_secure_x509_signature_data_length, + out, pub -> nx_secure_rsa_public_modulus_length); + CHECK(status == NX_CRYPTO_SUCCESS && out[0] == 0x00 && out[1] == 0x01 && out[2] == 0xFF, + "control: a real signature verifies with the CA's odd modulus"); + CHECK(h_calls == 1, "control: through one exponentiation"); +} + +/* Through the CRT with the device key's primes, then back with its public key. */ +static void control_crt(void) +{ + const NX_SECURE_RSA_PRIVATE_KEY *priv = &device.nx_secure_x509_private_key.rsa_private_key; + const NX_SECURE_RSA_PUBLIC_KEY *pub = &device.nx_secure_x509_public_key.rsa_public_key; + const UINT n = pub -> nx_secure_rsa_public_modulus_length; + static UCHAR in[H_MAX], sig[H_MAX], back[H_MAX]; + UINT status; + + memset(in, 0x11, sizeof(in)); + in[0] = 0x00; + in[1] = 0x01; + + h_calls = 0; + status = rsa_op(pub -> nx_secure_rsa_public_modulus, n, + priv -> nx_secure_rsa_private_exponent, priv -> nx_secure_rsa_private_exponent_length, + priv -> nx_secure_rsa_private_prime_p, priv -> nx_secure_rsa_private_prime_p_length, + priv -> nx_secure_rsa_private_prime_q, priv -> nx_secure_rsa_private_prime_q_length, + in, n, sig, n); + /* The accelerated CRT's two half-size powers are wrapped calls too. */ + CHECK(status == NX_CRYPTO_SUCCESS && h_calls >= 1, + "control: the device key's private operation runs through the CRT"); + + status = rsa_op(pub -> nx_secure_rsa_public_modulus, n, + pub -> nx_secure_rsa_public_exponent, pub -> nx_secure_rsa_public_exponent_length, + NX_NULL, 0, NX_NULL, 0, sig, n, back, n); + CHECK(status == NX_CRYPTO_SUCCESS && memcmp(back, in, n) == 0, + "control: and its public key inverts it"); +} + +static void run_mode(void) +{ + const NX_SECURE_RSA_PUBLIC_KEY *ca_pub = &ca.nx_secure_x509_public_key.rsa_public_key; + const NX_SECURE_RSA_PUBLIC_KEY *dev_pub = &device.nx_secure_x509_public_key.rsa_public_key; + static UCHAR zero[256]; + static UCHAR even[H_MAX]; + const UINT n = dev_pub -> nx_secure_rsa_public_modulus_length; + + control_public(); + control_crt(); + + /* What the certificate parser lets through: 0x00, then 128 zero bytes. */ + memset(zero, 0, sizeof(zero)); + refused("zero modulus, public", zero, 128, 0); + refused("zero modulus, CRT", zero, 128, 1); + + /* A real modulus with its low bit cleared: even, not zero. */ + memcpy(even, ca_pub -> nx_secure_rsa_public_modulus, ca_pub -> nx_secure_rsa_public_modulus_length); + even[ca_pub -> nx_secure_rsa_public_modulus_length - 1] &= (UCHAR)~1u; + refused("even CA modulus, public", even, ca_pub -> nx_secure_rsa_public_modulus_length, 0); + + memcpy(even, dev_pub -> nx_secure_rsa_public_modulus, n); + even[n - 1] &= (UCHAR)~1u; + refused("even device modulus, CRT", even, n, 1); +} + +int main(void) +{ + rsa = ami_rsa(); + if (!load_certs() || rsa == NX_NULL) + { + printf("FAIL fixture: certificates or the ami RSA method\n"); + return 1; + } + + ami_tls_crypto_set_crt(1); + + mode_name = "accelerated"; + ami_tls_crypto_set_arithmetic(AMI_TLS_ARITH_C68K); + run_mode(); + + mode_name = "reference"; + ami_tls_crypto_set_arithmetic(AMI_TLS_ARITH_REFERENCE); + run_mode(); + + printf("tls_rsa_modulus_parity: %d checks, %d failures\n", checks, failures); + return failures == 0 ? 0 : 1; +} diff --git a/tools/ci.sh b/tools/ci.sh index fbcd61abc..6e987ff61 100755 --- a/tools/ci.sh +++ b/tools/ci.sh @@ -1290,8 +1290,8 @@ ${rlwhy:+ -- }${rlwhy:-, see the log above}" ;; # the ones whose pointer round trips through 32-bit slots are load-bearing. HOST32_TEST_TARGETS=(fuzz_mdns fuzz_tls_crypto tls_rsa_key_regression test_tls_x509 test_tcp_handler test_transfer test_cork test_cork_fast - test_oob_filter tls_rsa_bounds) -HOST32_TEST_REGEX='(fuzz_mdns|fuzz_tls_crypto)_(seeds|sweep(_[0-9]+)?)$|^tls_rsa_key_regression$|tls_x509_checks$|^tcp_handler_packets$|^transfer_scatter_gather$|^cork_small_writes(_fastpath)?$|^oob_filter_payload_guard$|^tls_rsa_bounds$' + test_oob_filter tls_rsa_bounds tls_rsa_modulus_parity) +HOST32_TEST_REGEX='(fuzz_mdns|fuzz_tls_crypto)_(seeds|sweep(_[0-9]+)?)$|^tls_rsa_key_regression$|tls_x509_checks$|^tcp_handler_packets$|^transfer_scatter_gather$|^cork_small_writes(_fastpath)?$|^oob_filter_payload_guard$|^tls_rsa_bounds$|^tls_rsa_modulus_parity$' # 7 until fuzz_tls_crypto_sweep was split into four streams to get under the # ten-second budget; 10 then, 13 once fuzz_mdns_sweep followed it at 9.31 s of # that budget, and 21 when four streams each turned out to still be 8.14 s on @@ -1304,7 +1304,9 @@ HOST32_TEST_REGEX='(fuzz_mdns|fuzz_tls_crypto)_(seeds|sweep(_[0-9]+)?)$|^tls_rsa # 25 with oob_filter_payload_guard (N-074), oob.c as a 32-bit target. # 26 with tls_rsa_bounds (N-146), both RSA methods' modulus, output and # scratch bounds; 32-bit for tls_rsa_key_regression's reason. -HOST32_TESTS_EXPECTED=26 +# 27 with tls_rsa_modulus_parity (N-171), ours refusing an even modulus +# before any exponentiation; the same reason. +HOST32_TESTS_EXPECTED=27 stage_host32() { hr "host tests (32-bit: mDNS, TLS crypto, X.509, TCP:, transfer)"