Skip to content

Commit

Permalink
ssl/statem: Replace size_t with int and add the checks
Browse files Browse the repository at this point in the history
Replace the type of variables with int to avoid implicit conversion when it is assigned by EVP_MD_get_size().
Moreover, add the checks to avoid integer overflow.

Fixes: 6594189 ("Merge early_data_info extension into early_data")
Fixes: 9368f86 ("Add TLSv1.3 client side external PSK support")
Fixes: 1053a6e ("Implement Server side of PSK extension parsing")
Signed-off-by: Jiasheng Jiang <jiasheng@purdue.edu>

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #23937)
  • Loading branch information
JiangJias authored and t8m committed Apr 26, 2024
1 parent 6d01857 commit 48e3cf2
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
15 changes: 13 additions & 2 deletions ssl/statem/extensions_clnt.c
Original file line number Diff line number Diff line change
Expand Up @@ -979,8 +979,12 @@ EXT_RETURN tls_construct_ctos_padding(SSL_CONNECTION *s, WPACKET *pkt,
* Add the fixed PSK overhead, the identity length and the binder
* length.
*/
int md_size = EVP_MD_get_size(md);

if (md_size <= 0)
return EXT_RETURN_FAIL;
hlen += PSK_PRE_BINDER_OVERHEAD + s->session->ext.ticklen
+ EVP_MD_get_size(md);
+ md_size;
}
}

Expand Down Expand Up @@ -1019,7 +1023,8 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt,
{
#ifndef OPENSSL_NO_TLS1_3
uint32_t agesec, agems = 0;
size_t reshashsize = 0, pskhashsize = 0, binderoffset, msglen;
size_t binderoffset, msglen;
int reshashsize = 0, pskhashsize = 0;
unsigned char *resbinder = NULL, *pskbinder = NULL, *msgstart = NULL;
const EVP_MD *handmd = NULL, *mdres = NULL, *mdpsk = NULL;
int dores = 0;
Expand Down Expand Up @@ -1115,6 +1120,8 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt,
agems += s->session->ext.tick_age_add;

reshashsize = EVP_MD_get_size(mdres);
if (reshashsize <= 0)
goto dopsksess;
s->ext.tick_identity++;
dores = 1;
}
Expand Down Expand Up @@ -1144,6 +1151,10 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt,
}

pskhashsize = EVP_MD_get_size(mdpsk);
if (pskhashsize <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_PSK);
return EXT_RETURN_FAIL;
}
}

/* Create the extension, but skip over the binder for now */
Expand Down
7 changes: 5 additions & 2 deletions ssl/statem/extensions_srvr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,8 @@ int tls_parse_ctos_psk(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,
X509 *x, size_t chainidx)
{
PACKET identities, binders, binder;
size_t binderoffset, hashsize;
size_t binderoffset;
int hashsize;
SSL_SESSION *sess = NULL;
unsigned int id, i, ext = 0;
const EVP_MD *md = NULL;
Expand Down Expand Up @@ -1221,6 +1222,8 @@ int tls_parse_ctos_psk(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,

binderoffset = PACKET_data(pkt) - (const unsigned char *)s->init_buf->data;
hashsize = EVP_MD_get_size(md);
if (hashsize <= 0)
goto err;

if (!PACKET_get_length_prefixed_2(pkt, &binders)) {
SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
Expand All @@ -1234,7 +1237,7 @@ int tls_parse_ctos_psk(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,
}
}

if (PACKET_remaining(&binder) != hashsize) {
if (PACKET_remaining(&binder) != (size_t)hashsize) {
SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
goto err;
}
Expand Down

0 comments on commit 48e3cf2

Please sign in to comment.