diff --git a/nx_secure/inc/nx_secure_tls.h b/nx_secure/inc/nx_secure_tls.h index c07292867..5b21776f0 100644 --- a/nx_secure/inc/nx_secure_tls.h +++ b/nx_secure/inc/nx_secure_tls.h @@ -1407,6 +1407,8 @@ typedef struct NX_SECURE_TLS_SESSION_STRUCT #if (NX_SECURE_TLS_TLS_1_3_ENABLED) UINT _nx_secure_tls_1_3_crypto_init(NX_SECURE_TLS_SESSION *tls_session); +UINT _nx_secure_tls_1_3_strip_padding(NX_PACKET *decrypted_packet, USHORT *message_type_ptr, + UINT *message_length_ptr); UINT _nx_secure_tls_1_3_client_handshake(NX_SECURE_TLS_SESSION *tls_session, UCHAR *packet_buffer, UINT data_length, ULONG wait_option); UINT _nx_secure_tls_1_3_server_handshake(NX_SECURE_TLS_SESSION *tls_session, UCHAR *packet_buffer, diff --git a/nx_secure/src/nx_secure_tls_process_record.c b/nx_secure/src/nx_secure_tls_process_record.c index 6d2f7f339..c3c5f918e 100644 --- a/nx_secure/src/nx_secure_tls_process_record.c +++ b/nx_secure/src/nx_secure_tls_process_record.c @@ -276,22 +276,9 @@ NX_PACKET *decrypted_packet; if (status == NX_SECURE_TLS_SUCCESS) { - /* In TLS 1.3, encrypted records have a single byte at the - record that contains the message type (e.g. application data, - ect.), which is now the ACTUAL message type. */ - status = nx_packet_data_extract_offset(decrypted_packet, - decrypted_packet -> nx_packet_length - 1, - &message_type, 1, &bytes_copied); - if (status || (bytes_copied != 1)) - { - error_status = NX_SECURE_TLS_INVALID_PACKET; - } - - /* Remove the content type byte from the data length to process. */ - message_length = message_length - 1; - - /* Adjust packet length. */ - decrypted_packet -> nx_packet_length = message_length; + error_status = _nx_secure_tls_1_3_strip_padding(decrypted_packet, + &message_type, + &message_length); /* Increment the sequence number. This is done in the MAC verify step for 1.2 and earlier, but AEAD includes the MAC so we don't @@ -646,3 +633,78 @@ NX_PACKET *current_ptr; message_length -= (ULONG)(current_ptr -> nx_packet_append_ptr - current_ptr -> nx_packet_prepend_ptr); } } + + +#if (NX_SECURE_TLS_TLS_1_3_ENABLED) +/**************************************************************************/ +/* */ +/* FUNCTION RELEASE */ +/* */ +/* _nx_secure_tls_1_3_strip_padding PORTABLE C */ +/* */ +/* AUTHOR */ +/* */ +/* Edouard MALOT */ +/* */ +/* DESCRIPTION */ +/* */ +/* Recover the inner content type from a decrypted TLS 1.3 record and */ +/* strip the trailing zero-byte padding. Per RFC 8446 §5.4, the inner */ +/* content type is the last non-zero byte of the plaintext; all bytes */ +/* after it are padding. The scan visits every plaintext byte, so its */ +/* cost is independent of how much padding the peer added. */ +/* */ +/* INPUT */ +/* */ +/* decrypted_packet Decrypted record (possibly */ +/* spanning chained fragments) */ +/* message_type_ptr Set to the inner content type */ +/* message_length_ptr Set to the length excluding */ +/* the type byte and padding */ +/* */ +/* OUTPUT */ +/* */ +/* NX_SECURE_TLS_SUCCESS Inner type recovered */ +/* NX_SECURE_TLS_UNEXPECTED_MESSAGE Plaintext had no non-zero */ +/* byte (§5.4 violation) */ +/* */ +/**************************************************************************/ +UINT _nx_secure_tls_1_3_strip_padding(NX_PACKET *decrypted_packet, + USHORT *message_type_ptr, + UINT *message_length_ptr) +{ +NX_PACKET *scan_fragment; +UCHAR *scan_ptr; +ULONG running_offset = 0; +ULONG last_nonzero_offset = 0; +UCHAR last_nonzero_byte = 0; + + for (scan_fragment = decrypted_packet; scan_fragment != NX_NULL; scan_fragment = scan_fragment -> nx_packet_next) + { + for (scan_ptr = scan_fragment -> nx_packet_prepend_ptr; + scan_ptr < scan_fragment -> nx_packet_append_ptr; + scan_ptr++) + { + if (*scan_ptr != 0) + { + last_nonzero_byte = *scan_ptr; + last_nonzero_offset = running_offset; + } + running_offset++; + } + } + + if (last_nonzero_byte == 0) + { + *message_type_ptr = 0; + *message_length_ptr = 0; + decrypted_packet -> nx_packet_length = 0; + return(NX_SECURE_TLS_UNEXPECTED_MESSAGE); + } + + *message_type_ptr = (USHORT)last_nonzero_byte; + *message_length_ptr = last_nonzero_offset; + decrypted_packet -> nx_packet_length = last_nonzero_offset; + return(NX_SECURE_TLS_SUCCESS); +} +#endif /* NX_SECURE_TLS_TLS_1_3_ENABLED */ diff --git a/test/regression/nx_secure_test/nx_secure_tls_process_record_test.c b/test/regression/nx_secure_test/nx_secure_tls_process_record_test.c index 5ac49ef0c..00a8d1c27 100644 --- a/test/regression/nx_secure_test/nx_secure_tls_process_record_test.c +++ b/test/regression/nx_secure_test/nx_secure_tls_process_record_test.c @@ -229,6 +229,107 @@ UCHAR packet_buffer[100]; status = _nx_secure_tls_process_record(&tls_session, packet, &bytes_processed, 0); EXPECT_EQ(NX_SECURE_TLS_INVALID_PACKET, status); +#if (NX_SECURE_TLS_TLS_1_3_ENABLED) + /* Regression tests for the TLS 1.3 record de-padding (RFC 8446 §5.4). + Before the fix, the code read the literal last byte of the plaintext + as the inner content type; any padded record from a compliant peer + (JDK 11+, OpenSSL with padding on) was mishandled. */ + { + NX_PACKET *decrypted; + USHORT out_type; + UINT out_length; + UCHAR inner_plaintext[16]; + + /* Case 1: padded record. Plaintext = "hello" + type byte + 5 zeros. + The inner type must come back and the length must exclude both + the type byte and the padding. */ + tls_session.nx_secure_record_queue_header = NX_NULL; + status = nx_packet_allocate(&pool_0, &decrypted, NX_IPv4_TCP_PACKET, NX_WAIT_FOREVER); + EXPECT_EQ(NX_SUCCESS, status); + + memset(inner_plaintext, 0, sizeof(inner_plaintext)); + memcpy(inner_plaintext, "hello", 5); + inner_plaintext[5] = NX_SECURE_TLS_APPLICATION_DATA; + status = nx_packet_data_append(decrypted, inner_plaintext, 11, &pool_0, NX_WAIT_FOREVER); + EXPECT_EQ(NX_SUCCESS, status); + + out_type = 0; + out_length = 0; + status = _nx_secure_tls_1_3_strip_padding(decrypted, &out_type, &out_length); + EXPECT_EQ(NX_SECURE_TLS_SUCCESS, status); + EXPECT_EQ((USHORT)NX_SECURE_TLS_APPLICATION_DATA, out_type); + EXPECT_EQ((UINT)5, out_length); + EXPECT_EQ((ULONG)5, decrypted -> nx_packet_length); + + status = nx_packet_release(decrypted); + EXPECT_EQ(NX_SUCCESS, status); + + /* Case 2: all-zero plaintext. §5.4 says this is a peer protocol + violation and must yield unexpected_message. */ + status = nx_packet_allocate(&pool_0, &decrypted, NX_IPv4_TCP_PACKET, NX_WAIT_FOREVER); + EXPECT_EQ(NX_SUCCESS, status); + + memset(inner_plaintext, 0, sizeof(inner_plaintext)); + status = nx_packet_data_append(decrypted, inner_plaintext, 10, &pool_0, NX_WAIT_FOREVER); + EXPECT_EQ(NX_SUCCESS, status); + + out_type = 0xff; + out_length = 42; + status = _nx_secure_tls_1_3_strip_padding(decrypted, &out_type, &out_length); + EXPECT_EQ(NX_SECURE_TLS_UNEXPECTED_MESSAGE, status); + EXPECT_EQ((USHORT)0, out_type); + EXPECT_EQ((UINT)0, out_length); + + status = nx_packet_release(decrypted); + EXPECT_EQ(NX_SUCCESS, status); + + /* Case 3: unpadded record. Pre-fix arithmetic gave length - 1; the + new scan must return the exact same values so existing traffic is + unaffected. This is the regression-risk assertion. */ + status = nx_packet_allocate(&pool_0, &decrypted, NX_IPv4_TCP_PACKET, NX_WAIT_FOREVER); + EXPECT_EQ(NX_SUCCESS, status); + + memcpy(inner_plaintext, "hello", 5); + inner_plaintext[5] = NX_SECURE_TLS_APPLICATION_DATA; + status = nx_packet_data_append(decrypted, inner_plaintext, 6, &pool_0, NX_WAIT_FOREVER); + EXPECT_EQ(NX_SUCCESS, status); + + out_type = 0; + out_length = 0; + status = _nx_secure_tls_1_3_strip_padding(decrypted, &out_type, &out_length); + EXPECT_EQ(NX_SECURE_TLS_SUCCESS, status); + EXPECT_EQ((USHORT)NX_SECURE_TLS_APPLICATION_DATA, out_type); + EXPECT_EQ((UINT)5, out_length); + + status = nx_packet_release(decrypted); + EXPECT_EQ(NX_SUCCESS, status); + + /* Case 4: padded record that straddles a fragment boundary. The scan + must walk through the chained NX_PACKETs, not just the head. */ + status = nx_packet_allocate(&pool_0, &decrypted, NX_IPv4_TCP_PACKET, NX_WAIT_FOREVER); + EXPECT_EQ(NX_SUCCESS, status); + + /* Payload > single-packet capacity (1536) forces chaining. Fill with + a non-zero content byte, then the type, then trailing zeros — the + trailing zeros will land in a later fragment. */ + memset(data_buffer, 0xAB, 2000); + data_buffer[1999] = NX_SECURE_TLS_APPLICATION_DATA; + memset(&data_buffer[2000], 0, 500); + status = nx_packet_data_append(decrypted, data_buffer, 2500, &pool_0, NX_WAIT_FOREVER); + EXPECT_EQ(NX_SUCCESS, status); + + out_type = 0; + out_length = 0; + status = _nx_secure_tls_1_3_strip_padding(decrypted, &out_type, &out_length); + EXPECT_EQ(NX_SECURE_TLS_SUCCESS, status); + EXPECT_EQ((USHORT)NX_SECURE_TLS_APPLICATION_DATA, out_type); + EXPECT_EQ((UINT)1999, out_length); + + status = nx_packet_release(decrypted); + EXPECT_EQ(NX_SUCCESS, status); + } +#endif /* NX_SECURE_TLS_TLS_1_3_ENABLED */ + printf("SUCCESS!\n"); test_control_return(0); }