From e5086672c361cadfe86946d96de274bb5e9e3121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 15 Sep 2026 15:42:13 +0200 Subject: [PATCH 1/6] tls13: base the ECH acceptance check on the handshake header, not the buffer EchCalcAcceptance() hashes the message from its handshake header and reads the confirmation bytes at an offset into it, so its input must start at that header. DoTls13ServerHello() instead passed the buffer it had been handed together with an index measured from that buffer's base. The two coincide only when the ServerHello is the first, unfragmented handshake message of its record; otherwise the trailing hash length it computes is wrong. Derive the message start as input + args->begin - headerSz, the adjustment HashInput() already makes, and convert both the server-random offset and the HelloRetryRequest confBuf offset to be relative to it. That holds on every path reaching here: a record buffer, a reassembled TLS message, and the DTLS 1.3 reassembly path, which writes the handshake header immediately before the assembled message. The adjusted value goes into a local rather than back into args->acceptOffset, so re-entering TLS_ASYNC_FINALIZE gives the same result. EchCalcAcceptance() additionally rejects offsets that do not lie inside the message, so a future caller mistake cannot produce a negative hash length again. The server-side caller, EchWriteAcceptance(), already passes header-relative values and is unaffected. Behaviour change: a fragmented ServerHello or HelloRetryRequest no longer spuriously rejects ECH. A ServerHello coalesced behind a HelloRetryRequest that accepted ECH now fails with INVALID_PARAMETER, which is the RFC 9849 section 6.1.5 outcome. Tests: test_tls13_ech_hrr_coalesced_server_hello and test_tls13_ech_fragmented_server_hello, both failing before the fix and both needing HAVE_ECH, HAVE_SNI and the manual memio dependencies. --- src/tls13.c | 29 ++++- tests/api/test_tls13.c | 249 +++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls13.h | 4 + 3 files changed, 278 insertions(+), 4 deletions(-) diff --git a/src/tls13.c b/src/tls13.c index 0bf97c982a2..9fc251d06f9 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -4123,7 +4123,17 @@ static int EchCalcAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz, headerSz = HANDSHAKE_HEADER_SZ; #endif - if (isHrr) { + /* input starts at the handshake header, so the confirmation bytes must lie + * inside the message that follows it */ + if (acceptOffset < headerSz || helloSz < 0 || + (helloSz + headerSz) < ECH_ACCEPT_CONFIRMATION_SZ || + acceptOffset > (helloSz + headerSz) - + ECH_ACCEPT_CONFIRMATION_SZ) { + WOLFSSL_ERROR_VERBOSE(BUFFER_ERROR); + ret = BUFFER_ERROR; + } + + if (ret == 0 && isHrr) { /* the transcript hash of ClientHelloInner1 */ ret = GetMsgHash(ssl, clientHelloInnerHash); if (ret > 0) { @@ -6176,21 +6186,32 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, return ret; } else { + const byte* hsMsg; + int acceptOffset; + word32 headerSz = HANDSHAKE_HEADER_SZ; + #ifdef WOLFSSL_DTLS13 + if (ssl->options.dtls) + headerSz = DTLS13_HANDSHAKE_HEADER_SZ; + #endif + /* EchCheckAcceptance hashes from the handshake header, which + * always precedes input + args->begin (cf. HashInput) */ + hsMsg = input + args->begin - headerSz; /* account for hrr extension instead of server random */ if (args->extMsgType == hello_retry_request) { - args->acceptOffset = - (word32)(((WOLFSSL_ECH*)args->echX->data)->confBuf - input); + acceptOffset = + (int)(((WOLFSSL_ECH*)args->echX->data)->confBuf - hsMsg); args->acceptLabel = (byte*)echHrrAcceptConfirmationLabel; args->acceptLabelSz = ECH_HRR_ACCEPT_CONFIRMATION_LABEL_SZ; } else { + acceptOffset = (int)(input + args->acceptOffset - hsMsg); args->acceptLabel = (byte*)echAcceptConfirmationLabel; args->acceptLabelSz = ECH_ACCEPT_CONFIRMATION_LABEL_SZ; } /* check acceptance */ if (ret == 0) { ret = EchCheckAcceptance(ssl, args->acceptLabel, - args->acceptLabelSz, input, args->acceptOffset, helloSz, + args->acceptLabelSz, hsMsg, acceptOffset, helloSz, args->extMsgType); } if (ret != 0) diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index bba4b3b99a4..475d4c3745b 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -6926,6 +6926,255 @@ int test_tls13_duplicate_ech_extension(void) return EXPECT_RESULT(); } +#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH) && defined(HAVE_SNI) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_CERTS) && !defined(NO_FILESYSTEM) && !defined(NO_RSA) + +#define ECH_TEST_PUB_NAME "ech-public-name.com" +#define ECH_TEST_PRIV_NAME "ech-private-name.com" + +/* Bring an ECH client and server up to the point where the server has written + * a genuine, ECH-accepting HelloRetryRequest into the client's memio buffer. */ +static int EchHrrSetup(struct test_memio_ctx* test_ctx, WOLFSSL_CTX** ctx_c, + WOLFSSL_CTX** ctx_s, WOLFSSL** ssl_c, WOLFSSL** ssl_s) +{ + EXPECT_DECLS; + byte configs[512]; + word32 configsLen = (word32)sizeof(configs); + + XMEMSET(test_ctx, 0, sizeof(*test_ctx)); + ExpectIntEQ(test_memio_setup(test_ctx, ctx_c, ctx_s, ssl_c, ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0); + if (EXPECT_FAIL()) + return EXPECT_RESULT(); + + wolfSSL_set_verify(*ssl_c, WOLFSSL_VERIFY_NONE, NULL); + ExpectIntEQ(wolfSSL_CTX_GenerateEchConfig(*ctx_s, ECH_TEST_PUB_NAME, 0, 0, + 0), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_GetEchConfigs(*ctx_s, configs, &configsLen), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_SetEchConfigs(*ssl_c, configs, configsLen), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(*ssl_c, WOLFSSL_SNI_HOST_NAME, + ECH_TEST_PRIV_NAME, (word16)XSTRLEN(ECH_TEST_PRIV_NAME)), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(*ssl_s, WOLFSSL_SNI_HOST_NAME, + ECH_TEST_PRIV_NAME, (word16)XSTRLEN(ECH_TEST_PRIV_NAME)), + WOLFSSL_SUCCESS); + + /* No key share in ClientHello1, so the server answers with a + * HelloRetryRequest that carries a valid ECH confirmation. */ + ExpectIntEQ(wolfSSL_NoKeyShares(*ssl_c), WOLFSSL_SUCCESS); + + ExpectIntEQ(wolfSSL_connect(*ssl_c), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(*ssl_c, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(wolfSSL_accept(*ssl_s), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(*ssl_s, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(test_memio_msg_is_hello_retry_request(test_ctx), 1); + + return EXPECT_RESULT(); +} + +/* Build a ServerHello handshake message that echoes the session id and cipher + * suite of the HelloRetryRequest record in hrr, so the client's pre-ECH checks + * pass. Returns the message length, or -1. */ +static int EchBuildServerHello(const byte* hrr, int hrrSz, byte* out, + int outSz) +{ + int sessIdSz; + int csOff; + int bodySz; + int idx = 0; + + /* record header (5) + handshake header (4) + legacy_version (2) + + * random (32) is where the HelloRetryRequest's session id length sits. */ + if (hrrSz < 5 + 4 + 2 + RAN_LEN + 1) + return -1; + sessIdSz = hrr[5 + 4 + 2 + RAN_LEN]; + csOff = 5 + 4 + 2 + RAN_LEN + 1 + sessIdSz; + if (hrrSz < csOff + 2) + return -1; + + /* version + random + session id + cipher suite + compression + + * a 6-byte supported_versions extension block */ + bodySz = 2 + RAN_LEN + 1 + sessIdSz + 2 + 1 + 2 + 6; + if (outSz < bodySz + 4) + return -1; + + out[idx++] = server_hello; + out[idx++] = 0x00; + out[idx++] = (byte)(bodySz >> 8); + out[idx++] = (byte)bodySz; + out[idx++] = SSLv3_MAJOR; + out[idx++] = TLSv1_2_MINOR; + XMEMSET(out + idx, 0x41, RAN_LEN); + idx += RAN_LEN; + out[idx++] = (byte)sessIdSz; + XMEMCPY(out + idx, hrr + 5 + 4 + 2 + RAN_LEN + 1, (size_t)sessIdSz); + idx += sessIdSz; + out[idx++] = hrr[csOff]; + out[idx++] = hrr[csOff + 1]; + out[idx++] = 0x00; + out[idx++] = 0x00; + out[idx++] = 0x06; + out[idx++] = 0x00; + out[idx++] = TLSX_SUPPORTED_VERSIONS; + out[idx++] = 0x00; + out[idx++] = 0x02; + out[idx++] = SSLv3_MAJOR; + out[idx++] = TLSv1_3_MINOR; + + return idx; +} +#endif + +/* A ServerHello coalesced after the HelloRetryRequest in one record: the ECH + * acceptance check used to take its offsets from the record base, so the + * trailing hash length went negative and ran off the end of the record. */ +int test_tls13_ech_hrr_coalesced_server_hello(void) +{ + EXPECT_DECLS; +#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH) && defined(HAVE_SNI) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_CERTS) && !defined(NO_FILESYSTEM) && !defined(NO_RSA) + WOLFSSL_CTX* ctx_c = NULL; + WOLFSSL_CTX* ctx_s = NULL; + WOLFSSL* ssl_c = NULL; + WOLFSSL* ssl_s = NULL; + struct test_memio_ctx test_ctx; + const char* hrr = NULL; + int hrrSz = 0; + byte sh[128]; + int shSz = 0; + byte rec[512]; + int recLen; + + ExpectIntEQ(EchHrrSetup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s), + TEST_SUCCESS); + ExpectIntEQ(test_memio_get_message(&test_ctx, 1, &hrr, &hrrSz, 0), 0); + ExpectIntGT(shSz = EchBuildServerHello((const byte*)hrr, hrrSz, sh, + (int)sizeof(sh)), 0); + ExpectIntLE(hrrSz + shSz, (int)sizeof(rec)); + + if (EXPECT_SUCCESS()) { + XMEMCPY(rec, hrr, (size_t)hrrSz); + XMEMCPY(rec + hrrSz, sh, (size_t)shSz); + /* patch the record length to cover both handshake messages */ + recLen = ((rec[3] << 8) | rec[4]) + shSz; + rec[3] = (byte)(recLen >> 8); + rec[4] = (byte)recLen; + + test_memio_clear_buffer(&test_ctx, 1); + ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, (const char*)rec, + hrrSz + shSz), 0); + } + + /* RFC 9849 6.1.5: the confirmation fails on the appended ServerHello, so + * ECH is rejected after the HelloRetryRequest accepted it. */ + ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)), + WC_NO_ERR_TRACE(INVALID_PARAMETER)); + + wolfSSL_free(ssl_c); + wolfSSL_CTX_free(ctx_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + +/* The same offsets against a ServerHello reassembled from two records, where + * input points at the message body: ECH acceptance used to read 4 bytes past + * ssl->pendingMsg and report a rejection the server never sent. */ +int test_tls13_ech_fragmented_server_hello(void) +{ + EXPECT_DECLS; +#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH) && defined(HAVE_SNI) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_CERTS) && !defined(NO_FILESYSTEM) && !defined(NO_RSA) + WOLFSSL_CTX* ctx_c = NULL; + WOLFSSL_CTX* ctx_s = NULL; + WOLFSSL* ssl_c = NULL; + WOLFSSL* ssl_s = NULL; + struct test_memio_ctx test_ctx; + byte configs[512]; + word32 configsLen = (word32)sizeof(configs); + byte buf[TEST_MEMIO_BUF_SZ]; + int len = 0; + int shLen = 0; + int frag1 = 16; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0); + wolfSSL_set_verify(ssl_c, WOLFSSL_VERIFY_NONE, NULL); + + ExpectIntEQ(wolfSSL_CTX_GenerateEchConfig(ctx_s, ECH_TEST_PUB_NAME, 0, 0, + 0), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_GetEchConfigs(ctx_s, configs, &configsLen), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_SetEchConfigs(ssl_c, configs, configsLen), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, + ECH_TEST_PRIV_NAME, (word16)XSTRLEN(ECH_TEST_PRIV_NAME)), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(ssl_s, WOLFSSL_SNI_HOST_NAME, + ECH_TEST_PRIV_NAME, (word16)XSTRLEN(ECH_TEST_PRIV_NAME)), + WOLFSSL_SUCCESS); + + ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + + /* Record 0 of the server's flight is the plaintext ServerHello; split it + * into two records so the client has to reassemble it. */ + ExpectIntGT(test_ctx.c_len, 5); + if (EXPECT_SUCCESS()) { + shLen = (test_ctx.c_buff[3] << 8) | test_ctx.c_buff[4]; + ExpectIntGT(shLen, frag1); + } + if (EXPECT_SUCCESS()) { + XMEMCPY(buf, test_ctx.c_buff, 5); + buf[3] = (byte)(frag1 >> 8); + buf[4] = (byte)frag1; + len = 5; + XMEMCPY(buf + len, test_ctx.c_buff + 5, (size_t)frag1); + len += frag1; + XMEMCPY(buf + len, test_ctx.c_buff, 5); + buf[len + 3] = (byte)((shLen - frag1) >> 8); + buf[len + 4] = (byte)(shLen - frag1); + len += 5; + XMEMCPY(buf + len, test_ctx.c_buff + 5 + frag1, + (size_t)(shLen - frag1)); + len += shLen - frag1; + XMEMCPY(buf + len, test_ctx.c_buff + 5 + shLen, + (size_t)(test_ctx.c_len - 5 - shLen)); + len += test_ctx.c_len - 5 - shLen; + + test_memio_clear_buffer(&test_ctx, 1); + ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, (const char*)buf, + len), 0); + } + + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 20, NULL), 0); + ExpectIntEQ(wolfSSL_GetEchStatus(ssl_c), WOLFSSL_ECH_STATUS_ACCEPTED); + + wolfSSL_free(ssl_c); + wolfSSL_CTX_free(ctx_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + int test_key_share_mismatch(void) { diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index 50da06a7c02..d6072f534ca 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -51,6 +51,8 @@ int test_tls13_sg_missing(void); int test_tls13_ks_missing(void); int test_tls13_duplicate_extension(void); int test_tls13_duplicate_ech_extension(void); +int test_tls13_ech_hrr_coalesced_server_hello(void); +int test_tls13_ech_fragmented_server_hello(void); int test_key_share_mismatch(void); int test_key_share_mismatch_psk_dhe(void); int test_tls13_middlebox_compat_empty_session_id(void); @@ -181,6 +183,8 @@ int test_tls13_new_session_ticket_keeps_ems(void); TEST_DECL_GROUP("tls13", test_tls13_ks_missing), \ TEST_DECL_GROUP("tls13", test_tls13_duplicate_extension), \ TEST_DECL_GROUP("tls13", test_tls13_duplicate_ech_extension), \ + TEST_DECL_GROUP("tls13", test_tls13_ech_hrr_coalesced_server_hello), \ + TEST_DECL_GROUP("tls13", test_tls13_ech_fragmented_server_hello), \ TEST_DECL_GROUP("tls13", test_key_share_mismatch), \ TEST_DECL_GROUP("tls13", test_key_share_mismatch_psk_dhe), \ TEST_DECL_GROUP("tls13", test_tls13_middlebox_compat_empty_session_id), \ From 05449440a9e31560d4a6bd961d90f334c6dbbe11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 16 Sep 2026 10:16:35 +0200 Subject: [PATCH 2/6] Bound EVP_CipherUpdate output to inl + block size on decrypt In padded decrypt mode the function held two kinds of pending state at once: the most recent decrypted block in ctx->lastBlock for the padding check in Final, and a partial input block in ctx->buf. Nothing kept the two mutually exclusive, so a call that completed the partial block and carried further full blocks could write more than the inl + block_size that EVP_DecryptUpdate promises, which is what our own doxygen example for the function relies on. The library's own callers decrypt in single Update calls and are unaffected. A stored block is now output as soon as more input arrives, and the buffer completion path stores one only when the call ends on a block boundary, which is what OpenSSL does. Every call therefore stays within inl + block_size, the flush in the full blocks path becomes dead and is removed, and Final still sees either lastUsed or a non-zero bufUsed for truncated input. The same path also stored a block completed from ctx->buf when padding is disabled, where Final only checks that nothing is buffered and never looks at lastBlock, so that plaintext was silently dropped. The store is skipped with WOLFSSL_EVP_CIPH_NO_PADDING set, and Final now returns a block that was stored before padding was turned off. Behaviour change: which Update call returns a given block moves one call earlier for input that is not fed in block multiples, matching OpenSSL, so the three chunked decrypt checks in wolfcrypt/test/test.c are updated. Two of the three had no body on the if and had never checked anything. out must be at least inl + block size and must not overlap in; the doxygen documented neither and sized its example buffer at exactly inl. Strict in-place operation is not supported and never was. Tests: test_evp_cipher_update_chunked_bound, which decrypts a 64 byte AES-128-CBC ciphertext in every three way split against a buffer of exactly inl + block size followed by a canary, and test_evp_cipher_update_no_padding_buffered. --- doc/dox_comments/header_files/evp.h | 5 +- tests/api/test_evp_cipher.c | 215 ++++++++++++++++++++++++++++ tests/api/test_evp_cipher.h | 6 +- wolfcrypt/src/evp.c | 47 +++--- wolfcrypt/test/test.c | 18 ++- 5 files changed, 260 insertions(+), 31 deletions(-) diff --git a/doc/dox_comments/header_files/evp.h b/doc/dox_comments/header_files/evp.h index fb9c0803857..a6ec20d9392 100644 --- a/doc/dox_comments/header_files/evp.h +++ b/doc/dox_comments/header_files/evp.h @@ -229,7 +229,8 @@ int wolfSSL_EVP_DecryptInit_ex(WOLFSSL_EVP_CIPHER_CTX* ctx, \return SSL_FAILURE If not successful. \param ctx structure to get cipher type from. - \param out buffer to hold output. + \param out buffer to hold output, at least inl plus the cipher block size + bytes. It must not overlap in. \param outl adjusted to be size of output. \param in buffer to perform operation on. \param inl length of input buffer. @@ -237,7 +238,7 @@ int wolfSSL_EVP_DecryptInit_ex(WOLFSSL_EVP_CIPHER_CTX* ctx, _Example_ \code WOLFSSL_EVP_CIPHER_CTX* ctx = NULL; - unsigned char out[100]; + unsigned char out[100 + EVP_MAX_BLOCK_LENGTH]; int outl; unsigned char in[100]; int inl = 100; diff --git a/tests/api/test_evp_cipher.c b/tests/api/test_evp_cipher.c index c3de8b18b23..48e967eb75c 100644 --- a/tests/api/test_evp_cipher.c +++ b/tests/api/test_evp_cipher.c @@ -3008,3 +3008,218 @@ int test_evp_cipher_aead_aad_overflow(void) } +#if defined(OPENSSL_EXTRA) && !defined(NO_AES) && defined(HAVE_AES_CBC) && \ + defined(WOLFSSL_AES_128) + +#define EVP_CHUNK_CANARY_SZ 32 + +/* Decrypt with EVP_DecryptUpdate in the given chunk sizes, giving every call + * its own buffer of exactly inl + block size followed by a canary. */ +static int evp_chunked_decrypt(const byte* key, const byte* iv, + const byte* cipher, const int* chunks, int nchunks, int padding, + byte* plain, int* plainSz) +{ + EVP_CIPHER_CTX* ctx = NULL; + byte* out = NULL; + byte final[AES_BLOCK_SIZE]; + int ret = 0; + int offset = 0; + int total = 0; + int bound = 0; + int outl = 0; + int i = 0; + int j = 0; + + ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) + return -1; + + if (EVP_DecryptInit_ex(ctx, EVP_aes_128_cbc(), NULL, key, iv) != + WOLFSSL_SUCCESS) { + ret = -1; + } + if ((ret == 0) && (padding == 0) && + (EVP_CIPHER_CTX_set_padding(ctx, 0) != WOLFSSL_SUCCESS)) { + ret = -1; + } + + for (i = 0; (ret == 0) && (i < nchunks); i++) { + bound = chunks[i] + AES_BLOCK_SIZE; + out = (byte*)XMALLOC((size_t)(bound + EVP_CHUNK_CANARY_SZ), NULL, + DYNAMIC_TYPE_TMP_BUFFER); + if (out == NULL) { + ret = -1; + break; + } + XMEMSET(out, 0xA5, (size_t)(bound + EVP_CHUNK_CANARY_SZ)); + + outl = 0; + if (EVP_DecryptUpdate(ctx, out, &outl, cipher + offset, chunks[i]) != + WOLFSSL_SUCCESS) { + ret = -1; + } + else if ((outl < 0) || (outl > bound)) { + ret = -2; + } + else { + for (j = bound; j < bound + EVP_CHUNK_CANARY_SZ; j++) { + if (out[j] != 0xA5) + ret = -3; + } + } + if (ret == 0) { + XMEMCPY(plain + total, out, (size_t)outl); + total += outl; + offset += chunks[i]; + } + XFREE(out, NULL, DYNAMIC_TYPE_TMP_BUFFER); + out = NULL; + } + + if (ret == 0) { + outl = 0; + if (EVP_DecryptFinal_ex(ctx, final, &outl) != WOLFSSL_SUCCESS) { + ret = -1; + } + else { + XMEMCPY(plain + total, final, (size_t)outl); + total += outl; + } + } + + *plainSz = total; + EVP_CIPHER_CTX_free(ctx); + return ret; +} + +/* Encrypt 48 bytes of known plaintext into a 64 byte PKCS#7 padded + * ciphertext. */ +static int evp_chunked_setup(const byte* key, const byte* iv, byte* plain, + int plainSz, byte* cipher, int* cipherSz) +{ + EVP_CIPHER_CTX* ctx = NULL; + int ret = 0; + int outl = 0; + int total = 0; + int i = 0; + + for (i = 0; i < plainSz; i++) + plain[i] = (byte)i; + + ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) + return -1; + + if (EVP_EncryptInit_ex(ctx, EVP_aes_128_cbc(), NULL, key, iv) != + WOLFSSL_SUCCESS) { + ret = -1; + } + if ((ret == 0) && (EVP_EncryptUpdate(ctx, cipher, &outl, plain, plainSz) != + WOLFSSL_SUCCESS)) { + ret = -1; + } + if (ret == 0) { + total = outl; + if (EVP_EncryptFinal_ex(ctx, cipher + total, &outl) != + WOLFSSL_SUCCESS) { + ret = -1; + } + else { + total += outl; + } + } + + *cipherSz = total; + EVP_CIPHER_CTX_free(ctx); + return ret; +} + +#endif /* OPENSSL_EXTRA && !NO_AES && HAVE_AES_CBC && WOLFSSL_AES_128 */ + +int test_evp_cipher_update_chunked_bound(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_AES) && defined(HAVE_AES_CBC) && \ + defined(WOLFSSL_AES_128) + byte key[AES_BLOCK_SIZE]; + byte iv[AES_BLOCK_SIZE]; + byte plain[AES_BLOCK_SIZE * 3]; + byte cipher[AES_BLOCK_SIZE * 4]; + byte out[AES_BLOCK_SIZE * 4]; + int chunks[3]; + int cipherSz = 0; + int outSz = 0; + int i; + int j; + + XMEMSET(key, 0x0b, sizeof(key)); + XMEMSET(iv, 0x0c, sizeof(iv)); + + ExpectIntEQ(evp_chunked_setup(key, iv, plain, (int)sizeof(plain), cipher, + &cipherSz), 0); + ExpectIntEQ(cipherSz, (int)sizeof(cipher)); + + /* EVP_DecryptUpdate must never write more than inl + block size, + * whatever the input is split into */ + for (i = 1; EXPECT_SUCCESS() && (i < cipherSz); i++) { + for (j = i + 1; EXPECT_SUCCESS() && (j < cipherSz); j++) { + chunks[0] = i; + chunks[1] = j - i; + chunks[2] = cipherSz - j; + ExpectIntEQ(evp_chunked_decrypt(key, iv, cipher, chunks, 3, 1, out, + &outSz), 0); + ExpectIntEQ(outSz, (int)sizeof(plain)); + ExpectBufEQ(out, plain, sizeof(plain)); + } + } +#endif + return EXPECT_RESULT(); +} + +int test_evp_cipher_update_no_padding_buffered(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_AES) && defined(HAVE_AES_CBC) && \ + defined(WOLFSSL_AES_128) + byte key[AES_BLOCK_SIZE]; + byte iv[AES_BLOCK_SIZE]; + byte plain[AES_BLOCK_SIZE * 3]; + byte cipher[AES_BLOCK_SIZE * 4]; + byte out[AES_BLOCK_SIZE * 4]; + EVP_CIPHER_CTX* ctx = NULL; + int chunks[2]; + int cipherSz = 0; + int outSz = 0; + + XMEMSET(key, 0x0b, sizeof(key)); + XMEMSET(iv, 0x0c, sizeof(iv)); + + ExpectIntEQ(evp_chunked_setup(key, iv, plain, (int)sizeof(plain), cipher, + &cipherSz), 0); + + /* with padding disabled EVP_CipherFinal only checks that nothing is + * buffered, so a block completed from the buffer has to be returned here */ + chunks[0] = 6; + chunks[1] = AES_BLOCK_SIZE - 6; + ExpectIntEQ(evp_chunked_decrypt(key, iv, cipher, chunks, 2, 0, out, + &outSz), 0); + ExpectIntEQ(outSz, AES_BLOCK_SIZE); + ExpectBufEQ(out, plain, AES_BLOCK_SIZE); + + /* padding turned off after Update stored the block must not drop it */ + ExpectNotNull(ctx = EVP_CIPHER_CTX_new()); + ExpectIntEQ(EVP_DecryptInit_ex(ctx, EVP_aes_128_cbc(), NULL, key, iv), + WOLFSSL_SUCCESS); + outSz = -1; + ExpectIntEQ(EVP_DecryptUpdate(ctx, out, &outSz, cipher, AES_BLOCK_SIZE), + WOLFSSL_SUCCESS); + ExpectIntEQ(outSz, 0); + ExpectIntEQ(EVP_CIPHER_CTX_set_padding(ctx, 0), WOLFSSL_SUCCESS); + outSz = -1; + ExpectIntEQ(EVP_DecryptFinal_ex(ctx, out, &outSz), WOLFSSL_SUCCESS); + ExpectIntEQ(outSz, AES_BLOCK_SIZE); + ExpectBufEQ(out, plain, AES_BLOCK_SIZE); + EVP_CIPHER_CTX_free(ctx); +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_evp_cipher.h b/tests/api/test_evp_cipher.h index 7c8c1a0e1bd..058d6194300 100644 --- a/tests/api/test_evp_cipher.h +++ b/tests/api/test_evp_cipher.h @@ -67,6 +67,8 @@ int test_wolfSSL_EVP_des_ede3_ecb_no_key(void); int test_wolfSSL_EVP_Cipher_des_cbc_error(void); int test_evp_cipher_pkcs7_pad_zero(void); int test_evp_cipher_aead_aad_overflow(void); +int test_evp_cipher_update_chunked_bound(void); +int test_evp_cipher_update_no_padding_buffered(void); #define TEST_EVP_CIPHER_DECLS \ TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_CIPHER_CTX), \ @@ -111,6 +113,8 @@ int test_evp_cipher_aead_aad_overflow(void); TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_des_ede3_ecb_no_key), \ TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_Cipher_des_cbc_error), \ TEST_DECL_GROUP("evp_cipher", test_evp_cipher_pkcs7_pad_zero), \ - TEST_DECL_GROUP("evp_cipher", test_evp_cipher_aead_aad_overflow) + TEST_DECL_GROUP("evp_cipher", test_evp_cipher_aead_aad_overflow), \ + TEST_DECL_GROUP("evp_cipher", test_evp_cipher_update_chunked_bound), \ + TEST_DECL_GROUP("evp_cipher", test_evp_cipher_update_no_padding_buffered) #endif /* WOLFCRYPT_TEST_EVP_CIPHER_H */ diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index ca0ccdd7432..47040f756d9 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -1183,6 +1183,17 @@ int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, if (inl == 0) { return WOLFSSL_SUCCESS; } + + /* More input has arrived so a stored block is not the last one. Output it + * here: it must never coexist with a partial block in buf. */ + if ((ctx->enc == 0) && (ctx->lastUsed == 1)) { + PRINT_BUF(ctx->lastBlock, ctx->block_size); + XMEMCPY(out, ctx->lastBlock, (size_t)ctx->block_size); + *outl += ctx->block_size; + out += ctx->block_size; + ctx->lastUsed = 0; + } + if (ctx->bufUsed > 0) { /* concatenate them if there is anything */ int fill = fillBuff(ctx, in, inl); inl -= fill; @@ -1193,14 +1204,10 @@ int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, if (ctx->bufUsed == ctx->block_size) { byte* output = out; - /* During decryption we save the last block to check padding on Final. - * Update the last block stored if one has already been stored */ - if (ctx->enc == 0) { - if (ctx->lastUsed == 1) { - XMEMCPY(out, ctx->lastBlock, (size_t)ctx->block_size); - *outl+= ctx->block_size; - out += ctx->block_size; - } + /* Store the block for the padding check in Final only when it can + * still be the last one and Final would look at it. */ + if ((ctx->enc == 0) && (inl == 0) && (ctx->block_size != 1) && + !(ctx->flags & WOLFSSL_EVP_CIPH_NO_PADDING)) { output = ctx->lastBlock; /* redirect output to last block buffer */ ctx->lastUsed = 1; } @@ -1209,12 +1216,11 @@ int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, if (evpCipherBlock(ctx, output, ctx->buf, ctx->block_size) == 0) { return WOLFSSL_FAILURE; } - PRINT_BUF(out, ctx->block_size); + PRINT_BUF(output, ctx->block_size); ctx->bufUsed = 0; - /* if doing encryption update the new output block, decryption will - * always have the last block saved for when Final is called */ - if ((ctx->enc != 0)) { + /* nothing to report when the block was stored for Final */ + if (output == out) { *outl+= ctx->block_size; out += ctx->block_size; } @@ -1222,16 +1228,6 @@ int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, blocks = inl / ctx->block_size; if (blocks > 0) { - /* During decryption we save the last block to check padding on Final. - * Update the last block stored if one has already been stored */ - if ((ctx->enc == 0) && (ctx->lastUsed == 1)) { - PRINT_BUF(ctx->lastBlock, ctx->block_size); - XMEMCPY(out, ctx->lastBlock, (size_t)ctx->block_size); - *outl += ctx->block_size; - out += ctx->block_size; - ctx->lastUsed = 0; - } - /* process blocks */ if (evpCipherBlock(ctx, out, in, blocks * ctx->block_size) == 0) { return WOLFSSL_FAILURE; @@ -1665,6 +1661,13 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, unsigned char *out, if (ctx->flags & WOLFSSL_EVP_CIPH_NO_PADDING) { if (ctx->bufUsed != 0) return WOLFSSL_FAILURE; *outl = 0; + /* padding may have been turned off after Update stored a + * block; with no pad to strip it is all plaintext */ + if ((ctx->enc == 0) && (ctx->lastUsed == 1)) { + XMEMCPY(out, ctx->lastBlock, (size_t)ctx->block_size); + *outl = ctx->block_size; + ctx->lastUsed = 0; + } } else if (ctx->enc) { if (ctx->block_size == 1) { diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 61201f426e7..587c5611bca 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -35901,16 +35901,18 @@ static wc_test_ret_t openssl_aes_cbc_test(void) return WC_TEST_RET_ENC_NC; total += outlen; + /* the completed block is output as soon as more input follows it */ if (wolfSSL_EVP_CipherUpdate(de, (byte*)&plain[total], &outlen, (byte*)&cipher[6], 12) == 0) return WC_TEST_RET_ENC_NC; - if (outlen != 0) + if (outlen != 16) + return WC_TEST_RET_ENC_NC; total += outlen; if (wolfSSL_EVP_CipherUpdate(de, (byte*)&plain[total], &outlen, (byte*)&cipher[6+12], 14) == 0) return WC_TEST_RET_ENC_NC; - if (outlen != 16) + if (outlen != 0) return WC_TEST_RET_ENC_NC; total += outlen; @@ -37757,16 +37759,18 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t openssl_test(void) ERROR_OUT(WC_TEST_RET_ENC_NC, out); total += outlen; + /* the completed block is output as soon as more input follows it */ if (wolfSSL_EVP_CipherUpdate(de, (byte*)&plain[total], &outlen, (byte*)&cipher[6], 12) == 0) ERROR_OUT(WC_TEST_RET_ENC_NC, out); - if(outlen != 0) + if(outlen != 16) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); total += outlen; if (wolfSSL_EVP_CipherUpdate(de, (byte*)&plain[total], &outlen, (byte*)&cipher[6+12], 14) == 0) ERROR_OUT(WC_TEST_RET_ENC_NC, out); - if(outlen != 16) + if(outlen != 0) ERROR_OUT(WC_TEST_RET_ENC_NC, out); total += outlen; @@ -37823,14 +37827,16 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t openssl_test(void) ERROR_OUT(WC_TEST_RET_ENC_NC, out); total += outlen; + /* the completed block is output as soon as more input follows it */ if (wolfSSL_EVP_CipherUpdate(de, (byte*)&plain[total], &outlen, (byte*)&cipher[6], 12) == 0) ERROR_OUT(WC_TEST_RET_ENC_NC, out); - if(outlen != 0) + if(outlen != 16) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); total += outlen; if (wolfSSL_EVP_CipherUpdate(de, (byte*)&plain[total], &outlen, (byte*)&cipher[6+12], 14) == 0) ERROR_OUT(WC_TEST_RET_ENC_NC, out); - if(outlen != 16) + if(outlen != 0) ERROR_OUT(WC_TEST_RET_ENC_NC, out); total += outlen; From 845413ca1371eb7486d958b7588490bbb3c39058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 16 Sep 2026 11:34:32 +0200 Subject: [PATCH 3/6] pkcs7: bound the AuthEnvelopedData authTag copy by the remaining input In the WC_PKCS7_AUTHENV_ATRBEND state the streaming decoder guaranteed only MAX_LENGTH_SZ + ASN_TAG_SZ bytes before copying the authTag out of pkiMsg. Its re-buffer guard compared the tag size plus the OCTET STRING header against pkiMsgSz, the size of the whole buffer, instead of the bytes left after the header, so for a real message it never fired. Both explicit BUFFER_E bounds checks in front of the copy were compiled only under NO_PKCS7_STREAM, and no configure option defines that macro. Measure the re-buffer guard against pkiMsgSz - localIdx and, after a successful wc_PKCS7_AddDataToStream(), restore localIdx from the header offset and refresh pkiMsgSz, the idiom wc_PKCS7_DecodeEnvelopedData() already uses for encryptedContent. Make the pre-copy BUFFER_E check unconditional so the copy cannot exceed the available input whatever the streaming logic decides, and drop the weaker NO_PKCS7_STREAM-only check it subsumes. Two adjacent defects in the same state go with it. The state prologue never restored encOID, unlike WC_PKCS7_AUTHENV_5 and _6, so the AES-GCM and AES-CCM tag size gates below it compared against 0 on every re-entry, and the bound above makes that re-entry the normal streaming path rather than a rare corner. The re-buffer call also returned directly, so an error from it skipped the shared handler that releases the AAD and resets the stream; it now breaks into that handler, which already passes WC_PKCS7_WANT_READ_E through untouched. Behaviour change: input ending inside the tag returns WC_PKCS7_WANT_READ_E when streaming and BUFFER_E under NO_PKCS7_STREAM. A chunked delivery whose read boundary falls inside the tag now completes once the rest arrives; previously it failed with AES_GCM_AUTH_E. test_wc_PKCS7_DecodeAuthEnvelopedData_truncated was compiled only under NO_PKCS7_STREAM, so the default streaming decoder was never exercised, and it decoded from an oversized stack buffer. It now runs in streaming builds, decodes from exact sized heap copies, and sweeps truncations into the tag as well as the existing cut inside encryptedContent. --- tests/api/test_pkcs7.c | 61 +++++++++++++++++++++++++++++++++++------- wolfcrypt/src/pkcs7.c | 41 +++++++++++++--------------- 2 files changed, 69 insertions(+), 33 deletions(-) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index 982aab81bda..ecbbdec8484 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -3766,18 +3766,29 @@ int test_wc_PKCS7_DecodeEnvelopedData_constructedDefiniteOctet(void) } /* END test_wc_PKCS7_DecodeEnvelopedData_constructedDefiniteOctet() */ -/* Decoding an AuthEnvelopedData blob whose encryptedContent or authTag - * is truncated must return BUFFER_E rather than reading past pkiMsg. */ +/* Decoding an AuthEnvelopedData blob whose encryptedContent or authTag is + * truncated must reject it rather than reading past pkiMsg. */ int test_wc_PKCS7_DecodeAuthEnvelopedData_truncated(void) { EXPECT_DECLS; #if defined(HAVE_PKCS7) && defined(HAVE_AESGCM) && !defined(NO_RSA) && \ - !defined(NO_AES) && defined(WOLFSSL_AES_128) && defined(NO_PKCS7_STREAM) + !defined(NO_AES) && defined(WOLFSSL_AES_128) PKCS7* pkcs7 = NULL; byte enveloped[2048]; byte decoded[256]; byte data[] = "truncated authEnvelopedData test"; + byte* exact = NULL; + word32 inSz = 0; int encSz = 0; + int i; + /* 32 stops inside encryptedContent, which starves the stream before the + * tag; 1, 5 and 11 cut into the 16 byte tag itself */ + static const int truncBy[] = { 32, 1, 5, 11 }; +#ifdef NO_PKCS7_STREAM + int truncErr = WC_NO_ERR_TRACE(BUFFER_E); +#else + int truncErr = WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E); +#endif ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, (byte*)client_cert_der_2048, @@ -3788,13 +3799,13 @@ int test_wc_PKCS7_DecodeAuthEnvelopedData_truncated(void) pkcs7->contentOID = DATA; pkcs7->encryptOID = AES128GCMb; } - /* >32 so the encSz-32 / encSz-1 truncations below can't underflow */ + /* >32 so the truncations below can't underflow */ ExpectIntGT(encSz = wc_PKCS7_EncodeAuthEnvelopedData(pkcs7, enveloped, sizeof(enveloped)), 32); wc_PKCS7_Free(pkcs7); pkcs7 = NULL; - /* Truncate inside encryptedContent (encryptedContentSz check). */ + /* the untruncated message still decodes */ ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, (byte*)client_cert_der_2048, sizeof_client_cert_der_2048), 0); @@ -3803,12 +3814,37 @@ int test_wc_PKCS7_DecodeAuthEnvelopedData_truncated(void) pkcs7->privateKeySz = sizeof_client_key_der_2048; } ExpectIntEQ(wc_PKCS7_DecodeAuthEnvelopedData(pkcs7, enveloped, - (word32)encSz - 32, decoded, sizeof(decoded)), - WC_NO_ERR_TRACE(BUFFER_E)); + (word32)encSz, decoded, sizeof(decoded)), (int)sizeof(data)); + ExpectIntEQ(XMEMCMP(decoded, data, sizeof(data)), 0); wc_PKCS7_Free(pkcs7); pkcs7 = NULL; - /* Truncate one byte off the auth tag (authTagSz check). */ + /* exact sized copies, so that an over-read leaves the allocation */ + for (i = 0; i < (int)(sizeof(truncBy) / sizeof(truncBy[0])); i++) { + inSz = (word32)(encSz - truncBy[i]); + + ExpectNotNull(exact = (byte*)XMALLOC(inSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (exact != NULL) { + XMEMCPY(exact, enveloped, inSz); + } + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, (byte*)client_cert_der_2048, + sizeof_client_cert_der_2048), 0); + if (pkcs7 != NULL) { + pkcs7->privateKey = (byte*)client_key_der_2048; + pkcs7->privateKeySz = sizeof_client_key_der_2048; + } + ExpectIntEQ(wc_PKCS7_DecodeAuthEnvelopedData(pkcs7, exact, inSz, + decoded, sizeof(decoded)), truncErr); + wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + XFREE(exact, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + exact = NULL; + } + +#ifndef NO_PKCS7_STREAM + /* a read boundary inside the tag must ask for the rest, then decode */ ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, (byte*)client_cert_der_2048, sizeof_client_cert_der_2048), 0); @@ -3817,8 +3853,13 @@ int test_wc_PKCS7_DecodeAuthEnvelopedData_truncated(void) pkcs7->privateKeySz = sizeof_client_key_der_2048; } ExpectIntEQ(wc_PKCS7_DecodeAuthEnvelopedData(pkcs7, enveloped, - (word32)encSz - 1, decoded, sizeof(decoded)), - WC_NO_ERR_TRACE(BUFFER_E)); + (word32)encSz - 11, decoded, sizeof(decoded)), + WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); + ExpectIntEQ(wc_PKCS7_DecodeAuthEnvelopedData(pkcs7, + enveloped + encSz - 11, 11, decoded, sizeof(decoded)), + (int)sizeof(data)); + ExpectIntEQ(XMEMCMP(decoded, data, sizeof(data)), 0); +#endif wc_PKCS7_Free(pkcs7); #endif diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 6f63ba28354..8d3c4809d99 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -16226,7 +16226,7 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in, #ifndef NO_PKCS7_STREAM if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { - return ret; + break; } length = (int)pkcs7->stream->expected; @@ -16280,7 +16280,7 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in, #ifndef NO_PKCS7_STREAM if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz, pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { - return ret; + break; } pkiMsgSz = (pkcs7->stream->length > 0)? pkcs7->stream->length: inSz; @@ -16289,6 +16289,8 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in, encodedAttribs = pkcs7->stream->aad; } macSz = (int)pkcs7->stream->icvSz; + /* restore encOID for the authTag size gates below */ + wc_PKCS7_StreamGetVar(pkcs7, &encOID, NULL, NULL); #endif @@ -16344,42 +16346,35 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in, } #ifndef NO_PKCS7_STREAM - /* there might not be enough data for the auth tag too */ - if (ret == 0) { - if ((authTagSz + (localIdx - idx)) > pkcs7->stream->expected && - (authTagSz + (localIdx - idx)) > pkiMsgSz) { - pkcs7->stream->expected = authTagSz + - (localIdx - idx); - if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz, - pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { - return ret; - } + /* measure the tag against what is left after the OCTET STRING + * header, not against the size of the whole buffer */ + if (ret == 0 && + (localIdx > pkiMsgSz || authTagSz > pkiMsgSz - localIdx)) { + word32 ofsetIdx = localIdx - idx; + + pkcs7->stream->expected = authTagSz + ofsetIdx; + if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz, + pkcs7->stream->expected, &pkiMsg, &idx)) != 0) { + break; } + localIdx = idx + ofsetIdx; + pkiMsgSz = (pkcs7->stream->length > 0)? pkcs7->stream->length: + inSz; } #endif idx = localIdx; - #ifdef NO_PKCS7_STREAM - if (ret == 0 && authTagSz > (word32)(pkiMsgSz - idx)) { - ret = BUFFER_E; - } - #endif - if (ret == 0 && authTagSz > (word32)sizeof(authTag)) { WOLFSSL_MSG("AuthEnvelopedData authTag too large for buffer"); ret = ASN_PARSE_E; } - #ifdef NO_PKCS7_STREAM - /* In the streaming build the block above re-buffers enough data for - * the auth tag. Without streaming, verify the tag fits within the - * provided input before copying. */ + /* verify the tag fits in the available input before copying */ if (ret == 0 && (idx > pkiMsgSz || authTagSz > pkiMsgSz - idx)) { WOLFSSL_MSG("AuthEnvelopedData authTag exceeds input buffer"); ret = BUFFER_E; } - #endif if (ret == 0) { XMEMCPY(authTag, &pkiMsg[idx], authTagSz); From bc4a9126290c854662ceadcbcba60833d83d140b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 16 Sep 2026 11:58:14 +0200 Subject: [PATCH 4/6] tls: reject TLS 1.3 cipher suites below TLS 1.3 on the client DoServerHello() checked only that the ServerHello's cipher suite appeared in the client's own list, never that it is usable at the negotiated version. VerifyServerSuite() has exactly that check on the server, and the client had no equivalent. A resuming ClientHello is pinned to the cached session's version by wolfSSL_set_session() but still carries the full configured suite list, so a downgrade-capable client resuming a TLS 1.2 session by session ID offers TLS 1.3-only suites inside a TLS 1.2 hello. With HAVE_NULL_CIPHER built, TLS_SHA384_SHA384 is in the default client list without any application opt-in. Selecting such a suite gives SetCipherSpecs() sizes whose TLS 1.2 key block does not fit key_dig, on a resumption path that derives keys before the peer is authenticated. Reject the suite in SetCipherSpecs(), where the specs become authoritative: specs.kea == any_kea marks exactly the suites VerifyServerSuite() rejects, so one check covers both DoServerHello() paths as well as the DTLS key-import API and the sniffer. Every other caller already runs with ssl->version at TLS 1.3 when a TLS 1.3 suite is legitimate. DeriveTlsKeys() additionally returns BUFFER_E for a key block larger than its buffer. wc_PRF_TLS() keeps no MAX_PRF_DIG bound because wolfSSL_export_keying_material() and the EAP key export legitimately ask for more. Stop offering the suites in the first place: SendClientHello() is reached only below TLS 1.3, since SendTls13ClientHello() takes over above it, so filter TLS 1.3-only suites out of the cipher_suites it writes and fail with SUITES_ERROR if nothing is left. VerifyServerSuite() already carried IsTls13CipherSuite(), a second copy of the same predicate; the two are merged into IsTls13OnlySuite(), keeping the HAVE_NULL_CIPHER and SM4 guards, so a future TLS 1.3 suite has one list to be added to. SetCipherSpecs() zeroes ssl->specs before returning UNSUPPORTED_SUITE. GetCipherSpec() has already written the rejected suite's key, hash and IV sizes there, and ssl->options.tls, ssl->hmac and the DTLS fields are not updated on that path, so a caller that tolerated the failure would run TLS 1.2 with a TLS 1.3 key block. Behaviour change: a TLS 1.2 or DTLS 1.2 peer that picks a TLS 1.3 suite now fails with UNSUPPORTED_SUITE at ServerHello rather than BAD_KEA_TYPE_E or a Finished failure. Tests: test_tls12_server_hello_tls13_suite rewrites the suite of a resuming ServerHello and expects UNSUPPORTED_SUITE; test_tls12_client_hello_no_tls13_suites checks that the resuming ClientHello drops the TLS 1.3 suites the non-resuming one offers, and still resumes. --- src/internal.c | 105 +++++++++++++--------- src/keys.c | 12 +++ src/tls.c | 6 ++ tests/api/test_tls.c | 208 +++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls.h | 4 + 5 files changed, 294 insertions(+), 41 deletions(-) diff --git a/src/internal.c b/src/internal.c index 69b12575596..9ad416b1f99 100644 --- a/src/internal.c +++ b/src/internal.c @@ -34539,6 +34539,35 @@ static void MakePSKPreMasterSecret(Arrays* arrays, byte use_psk_key) } #endif +#if defined(WOLFSSL_TLS13) && \ + ((!defined(NO_WOLFSSL_CLIENT) && !defined(NO_TLS) && \ + !defined(WOLFSSL_NO_TLS12)) || \ + (!defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS))) +/* Is this one of the cipher suites that can only be negotiated at TLS 1.3? */ +static int IsTls13OnlySuite(byte first, byte second) +{ + (void)second; + + if (first == TLS13_BYTE) + return 1; + +#ifdef HAVE_NULL_CIPHER + if ((first == ECC_BYTE) && ((second == TLS_SHA256_SHA256) || + (second == TLS_SHA384_SHA384))) + return 1; +#endif + +#if (defined(WOLFSSL_SM4_GCM) || defined(WOLFSSL_SM4_CCM)) && \ + defined(WOLFSSL_SM3) + if ((first == CIPHER_BYTE) && ((second == TLS_SM4_GCM_SM3) || + (second == TLS_SM4_CCM_SM3))) + return 1; +#endif + + return 0; +} +#endif + /* client only parts */ #if !defined(NO_WOLFSSL_CLIENT) && !defined(NO_TLS) @@ -34568,6 +34597,10 @@ static void MakePSKPreMasterSecret(Arrays* arrays, byte use_psk_key) int ret; word32 extSz = 0; const Suites* suites; + word16 suiteSz; +#ifdef WOLFSSL_TLS13 + word16 i; +#endif if (ssl == NULL) { return BAD_FUNC_ARG; @@ -34608,6 +34641,22 @@ static void MakePSKPreMasterSecret(Arrays* arrays, byte use_psk_key) return SUITES_ERROR; } +#ifdef WOLFSSL_TLS13 + /* Reached only below TLS 1.3, where a TLS 1.3 suite the server picked + * from the list could not be used to derive keys. */ + suiteSz = 0; + for (i = 0; i + SUITE_LEN <= suites->suiteSz; i += SUITE_LEN) { + if (!IsTls13OnlySuite(suites->suites[i], suites->suites[i + 1])) + suiteSz += SUITE_LEN; + } + if (suiteSz == 0) { + WOLFSSL_MSG("No cipher suite valid for version in ClientHello"); + return SUITES_ERROR; + } +#else + suiteSz = suites->suiteSz; +#endif + #ifdef HAVE_SESSION_TICKET if (ssl->options.resuming && ssl->session->ticketLen > 0) { SessionTicket* ticket; @@ -34636,7 +34685,7 @@ static void MakePSKPreMasterSecret(Arrays* arrays, byte use_psk_key) length += SUITE_LEN; else #endif - length += suites->suiteSz; + length += suiteSz; #ifdef HAVE_TLS_EXTENSIONS /* auto populate extensions supported unless user defined */ @@ -34742,10 +34791,19 @@ static void MakePSKPreMasterSecret(Arrays* arrays, byte use_psk_key) #endif /* NO_FORCE_SCR_SAME_SUITE */ { /* then cipher suites */ - c16toa(suites->suiteSz, output + idx); + c16toa(suiteSz, output + idx); idx += OPAQUE16_LEN; - XMEMCPY(output + idx, &suites->suites, suites->suiteSz); - idx += suites->suiteSz; +#ifdef WOLFSSL_TLS13 + for (i = 0; i + SUITE_LEN <= suites->suiteSz; i += SUITE_LEN) { + if (IsTls13OnlySuite(suites->suites[i], suites->suites[i + 1])) + continue; + output[idx++] = suites->suites[i]; + output[idx++] = suites->suites[i + 1]; + } +#else + XMEMCPY(output + idx, &suites->suites, suiteSz); + idx += suiteSz; +#endif } /* last, compression. RFC 5246 7.4.1.2 requires the list to always @@ -40773,37 +40831,6 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) #endif /* !WOLFSSL_NO_TLS12 */ -#ifdef WOLFSSL_TLS13 - /* Check if a cipher suite is a TLS 1.3 cipher suite - * Returns 1 if TLS 1.3 cipher suite, 0 otherwise - */ - static WC_INLINE int IsTls13CipherSuite(byte first, byte second) - { - (void)second; /* Suppress unused parameter warning */ - - /* TLS 1.3 cipher suites use TLS13_BYTE (0x13) as first byte */ - if (first == TLS13_BYTE) - return 1; - -#ifdef HAVE_NULL_CIPHER - /* Special cases for integrity-only cipher suites */ - if (first == ECC_BYTE && (second == TLS_SHA256_SHA256 || - second == TLS_SHA384_SHA384)) - return 1; -#endif - -#if (defined(WOLFSSL_SM4_GCM) || defined(WOLFSSL_SM4_CCM)) && \ - defined(WOLFSSL_SM3) - /* SM4 cipher suites for TLS 1.3 */ - if (first == CIPHER_BYTE && (second == TLS_SM4_GCM_SM3 || - second == TLS_SM4_CCM_SM3)) - return 1; -#endif - - return 0; - } -#endif /* WOLFSSL_TLS13 */ - /* Make sure server cert/key are valid for this suite, true on success * Returns 1 for valid server suite or 0 if not found * For asynchronous this can return WC_PENDING_E @@ -40834,7 +40861,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) /* When negotiating TLS 1.3, reject non-TLS 1.3 cipher suites */ if (IsAtLeastTLSv1_3(ssl->version) && ssl->options.side == WOLFSSL_SERVER_END) { - if (!IsTls13CipherSuite(first, second)) { + if (!IsTls13OnlySuite(first, second)) { WOLFSSL_MSG("TLS 1.2 cipher suite not valid for TLS 1.3"); return 0; } @@ -40944,11 +40971,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) return 0; /* not found */ #endif /* HAVE_SUPPORTED_CURVES */ } - else if ((first == TLS13_BYTE) || ((first == ECC_BYTE) && - ((second == TLS_SHA256_SHA256) || - (second == TLS_SHA384_SHA384))) || - ((first == CIPHER_BYTE) && ((second == TLS_SM4_GCM_SM3) || - (second == TLS_SM4_CCM_SM3)))) { + else if (IsTls13OnlySuite(first, second)) { /* Can't negotiate TLS 1.3 cipher suites with lower protocol * version. */ return 0; diff --git a/src/keys.c b/src/keys.c index 1d7ae76ff0c..7ceed5798bf 100644 --- a/src/keys.c +++ b/src/keys.c @@ -50,6 +50,18 @@ int SetCipherSpecs(WOLFSSL* ssl) ssl->options.cipherSuite, &ssl->specs, &ssl->options); if (ret == 0) { + #ifdef WOLFSSL_TLS13 + /* The client only checks that the ServerHello suite was offered, and + * the TLS 1.2 key block of TLS_SHA384_SHA384 exceeds MAX_PRF_DIG. */ + if (ssl->specs.kea == any_kea && !IsAtLeastTLSv1_3(ssl->version)) { + WOLFSSL_MSG("TLS 1.3 cipher suite not valid for lower version"); + WOLFSSL_ERROR_VERBOSE(UNSUPPORTED_SUITE); + /* GetCipherSpec already wrote the rejected suite's sizes. */ + XMEMSET(&ssl->specs, 0, sizeof(ssl->specs)); + return UNSUPPORTED_SUITE; + } + #endif /* WOLFSSL_TLS13 */ + #ifdef WOLFSSL_ALLOW_SSLV3 /* SSLv3 (RFC 6101) defines MAC algorithms as MD5 and SHA-1. SHA-256 * was introduced in TLS 1.2 (RFC 5246). SSL_hmac for old SSLv3 diff --git a/src/tls.c b/src/tls.c index 9a6c3ccaf98..0d6a2b91597 100644 --- a/src/tls.c +++ b/src/tls.c @@ -563,6 +563,12 @@ int DeriveTlsKeys(WOLFSSL* ssl) 2 * ssl->specs.iv_size; WC_DECLARE_VAR(key_dig, byte, MAX_PRF_DIG, 0); + if (key_dig_len <= 0 || key_dig_len > MAX_PRF_DIG) { + WOLFSSL_MSG("Key block too large for PRF digest buffer"); + WOLFSSL_ERROR_VERBOSE(BUFFER_E); + return BUFFER_E; + } + WC_ALLOC_VAR_EX(key_dig, byte, MAX_PRF_DIG, ssl->heap, DYNAMIC_TYPE_DIGEST, return MEMORY_E); diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index 67233e17e7a..9b47f037fd2 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -2234,6 +2234,214 @@ int test_tls12_resume_ticket_decline_fallback(void) return EXPECT_RESULT(); } +/* A TLS 1.2 ServerHello must not be able to select a TLS 1.3-only suite. The + * suite was offered, so DoServerHello() accepted it, and the TLS 1.2 key block + * of TLS_SHA384_SHA384 (2 * (48 + 48 + 48)) overran key_dig[MAX_PRF_DIG] in + * DeriveTlsKeys(). Resume a TLS 1.2 session, rewrite the suite in the server's + * resuming ServerHello and expect UNSUPPORTED_SUITE before any key derivation. */ +int test_tls12_server_hello_tls13_suite(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(WOLFSSL_TLS13) && \ + !defined(NO_SESSION_CACHE) && !defined(NO_RSA) && defined(HAVE_ECC) && \ + !defined(NO_AES) && defined(HAVE_AESGCM) && !defined(NO_SHA256) && \ + !defined(WOLFSSL_NO_STRICT_CIPHER_SUITE) && \ + defined(BUILD_TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) && \ + defined(BUILD_TLS_AES_128_GCM_SHA256) + const char* suite12 = "ECDHE-RSA-AES128-GCM-SHA256"; + const char* suites_c = +#if defined(HAVE_NULL_CIPHER) && defined(BUILD_TLS_SHA384_SHA384) + "TLS13-SHA384-SHA384:" +#endif + "TLS13-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256"; + const byte tls13Suites[][SUITE_LEN] = { +#if defined(HAVE_NULL_CIPHER) && defined(BUILD_TLS_SHA384_SHA384) + { ECC_BYTE, TLS_SHA384_SHA384 }, +#endif + { TLS13_BYTE, TLS_AES_128_GCM_SHA256 }, + }; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + WOLFSSL_SESSION *sess = NULL; + struct test_memio_ctx test_ctx; + word32 i; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfSSLv23_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, suites_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, suite12), WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectNotNull(sess = wolfSSL_get1_session(ssl_c)); + wolfSSL_free(ssl_c); + ssl_c = NULL; + wolfSSL_free(ssl_s); + ssl_s = NULL; + + for (i = 0; i < sizeof(tls13Suites) / sizeof(tls13Suites[0]); i++) { + int sidOff = RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ + VERSION_SZ + + RAN_LEN; + int suiteOff = 0; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, + &ssl_s, wolfSSLv23_client_method, + wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, suites_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, suite12), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + + ExpectIntGT(test_ctx.c_len, sidOff + ENUM_LEN); + ExpectIntEQ(test_ctx.c_buff[0], handshake); + ExpectIntEQ(test_ctx.c_buff[RECORD_HEADER_SZ], server_hello); + if (EXPECT_SUCCESS()) { + suiteOff = sidOff + ENUM_LEN + test_ctx.c_buff[sidOff]; + ExpectIntGT(test_ctx.c_len, suiteOff + SUITE_LEN); + } + if (EXPECT_SUCCESS()) { + ExpectIntEQ(test_ctx.c_buff[suiteOff], ECC_BYTE); + ExpectIntEQ(test_ctx.c_buff[suiteOff + 1], + TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256); + test_ctx.c_buff[suiteOff] = tls13Suites[i][0]; + test_ctx.c_buff[suiteOff + 1] = tls13Suites[i][1]; + } + + ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WC_NO_ERR_TRACE(UNSUPPORTED_SUITE)); + + wolfSSL_free(ssl_c); + ssl_c = NULL; + wolfSSL_free(ssl_s); + ssl_s = NULL; + } + + wolfSSL_SESSION_free(sess); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(WOLFSSL_TLS13) && \ + !defined(NO_SESSION_CACHE) && !defined(NO_RSA) && defined(HAVE_ECC) && \ + !defined(NO_AES) && defined(HAVE_AESGCM) && !defined(NO_SHA256) && \ + defined(BUILD_TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) && \ + defined(BUILD_TLS_AES_128_GCM_SHA256) +/* Count occurrences of suite in the cipher_suites list of the ClientHello in + * buf, or -1 if the message is not a parseable ClientHello. */ +static int test_client_hello_count_suite(const byte* buf, int len, + const byte* suite) +{ + int idx = RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ + VERSION_SZ + RAN_LEN; + int suiteSz; + int found = 0; + + if (len < idx + ENUM_LEN || buf[0] != handshake || + buf[RECORD_HEADER_SZ] != client_hello) { + return -1; + } + idx += ENUM_LEN + buf[idx]; + if (len < idx + OPAQUE16_LEN) + return -1; + suiteSz = (buf[idx] << 8) | buf[idx + 1]; + idx += OPAQUE16_LEN; + if (suiteSz <= 0 || len < idx + suiteSz) + return -1; + + for (; suiteSz >= SUITE_LEN; suiteSz -= SUITE_LEN, idx += SUITE_LEN) { + if (buf[idx] == suite[0] && buf[idx + 1] == suite[1]) + found++; + } + return found; +} +#endif + +/* A ClientHello pinned below TLS 1.3 by wolfSSL_set_session() must not offer + * TLS 1.3-only suites, which the server can only answer with a suite the + * negotiated version cannot use. */ +int test_tls12_client_hello_no_tls13_suites(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(WOLFSSL_TLS13) && \ + !defined(NO_SESSION_CACHE) && !defined(NO_RSA) && defined(HAVE_ECC) && \ + !defined(NO_AES) && defined(HAVE_AESGCM) && !defined(NO_SHA256) && \ + defined(BUILD_TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) && \ + defined(BUILD_TLS_AES_128_GCM_SHA256) + const char* suite12 = "ECDHE-RSA-AES128-GCM-SHA256"; + const char* suites_c = +#if defined(HAVE_NULL_CIPHER) && defined(BUILD_TLS_SHA384_SHA384) + "TLS13-SHA384-SHA384:" +#endif + "TLS13-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256"; + const byte suite13[SUITE_LEN] = { TLS13_BYTE, TLS_AES_128_GCM_SHA256 }; +#if defined(HAVE_NULL_CIPHER) && defined(BUILD_TLS_SHA384_SHA384) + const byte suiteNull[SUITE_LEN] = { ECC_BYTE, TLS_SHA384_SHA384 }; +#endif + const byte suiteEcc[SUITE_LEN] = + { ECC_BYTE, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 }; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + WOLFSSL_SESSION *sess = NULL; + struct test_memio_ctx test_ctx; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfSSLv23_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, suites_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, suite12), WOLFSSL_SUCCESS); + /* Not pinned: the TLS 1.3 suites are offered. */ + ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(test_client_hello_count_suite(test_ctx.s_buff, test_ctx.s_len, + suite13), 1); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectNotNull(sess = wolfSSL_get1_session(ssl_c)); + wolfSSL_free(ssl_c); + ssl_c = NULL; + wolfSSL_free(ssl_s); + ssl_s = NULL; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfSSLv23_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, suites_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, suite12), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(test_client_hello_count_suite(test_ctx.s_buff, test_ctx.s_len, + suite13), 0); +#if defined(HAVE_NULL_CIPHER) && defined(BUILD_TLS_SHA384_SHA384) + ExpectIntEQ(test_client_hello_count_suite(test_ctx.s_buff, test_ctx.s_len, + suiteNull), 0); +#endif + ExpectIntEQ(test_client_hello_count_suite(test_ctx.s_buff, test_ctx.s_len, + suiteEcc), 1); + /* The resumption itself must still work. */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(wolfSSL_session_reused(ssl_c), 1); + + wolfSSL_SESSION_free(sess); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + /* wolfSSL_set_session() must reject a TLS 1.2 session when minDowngrade is * set to TLS 1.3. */ int test_tls_set_session_min_downgrade(void) diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index 6c3fda89962..deb08880236 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -49,6 +49,8 @@ int test_tls_version_error_alert_mapping(void); int test_tls12_etm_failed_resumption(void); int test_tls12_resume_ticket_wrong_suite(void); int test_tls12_resume_ticket_decline_fallback(void); +int test_tls12_server_hello_tls13_suite(void); +int test_tls12_client_hello_no_tls13_suites(void); int test_tls_set_session_min_downgrade(void); int test_tls12_session_id_resumption_sni_mismatch(void); int test_tls13_session_resumption_sni_mismatch(void); @@ -96,6 +98,8 @@ int test_wolfSSL_get_shared_ciphers(void); TEST_DECL_GROUP("tls", test_tls12_etm_failed_resumption), \ TEST_DECL_GROUP("tls", test_tls12_resume_ticket_wrong_suite), \ TEST_DECL_GROUP("tls", test_tls12_resume_ticket_decline_fallback), \ + TEST_DECL_GROUP("tls", test_tls12_server_hello_tls13_suite), \ + TEST_DECL_GROUP("tls", test_tls12_client_hello_no_tls13_suites), \ TEST_DECL_GROUP("tls", test_tls_set_session_min_downgrade), \ TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_sni_mismatch), \ TEST_DECL_GROUP("tls", test_tls13_session_resumption_sni_mismatch), \ From 4d73099c851cd1f678f4353bb9b1bde50b971f8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 16 Sep 2026 14:10:32 +0200 Subject: [PATCH 5/6] ecc: bound explicit EC domain parameters to MAX_ECC_BYTES EccSpecifiedECDomainDecode() took the curve size straight from the DER length of the prime INTEGER with no upper bound, and wc_EccPublicKeyDecode() then installed that curve on the caller's key. Every fixed buffer in ecc.c is dimensioned for MAX_ECC_BYTES, so a curve larger than that left the export and encode paths padding a coordinate outside its buffer, with both the offset and the bytes written taken from the key. One of those paths reaches it through the library's own allocation, with no output buffer from the caller at all. Reject the parameters where the size is taken from input, which is where the static ecc_sets[] table establishes the same invariant for every named curve. wc_ecc_set_custom_curve() applies the bound too, so a curve installed through the API cannot exceed the fixed buffers either, and the WOLFSSL_ECC_CURVE_STATIC branch bounds the a/b/order lengths, which are hex-encoded into MAX_ECC_STRING arrays from their own unbounded DER lengths. wc_ecc_set_custom_curve() rejects a non-positive size as well, matching the decoder: size 0 let a one byte 0x04 satisfy the importer's length check and yield a degenerate public key. Both reject paths log. Behaviour change: explicit-parameter EC keys and certificates whose prime is longer than MAX_ECC_BYTES (66 bytes without SAKKE, 128 with it) now fail to decode with ASN_PARSE_E instead of being accepted with a dp the rest of the library cannot handle safely. Every standard prime curve up to P-521 still decodes. Known limitation in WOLFSSL_ECC_CURVE_STATIC builds: the order bound is MAX_ECC_BYTES, but the Koblitz curves carry an order one byte wider than their prime, so in a build whose MAX_ECC_BYTES equals that prime size an explicit parameter key for one of them is now refused. MAX_ECC_STRING cannot hold its hex form, which is what the unbounded DataToHexString() overflowed before, so this fails closed where it used to corrupt the struct. Test: test_wc_EccPublicKeyDecode_explicit_curve_size, which builds the SubjectPublicKeyInfo at run time so the control prime tracks MAX_ECC_BYTES in whatever configuration it is built for. --- tests/api/test_ecc.c | 162 +++++++++++++++++++++++++++++++++++++++++++ tests/api/test_ecc.h | 5 +- wolfcrypt/src/asn.c | 19 +++++ wolfcrypt/src/ecc.c | 5 ++ 4 files changed, 190 insertions(+), 1 deletion(-) diff --git a/tests/api/test_ecc.c b/tests/api/test_ecc.c index f841b71421d..11f610b3c7d 100644 --- a/tests/api/test_ecc.c +++ b/tests/api/test_ecc.c @@ -4136,3 +4136,165 @@ int test_wc_EccDecisionCoverage4(void) #endif /* HAVE_ECC && !WC_NO_RNG && !WOLF_CRYPTO_CB_ONLY_ECC */ return EXPECT_RESULT(); } /* END test_wc_EccDecisionCoverage4 */ + +#if defined(HAVE_ECC) && defined(WOLFSSL_CUSTOM_CURVES) && \ + defined(WOLFSSL_ASN_TEMPLATE) && defined(HAVE_ECC_KEY_IMPORT) && \ + defined(HAVE_ECC_KEY_EXPORT) && !defined(NO_ASN) + +#define ECC_SPEC_COORD_SZ MAX_ECC_BYTES +#define ECC_SPEC_MAX_PRIME 300 +#define ECC_SPEC_BUF_SZ ((ECC_SPEC_MAX_PRIME * 7) + 512) + +static void EccSpecFill(byte* buf, word32* idx, byte val, word32 len) +{ + *idx -= len; + XMEMSET(buf + *idx, val, len); +} + +static void EccSpecHdr(byte* buf, word32* idx, byte tag, word32 len) +{ + if (len < 128) { + buf[--(*idx)] = (byte)len; + } + else if (len < 256) { + buf[--(*idx)] = (byte)len; + buf[--(*idx)] = 0x81; + } + else { + buf[--(*idx)] = (byte)len; + buf[--(*idx)] = (byte)(len >> 8); + buf[--(*idx)] = 0x82; + } + buf[--(*idx)] = tag; +} + +/* Build an ecPublicKey SubjectPublicKeyInfo carrying explicit + * SpecifiedECDomain parameters with a prime of primeSz bytes. Encoded back to + * front, so each item is wrapped once its content is in place. */ +static word32 EccSpecifiedSpki(byte* buf, word32 primeSz) +{ + static const byte ecPubKeyOid[] = { + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01 + }; + static const byte primeFldOid[] = { + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x01, 0x01 + }; + word32 idx = ECC_SPEC_BUF_SZ; + word32 end = ECC_SPEC_BUF_SZ; + word32 domEnd; + + /* Subject public key: BIT STRING of 0x04 . */ + EccSpecFill(buf, &idx, 0x42, ECC_SPEC_COORD_SZ); + EccSpecFill(buf, &idx, 0x41, ECC_SPEC_COORD_SZ); + buf[--idx] = 0x04; + buf[--idx] = 0x00; + EccSpecHdr(buf, &idx, ASN_BIT_STRING, end - idx); + domEnd = idx; + + /* SpecifiedECDomain: version, field, curve, base point, order. */ + EccSpecFill(buf, &idx, 0x7f, primeSz); + EccSpecHdr(buf, &idx, ASN_INTEGER, primeSz); + end = idx; + EccSpecFill(buf, &idx, 0x03, primeSz * 2); + buf[--idx] = 0x04; + EccSpecHdr(buf, &idx, ASN_OCTET_STRING, end - idx); + end = idx; + EccSpecFill(buf, &idx, 0x02, primeSz); + EccSpecHdr(buf, &idx, ASN_OCTET_STRING, primeSz); + EccSpecFill(buf, &idx, 0x01, primeSz); + EccSpecHdr(buf, &idx, ASN_OCTET_STRING, primeSz); + EccSpecHdr(buf, &idx, ASN_SEQUENCE | ASN_CONSTRUCTED, end - idx); + end = idx; + EccSpecFill(buf, &idx, 0x7f, primeSz); + EccSpecHdr(buf, &idx, ASN_INTEGER, primeSz); + idx -= sizeof(primeFldOid); + XMEMCPY(buf + idx, primeFldOid, sizeof(primeFldOid)); + EccSpecHdr(buf, &idx, ASN_OBJECT_ID, sizeof(primeFldOid)); + EccSpecHdr(buf, &idx, ASN_SEQUENCE | ASN_CONSTRUCTED, end - idx); + buf[--idx] = 0x01; + EccSpecHdr(buf, &idx, ASN_INTEGER, 1); + EccSpecHdr(buf, &idx, ASN_SEQUENCE | ASN_CONSTRUCTED, domEnd - idx); + + /* AlgorithmIdentifier and the enclosing SubjectPublicKeyInfo. */ + idx -= sizeof(ecPubKeyOid); + XMEMCPY(buf + idx, ecPubKeyOid, sizeof(ecPubKeyOid)); + EccSpecHdr(buf, &idx, ASN_OBJECT_ID, sizeof(ecPubKeyOid)); + EccSpecHdr(buf, &idx, ASN_SEQUENCE | ASN_CONSTRUCTED, domEnd - idx); + EccSpecHdr(buf, &idx, ASN_SEQUENCE | ASN_CONSTRUCTED, + ECC_SPEC_BUF_SZ - idx); + + XMEMMOVE(buf, buf + idx, ECC_SPEC_BUF_SZ - idx); + return ECC_SPEC_BUF_SZ - idx; +} +#endif + +int test_wc_EccPublicKeyDecode_explicit_curve_size(void) +{ + EXPECT_DECLS; +#if defined(HAVE_ECC) && defined(WOLFSSL_CUSTOM_CURVES) && \ + defined(WOLFSSL_ASN_TEMPLATE) && defined(HAVE_ECC_KEY_IMPORT) && \ + defined(HAVE_ECC_KEY_EXPORT) && !defined(NO_ASN) + ecc_key key; + byte* der = NULL; + byte* out = NULL; + word32 derSz; + word32 idx; +#ifndef WOLFSSL_VALIDATE_ECC_IMPORT + word32 outSz; +#endif + + ExpectNotNull(der = (byte*)XMALLOC(ECC_SPEC_BUF_SZ, NULL, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectNotNull(out = (byte*)XMALLOC((ECC_SPEC_MAX_PRIME * 2) + 1, NULL, + DYNAMIC_TYPE_TMP_BUFFER)); + +#ifndef WOLFSSL_VALIDATE_ECC_IMPORT + /* A prime the ECC code is dimensioned for still decodes and exports. The + * fabricated point is not on the fabricated curve, so import validation + * would reject it. */ + if (EXPECT_SUCCESS()) { + XMEMSET(&key, 0, sizeof(key)); + derSz = EccSpecifiedSpki(der, MAX_ECC_BYTES); + idx = 0; + ExpectIntEQ(wc_ecc_init(&key), 0); + ExpectIntEQ(wc_EccPublicKeyDecode(der, &idx, &key, derSz), 0); + ExpectNotNull(key.dp); + ExpectIntEQ(key.dp != NULL ? key.dp->size : 0, MAX_ECC_BYTES); + outSz = (MAX_ECC_BYTES * 2) + 1; + ExpectIntEQ(wc_ecc_export_x963(&key, out, &outSz), 0); + ExpectIntEQ(outSz, (MAX_ECC_BYTES * 2) + 1); + wc_ecc_free(&key); + } +#endif + + /* A prime past MAX_ECC_BYTES must be rejected at decode: the curve size + * drives the padding offset into a fixed ECC_BUFSIZE stack buffer in + * _ecc_export_x963(). */ + if (EXPECT_SUCCESS()) { + XMEMSET(&key, 0, sizeof(key)); + derSz = EccSpecifiedSpki(der, ECC_SPEC_MAX_PRIME); + idx = 0; + ExpectIntEQ(wc_ecc_init(&key), 0); + ExpectIntEQ(wc_EccPublicKeyDecode(der, &idx, &key, derSz), + WC_NO_ERR_TRACE(ASN_PARSE_E)); + wc_ecc_free(&key); + } + + /* The same bound applies to a custom curve set through the API. */ + if (EXPECT_SUCCESS()) { + ecc_set_type dp; + + XMEMSET(&key, 0, sizeof(key)); + XMEMSET(&dp, 0, sizeof(dp)); + dp.size = ECC_SPEC_MAX_PRIME; + ExpectIntEQ(wc_ecc_init(&key), 0); + ExpectIntEQ(wc_ecc_set_custom_curve(&key, &dp), + WC_NO_ERR_TRACE(ECC_BAD_ARG_E)); + wc_ecc_free(&key); + } + + XFREE(out, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER); +#endif + return EXPECT_RESULT(); +} /* END test_wc_EccPublicKeyDecode_explicit_curve_size */ diff --git a/tests/api/test_ecc.h b/tests/api/test_ecc.h index acaa4bb349b..ddce59e3e9f 100644 --- a/tests/api/test_ecc.h +++ b/tests/api/test_ecc.h @@ -75,6 +75,7 @@ int test_wc_EccDecisionCoverage(void); int test_wc_EccDecisionCoverage2(void); int test_wc_EccDecisionCoverage3(void); int test_wc_EccDecisionCoverage4(void); +int test_wc_EccPublicKeyDecode_explicit_curve_size(void); #define TEST_ECC_DECLS \ TEST_DECL_GROUP("ecc", test_wc_ecc_get_curve_size_from_name), \ @@ -127,6 +128,8 @@ int test_wc_EccDecisionCoverage4(void); TEST_DECL_GROUP("ecc", test_wc_EccDecisionCoverage), \ TEST_DECL_GROUP("ecc", test_wc_EccDecisionCoverage2), \ TEST_DECL_GROUP("ecc", test_wc_EccDecisionCoverage3), \ - TEST_DECL_GROUP("ecc", test_wc_EccDecisionCoverage4) + TEST_DECL_GROUP("ecc", test_wc_EccDecisionCoverage4), \ + TEST_DECL_GROUP("ecc", \ + test_wc_EccPublicKeyDecode_explicit_curve_size) #endif /* WOLFCRYPT_TEST_ECC_H */ diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 2df6050a862..4dc63d7c314 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -34257,6 +34257,14 @@ static int EccSpecifiedECDomainDecode(const byte* input, word32 inSz, /* Length of the prime in bytes is the curve size. */ curve->size = (int)dataASN[ECCSPECIFIEDASN_IDX_PRIME_P].data.ref.length; + /* Curve size must fit the fixed size buffers in the ECC code. */ + if ((curve->size <= 0) || (curve->size > MAX_ECC_BYTES) || + (curve->size > ECC_MAXSIZE)) { + WOLFSSL_MSG("ECC explicit domain prime outside MAX_ECC_BYTES"); + ret = ASN_PARSE_E; + } + } + if (ret == 0) { /* Base point: 0x04 (must be uncompressed). */ GetASN_GetConstRef(&dataASN[ECCSPECIFIEDASN_IDX_BASE], &base, &baseLen); @@ -34322,6 +34330,17 @@ static int EccSpecifiedECDomainDecode(const byte* input, word32 inSz, curve->order = curve_order; } #else + /* Hex strings are held in fixed size MAX_ECC_STRING buffers. */ + if ((ret == 0) && + ((dataASN[ECCSPECIFIEDASN_IDX_PARAM_A].data.ref.length > + (word32)MAX_ECC_BYTES) || + (dataASN[ECCSPECIFIEDASN_IDX_PARAM_B].data.ref.length > + (word32)MAX_ECC_BYTES) || + (dataASN[ECCSPECIFIEDASN_IDX_ORDER].data.ref.length > + (word32)MAX_ECC_BYTES))) { + WOLFSSL_MSG("ECC explicit domain parameter too long for hex buffer"); + ret = ASN_PARSE_E; + } if (ret == 0) { /* Base X-ordinate */ DataToHexString(base + 1, (word32)curve->size, (char *)curve->Gx); diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 4609def586c..c66eef92ead 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -17293,6 +17293,11 @@ int wc_ecc_set_custom_curve(ecc_key* key, const ecc_set_type* dp) if (key == NULL || dp == NULL) { return BAD_FUNC_ARG; } + /* Curve size must fit the fixed size buffers in the ECC code. */ + if ((dp->size <= 0) || (dp->size > MAX_ECC_BYTES) || + (dp->size > ECC_MAXSIZE)) { + return ECC_BAD_ARG_E; + } key->idx = ECC_CUSTOM_IDX; key->dp = dp; From eda7614f123e58a44963c19ebe9e205a4f3e50ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 16 Sep 2026 15:59:16 +0200 Subject: [PATCH 6/6] ecc: bound X9.63 ordinates to the curve and range check SP points _ecc_import_x963_ex2() took the ordinate width from the input length and never compared it with the curve, so a SubjectPublicKeyInfo naming one curve but carrying wider ordinates became a key on that curve holding oversized ordinates. wc_ecc_set_curve() bounds the size only by ECC_MAXSIZE and, when given a curve id, selects the curve by id without comparing sizes. The untrusted-import validation did not catch it in SP builds: sp_ecc_is_point_256/384/521() copy only the low limbs out of the mp_int, so the truncated point is on the curve and the import succeeds, where the generic wc_ecc_is_point() returns ECC_OUT_OF_RANGE_E. Every caller of the importer is affected, including wc_EccPublicKeyDecode(), the TLS 1.2 and TLS 1.3 key exchange, PKCS#7 KARI, HPKE and the compat d2i_* functions; none of them compares the wire length with the curve size first. wolfSSL_EC_POINT_point2hex() then placed the x-ordinate at an offset derived from the curve size minus the ordinate size, which goes negative once an ordinate is wider than the curve. The compressed form and the y ordinate degenerate the same way. _ecc_import_x963_ex2() now requires the ordinate size to equal key->dp->size, returning ECC_BAD_ARG_E otherwise. This is the check wc_ecc_import_point_der_ex() already has, placed in the shared importer so it covers every caller and every math back end. wolfSSL_EC_POINT_point2hex() refuses ordinates larger than the curve before computing any offset, mirroring wc_ecc_export_point_der(), whose guard wolfSSL_EC_POINT_point2oct() already inherits, and picks up the NULL ordinate check that its new bound would otherwise be the first to dereference past. sp_ecc_is_point_256/384/521/1024() in all eight SP back ends gain the mp_count_bits() and modulus comparison that the neighbouring sp_ecc_check_key_*() already carries, so SP builds no longer accept a same-length non-canonical encoding that every other back end rejects. Both functions also gain the mp_isneg() test the generic implementation has: sp__from_mp() ignores the sign, so a negative ordinate was read as its magnitude. sp_ecc_check_key_*() lacked that check before this change as well. The sp_*.c files are generator output; the change was made in wolfSSL/scripts sp/ecc.rb and the files regenerated, and the result is byte for byte what the modified generator produces. Behaviour change: X9.63 encodings shorter than the curve size are rejected as well. They are non-canonical under SEC 1 section 2.3.4 and RFC 5480 and OpenSSL rejects them too. With ECC_CURVE_DEF the size-based curve lookup still resolves exact-size encodings as before. Tests: test_wc_ecc_import_x963_oversized and test_wc_ecc_import_x963_non_canonical cover the importer, test_wolfSSL_EC_POINT_point2hex_oversized covers the sink, and all three fail on the unfixed tree. The SP MC/DC drivers gain the new bit-length and sign branches and an operand exactly the field's width but not below the modulus, without which the new range compare was only ever reached with the bit-length guard already true. Those rows now assert ECC_OUT_OF_RANGE_E rather than discarding the result, and report a failure separately from a skipped row; removing either guard from a back end makes them fail. --- src/pk_ec.c | 11 ++ tests/api/test_ecc.c | 146 +++++++++++++++++++++++ tests/api/test_ecc.h | 4 + tests/api/test_ossl_ec.c | 76 ++++++++++++ tests/api/test_ossl_ec.h | 4 +- tests/unit-mcdc/test_sp_arm64_whitebox.c | 89 ++++++++++++-- tests/unit-mcdc/test_sp_c64_whitebox.c | 89 ++++++++++++-- wolfcrypt/src/ecc.c | 8 ++ wolfcrypt/src/sp_arm32.c | 100 +++++++++++++--- wolfcrypt/src/sp_arm64.c | 100 +++++++++++++--- wolfcrypt/src/sp_armthumb.c | 100 +++++++++++++--- wolfcrypt/src/sp_c32.c | 100 +++++++++++++--- wolfcrypt/src/sp_c64.c | 100 +++++++++++++--- wolfcrypt/src/sp_cortexm.c | 100 +++++++++++++--- wolfcrypt/src/sp_riscv64.c | 100 +++++++++++++--- wolfcrypt/src/sp_x86_64.c | 100 +++++++++++++--- 16 files changed, 1084 insertions(+), 143 deletions(-) diff --git a/src/pk_ec.c b/src/pk_ec.c index f6df291a89a..c3b306d3ebe 100644 --- a/src/pk_ec.c +++ b/src/pk_ec.c @@ -1249,6 +1249,17 @@ char* wolfSSL_EC_POINT_point2hex(const WOLFSSL_EC_GROUP* group, err = 1; } } + /* A point that has never been set carries NULL ordinates. */ + if ((!err) && ((point->X == NULL) || (point->Y == NULL) || + (point->X->internal == NULL) || (point->Y->internal == NULL))) { + err = 1; + } + /* Ordinates wider than the curve would make the offsets below negative. */ + if ((!err) && + ((mp_unsigned_bin_size((mp_int*)point->X->internal) > sz) || + (mp_unsigned_bin_size((mp_int*)point->Y->internal) > sz))) { + err = 1; + } if (!err) { /* [] */ len = sz + 1; diff --git a/tests/api/test_ecc.c b/tests/api/test_ecc.c index 11f610b3c7d..2b88c0cab98 100644 --- a/tests/api/test_ecc.c +++ b/tests/api/test_ecc.c @@ -1120,6 +1120,152 @@ int test_wc_ecc_import_x963_off_curve(void) return EXPECT_RESULT(); } /* END test_wc_ecc_import_x963_off_curve */ +/* + * testing wc_ecc_import_x963() rejects a point whose ordinates are not the + * size of the curve. + * + * _ecc_import_x963_ex2() took the ordinate size from the input length, so a + * P-256 point carrying 64-byte ordinates became a P-256 key holding 512-bit + * ordinates. SP builds accepted it because sp_ecc_is_point_256() looked only + * at the low limbs, and wolfSSL_EC_POINT_point2hex() then wrote before its + * buffer. + */ +int test_wc_ecc_import_x963_oversized(void) +{ + EXPECT_DECLS; +#if defined(HAVE_ECC) && defined(HAVE_ECC_KEY_IMPORT) && \ + defined(HAVE_ECC_KEY_EXPORT) && \ + !defined(NO_ECC256) && !defined(NO_ECC_SECP) && \ + (!defined(HAVE_FIPS) || FIPS_VERSION_GE(7,0)) && !defined(HAVE_SELFTEST) && \ + !defined(WOLF_CRYPTO_CB_ONLY_ECC) + ecc_key pubKey; + byte out[80]; + word32 outLen = (word32)sizeof(out); + /* The P-256 generator, canonically encoded. */ + static const byte canonX963[] = { + 0x04, + 0x6B, 0x17, 0xD1, 0xF2, 0xE1, 0x2C, 0x42, 0x47, + 0xF8, 0xBC, 0xE6, 0xE5, 0x63, 0xA4, 0x40, 0xF2, + 0x77, 0x03, 0x7D, 0x81, 0x2D, 0xEB, 0x33, 0xA0, + 0xF4, 0xA1, 0x39, 0x45, 0xD8, 0x98, 0xC2, 0x96, + 0x4F, 0xE3, 0x42, 0xE2, 0xFE, 0x1A, 0x7F, 0x9B, + 0x8E, 0xE7, 0xEB, 0x4A, 0x7C, 0x0F, 0x9E, 0x16, + 0x2B, 0xCE, 0x33, 0x57, 0x6B, 0x31, 0x5E, 0xCE, + 0xCB, 0xB6, 0x40, 0x68, 0x37, 0xBF, 0x51, 0xF5 + }; + /* The same point with each ordinate left-padded to 64 bytes. The + * zero byte below each ordinate keeps the truncated point on the curve + * for the 5 x 52-bit back end, so the padding alone decides the result. */ + static const byte oversizedX963[] = { + 0x04, + 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, + 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, + 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, + 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x00, + 0x6B, 0x17, 0xD1, 0xF2, 0xE1, 0x2C, 0x42, 0x47, + 0xF8, 0xBC, 0xE6, 0xE5, 0x63, 0xA4, 0x40, 0xF2, + 0x77, 0x03, 0x7D, 0x81, 0x2D, 0xEB, 0x33, 0xA0, + 0xF4, 0xA1, 0x39, 0x45, 0xD8, 0x98, 0xC2, 0x96, + 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, + 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, + 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, + 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x00, + 0x4F, 0xE3, 0x42, 0xE2, 0xFE, 0x1A, 0x7F, 0x9B, + 0x8E, 0xE7, 0xEB, 0x4A, 0x7C, 0x0F, 0x9E, 0x16, + 0x2B, 0xCE, 0x33, 0x57, 0x6B, 0x31, 0x5E, 0xCE, + 0xCB, 0xB6, 0x40, 0x68, 0x37, 0xBF, 0x51, 0xF5 + }; + + XMEMSET(&pubKey, 0, sizeof(ecc_key)); + ExpectIntEQ(wc_ecc_init(&pubKey), 0); + ExpectIntEQ(wc_ecc_import_x963_ex(canonX963, (word32)sizeof(canonX963), + &pubKey, ECC_SECP256R1), 0); + ExpectIntEQ(wc_ecc_export_x963(&pubKey, out, &outLen), 0); + ExpectIntEQ(outLen, (word32)sizeof(canonX963)); + wc_ecc_free(&pubKey); + + XMEMSET(&pubKey, 0, sizeof(ecc_key)); + ExpectIntEQ(wc_ecc_init(&pubKey), 0); + ExpectIntEQ(wc_ecc_import_x963_ex(oversizedX963, + (word32)sizeof(oversizedX963), &pubKey, ECC_SECP256R1), + WC_NO_ERR_TRACE(ECC_BAD_ARG_E)); + wc_ecc_free(&pubKey); + + /* The size-based curve lookup must reject it as well. */ + XMEMSET(&pubKey, 0, sizeof(ecc_key)); + ExpectIntEQ(wc_ecc_init(&pubKey), 0); + ExpectIntNE(wc_ecc_import_x963(oversizedX963, + (word32)sizeof(oversizedX963), &pubKey), 0); + wc_ecc_free(&pubKey); + +#ifdef FP_ECC + wc_ecc_fp_free(); +#endif +#endif + return EXPECT_RESULT(); +} /* END test_wc_ecc_import_x963_oversized */ + +/* + * testing wc_ecc_import_x963() rejects an ordinate that is not less than the + * field prime. + * + * sp_ecc_is_point_256/384/521() reduced the ordinates implicitly, so SP builds + * accepted the non-canonical encoding 04 || (x + p) || y of a valid point that + * every other back end, and SEC 1 section 2.3.4, rejects. + */ +int test_wc_ecc_import_x963_non_canonical(void) +{ + EXPECT_DECLS; +#if defined(HAVE_ECC) && defined(HAVE_ECC_KEY_IMPORT) && \ + !defined(NO_ECC256) && !defined(NO_ECC_SECP) && \ + (!defined(HAVE_FIPS) || FIPS_VERSION_GE(7,0)) && !defined(HAVE_SELFTEST) && \ + !defined(WOLF_CRYPTO_CB_ONLY_ECC) + ecc_key pubKey; + /* P-256 point (5, y). */ + static const byte canonX963[] = { + 0x04, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, + 0x45, 0x92, 0x43, 0xB9, 0xAA, 0x58, 0x18, 0x06, + 0xFE, 0x91, 0x3B, 0xCE, 0x99, 0x81, 0x7A, 0xDE, + 0x11, 0xCA, 0x50, 0x3C, 0x64, 0xD9, 0xA3, 0xC5, + 0x33, 0x41, 0x5C, 0x08, 0x32, 0x48, 0xFB, 0xCC + }; + /* The same point with x replaced by x + p; still 32 bytes wide. */ + static const byte nonCanonX963[] = { + 0x04, + 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, + 0x45, 0x92, 0x43, 0xB9, 0xAA, 0x58, 0x18, 0x06, + 0xFE, 0x91, 0x3B, 0xCE, 0x99, 0x81, 0x7A, 0xDE, + 0x11, 0xCA, 0x50, 0x3C, 0x64, 0xD9, 0xA3, 0xC5, + 0x33, 0x41, 0x5C, 0x08, 0x32, 0x48, 0xFB, 0xCC + }; + + XMEMSET(&pubKey, 0, sizeof(ecc_key)); + ExpectIntEQ(wc_ecc_init(&pubKey), 0); + ExpectIntEQ(wc_ecc_import_x963_ex(canonX963, (word32)sizeof(canonX963), + &pubKey, ECC_SECP256R1), 0); + wc_ecc_free(&pubKey); + + XMEMSET(&pubKey, 0, sizeof(ecc_key)); + ExpectIntEQ(wc_ecc_init(&pubKey), 0); + ExpectIntEQ(wc_ecc_import_x963_ex(nonCanonX963, + (word32)sizeof(nonCanonX963), &pubKey, ECC_SECP256R1), + WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)); + wc_ecc_free(&pubKey); + +#ifdef FP_ECC + wc_ecc_fp_free(); +#endif +#endif + return EXPECT_RESULT(); +} /* END test_wc_ecc_import_x963_non_canonical */ + /* * testing wc_ecc_import_private_key() */ diff --git a/tests/api/test_ecc.h b/tests/api/test_ecc.h index ddce59e3e9f..7f19e1ef10a 100644 --- a/tests/api/test_ecc.h +++ b/tests/api/test_ecc.h @@ -43,6 +43,8 @@ int test_wc_ecc_export_x963(void); int test_wc_ecc_export_x963_ex(void); int test_wc_ecc_import_x963(void); int test_wc_ecc_import_x963_off_curve(void); +int test_wc_ecc_import_x963_oversized(void); +int test_wc_ecc_import_x963_non_canonical(void); int test_wc_ecc_import_private_key(void); int test_wc_ecc_export_private_only(void); int test_wc_ecc_rs_to_sig(void); @@ -97,6 +99,8 @@ int test_wc_EccPublicKeyDecode_explicit_curve_size(void); TEST_DECL_GROUP("ecc", test_wc_ecc_export_x963_ex), \ TEST_DECL_GROUP("ecc", test_wc_ecc_import_x963), \ TEST_DECL_GROUP("ecc", test_wc_ecc_import_x963_off_curve), \ + TEST_DECL_GROUP("ecc", test_wc_ecc_import_x963_oversized), \ + TEST_DECL_GROUP("ecc", test_wc_ecc_import_x963_non_canonical), \ TEST_DECL_GROUP("ecc", test_wc_ecc_import_private_key), \ TEST_DECL_GROUP("ecc", test_wc_ecc_export_private_only), \ TEST_DECL_GROUP("ecc", test_wc_ecc_rs_to_sig), \ diff --git a/tests/api/test_ossl_ec.c b/tests/api/test_ossl_ec.c index 5015658ccac..50b8f523d0a 100644 --- a/tests/api/test_ossl_ec.c +++ b/tests/api/test_ossl_ec.c @@ -1699,6 +1699,82 @@ int test_ECDH_compute_key(void) return EXPECT_RESULT(); } +/* + * testing EC_POINT_point2hex() rejects a point whose ordinates are wider than + * the curve. + * + * The x-ordinate offset was computed as sz - mp_unsigned_bin_size(X) + 1, + * which goes negative once an ordinate exceeds the curve size, writing + * attacker-chosen bytes in front of the freshly allocated buffer. + */ +int test_wolfSSL_EC_POINT_point2hex_oversized(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_ECC256) && !defined(NO_ECC_SECP) && \ + !defined(WOLFSSL_SP_MATH) && \ + (!defined(HAVE_FIPS) || (defined(HAVE_FIPS_VERSION) && \ + (HAVE_FIPS_VERSION > 2))) + EC_GROUP* group = NULL; + EC_POINT* point = NULL; + BIGNUM* x = NULL; + BIGNUM* y = NULL; + char* hexStr = NULL; + /* P-256 point (5, y). */ + static const byte smallX[32] = { + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05 + }; + static const byte smallY[32] = { + 0x45, 0x92, 0x43, 0xB9, 0xAA, 0x58, 0x18, 0x06, + 0xFE, 0x91, 0x3B, 0xCE, 0x99, 0x81, 0x7A, 0xDE, + 0x11, 0xCA, 0x50, 0x3C, 0x64, 0xD9, 0xA3, 0xC5, + 0x33, 0x41, 0x5C, 0x08, 0x32, 0x48, 0xFB, 0xCC + }; + /* 64 bytes: twice the P-256 ordinate size. */ + static const byte wideOrdinate[64] = { + 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, + 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, + 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, + 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x00, + 0x6B, 0x17, 0xD1, 0xF2, 0xE1, 0x2C, 0x42, 0x47, + 0xF8, 0xBC, 0xE6, 0xE5, 0x63, 0xA4, 0x40, 0xF2, + 0x77, 0x03, 0x7D, 0x81, 0x2D, 0xEB, 0x33, 0xA0, + 0xF4, 0xA1, 0x39, 0x45, 0xD8, 0x98, 0xC2, 0x96 + }; + + ExpectNotNull(group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1)); + ExpectNotNull(point = EC_POINT_new(group)); + ExpectNotNull(x = BN_bin2bn(smallX, (int)sizeof(smallX), NULL)); + ExpectNotNull(y = BN_bin2bn(smallY, (int)sizeof(smallY), NULL)); + ExpectIntEQ(EC_POINT_set_affine_coordinates_GFp(group, point, x, y, NULL), + 1); + + /* Control: a well-formed point converts. */ + ExpectNotNull(hexStr = EC_POINT_point2hex(group, point, + POINT_CONVERSION_UNCOMPRESSED, NULL)); + XFREE(hexStr, NULL, DYNAMIC_TYPE_ECC); + hexStr = NULL; + + /* Widen the x-ordinate the way an over-long X9.63 import used to. */ + ExpectNotNull(BN_bin2bn(wideOrdinate, (int)sizeof(wideOrdinate), point->X)); + ExpectNull(hexStr = EC_POINT_point2hex(group, point, + POINT_CONVERSION_UNCOMPRESSED, NULL)); + XFREE(hexStr, NULL, DYNAMIC_TYPE_ECC); + hexStr = NULL; + ExpectNull(hexStr = EC_POINT_point2hex(group, point, + POINT_CONVERSION_COMPRESSED, NULL)); + XFREE(hexStr, NULL, DYNAMIC_TYPE_ECC); + + BN_free(y); + BN_free(x); + EC_POINT_free(point); + EC_GROUP_free(group); +#endif + return EXPECT_RESULT(); +} + /* Test that d2i_ECPrivateKey derives the public point when the optional * publicKey [1] field is absent from the RFC 5915 DER encoding. * diff --git a/tests/api/test_ossl_ec.h b/tests/api/test_ossl_ec.h index ddb81966f6c..3bfbdc9d1c7 100644 --- a/tests/api/test_ossl_ec.h +++ b/tests/api/test_ossl_ec.h @@ -45,6 +45,7 @@ int test_wolfSSL_ECDSA_SIG(void); int test_ECDSA_size_sign(void); int test_ECDH_compute_key(void); int test_d2i_ECPrivateKey_no_pubkey(void); +int test_wolfSSL_EC_POINT_point2hex_oversized(void); #define TEST_OSSL_EC_DECLS \ @@ -66,7 +67,8 @@ int test_d2i_ECPrivateKey_no_pubkey(void); TEST_DECL_GROUP("ossl_ec", test_wolfSSL_ECDSA_SIG), \ TEST_DECL_GROUP("ossl_ec", test_ECDSA_size_sign), \ TEST_DECL_GROUP("ossl_ec", test_ECDH_compute_key), \ - TEST_DECL_GROUP("ossl_ec", test_d2i_ECPrivateKey_no_pubkey) + TEST_DECL_GROUP("ossl_ec", test_d2i_ECPrivateKey_no_pubkey), \ + TEST_DECL_GROUP("ossl_ec", test_wolfSSL_EC_POINT_point2hex_oversized) #endif diff --git a/tests/unit-mcdc/test_sp_arm64_whitebox.c b/tests/unit-mcdc/test_sp_arm64_whitebox.c index a564ae241f1..1528c507865 100644 --- a/tests/unit-mcdc/test_sp_arm64_whitebox.c +++ b/tests/unit-mcdc/test_sp_arm64_whitebox.c @@ -131,6 +131,8 @@ #include static int wb_fail = 0; +/* A guard that did not fire, as opposed to a row wb_fail marks as skipped. */ +static int wb_bad = 0; #define WB_NOTE(msg) do { printf(" [wb] %s\n", (msg)); } while (0) /* Crafted-input driver shared with the SP host-backend white-boxes. The four @@ -434,6 +436,17 @@ static void wb_run_mulmod_add_all(void) * mp_int inputs (no need for a valid key -- these functions only inspect * the ordinates handed to them). * ----------------------------------------------------------------------- */ +/* A row that targets a range or sign guard must be rejected by it; anything + * else means that guard was lost in a regeneration of the SP back end. */ +static void wb_expect_range(int ret, const char* what) +{ + if (ret != WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)) { + printf(" [wb][FAIL] %s did not return ECC_OUT_OF_RANGE_E (got %d)\n", + what, ret); + wb_bad = 1; + } +} + static void wb_run_point_specials(int fieldBits, const char* label, int (*is_point)(const mp_int*, const mp_int*), int (*check_key)(const mp_int*, const mp_int*, const mp_int*, void*)) @@ -441,13 +454,23 @@ static void wb_run_point_specials(int fieldBits, const char* label, mp_int zero; mp_int small; mp_int big; + mp_int atMax; + mp_int neg; + int haveNeg; byte bigbuf[96]; + byte maxbuf[128]; int nbytes = fieldBits / 8 + 9; /* comfortably more bits than fieldBits */ + int maxbytes = (fieldBits + 7) / 8; + int topbits = fieldBits - ((maxbytes - 1) * 8); if (nbytes > (int)sizeof(bigbuf)) { nbytes = (int)sizeof(bigbuf); } XMEMSET(bigbuf, 0xFF, sizeof(bigbuf)); + /* Exactly fieldBits wide and all ones, so it is not less than any of the + * moduli here while still passing the bit-length guard. */ + XMEMSET(maxbuf, 0xFF, sizeof(maxbuf)); + maxbuf[0] = (byte)((1u << topbits) - 1u); if (mp_init(&zero) != MP_OKAY) { WB_NOTE("mp_init(zero) failed (point specials)"); @@ -467,21 +490,62 @@ static void wb_run_point_specials(int fieldBits, const char* label, mp_clear(&small); return; } + if (mp_init(&atMax) != MP_OKAY) { + WB_NOTE("mp_init(atMax) failed (point specials)"); + wb_fail = 1; + mp_clear(&zero); + mp_clear(&small); + mp_clear(&big); + return; + } + if (mp_init(&neg) != MP_OKAY) { + WB_NOTE("mp_init(neg) failed (point specials)"); + wb_fail = 1; + mp_clear(&zero); + mp_clear(&small); + mp_clear(&big); + mp_clear(&atMax); + return; + } mp_set(&small, 3); - if (mp_read_unsigned_bin(&big, bigbuf, (word32)nbytes) != MP_OKAY) { - WB_NOTE("mp_read_unsigned_bin(big) failed (point specials)"); + /* mp_setneg() is a no-op unless the math back end was built with negative + * support, so ask the value itself rather than the build macros. */ + mp_set(&neg, 3); + mp_setneg(&neg); + haveNeg = mp_isneg(&neg) ? 1 : 0; + if ((mp_read_unsigned_bin(&big, bigbuf, (word32)nbytes) != MP_OKAY) || + (mp_read_unsigned_bin(&atMax, maxbuf, (word32)maxbytes) + != MP_OKAY)) { + WB_NOTE("mp_read_unsigned_bin failed (point specials)"); wb_fail = 1; } else { - /* Point at infinity (x == 0 && y == 0). is_point() has no - * bit-length guard, so this only drives its general field math - * with a degenerate operand -- it is check_key() below that has - * the explicit "point at infinity" branch. */ + /* Point at infinity (x == 0 && y == 0). is_point() has no explicit + * "point at infinity" branch, so this drives its general field math + * with a degenerate operand; check_key() below has that branch. */ (void)is_point(&zero, &zero); /* A small, well-formed, off-curve pair: exercises the same field * math with a non-degenerate, non-infinity operand. */ (void)is_point(&small, &small); + /* mp_count_bits(pX) > fieldBits, independently true. */ + wb_expect_range(is_point(&big, &small), "is_point(big, small)"); + /* mp_count_bits(pY) > fieldBits, independently true. */ + wb_expect_range(is_point(&small, &big), "is_point(small, big)"); + /* The bit-length guard passes, so only sp__cmp_(pub->x, mod) + * >= 0 can reject: that arm independently true. */ + wb_expect_range(is_point(&atMax, &small), "is_point(atMax, small)"); + /* Same for the pub->y arm of that compare. */ + wb_expect_range(is_point(&small, &atMax), "is_point(small, atMax)"); + if (haveNeg) { + /* mp_isneg(pX), then mp_isneg(pY), independently true. */ + wb_expect_range(is_point(&neg, &small), "is_point(neg, small)"); + wb_expect_range(is_point(&small, &neg), "is_point(small, neg)"); + } + else { + WB_NOTE("math back end has no negative support; sign rows " + "skipped"); + } if (check_key != NULL) { /* (sp__iszero_(pub->x) != 0) && @@ -499,6 +563,13 @@ static void wb_run_point_specials(int fieldBits, const char* label, * not on the curve, so this reaches (and cleanly fails) that * logic without needing a real key. */ (void)check_key(&small, &small, &small, NULL); + if (haveNeg) { + /* mp_isneg(pX), then mp_isneg(pY), independently true. */ + wb_expect_range(check_key(&neg, &small, NULL, NULL), + "check_key(neg, small)"); + wb_expect_range(check_key(&small, &neg, NULL, NULL), + "check_key(small, neg)"); + } } else { WB_NOTE("check_key needs HAVE_ECC_CHECK_KEY || " @@ -506,6 +577,8 @@ static void wb_run_point_specials(int fieldBits, const char* label, } } + mp_clear(&neg); + mp_clear(&atMax); mp_clear(&big); mp_clear(&small); mp_clear(&zero); @@ -1565,10 +1638,12 @@ int main(void) wb_run_mod_inv(); wb_spc_all(); - printf("done (%s)\n", wb_fail ? "with skips" : "ok"); + printf("done (%s)\n", wb_bad ? "with failures" : + (wb_fail ? "with skips" : "ok")); #else printf(" no SP feature; nothing to exercise\n"); #endif (void)wb_fail; + (void)wb_bad; return 0; } diff --git a/tests/unit-mcdc/test_sp_c64_whitebox.c b/tests/unit-mcdc/test_sp_c64_whitebox.c index 2de781e2727..819f440ca6f 100644 --- a/tests/unit-mcdc/test_sp_c64_whitebox.c +++ b/tests/unit-mcdc/test_sp_c64_whitebox.c @@ -131,6 +131,8 @@ #include static int wb_fail = 0; +/* A guard that did not fire, as opposed to a row wb_fail marks as skipped. */ +static int wb_bad = 0; #define WB_NOTE(msg) do { printf(" [wb] %s\n", (msg)); } while (0) /* Crafted-input driver shared with the other two SP host-backend @@ -392,6 +394,17 @@ static void wb_run_mulmod_add_all(void) * mp_int inputs (no need for a valid key -- these functions only inspect * the ordinates handed to them). * ----------------------------------------------------------------------- */ +/* A row that targets a range or sign guard must be rejected by it; anything + * else means that guard was lost in a regeneration of the SP back end. */ +static void wb_expect_range(int ret, const char* what) +{ + if (ret != WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)) { + printf(" [wb][FAIL] %s did not return ECC_OUT_OF_RANGE_E (got %d)\n", + what, ret); + wb_bad = 1; + } +} + static void wb_run_point_specials(int fieldBits, const char* label, int (*is_point)(const mp_int*, const mp_int*), int (*check_key)(const mp_int*, const mp_int*, const mp_int*, void*)) @@ -399,13 +412,23 @@ static void wb_run_point_specials(int fieldBits, const char* label, mp_int zero; mp_int small; mp_int big; + mp_int atMax; + mp_int neg; + int haveNeg; byte bigbuf[96]; + byte maxbuf[128]; int nbytes = fieldBits / 8 + 9; /* comfortably more bits than fieldBits */ + int maxbytes = (fieldBits + 7) / 8; + int topbits = fieldBits - ((maxbytes - 1) * 8); if (nbytes > (int)sizeof(bigbuf)) { nbytes = (int)sizeof(bigbuf); } XMEMSET(bigbuf, 0xFF, sizeof(bigbuf)); + /* Exactly fieldBits wide and all ones, so it is not less than any of the + * moduli here while still passing the bit-length guard. */ + XMEMSET(maxbuf, 0xFF, sizeof(maxbuf)); + maxbuf[0] = (byte)((1u << topbits) - 1u); if (mp_init(&zero) != MP_OKAY) { WB_NOTE("mp_init(zero) failed (point specials)"); @@ -425,21 +448,62 @@ static void wb_run_point_specials(int fieldBits, const char* label, mp_clear(&small); return; } + if (mp_init(&atMax) != MP_OKAY) { + WB_NOTE("mp_init(atMax) failed (point specials)"); + wb_fail = 1; + mp_clear(&zero); + mp_clear(&small); + mp_clear(&big); + return; + } + if (mp_init(&neg) != MP_OKAY) { + WB_NOTE("mp_init(neg) failed (point specials)"); + wb_fail = 1; + mp_clear(&zero); + mp_clear(&small); + mp_clear(&big); + mp_clear(&atMax); + return; + } mp_set(&small, 3); - if (mp_read_unsigned_bin(&big, bigbuf, (word32)nbytes) != MP_OKAY) { - WB_NOTE("mp_read_unsigned_bin(big) failed (point specials)"); + /* mp_setneg() is a no-op unless the math back end was built with negative + * support, so ask the value itself rather than the build macros. */ + mp_set(&neg, 3); + mp_setneg(&neg); + haveNeg = mp_isneg(&neg) ? 1 : 0; + if ((mp_read_unsigned_bin(&big, bigbuf, (word32)nbytes) != MP_OKAY) || + (mp_read_unsigned_bin(&atMax, maxbuf, (word32)maxbytes) + != MP_OKAY)) { + WB_NOTE("mp_read_unsigned_bin failed (point specials)"); wb_fail = 1; } else { - /* Point at infinity (x == 0 && y == 0). is_point() has no - * bit-length guard, so this only drives its general field math - * with a degenerate operand -- it is check_key() below that has - * the explicit "point at infinity" branch. */ + /* Point at infinity (x == 0 && y == 0). is_point() has no explicit + * "point at infinity" branch, so this drives its general field math + * with a degenerate operand; check_key() below has that branch. */ (void)is_point(&zero, &zero); /* A small, well-formed, off-curve pair: exercises the same field * math with a non-degenerate, non-infinity operand. */ (void)is_point(&small, &small); + /* mp_count_bits(pX) > fieldBits, independently true. */ + wb_expect_range(is_point(&big, &small), "is_point(big, small)"); + /* mp_count_bits(pY) > fieldBits, independently true. */ + wb_expect_range(is_point(&small, &big), "is_point(small, big)"); + /* The bit-length guard passes, so only sp__cmp_(pub->x, mod) + * >= 0 can reject: that arm independently true. */ + wb_expect_range(is_point(&atMax, &small), "is_point(atMax, small)"); + /* Same for the pub->y arm of that compare. */ + wb_expect_range(is_point(&small, &atMax), "is_point(small, atMax)"); + if (haveNeg) { + /* mp_isneg(pX), then mp_isneg(pY), independently true. */ + wb_expect_range(is_point(&neg, &small), "is_point(neg, small)"); + wb_expect_range(is_point(&small, &neg), "is_point(small, neg)"); + } + else { + WB_NOTE("math back end has no negative support; sign rows " + "skipped"); + } if (check_key != NULL) { /* (sp__iszero_(pub->x) != 0) && @@ -457,6 +521,13 @@ static void wb_run_point_specials(int fieldBits, const char* label, * not on the curve, so this reaches (and cleanly fails) that * logic without needing a real key. */ (void)check_key(&small, &small, &small, NULL); + if (haveNeg) { + /* mp_isneg(pX), then mp_isneg(pY), independently true. */ + wb_expect_range(check_key(&neg, &small, NULL, NULL), + "check_key(neg, small)"); + wb_expect_range(check_key(&small, &neg, NULL, NULL), + "check_key(small, neg)"); + } } else { WB_NOTE("check_key needs HAVE_ECC_CHECK_KEY || " @@ -464,6 +535,8 @@ static void wb_run_point_specials(int fieldBits, const char* label, } } + mp_clear(&neg); + mp_clear(&atMax); mp_clear(&big); mp_clear(&small); mp_clear(&zero); @@ -892,10 +965,12 @@ int main(void) wb_run_point_specials_all(); wb_spc_all(); - printf("done (%s)\n", wb_fail ? "with skips" : "ok"); + printf("done (%s)\n", wb_bad ? "with failures" : + (wb_fail ? "with skips" : "ok")); #else printf(" no SP feature; nothing to exercise\n"); #endif (void)wb_fail; + (void)wb_bad; return 0; } diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index c66eef92ead..2ecf9d520f1 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -11682,6 +11682,14 @@ static int _ecc_import_x963_ex2(const byte* in, word32 inLen, ecc_key* key, } } + /* Ordinates must be exactly the curve's size, as wc_ecc_set_curve() only + * bounds keysize by ECC_MAXSIZE when a curve id is given. */ + if (err == MP_OKAY) { + if ((key->dp == NULL) || (keysize != key->dp->size)) { + err = ECC_BAD_ARG_E; + } + } + /* read data */ if (err == MP_OKAY) err = mp_read_unsigned_bin(key->pubkey.x, in, (word32)keysize); diff --git a/wolfcrypt/src/sp_arm32.c b/wolfcrypt/src/sp_arm32.c index 1b85225729a..32d8939d54c 100644 --- a/wolfcrypt/src/sp_arm32.c +++ b/wolfcrypt/src/sp_arm32.c @@ -81826,6 +81826,7 @@ static int sp_256_ecc_is_point_8(const sp_point_256* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) { @@ -81833,12 +81834,28 @@ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_256, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_256_from_mp(pub->x, 8, pX); sp_256_from_mp(pub->y, 8, pY); sp_256_from_bin(pub->z, 8, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_256_cmp_8(pub->x, p256_mod) >= 0) || + (sp_256_cmp_8(pub->y, p256_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_256_ecc_is_point_8(pub, NULL); } @@ -81873,12 +81890,12 @@ int sp_ecc_check_key_256(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || - ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + ((privm != NULL) && (mp_count_bits(privm) > 256)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -100374,6 +100391,7 @@ static int sp_384_ecc_is_point_12(const sp_point_384* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) { @@ -100381,12 +100399,28 @@ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_384, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_384_from_mp(pub->x, 12, pX); sp_384_from_mp(pub->y, 12, pY); sp_384_from_bin(pub->z, 12, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_384_cmp_12(pub->x, p384_mod) >= 0) || + (sp_384_cmp_12(pub->y, p384_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_384_ecc_is_point_12(pub, NULL); } @@ -100421,12 +100455,12 @@ int sp_ecc_check_key_384(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || - ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + ((privm != NULL) && (mp_count_bits(privm) > 384)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -129602,6 +129636,7 @@ static int sp_521_ecc_is_point_17(const sp_point_521* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) { @@ -129609,12 +129644,28 @@ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_521, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_521_from_mp(pub->x, 17, pX); sp_521_from_mp(pub->y, 17, pY); sp_521_from_bin(pub->z, 17, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_521_cmp_17(pub->x, p521_mod) >= 0) || + (sp_521_cmp_17(pub->y, p521_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_521_ecc_is_point_17(pub, NULL); } @@ -129649,12 +129700,12 @@ int sp_ecc_check_key_521(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || - ((privm != NULL) && (mp_count_bits(privm) > 521)))) { + ((privm != NULL) && (mp_count_bits(privm) > 521)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -160210,6 +160261,7 @@ static int sp_1024_ecc_is_point_32(const sp_point_1024* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) { @@ -160217,12 +160269,28 @@ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_1024, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_1024_from_mp(pub->x, 32, pX); sp_1024_from_mp(pub->y, 32, pY); sp_1024_from_bin(pub->z, 32, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_1024_cmp_32(pub->x, p1024_mod) >= 0) || + (sp_1024_cmp_32(pub->y, p1024_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_1024_ecc_is_point_32(pub, NULL); } @@ -160257,12 +160325,12 @@ int sp_ecc_check_key_1024(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || - ((privm != NULL) && (mp_count_bits(privm) > 1024)))) { + ((privm != NULL) && (mp_count_bits(privm) > 1024)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } diff --git a/wolfcrypt/src/sp_arm64.c b/wolfcrypt/src/sp_arm64.c index 4f013db6c94..e0daf17c98b 100644 --- a/wolfcrypt/src/sp_arm64.c +++ b/wolfcrypt/src/sp_arm64.c @@ -58172,6 +58172,7 @@ static int sp_256_ecc_is_point_4(const sp_point_256* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) { @@ -58179,12 +58180,28 @@ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_256, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_256_from_mp(pub->x, 4, pX); sp_256_from_mp(pub->y, 4, pY); sp_256_from_bin(pub->z, 4, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_256_cmp_4(pub->x, p256_mod) >= 0) || + (sp_256_cmp_4(pub->y, p256_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_256_ecc_is_point_4(pub, NULL); } @@ -58219,12 +58236,12 @@ int sp_ecc_check_key_256(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || - ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + ((privm != NULL) && (mp_count_bits(privm) > 256)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -84087,6 +84104,7 @@ static int sp_384_ecc_is_point_6(const sp_point_384* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) { @@ -84094,12 +84112,28 @@ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_384, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_384_from_mp(pub->x, 6, pX); sp_384_from_mp(pub->y, 6, pY); sp_384_from_bin(pub->z, 6, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_384_cmp_6(pub->x, p384_mod) >= 0) || + (sp_384_cmp_6(pub->y, p384_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_384_ecc_is_point_6(pub, NULL); } @@ -84134,12 +84168,12 @@ int sp_ecc_check_key_384(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || - ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + ((privm != NULL) && (mp_count_bits(privm) > 384)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -128904,6 +128938,7 @@ static int sp_521_ecc_is_point_9(const sp_point_521* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) { @@ -128911,12 +128946,28 @@ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_521, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_521_from_mp(pub->x, 9, pX); sp_521_from_mp(pub->y, 9, pY); sp_521_from_bin(pub->z, 9, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_521_cmp_9(pub->x, p521_mod) >= 0) || + (sp_521_cmp_9(pub->y, p521_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_521_ecc_is_point_9(pub, NULL); } @@ -128951,12 +129002,12 @@ int sp_ecc_check_key_521(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || - ((privm != NULL) && (mp_count_bits(privm) > 521)))) { + ((privm != NULL) && (mp_count_bits(privm) > 521)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -140924,6 +140975,7 @@ static int sp_1024_ecc_is_point_16(const sp_point_1024* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) { @@ -140931,12 +140983,28 @@ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_1024, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_1024_from_mp(pub->x, 16, pX); sp_1024_from_mp(pub->y, 16, pY); sp_1024_from_bin(pub->z, 16, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_1024_cmp_16(pub->x, p1024_mod) >= 0) || + (sp_1024_cmp_16(pub->y, p1024_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_1024_ecc_is_point_16(pub, NULL); } @@ -140971,12 +141039,12 @@ int sp_ecc_check_key_1024(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || - ((privm != NULL) && (mp_count_bits(privm) > 1024)))) { + ((privm != NULL) && (mp_count_bits(privm) > 1024)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } diff --git a/wolfcrypt/src/sp_armthumb.c b/wolfcrypt/src/sp_armthumb.c index 0a77bddf1c4..bf2768f820b 100644 --- a/wolfcrypt/src/sp_armthumb.c +++ b/wolfcrypt/src/sp_armthumb.c @@ -108337,6 +108337,7 @@ static int sp_256_ecc_is_point_8(const sp_point_256* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) { @@ -108344,12 +108345,28 @@ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_256, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_256_from_mp(pub->x, 8, pX); sp_256_from_mp(pub->y, 8, pY); sp_256_from_bin(pub->z, 8, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_256_cmp_8(pub->x, p256_mod) >= 0) || + (sp_256_cmp_8(pub->y, p256_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_256_ecc_is_point_8(pub, NULL); } @@ -108384,12 +108401,12 @@ int sp_ecc_check_key_256(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || - ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + ((privm != NULL) && (mp_count_bits(privm) > 256)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -119388,6 +119405,7 @@ static int sp_384_ecc_is_point_12(const sp_point_384* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) { @@ -119395,12 +119413,28 @@ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_384, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_384_from_mp(pub->x, 12, pX); sp_384_from_mp(pub->y, 12, pY); sp_384_from_bin(pub->z, 12, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_384_cmp_12(pub->x, p384_mod) >= 0) || + (sp_384_cmp_12(pub->y, p384_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_384_ecc_is_point_12(pub, NULL); } @@ -119435,12 +119469,12 @@ int sp_ecc_check_key_384(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || - ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + ((privm != NULL) && (mp_count_bits(privm) > 384)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -136239,6 +136273,7 @@ static int sp_521_ecc_is_point_17(const sp_point_521* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) { @@ -136246,12 +136281,28 @@ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_521, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_521_from_mp(pub->x, 17, pX); sp_521_from_mp(pub->y, 17, pY); sp_521_from_bin(pub->z, 17, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_521_cmp_17(pub->x, p521_mod) >= 0) || + (sp_521_cmp_17(pub->y, p521_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_521_ecc_is_point_17(pub, NULL); } @@ -136286,12 +136337,12 @@ int sp_ecc_check_key_521(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || - ((privm != NULL) && (mp_count_bits(privm) > 521)))) { + ((privm != NULL) && (mp_count_bits(privm) > 521)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -218858,6 +218909,7 @@ static int sp_1024_ecc_is_point_32(const sp_point_1024* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) { @@ -218865,12 +218917,28 @@ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_1024, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_1024_from_mp(pub->x, 32, pX); sp_1024_from_mp(pub->y, 32, pY); sp_1024_from_bin(pub->z, 32, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_1024_cmp_32(pub->x, p1024_mod) >= 0) || + (sp_1024_cmp_32(pub->y, p1024_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_1024_ecc_is_point_32(pub, NULL); } @@ -218905,12 +218973,12 @@ int sp_ecc_check_key_1024(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || - ((privm != NULL) && (mp_count_bits(privm) > 1024)))) { + ((privm != NULL) && (mp_count_bits(privm) > 1024)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } diff --git a/wolfcrypt/src/sp_c32.c b/wolfcrypt/src/sp_c32.c index 8352093aa73..28c92b50f38 100644 --- a/wolfcrypt/src/sp_c32.c +++ b/wolfcrypt/src/sp_c32.c @@ -27903,6 +27903,7 @@ static int sp_256_ecc_is_point_9(const sp_point_256* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) { @@ -27910,12 +27911,28 @@ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_256, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_256_from_mp(pub->x, 9, pX); sp_256_from_mp(pub->y, 9, pY); sp_256_from_bin(pub->z, 9, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_256_cmp_9(pub->x, p256_mod) >= 0) || + (sp_256_cmp_9(pub->y, p256_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_256_ecc_is_point_9(pub, NULL); } @@ -27950,12 +27967,12 @@ int sp_ecc_check_key_256(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || - ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + ((privm != NULL) && (mp_count_bits(privm) > 256)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -35851,6 +35868,7 @@ static int sp_384_ecc_is_point_15(const sp_point_384* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) { @@ -35858,12 +35876,28 @@ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_384, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_384_from_mp(pub->x, 15, pX); sp_384_from_mp(pub->y, 15, pY); sp_384_from_bin(pub->z, 15, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_384_cmp_15(pub->x, p384_mod) >= 0) || + (sp_384_cmp_15(pub->y, p384_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_384_ecc_is_point_15(pub, NULL); } @@ -35898,12 +35932,12 @@ int sp_ecc_check_key_384(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || - ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + ((privm != NULL) && (mp_count_bits(privm) > 384)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -43908,6 +43942,7 @@ static int sp_521_ecc_is_point_21(const sp_point_521* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) { @@ -43915,12 +43950,28 @@ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_521, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_521_from_mp(pub->x, 21, pX); sp_521_from_mp(pub->y, 21, pY); sp_521_from_bin(pub->z, 21, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_521_cmp_21(pub->x, p521_mod) >= 0) || + (sp_521_cmp_21(pub->y, p521_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_521_ecc_is_point_21(pub, NULL); } @@ -43955,12 +44006,12 @@ int sp_ecc_check_key_521(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || - ((privm != NULL) && (mp_count_bits(privm) > 521)))) { + ((privm != NULL) && (mp_count_bits(privm) > 521)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -55648,6 +55699,7 @@ static int sp_1024_ecc_is_point_42(const sp_point_1024* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) { @@ -55655,12 +55707,28 @@ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_1024, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_1024_from_mp(pub->x, 42, pX); sp_1024_from_mp(pub->y, 42, pY); sp_1024_from_bin(pub->z, 42, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_1024_cmp_42(pub->x, p1024_mod) >= 0) || + (sp_1024_cmp_42(pub->y, p1024_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_1024_ecc_is_point_42(pub, NULL); } @@ -55695,12 +55763,12 @@ int sp_ecc_check_key_1024(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || - ((privm != NULL) && (mp_count_bits(privm) > 1024)))) { + ((privm != NULL) && (mp_count_bits(privm) > 1024)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } diff --git a/wolfcrypt/src/sp_c64.c b/wolfcrypt/src/sp_c64.c index 147dc8ddadb..692f9b2db46 100644 --- a/wolfcrypt/src/sp_c64.c +++ b/wolfcrypt/src/sp_c64.c @@ -28686,6 +28686,7 @@ static int sp_256_ecc_is_point_5(const sp_point_256* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) { @@ -28693,12 +28694,28 @@ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_256, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_256_from_mp(pub->x, 5, pX); sp_256_from_mp(pub->y, 5, pY); sp_256_from_bin(pub->z, 5, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_256_cmp_5(pub->x, p256_mod) >= 0) || + (sp_256_cmp_5(pub->y, p256_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_256_ecc_is_point_5(pub, NULL); } @@ -28733,12 +28750,12 @@ int sp_ecc_check_key_256(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || - ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + ((privm != NULL) && (mp_count_bits(privm) > 256)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -36021,6 +36038,7 @@ static int sp_384_ecc_is_point_7(const sp_point_384* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) { @@ -36028,12 +36046,28 @@ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_384, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_384_from_mp(pub->x, 7, pX); sp_384_from_mp(pub->y, 7, pY); sp_384_from_bin(pub->z, 7, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_384_cmp_7(pub->x, p384_mod) >= 0) || + (sp_384_cmp_7(pub->y, p384_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_384_ecc_is_point_7(pub, NULL); } @@ -36068,12 +36102,12 @@ int sp_ecc_check_key_384(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || - ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + ((privm != NULL) && (mp_count_bits(privm) > 384)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -43388,6 +43422,7 @@ static int sp_521_ecc_is_point_9(const sp_point_521* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) { @@ -43395,12 +43430,28 @@ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_521, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_521_from_mp(pub->x, 9, pX); sp_521_from_mp(pub->y, 9, pY); sp_521_from_bin(pub->z, 9, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_521_cmp_9(pub->x, p521_mod) >= 0) || + (sp_521_cmp_9(pub->y, p521_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_521_ecc_is_point_9(pub, NULL); } @@ -43435,12 +43486,12 @@ int sp_ecc_check_key_521(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || - ((privm != NULL) && (mp_count_bits(privm) > 521)))) { + ((privm != NULL) && (mp_count_bits(privm) > 521)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -54121,6 +54172,7 @@ static int sp_1024_ecc_is_point_18(const sp_point_1024* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) { @@ -54128,12 +54180,28 @@ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_1024, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_1024_from_mp(pub->x, 18, pX); sp_1024_from_mp(pub->y, 18, pY); sp_1024_from_bin(pub->z, 18, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_1024_cmp_18(pub->x, p1024_mod) >= 0) || + (sp_1024_cmp_18(pub->y, p1024_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_1024_ecc_is_point_18(pub, NULL); } @@ -54168,12 +54236,12 @@ int sp_ecc_check_key_1024(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || - ((privm != NULL) && (mp_count_bits(privm) > 1024)))) { + ((privm != NULL) && (mp_count_bits(privm) > 1024)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } diff --git a/wolfcrypt/src/sp_cortexm.c b/wolfcrypt/src/sp_cortexm.c index 8f8ca66052a..e54dee258a3 100644 --- a/wolfcrypt/src/sp_cortexm.c +++ b/wolfcrypt/src/sp_cortexm.c @@ -43330,6 +43330,7 @@ static int sp_256_ecc_is_point_8(const sp_point_256* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) { @@ -43337,12 +43338,28 @@ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_256, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_256_from_mp(pub->x, 8, pX); sp_256_from_mp(pub->y, 8, pY); sp_256_from_bin(pub->z, 8, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_256_cmp_8(pub->x, p256_mod) >= 0) || + (sp_256_cmp_8(pub->y, p256_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_256_ecc_is_point_8(pub, NULL); } @@ -43377,12 +43394,12 @@ int sp_ecc_check_key_256(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || - ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + ((privm != NULL) && (mp_count_bits(privm) > 256)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -53695,6 +53712,7 @@ static int sp_384_ecc_is_point_12(const sp_point_384* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) { @@ -53702,12 +53720,28 @@ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_384, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_384_from_mp(pub->x, 12, pX); sp_384_from_mp(pub->y, 12, pY); sp_384_from_bin(pub->z, 12, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_384_cmp_12(pub->x, p384_mod) >= 0) || + (sp_384_cmp_12(pub->y, p384_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_384_ecc_is_point_12(pub, NULL); } @@ -53742,12 +53776,12 @@ int sp_ecc_check_key_384(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || - ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + ((privm != NULL) && (mp_count_bits(privm) > 384)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -67390,6 +67424,7 @@ static int sp_521_ecc_is_point_17(const sp_point_521* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) { @@ -67397,12 +67432,28 @@ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_521, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_521_from_mp(pub->x, 17, pX); sp_521_from_mp(pub->y, 17, pY); sp_521_from_bin(pub->z, 17, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_521_cmp_17(pub->x, p521_mod) >= 0) || + (sp_521_cmp_17(pub->y, p521_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_521_ecc_is_point_17(pub, NULL); } @@ -67437,12 +67488,12 @@ int sp_ecc_check_key_521(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || - ((privm != NULL) && (mp_count_bits(privm) > 521)))) { + ((privm != NULL) && (mp_count_bits(privm) > 521)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -83930,6 +83981,7 @@ static int sp_1024_ecc_is_point_32(const sp_point_1024* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) { @@ -83937,12 +83989,28 @@ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_1024, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_1024_from_mp(pub->x, 32, pX); sp_1024_from_mp(pub->y, 32, pY); sp_1024_from_bin(pub->z, 32, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_1024_cmp_32(pub->x, p1024_mod) >= 0) || + (sp_1024_cmp_32(pub->y, p1024_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_1024_ecc_is_point_32(pub, NULL); } @@ -83977,12 +84045,12 @@ int sp_ecc_check_key_1024(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || - ((privm != NULL) && (mp_count_bits(privm) > 1024)))) { + ((privm != NULL) && (mp_count_bits(privm) > 1024)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } diff --git a/wolfcrypt/src/sp_riscv64.c b/wolfcrypt/src/sp_riscv64.c index 1f9309b0791..797fd0cbb30 100644 --- a/wolfcrypt/src/sp_riscv64.c +++ b/wolfcrypt/src/sp_riscv64.c @@ -44539,6 +44539,7 @@ static int sp_256_ecc_is_point_4(const sp_point_256* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) { @@ -44546,12 +44547,28 @@ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_256, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_256_from_mp(pub->x, 4, pX); sp_256_from_mp(pub->y, 4, pY); sp_256_from_bin(pub->z, 4, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_256_cmp_4(pub->x, p256_mod) >= 0) || + (sp_256_cmp_4(pub->y, p256_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_256_ecc_is_point_4(pub, NULL); } @@ -44586,12 +44603,12 @@ int sp_ecc_check_key_256(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || - ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + ((privm != NULL) && (mp_count_bits(privm) > 256)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -54685,6 +54702,7 @@ static int sp_384_ecc_is_point_6(const sp_point_384* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) { @@ -54692,12 +54710,28 @@ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_384, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_384_from_mp(pub->x, 6, pX); sp_384_from_mp(pub->y, 6, pY); sp_384_from_bin(pub->z, 6, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_384_cmp_6(pub->x, p384_mod) >= 0) || + (sp_384_cmp_6(pub->y, p384_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_384_ecc_is_point_6(pub, NULL); } @@ -54732,12 +54766,12 @@ int sp_ecc_check_key_384(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || - ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + ((privm != NULL) && (mp_count_bits(privm) > 384)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -65097,6 +65131,7 @@ static int sp_521_ecc_is_point_9(const sp_point_521* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) { @@ -65104,12 +65139,28 @@ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_521, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_521_from_mp(pub->x, 9, pX); sp_521_from_mp(pub->y, 9, pY); sp_521_from_bin(pub->z, 9, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_521_cmp_9(pub->x, p521_mod) >= 0) || + (sp_521_cmp_9(pub->y, p521_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_521_ecc_is_point_9(pub, NULL); } @@ -65144,12 +65195,12 @@ int sp_ecc_check_key_521(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || - ((privm != NULL) && (mp_count_bits(privm) > 521)))) { + ((privm != NULL) && (mp_count_bits(privm) > 521)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -82847,6 +82898,7 @@ static int sp_1024_ecc_is_point_16(const sp_point_1024* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) { @@ -82854,12 +82906,28 @@ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_1024, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_1024_from_mp(pub->x, 16, pX); sp_1024_from_mp(pub->y, 16, pY); sp_1024_from_bin(pub->z, 16, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_1024_cmp_16(pub->x, p1024_mod) >= 0) || + (sp_1024_cmp_16(pub->y, p1024_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_1024_ecc_is_point_16(pub, NULL); } @@ -82894,12 +82962,12 @@ int sp_ecc_check_key_1024(const mp_int* pX, const mp_int* pY, int err = MP_OKAY; - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || - ((privm != NULL) && (mp_count_bits(privm) > 1024)))) { + ((privm != NULL) && (mp_count_bits(privm) > 1024)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } diff --git a/wolfcrypt/src/sp_x86_64.c b/wolfcrypt/src/sp_x86_64.c index ca0abb541e7..9ff5aea7499 100644 --- a/wolfcrypt/src/sp_x86_64.c +++ b/wolfcrypt/src/sp_x86_64.c @@ -26159,6 +26159,7 @@ static int sp_256_ecc_is_point_4(const sp_point_256* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) { @@ -26166,12 +26167,28 @@ int sp_ecc_is_point_256(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_256, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_256_from_mp(pub->x, 4, pX); sp_256_from_mp(pub->y, 4, pY); sp_256_from_bin(pub->z, 4, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_256_cmp_4(pub->x, p256_mod) >= 0) || + (sp_256_cmp_4(pub->y, p256_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_256_ecc_is_point_4(pub, NULL); } @@ -26209,12 +26226,12 @@ int sp_ecc_check_key_256(const mp_int* pX, const mp_int* pY, #endif - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 256) || (mp_count_bits(pY) > 256) || - ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + ((privm != NULL) && (mp_count_bits(privm) > 256)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -50975,6 +50992,7 @@ static int sp_384_ecc_is_point_6(const sp_point_384* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) { @@ -50982,12 +51000,28 @@ int sp_ecc_is_point_384(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_384, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_384_from_mp(pub->x, 6, pX); sp_384_from_mp(pub->y, 6, pY); sp_384_from_bin(pub->z, 6, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_384_cmp_6(pub->x, p384_mod) >= 0) || + (sp_384_cmp_6(pub->y, p384_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_384_ecc_is_point_6(pub, NULL); } @@ -51025,12 +51059,12 @@ int sp_ecc_check_key_384(const mp_int* pX, const mp_int* pY, #endif - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 384) || (mp_count_bits(pY) > 384) || - ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + ((privm != NULL) && (mp_count_bits(privm) > 384)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -91996,6 +92030,7 @@ static int sp_521_ecc_is_point_9(const sp_point_521* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) { @@ -92003,12 +92038,28 @@ int sp_ecc_is_point_521(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_521, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_521_from_mp(pub->x, 9, pX); sp_521_from_mp(pub->y, 9, pY); sp_521_from_bin(pub->z, 9, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_521_cmp_9(pub->x, p521_mod) >= 0) || + (sp_521_cmp_9(pub->y, p521_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_521_ecc_is_point_9(pub, NULL); } @@ -92046,12 +92097,12 @@ int sp_ecc_check_key_521(const mp_int* pX, const mp_int* pY, #endif - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 521) || (mp_count_bits(pY) > 521) || - ((privm != NULL) && (mp_count_bits(privm) > 521)))) { + ((privm != NULL) && (mp_count_bits(privm) > 521)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; } @@ -104764,6 +104815,7 @@ static int sp_1024_ecc_is_point_16(const sp_point_1024* point, * @return MP_OKAY otherwise. * @return MEMORY_E when dynamic memory allocation fails. * @return MP_VAL when the point is not on the curve. + * @return ECC_OUT_OF_RANGE_E when an ordinate is not less than the modulus. */ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) { @@ -104771,12 +104823,28 @@ int sp_ecc_is_point_1024(const mp_int* pX, const mp_int* pY) const byte one[1] = { 1 }; int err = MP_OKAY; + /* Quick check the public key ordinates are not negative and their lengths + * are in range; proper check later. */ + if ((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || + mp_isneg(pX) || mp_isneg(pY)) { + err = ECC_OUT_OF_RANGE_E; + } + SP_ALLOC_VAR(sp_point_1024, pub, 1, NULL, DYNAMIC_TYPE_ECC); if (err == MP_OKAY) { sp_1024_from_mp(pub->x, 16, pX); sp_1024_from_mp(pub->y, 16, pY); sp_1024_from_bin(pub->z, 16, one, (int)sizeof(one)); + } + /* Check range of X and Y */ + if ((err == MP_OKAY) && + ((sp_1024_cmp_16(pub->x, p1024_mod) >= 0) || + (sp_1024_cmp_16(pub->y, p1024_mod) >= 0))) { + err = ECC_OUT_OF_RANGE_E; + } + + if (err == MP_OKAY) { err = sp_1024_ecc_is_point_16(pub, NULL); } @@ -104814,12 +104882,12 @@ int sp_ecc_check_key_1024(const mp_int* pX, const mp_int* pY, #endif - /* Quick check the lengs of public key ordinates and private key are in - * range. Proper check later. - */ + /* Quick check the public key ordinates are not negative and that their + * lengths and the private key length are in range. Proper check later. */ if (((mp_count_bits(pX) > 1024) || (mp_count_bits(pY) > 1024) || - ((privm != NULL) && (mp_count_bits(privm) > 1024)))) { + ((privm != NULL) && (mp_count_bits(privm) > 1024)) || + mp_isneg(pX) || mp_isneg(pY))) { err = ECC_OUT_OF_RANGE_E; }