From 636e2132bf375669841ba892203d09e2681ad9d3 Mon Sep 17 00:00:00 2001 From: Tinic Uro Date: Sat, 3 Oct 2026 00:38:18 +0000 Subject: [PATCH 1/2] tls: ami RSA method checks its output and scratch; RSA bounds test (N-146) ami_crypto_method_rsa (src/tls/ami_tls_crypto.c) had the vendored method's shape: the output compared with the exponent's length, the scratch length never given to ami_rsa_exponentiate(), the setup and extract statuses dropped. It now checks the output against the modulus, passes and checks the scratch in the vendored operation's units and formula, and returns those statuses. The modulus ceiling comes with the vendored init it delegates to. tests/fuzz/tls_rsa_bounds (32-bit, beside tls_rsa_key_regression) runs the same contracts on crypto_method_rsa and ours: a 4096-bit modulus accepted and a 4104-bit or zero one refused with the context untouched; a real 2048-bit signature verified into an output exactly the modulus long; one byte short refused with nothing written at or past it (a canary inside the test's own array); an overlong exponent an error; and the vendored operation's scratch one USHORT short refused, exactly enough used. Benign only: no oversized exponentiation is run. Pairs with tinic/netxduo b96bbb26 (local, not pushed), off fork master defce9de; the gitlink is NOT moved. Unfixed (defce9de + main's ami_tls_crypto.c): 13 of 22 checks fail. Fixed: 22/22. HOST32 25 -> 26. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + src/tls/ami_tls_crypto.c | 51 ++++++-- tests/fuzz/CMakeLists.txt | 14 +++ tests/fuzz/tls_rsa_bounds.c | 243 ++++++++++++++++++++++++++++++++++++ tools/ci.sh | 8 +- 5 files changed, 304 insertions(+), 13 deletions(-) create mode 100644 tests/fuzz/tls_rsa_bounds.c diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bea4debd..1a1476a7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Add new entries under `Unreleased`; published release sections are history. ## Unreleased +- RSA keys over 4096 bits are refused (N-146). - TLS rejects HelloRetryRequest cookies and ClientHello messages that do not fit their buffers, and reports failed key-share construction. - TLS 1.3 handshake builder failures preserve the original error and release diff --git a/src/tls/ami_tls_crypto.c b/src/tls/ami_tls_crypto.c index 400a9e6a5..b676253aa 100644 --- a/src/tls/ami_tls_crypto.c +++ b/src/tls/ami_tls_crypto.c @@ -470,14 +470,32 @@ static UINT ami_rsa_exponentiate(const UCHAR *exponent, UINT exponent_length, const UCHAR *q, UINT q_length, const UCHAR *input, UINT input_length, UCHAR *output, - USHORT *scratch_buf_ptr, + USHORT *scratch_buf_ptr, UINT scratch_buf_length, HN_UBASE *powm_scratch, UINT powm_scratch_limbs) { HN_UBASE *scratch; UINT mod_length; +UINT status; +ULONG scratch_needed; NX_CRYPTO_HUGE_NUMBER modulus_hn, exponent_hn, input_hn, output_hn, p_hn, q_hn; + /* As the vendored operation now does (N-146): the carve's need against + the scratch, in bytes, before any of it is touched; scratch_buf_length + counts USHORTs. The numbers' own bounds are setup()'s, whose status + is returned. */ + if (modulus_length == 0) + { + return(NX_CRYPTO_SIZE_ERROR); + } + scratch_needed = ((p != NX_CRYPTO_NULL) && (q != NX_CRYPTO_NULL)) + ? ((10UL * modulus_length) + 24UL) + : ((7UL * modulus_length) + 8UL); + if (((ULONG)scratch_buf_length * sizeof(USHORT)) < scratch_needed) + { + return(NX_CRYPTO_SIZE_ERROR); + } + scratch = (HN_UBASE *)scratch_buf_ptr; @@ -486,17 +504,28 @@ NX_CRYPTO_HUGE_NUMBER modulus_hn, exponent_hn, input_hn, output_hn, p_hn, q_hn NX_CRYPTO_HUGE_NUMBER_INITIALIZE(&exponent_hn, scratch, modulus_length); NX_CRYPTO_HUGE_NUMBER_INITIALIZE(&output_hn, scratch, modulus_length << 1); - _nx_crypto_huge_number_setup(&exponent_hn, exponent, exponent_length); - _nx_crypto_huge_number_setup(&input_hn, input, input_length); - _nx_crypto_huge_number_setup(&modulus_hn, modulus, modulus_length); + status = _nx_crypto_huge_number_setup(&exponent_hn, exponent, exponent_length); + if (status == NX_CRYPTO_SUCCESS) + status = _nx_crypto_huge_number_setup(&input_hn, input, input_length); + if (status == NX_CRYPTO_SUCCESS) + status = _nx_crypto_huge_number_setup(&modulus_hn, modulus, modulus_length); + if (status != NX_CRYPTO_SUCCESS) + { + return(status); + } if ((p != NX_CRYPTO_NULL) && (q != NX_CRYPTO_NULL)) { NX_CRYPTO_HUGE_NUMBER_INITIALIZE(&p_hn, scratch, modulus_length >> 1); NX_CRYPTO_HUGE_NUMBER_INITIALIZE(&q_hn, scratch, modulus_length >> 1); - _nx_crypto_huge_number_setup(&p_hn, p, p_length); - _nx_crypto_huge_number_setup(&q_hn, q, q_length); + status = _nx_crypto_huge_number_setup(&p_hn, p, p_length); + if (status == NX_CRYPTO_SUCCESS) + status = _nx_crypto_huge_number_setup(&q_hn, q, q_length); + if (status != NX_CRYPTO_SUCCESS) + { + return(status); + } if (ami_arithmetic == AMI_TLS_ARITH_REFERENCE) { @@ -527,9 +556,7 @@ NX_CRYPTO_HUGE_NUMBER modulus_hn, exponent_hn, input_hn, output_hn, p_hn, q_hn } } - _nx_crypto_huge_number_extract(&output_hn, output, modulus_length, &mod_length); - - return(NX_CRYPTO_SUCCESS); + return(_nx_crypto_huge_number_extract(&output_hn, output, modulus_length, &mod_length)); } static UINT ami_crypto_method_rsa_init(struct NX_CRYPTO_METHOD_STRUCT *method, @@ -640,7 +667,10 @@ ULONG elapsed; return(NX_CRYPTO_PTR_ERROR); } - if (output_length_in_byte < (ULONG)(key_size_in_bits >> 3)) + /* The result is the modulus's length; key_size_in_bits is the + exponent's, 24 bits for 65537, so it bounded nothing (N-146). */ + if ((base -> nx_crypto_rsa_modulus_length == 0) || + (output_length_in_byte < (ULONG)(base -> nx_crypto_rsa_modulus_length))) { return(NX_CRYPTO_INVALID_BUFFER_SIZE); } @@ -692,6 +722,7 @@ ULONG elapsed; input, (UINT)input_length_in_byte, output, base -> nx_crypto_rsa_scratch_buffer, + NX_CRYPTO_RSA_SCRATCH_BUFFER_SIZE, ctx -> ami_rsa_powm_scratch, AMI_TLS_POWM_SCRATCH_LIMBS); diff --git a/tests/fuzz/CMakeLists.txt b/tests/fuzz/CMakeLists.txt index 9aaafb636..0a8bd5051 100644 --- a/tests/fuzz/CMakeLists.txt +++ b/tests/fuzz/CMakeLists.txt @@ -593,6 +593,19 @@ if(CMAKE_SIZEOF_VOID_P EQUAL 4) target_link_libraries(tls_rsa_key_regression 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-146: both RSA methods refuse a modulus over NX_CRYPTO_MAX_RSA_MODULUS_SIZE, + # check the output against the modulus, and honour the scratch length. + add_executable(tls_rsa_bounds + tls_rsa_bounds.c + fuzz_nxstub.c + fuzz_txstub.c) + target_include_directories(tls_rsa_bounds PRIVATE ${_fuzz_tls_crypto_inc}) + target_compile_definitions(tls_rsa_bounds PRIVATE ${_fuzz_tls_def}) + target_compile_options(tls_rsa_bounds PRIVATE -UAMINETXDUO_IPV6 ${_fuzz_san} -Wall -Wextra) + target_link_options(tls_rsa_bounds PRIVATE ${_fuzz_san}) + 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) endif() @@ -716,6 +729,7 @@ if(CMAKE_SIZEOF_VOID_P EQUAL 4) COMMAND fuzz_tls_crypto -s) add_test(NAME tls_rsa_key_regression COMMAND tls_rsa_key_regression) + add_test(NAME tls_rsa_bounds COMMAND tls_rsa_bounds) # 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_bounds.c b/tests/fuzz/tls_rsa_bounds.c new file mode 100644 index 000000000..9d9dba9a8 --- /dev/null +++ b/tests/fuzz/tls_rsa_bounds.c @@ -0,0 +1,243 @@ +/* + * N-146: the RSA methods - the vendored crypto_method_rsa and ours, + * ami_crypto_method_rsa (src/tls/ami_tls_crypto.c) - must refuse a modulus + * larger than the scratch and every caller's output buffer are sized for, + * check the output against the MODULUS (the key_size they are handed is the + * exponent's), use the scratch length they are given, in the right units, + * and report a number that does not fit rather than compute on a stale one. + * + * Benign contracts only: no oversized exponentiation is ever run, and an + * output that is too short sits inside a larger array whose tail is a + * canary, so the old code's overrun lands in the test's own memory. A real + * signature is decrypted with the CA's 2048-bit key to show a valid + * operation still works with an output exactly the modulus long. + * + * 32-BIT ONLY, for tls_rsa_key_regression's reason (tests/fuzz/CMakeLists.txt). + * + * SPDX-License-Identifier: MIT + */ + +#include +#include +#include + +#include "nx_secure_tls.h" +#include "nx_secure_x509.h" +#include "nx_crypto_rsa.h" +#include "ami_tls_crypto.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; } + +/* The header's device key is not needed here; it is static, so name it once. */ +static const void *const unused_key[] = { test_device_cert_key_der, &test_device_cert_key_der_len }; + +static int checks; +static int failures; + +#define CHECK(c, what) do { checks++; if (!(c)) { failures++; \ + printf("FAIL %s: %s\n", method_name, what); } } while (0) + +static const char *method_name = ""; + +extern NX_CRYPTO_METHOD crypto_method_rsa; + +/* Metadata big enough for either context (ours embeds NX_CRYPTO_RSA first). */ +static ULONG metadata_words[32768 / sizeof(ULONG)]; +#define METADATA ((UCHAR *)metadata_words) +#define METADATA_SIZE ((ULONG)sizeof(metadata_words)) + +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; +static UCHAR ca_der[2048], leaf_der[2048]; + +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); + 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; +} + +/* ------------------------------------------------------------ contracts -- */ + +static void init_ceiling(const NX_CRYPTO_METHOD *m) +{ + static UCHAR modulus[600]; + VOID *handler = NX_NULL; + UINT status; + ULONG i; + int untouched = 1; + + memset(modulus, 0xC3, sizeof(modulus)); + + status = m->nx_crypto_init((NX_CRYPTO_METHOD *)m, modulus, 4096, &handler, + METADATA, METADATA_SIZE); + CHECK(status == NX_CRYPTO_SUCCESS, "a 4096-bit modulus is accepted"); + + memset(METADATA, 0xA5, 64); + status = m->nx_crypto_init((NX_CRYPTO_METHOD *)m, modulus, 4096 + 8, &handler, + METADATA, METADATA_SIZE); + CHECK(status == NX_CRYPTO_UNSUPPORTED_KEY_SIZE, "a 4104-bit modulus is refused"); + for (i = 0; i < 64; i++) + untouched &= (METADATA[i] == 0xA5); + CHECK(untouched, "and the context is not written"); + + status = m->nx_crypto_init((NX_CRYPTO_METHOD *)m, modulus, 0, &handler, + METADATA, METADATA_SIZE); + CHECK(status == NX_CRYPTO_UNSUPPORTED_KEY_SIZE, "a zero-length modulus is refused"); +} + +/* The leaf's signature, decrypted with the CA's public key: PKCS#1 type 1. */ +static UINT verify_into(const NX_CRYPTO_METHOD *m, UCHAR *out, ULONG out_len) +{ + const NX_SECURE_RSA_PUBLIC_KEY *pub = &ca.nx_secure_x509_public_key.rsa_public_key; + VOID *handler = NX_NULL; + UINT status; + + status = m->nx_crypto_init((NX_CRYPTO_METHOD *)m, + (UCHAR *)pub->nx_secure_rsa_public_modulus, + (NX_CRYPTO_KEY_SIZE)(pub->nx_secure_rsa_public_modulus_length << 3), + &handler, METADATA, METADATA_SIZE); + if (status != NX_CRYPTO_SUCCESS) + return status; + + return m->nx_crypto_operation(NX_CRYPTO_DECRYPT, handler, (NX_CRYPTO_METHOD *)m, + (UCHAR *)pub->nx_secure_rsa_public_exponent, + (NX_CRYPTO_KEY_SIZE)(pub->nx_secure_rsa_public_exponent_length << 3), + (UCHAR *)leaf.nx_secure_x509_signature_data, + leaf.nx_secure_x509_signature_data_length, + NX_NULL, out, out_len, METADATA, METADATA_SIZE, NX_NULL, NX_NULL); +} + +static void output_capacity(const NX_CRYPTO_METHOD *m) +{ + const UINT mod = ca.nx_secure_x509_public_key.rsa_public_key.nx_secure_rsa_public_modulus_length; + static UCHAR out[1024]; + UINT status, i; + int canary = 1; + + /* Exactly the modulus: a valid operation, unchanged. */ + memset(out, 0, sizeof(out)); + status = verify_into(m, out, mod); + CHECK(status == NX_CRYPTO_SUCCESS, "an output exactly the modulus long works"); + CHECK(out[0] == 0x00 && out[1] == 0x01 && out[2] == 0xFF, + "and holds the PKCS#1 block of the real signature"); + + /* One byte short: refused, nothing written past it. The array goes on, + so the old code's full-length write lands in the canary, not beyond. */ + memset(out, 0x5C, sizeof(out)); + status = verify_into(m, out, mod - 1); + CHECK(status == NX_CRYPTO_INVALID_BUFFER_SIZE, "an output one byte short is refused"); + for (i = mod - 1; i < mod + 16; i++) + canary &= (out[i] == 0x5C); + CHECK(canary, "and nothing is written at or past its end"); +} + +/* An exponent whose significant bytes exceed the number carved for it: + setup() refuses it, and that refusal must come back. */ +static void exponent_overlong(const NX_CRYPTO_METHOD *m) +{ + const NX_SECURE_RSA_PUBLIC_KEY *pub = &ca.nx_secure_x509_public_key.rsa_public_key; + static UCHAR exponent[512]; + static UCHAR out[1024]; + VOID *handler = NX_NULL; + UINT status; + + memset(exponent, 0x11, sizeof(exponent)); + status = m->nx_crypto_init((NX_CRYPTO_METHOD *)m, + (UCHAR *)pub->nx_secure_rsa_public_modulus, + (NX_CRYPTO_KEY_SIZE)(pub->nx_secure_rsa_public_modulus_length << 3), + &handler, METADATA, METADATA_SIZE); + CHECK(status == NX_CRYPTO_SUCCESS, "init for the overlong exponent"); + + status = m->nx_crypto_operation(NX_CRYPTO_DECRYPT, handler, (NX_CRYPTO_METHOD *)m, + exponent, + (NX_CRYPTO_KEY_SIZE)((pub->nx_secure_rsa_public_modulus_length + 8) << 3), + (UCHAR *)leaf.nx_secure_x509_signature_data, + leaf.nx_secure_x509_signature_data_length, + NX_NULL, out, sizeof(out), METADATA, METADATA_SIZE, + NX_NULL, NX_NULL); + CHECK(status == NX_CRYPTO_SIZE_ERROR, + "an exponent longer than the modulus is an error, not a stale computation"); +} + +/* The vendored operation directly: the scratch length it is given counts. */ +static void scratch_units(void) +{ + const NX_SECURE_RSA_PUBLIC_KEY *pub = &ca.nx_secure_x509_public_key.rsa_public_key; + const UINT mod = pub->nx_secure_rsa_public_modulus_length; + static USHORT scratch[4096]; + static UCHAR out[1024]; + UINT status; + + method_name = "_nx_crypto_rsa_operation"; + + /* Needs 7 * mod + 8 bytes; say there is one USHORT less than that. */ + status = _nx_crypto_rsa_operation(pub->nx_secure_rsa_public_exponent, + pub->nx_secure_rsa_public_exponent_length, + pub->nx_secure_rsa_public_modulus, mod, + NX_NULL, 0, NX_NULL, 0, + leaf.nx_secure_x509_signature_data, + leaf.nx_secure_x509_signature_data_length, + out, scratch, ((7u * mod + 8u) / sizeof(USHORT)) - 1u); + CHECK(status == NX_CRYPTO_SIZE_ERROR, "a scratch one USHORT short is refused"); + + status = _nx_crypto_rsa_operation(pub->nx_secure_rsa_public_exponent, + pub->nx_secure_rsa_public_exponent_length, + pub->nx_secure_rsa_public_modulus, mod, + NX_NULL, 0, NX_NULL, 0, + leaf.nx_secure_x509_signature_data, + leaf.nx_secure_x509_signature_data_length, + out, scratch, (7u * mod + 8u) / sizeof(USHORT)); + CHECK(status == NX_CRYPTO_SUCCESS, "and exactly enough is used"); +} + +int main(void) +{ + const NX_CRYPTO_METHOD *ours = ami_rsa(); + + (void)unused_key; + + if (!load_certs() || ours == NX_NULL) + { + printf("FAIL fixture: certificates or the ami RSA method\n"); + return 1; + } + + method_name = "crypto_method_rsa"; + init_ceiling(&crypto_method_rsa); + output_capacity(&crypto_method_rsa); + exponent_overlong(&crypto_method_rsa); + + method_name = "ami_crypto_method_rsa"; + init_ceiling(ours); + output_capacity(ours); + exponent_overlong(ours); + + scratch_units(); + + printf("tls_rsa_bounds: %d checks, %d failures\n", checks, failures); + return failures == 0 ? 0 : 1; +} diff --git a/tools/ci.sh b/tools/ci.sh index a58fbabf1..011d18fe3 100755 --- a/tools/ci.sh +++ b/tools/ci.sh @@ -1277,8 +1277,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) -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$' + 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$' # 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 @@ -1289,7 +1289,9 @@ HOST32_TEST_REGEX='(fuzz_mdns|fuzz_tls_crypto)_(seeds|sweep(_[0-9]+)?)$|^tls_rsa # around it, whose ABI asserts make it 32-bit, as test_transfer is. # 24 with tls_rsa_key_regression (F-276), ami_tls_crypto.c as fuzz_tls_crypto. # 25 with oob_filter_payload_guard (N-074), oob.c as a 32-bit target. -HOST32_TESTS_EXPECTED=25 +# 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 stage_host32() { hr "host tests (32-bit: mDNS, TLS crypto, X.509, TCP:, transfer)" From b7c2f94fcdaf1e88b7dcedb7901a664786fee2e3 Mon Sep 17 00:00:00 2001 From: Tinic Uro Date: Sat, 3 Oct 2026 00:59:30 +0000 Subject: [PATCH 2/2] tls: ami RSA init caps at its own 4096 bits; rounded scratch; more controls (N-146 amendment) ami_crypto_method_rsa_init() now refuses 0, non-byte and over AMI_TLS_RSA_MAX_MODULUS_BITS (4096) itself, before the vendored init: that ceiling, NX_CRYPTO_MAX_RSA_MODULUS_SIZE, may be raised by a build, but AMI_TLS_POWM_SCRATCH_LIMBS is fixed, and a _Static_assert now ties the two (C68K_POWM_SCRATCH_LIMBS(128, 1) = 2704). ami_rsa_exponentiate() counts the scratch in limb-rounded lengths, as the vendored operation now does. tls_rsa_bounds adds: a 2047-bit (non-byte) modulus refused by both methods; a valid public exponent zero-padded past the modulus length still verifying (the reason there is no explicit exponent <= modulus check); and the vendored operation with a 257-byte modulus refusing 913 USHORTs and accepting 914 (7 * 260 + 8 bytes). Coverage limits: the CRT path is exercised by tls_rsa_key_regression's signing only, not by this test; the extract-status propagation is source-only. Pairs with tinic/netxduo 4bf8d7e1. Unfixed: 18 of 30 checks fail; fixed 30/30; host32 26/26. CHANGELOG: the N-146 entry from 636e2132 covers this. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/tls/ami_tls_crypto.c | 37 ++++++++++++++++++++++--- tests/fuzz/tls_rsa_bounds.c | 54 +++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/tls/ami_tls_crypto.c b/src/tls/ami_tls_crypto.c index b676253aa..30c5cc993 100644 --- a/src/tls/ami_tls_crypto.c +++ b/src/tls/ami_tls_crypto.c @@ -446,6 +446,20 @@ UINT i; */ #define AMI_TLS_POWM_SCRATCH_LIMBS 2704u +/* + * The modulus that scratch is sized for, in bits. The vendored init's ceiling + * is NX_CRYPTO_MAX_RSA_MODULUS_SIZE, which a build may raise; ours may not + * follow it, because the area above is fixed (N-146). + */ +#define AMI_TLS_RSA_MAX_MODULUS_BITS 4096u + +/* m_len counts HN_UBASE limbs, 32 bits each here; window 1 is the least the + powm accepts. The cap and the area are one decision. */ +_Static_assert(AMI_TLS_POWM_SCRATCH_LIMBS >= + C68K_POWM_SCRATCH_LIMBS(AMI_TLS_RSA_MAX_MODULUS_BITS / 32u, 1u), + "AMI_TLS_RSA_MAX_MODULUS_BITS needs more powm scratch than " + "AMI_TLS_POWM_SCRATCH_LIMBS provides"); + typedef struct AMI_CRYPTO_RSA_STRUCT { NX_CRYPTO_RSA ami_rsa_base; @@ -488,9 +502,18 @@ NX_CRYPTO_HUGE_NUMBER modulus_hn, exponent_hn, input_hn, output_hn, p_hn, q_hn { return(NX_CRYPTO_SIZE_ERROR); } - scratch_needed = ((p != NX_CRYPTO_NULL) && (q != NX_CRYPTO_NULL)) - ? ((10UL * modulus_length) + 24UL) - : ((7UL * modulus_length) + 8UL); + /* Rounded to whole limbs as the carve is; the vendored operation's + comment has the arithmetic. */ + { + ULONG rounded = ((ULONG)modulus_length + HN_SIZE_ROUND) & ~(ULONG)HN_SIZE_ROUND; + + if ((p != NX_CRYPTO_NULL) && (q != NX_CRYPTO_NULL)) + scratch_needed = ((modulus_length % (2u * (HN_SIZE_ROUND + 1u))) == 0) + ? ((10UL * modulus_length) + 24UL) + : ((10UL * rounded) + 40UL); + else + scratch_needed = (7UL * rounded) + 8UL; + } if (((ULONG)scratch_buf_length * sizeof(USHORT)) < scratch_needed) { return(NX_CRYPTO_SIZE_ERROR); @@ -577,6 +600,14 @@ static UINT ami_crypto_method_rsa_init(struct NX_CRYPTO_METHOD_STRUCT *method, return(NX_CRYPTO_PTR_ERROR); } + /* The accelerated path's own capacity, whatever the vendored ceiling is + set to; refused before the context is touched (N-146). */ + if ((key_size_in_bits == 0) || ((key_size_in_bits & 7u) != 0) || + (key_size_in_bits > AMI_TLS_RSA_MAX_MODULUS_BITS)) + { + return(NX_CRYPTO_UNSUPPORTED_KEY_SIZE); + } + return(_nx_crypto_method_rsa_init(method, key, key_size_in_bits, handle, crypto_metadata, crypto_metadata_size)); } diff --git a/tests/fuzz/tls_rsa_bounds.c b/tests/fuzz/tls_rsa_bounds.c index 9d9dba9a8..24dd9969b 100644 --- a/tests/fuzz/tls_rsa_bounds.c +++ b/tests/fuzz/tls_rsa_bounds.c @@ -107,6 +107,41 @@ static void init_ceiling(const NX_CRYPTO_METHOD *m) status = m->nx_crypto_init((NX_CRYPTO_METHOD *)m, modulus, 0, &handler, METADATA, METADATA_SIZE); CHECK(status == NX_CRYPTO_UNSUPPORTED_KEY_SIZE, "a zero-length modulus is refused"); + + status = m->nx_crypto_init((NX_CRYPTO_METHOD *)m, modulus, 2047, &handler, + METADATA, METADATA_SIZE); + CHECK(status == NX_CRYPTO_UNSUPPORTED_KEY_SIZE, "a modulus that is not whole bytes is refused"); +} + +/* A valid key whose DER integer carries leading zero bytes, so its length is + past the modulus's: setup() strips them, and the operation must still work + (no explicit exponent <= modulus check may reject it). */ +static void exponent_leading_zeros(const NX_CRYPTO_METHOD *m) +{ + const NX_SECURE_RSA_PUBLIC_KEY *pub = &ca.nx_secure_x509_public_key.rsa_public_key; + const UINT mod = pub->nx_secure_rsa_public_modulus_length; + static UCHAR exponent[600]; + static UCHAR out[1024]; + UINT pad = mod + 2 - pub->nx_secure_rsa_public_exponent_length; + VOID *handler = NX_NULL; + UINT status; + + memset(exponent, 0x00, sizeof(exponent)); + memcpy(&exponent[pad], pub->nx_secure_rsa_public_exponent, pub->nx_secure_rsa_public_exponent_length); + + status = m->nx_crypto_init((NX_CRYPTO_METHOD *)m, + (UCHAR *)pub->nx_secure_rsa_public_modulus, (NX_CRYPTO_KEY_SIZE)(mod << 3), + &handler, METADATA, METADATA_SIZE); + CHECK(status == NX_CRYPTO_SUCCESS, "init for the zero-padded exponent"); + + memset(out, 0, sizeof(out)); + status = m->nx_crypto_operation(NX_CRYPTO_DECRYPT, handler, (NX_CRYPTO_METHOD *)m, + exponent, (NX_CRYPTO_KEY_SIZE)((mod + 2) << 3), + (UCHAR *)leaf.nx_secure_x509_signature_data, + leaf.nx_secure_x509_signature_data_length, + NX_NULL, out, mod, METADATA, METADATA_SIZE, NX_NULL, NX_NULL); + CHECK(status == NX_CRYPTO_SUCCESS && out[0] == 0x00 && out[1] == 0x01 && out[2] == 0xFF, + "an exponent with leading zero bytes past the modulus length still verifies"); } /* The leaf's signature, decrypted with the CA's public key: PKCS#1 type 1. */ @@ -212,6 +247,23 @@ static void scratch_units(void) leaf.nx_secure_x509_signature_data_length, out, scratch, (7u * mod + 8u) / sizeof(USHORT)); CHECK(status == NX_CRYPTO_SUCCESS, "and exactly enough is used"); + + /* A modulus that is not a whole number of limbs: every carve rounds up, + so the requirement is 7 * round4(m) + 8, not 7 * m + 8. 257 bytes + needs 7 * 260 + 8 = 1828 bytes = 914 USHORTs. */ + { + static UCHAR odd_modulus[257]; + static UCHAR one[1] = { 0x03 }; + + memset(odd_modulus, 0xC3, sizeof(odd_modulus)); + status = _nx_crypto_rsa_operation(one, 1, odd_modulus, 257, NX_NULL, 0, NX_NULL, 0, + one, 1, out, scratch, 913u); + CHECK(status == NX_CRYPTO_SIZE_ERROR, + "an odd 257-byte modulus wants its rounded size: 913 USHORTs refused"); + status = _nx_crypto_rsa_operation(one, 1, odd_modulus, 257, NX_NULL, 0, NX_NULL, 0, + one, 1, out, scratch, 914u); + CHECK(status == NX_CRYPTO_SUCCESS, "and 914 accepted"); + } } int main(void) @@ -230,11 +282,13 @@ int main(void) init_ceiling(&crypto_method_rsa); output_capacity(&crypto_method_rsa); exponent_overlong(&crypto_method_rsa); + exponent_leading_zeros(&crypto_method_rsa); method_name = "ami_crypto_method_rsa"; init_ceiling(ours); output_capacity(ours); exponent_overlong(ours); + exponent_leading_zeros(ours); scratch_units();