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.
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index a8aa1c0..b977cfb 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1203,9 +1203,11 @@
/*
* SSLv3.0 MAC functions
*/
-static void ssl_mac( mbedtls_md_context_t *md_ctx, unsigned char *secret,
- unsigned char *buf, size_t len,
- unsigned char *ctr, int type )
+static void ssl_mac( mbedtls_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];
@@ -1230,14 +1232,14 @@
mbedtls_md_update( md_ctx, padding, padlen );
mbedtls_md_update( md_ctx, header, 11 );
mbedtls_md_update( md_ctx, buf, len );
- mbedtls_md_finish( md_ctx, buf + len );
+ mbedtls_md_finish( md_ctx, out );
memset( padding, 0x5C, padlen );
mbedtls_md_starts( md_ctx );
mbedtls_md_update( md_ctx, secret, md_size );
mbedtls_md_update( md_ctx, padding, padlen );
- mbedtls_md_update( md_ctx, buf + len, md_size );
- mbedtls_md_finish( md_ctx, buf + len );
+ mbedtls_md_update( md_ctx, out, md_size );
+ mbedtls_md_finish( md_ctx, out );
}
#endif /* MBEDTLS_SSL_PROTO_SSL3 */
@@ -1282,10 +1284,15 @@
#if defined(MBEDTLS_SSL_PROTO_SSL3)
if( ssl->minor_ver == MBEDTLS_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
@@ -1932,7 +1939,8 @@
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 /* MBEDTLS_SSL_PROTO_SSL3 */