[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 <mcb30@ipxe.org>

1 files changed, 20 insertions, 10 deletions

diff --git a/src/net/tls.c b/src/net/tls.c
index 58fad65e1..5f89be452 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;
 }