From 3ea75b3a9b16855ab610af4df84acaed8c0ae9a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 18 Dec 2017 18:04:59 +0100 Subject: [PATCH 1/2] Fix SSLv3 MAC computation In a previous PR (Fix heap corruption in implementation of truncated HMAC extension #425) the place where MAC is computed was changed from the end of the SSL I/O buffer to a local buffer (then (part of) the content of the local buffer is either copied to the output buffer of compare to the input buffer). Unfortunately, this change was made only for TLS 1.0 and later, leaving SSL 3.0 in an inconsistent state due to ssl_mac() still writing to the old, hard-coded location, which, for MAC verification, resulted in later comparing the end of the input buffer (containing the computed MAC) to the local buffer (uninitialised), most likely resulting in MAC verification failure, hence no interop (even with ourselves). This commit completes the move to using a local buffer by using this strategy for SSL 3.0 too. Fortunately ssl_mac() was static so it's not a problem to change its signature. --- library/ssl_tls.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ef8cd203e..1018270f9 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1050,9 +1050,11 @@ int ssl_psk_derive_premaster( ssl_context *ssl, key_exchange_type_t key_ex ) /* * SSLv3.0 MAC functions */ -static void ssl_mac( md_context_t *md_ctx, unsigned char *secret, - unsigned char *buf, size_t len, - unsigned char *ctr, int type ) +static void ssl_mac( md_context_t *md_ctx, + const unsigned char *secret, + const unsigned char *buf, size_t len, + const unsigned char *ctr, int type, + unsigned char out[20] ) { unsigned char header[11]; unsigned char padding[48]; @@ -1077,14 +1079,14 @@ static void ssl_mac( md_context_t *md_ctx, unsigned char *secret, md_update( md_ctx, padding, padlen ); md_update( md_ctx, header, 11 ); md_update( md_ctx, buf, len ); - md_finish( md_ctx, buf + len ); + md_finish( md_ctx, out ); memset( padding, 0x5C, padlen ); md_starts( md_ctx ); md_update( md_ctx, secret, md_size ); md_update( md_ctx, padding, padlen ); - md_update( md_ctx, buf + len, md_size ); - md_finish( md_ctx, buf + len ); + md_update( md_ctx, out, md_size ); + md_finish( md_ctx, out ); } #endif /* POLARSSL_SSL_PROTO_SSL3 */ @@ -1130,10 +1132,15 @@ static int ssl_encrypt_buf( ssl_context *ssl ) #if defined(POLARSSL_SSL_PROTO_SSL3) if( ssl->minor_ver == SSL_MINOR_VERSION_0 ) { + unsigned char mac[20]; /* SHA-1 at most */ + ssl_mac( &ssl->transform_out->md_ctx_enc, ssl->transform_out->mac_enc, ssl->out_msg, ssl->out_msglen, - ssl->out_ctr, ssl->out_msgtype ); + ssl->out_ctr, ssl->out_msgtype, + mac ); + + memcpy( ssl->out_msg + ssl->out_msglen, mac, ssl->transform_out->maclen ); } else #endif @@ -1790,7 +1797,8 @@ static int ssl_decrypt_buf( ssl_context *ssl ) ssl_mac( &ssl->transform_in->md_ctx_dec, ssl->transform_in->mac_dec, ssl->in_msg, ssl->in_msglen, - ssl->in_ctr, ssl->in_msgtype ); + ssl->in_ctr, ssl->in_msgtype, + mac_expect ); } else #endif /* POLARSSL_SSL_PROTO_SSL3 */ From 921eb599f6934eee2f82bcba92b1a2b710746f46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 19 Dec 2017 10:03:46 +0100 Subject: [PATCH 2/2] Fix magic constant in previous commit --- library/ssl_tls.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1018270f9..855872b4f 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1050,11 +1050,12 @@ int ssl_psk_derive_premaster( ssl_context *ssl, key_exchange_type_t key_ex ) /* * SSLv3.0 MAC functions */ +#define SSL_MAC_MAX_BYTES 20 /* MD-5 or SHA-1 */ static void ssl_mac( md_context_t *md_ctx, const unsigned char *secret, const unsigned char *buf, size_t len, const unsigned char *ctr, int type, - unsigned char out[20] ) + unsigned char out[SSL_MAC_MAX_BYTES] ) { unsigned char header[11]; unsigned char padding[48]; @@ -1132,7 +1133,7 @@ static int ssl_encrypt_buf( ssl_context *ssl ) #if defined(POLARSSL_SSL_PROTO_SSL3) if( ssl->minor_ver == SSL_MINOR_VERSION_0 ) { - unsigned char mac[20]; /* SHA-1 at most */ + unsigned char mac[SSL_MAC_MAX_BYTES]; ssl_mac( &ssl->transform_out->md_ctx_enc, ssl->transform_out->mac_enc,