From 0cc0f47443ef9711775a748c2b0fb40e38643733 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Wed, 31 Jan 2024 13:49:35 +0000 Subject: [tls] Tidy up error handling flow in tls_send_plaintext() Coverity reported that tls_send_plaintext() failed to check the return status from tls_generate_random(), which could potentially result in uninitialised random data being used as the block initialisation vector (instead of intentionally random data). Add the missing return status check, and separate out the error handling code paths (since on the successful exit code path there will be no need to free either the plaintext or the ciphertext anyway). Signed-off-by: Michael Brown --- src/net/tls.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/net/tls.c b/src/net/tls.c index 58fad65..5f89be4 100644 --- a/src/net/tls.c +++ b/src/net/tls.c @@ -2945,9 +2945,9 @@ static int tls_send_plaintext ( struct tls_connection *tls, unsigned int type, } __attribute__ (( packed )) iv; struct tls_auth_header authhdr; struct tls_header *tlshdr; - void *plaintext = NULL; - size_t plaintext_len = len; - struct io_buffer *ciphertext = NULL; + void *plaintext; + size_t plaintext_len; + struct io_buffer *ciphertext; size_t ciphertext_len; size_t padding_len; uint8_t mac[digest->digestsize]; @@ -2956,7 +2956,10 @@ static int tls_send_plaintext ( struct tls_connection *tls, unsigned int type, /* Construct initialisation vector */ memcpy ( iv.fixed, cipherspec->fixed_iv, sizeof ( iv.fixed ) ); - tls_generate_random ( tls, iv.record, sizeof ( iv.record ) ); + if ( ( rc = tls_generate_random ( tls, iv.record, + sizeof ( iv.record ) ) ) != 0 ) { + goto err_random; + } /* Construct authentication data */ authhdr.seq = cpu_to_be64 ( tls->tx_seq ); @@ -2965,7 +2968,7 @@ static int tls_send_plaintext ( struct tls_connection *tls, unsigned int type, authhdr.header.length = htons ( len ); /* Calculate padding length */ - plaintext_len += suite->mac_len; + plaintext_len = ( len + suite->mac_len ); if ( is_block_cipher ( cipher ) ) { padding_len = ( ( ( cipher->blocksize - 1 ) & -( plaintext_len + 1 ) ) + 1 ); @@ -2980,7 +2983,7 @@ static int tls_send_plaintext ( struct tls_connection *tls, unsigned int type, DBGC ( tls, "TLS %p could not allocate %zd bytes for " "plaintext\n", tls, plaintext_len ); rc = -ENOMEM_TX_PLAINTEXT; - goto done; + goto err_plaintext; } /* Assemble plaintext */ @@ -3014,7 +3017,7 @@ static int tls_send_plaintext ( struct tls_connection *tls, unsigned int type, DBGC ( tls, "TLS %p could not allocate %zd bytes for " "ciphertext\n", tls, ciphertext_len ); rc = -ENOMEM_TX_CIPHERTEXT; - goto done; + goto err_ciphertext; } /* Assemble ciphertext */ @@ -3039,15 +3042,22 @@ static int tls_send_plaintext ( struct tls_connection *tls, unsigned int type, iob_disown ( ciphertext ) ) ) != 0 ) { DBGC ( tls, "TLS %p could not deliver ciphertext: %s\n", tls, strerror ( rc ) ); - goto done; + goto err_deliver; } /* Update TX state machine to next record */ tls->tx_seq += 1; - done: - free ( plaintext ); + assert ( plaintext == NULL ); + assert ( ciphertext == NULL ); + return 0; + + err_deliver: free_iob ( ciphertext ); + err_ciphertext: + free ( plaintext ); + err_plaintext: + err_random: return rc; } -- cgit v1.1