Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions nx_secure/src/nx_secure_generate_premaster_secret.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ UINT pre_master_secret_size;
#if defined(NX_SECURE_ENABLE_ECC_CIPHERSUITE) && !defined(NX_SECURE_DISABLE_X509)
if (ciphersuite -> nx_secure_tls_public_cipher -> nx_crypto_algorithm == NX_CRYPTO_KEY_EXCHANGE_ECDHE)
{
#ifdef NX_SECURE_ENABLE_PSK_CIPHERSUITES
#ifdef NX_SECURE_ENABLE_PSK_CIPHERSUITES
if(ciphersuite->nx_secure_tls_public_auth->nx_crypto_algorithm == NX_CRYPTO_KEY_EXCHANGE_PSK)
{
/* From RFC 5489:
Expand Down Expand Up @@ -280,9 +280,24 @@ UINT pre_master_secret_size;
/* Now, using the identity as a key, find the PSK in our PSK store. */
if (session_type == NX_SECURE_TLS_SESSION_TYPE_SERVER)
{
/* Server just uses its PSK. */
psk_data = tls_credentials -> nx_secure_tls_psk_store[0].nx_secure_tls_psk_data;
psk_length = tls_credentials -> nx_secure_tls_psk_store[0].nx_secure_tls_psk_data_size;
/* Attempt to find the PSK ID requested by the client in the ClientKeyExchange message. */
psk_length = 0;
const UINT client_psk_id_size = tls_credentials -> nx_secure_tls_remote_psk_id_size;
const UCHAR* client_psk_id = tls_credentials -> nx_secure_tls_remote_psk_id;
for (i = 0; i < NX_SECURE_TLS_MAX_PSK_KEYS; i++)
{
if ((client_psk_id_size == tls_credentials -> nx_secure_tls_psk_store[i].nx_secure_tls_psk_id_size) &&
(NX_SECURE_MEMCMP(client_psk_id, tls_credentials -> nx_secure_tls_psk_store[i].nx_secure_tls_psk_id, client_psk_id_size) == 0))
{
psk_data = tls_credentials -> nx_secure_tls_psk_store[i].nx_secure_tls_psk_data;
psk_length = tls_credentials -> nx_secure_tls_psk_store[i].nx_secure_tls_psk_data_size;
break;
}
}
if (psk_length == 0)
{
return(NX_SECURE_TLS_NO_MATCHING_PSK);
}
}
else
{
Expand Down
16 changes: 16 additions & 0 deletions nx_secure/src/nx_secure_process_client_key_exchange.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,22 @@ UINT private_key_length;
/* Check for PSK ciphersuites and generate the pre-master-secret. */
if (ciphersuite -> nx_secure_tls_public_auth -> nx_crypto_algorithm == NX_CRYPTO_KEY_EXCHANGE_PSK)
{
/* Store the requested PSK ID in the TLS credentials. */
if (message_length < 2)
{
return NX_INVALID_PACKET;
}
tls_credentials -> nx_secure_tls_remote_psk_id_size = ((packet_buffer[0] << 8) | packet_buffer[1]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you read past the handshake buffer for truncated PSK identities. The code checks message_length >= 2 and psk_id_size <= NX_SECURE_TLS_MAX_PSK_ID_SIZE, but never checks 2 + psk_id_size <= message_length before copying from &packet_buffer[2]. A malformed ClientKeyExchange can advertise a valid-sized identity but provide fewer bytes. Return NX_SECURE_TLS_INCORRECT_MESSAGE_LENGTH before the copy.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, now returns NX_SECURE_TLS_INCORRECT_MESSAGE_LENGTH if message_length < 2 + tls_credentials -> nx_secure_tls_remote_psk_id_size

if (tls_credentials -> nx_secure_tls_remote_psk_id_size > NX_SECURE_TLS_MAX_PSK_ID_SIZE)
{
return NX_SIZE_ERROR;
}
if (message_length < 2 + tls_credentials -> nx_secure_tls_remote_psk_id_size)
{
return NX_SECURE_TLS_INCORRECT_MESSAGE_LENGTH;
}
NX_SECURE_MEMCPY(tls_credentials -> nx_secure_tls_remote_psk_id, &packet_buffer[2], tls_credentials -> nx_secure_tls_remote_psk_id_size);

status = _nx_secure_generate_premaster_secret(ciphersuite, protocol_version, tls_key_material, tls_credentials,
NX_SECURE_TLS_SESSION_TYPE_SERVER, received_remote_credentials,
public_cipher_metadata, public_cipher_metadata_size, tls_ecc_curves);
Expand Down