diff options
| author | Matt Caswell <matt@openssl.org> | 2022-10-27 14:14:53 +0100 |
|---|---|---|
| committer | Matt Caswell <matt@openssl.org> | 2022-12-05 11:10:55 +0000 |
| commit | 31efcf2c872f8f4d09ad5209ccbf1ada73436775 (patch) | |
| tree | 4bde433371b4781aeaedbbb8e01255b53e704055 /ssl | |
| parent | f868abcc5dbcbed6ca2e33bdb9bf06c817a4cce3 (diff) | |
Fix the ceiling on how much encryption growth we can have
Stitched ciphersuites can grow by more during encryption than the code
allowed for. We fix the calculation and add an assert to check we go it
right.
Also if we are adding the MAC independently of the cipher algorithm then
the encryption growth will not include that MAC so we should remove it
from the amount of bytes that we reserve for that growth. Otherwise we
might exceed our buffer size and the WPACKET_reserve operation will
fail.
Note that this is not a security issue. Even though we can overflow the
amount of bytes reserved in the WPACKET for the encryption, the underlying
buffer is still big enough.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19585)
Diffstat (limited to 'ssl')
| -rw-r--r-- | ssl/record/rec_layer_s3.c | 17 |
1 files changed, 12 insertions, 5 deletions
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index fe4abd6723..1db1712a09 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -16,6 +16,7 @@ #include <openssl/rand.h> #include "record_local.h" #include "../packet_local.h" +#include "internal/cryptlib.h" #if defined(OPENSSL_SMALL_FOOTPRINT) || \ !( defined(AESNI_ASM) && ( \ @@ -983,11 +984,14 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, } /* - * Reserve some bytes for any growth that may occur during encryption. - * This will be at most one cipher block or the tag length if using - * AEAD. SSL_RT_MAX_CIPHER_BLOCK_SIZE covers either case. - */ - if (!WPACKET_reserve_bytes(thispkt, SSL_RT_MAX_CIPHER_BLOCK_SIZE, + * Reserve some bytes for any growth that may occur during encryption. If + * we are adding the MAC independently of the cipher algorithm, then the + * max encrypted overhead does not need to include an allocation for that + * MAC + */ + if (!WPACKET_reserve_bytes(thispkt, + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD + - mac_size, NULL) /* * We also need next the amount of bytes written to this @@ -1037,6 +1041,9 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, /* Allocate bytes for the encryption overhead */ if (!WPACKET_get_length(thispkt, &origlen) + /* Check we allowed enough room for the encryption growth */ + || !ossl_assert(origlen + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD + - mac_size >= thiswr->length) /* Encryption should never shrink the data! */ || origlen > thiswr->length || (thiswr->length > origlen |