diff options
| author | Joel Sing <jsing@cvs.openbsd.org> | 2014-09-22 12:36:07 +0000 |
|---|---|---|
| committer | Joel Sing <jsing@cvs.openbsd.org> | 2014-09-22 12:36:07 +0000 |
| commit | e4cae058f8495b54a602f68f62df47033447c0ae (patch) | |
| tree | 5a6a5adfdd55e30fec0c48ad33831f6bac65a6b5 /lib | |
| parent | 1bddfd9c149d915055fd10dd3c079af1001ee228 (diff) | |
It is possible (although unlikely in practice) for peer_finish_md_len to
end up with a value of zero, primarily since ssl3_take_mac() fails to check
the return value from the final_finish_mac() call. This would then mean that
an SSL finished message with a zero-byte payload would successfully match
against the calculated finish MAC.
Avoid this by checking the length of peer_finish_md_len and the SSL
finished message payload, against the known length already stored in
the SSL3_ENC_METHOD finish_mac_length field (making use of a previously
unused field).
ok miod@ (a little while back)
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/libssl/src/ssl/s3_both.c | 24 |
1 files changed, 11 insertions, 13 deletions
diff --git a/lib/libssl/src/ssl/s3_both.c b/lib/libssl/src/ssl/s3_both.c index 6ba3d4bfcef..17368f1107f 100644 --- a/lib/libssl/src/ssl/s3_both.c +++ b/lib/libssl/src/ssl/s3_both.c @@ -1,4 +1,4 @@ -/* $OpenBSD: s3_both.c,v 1.28 2014/08/07 20:02:23 miod Exp $ */ +/* $OpenBSD: s3_both.c,v 1.29 2014/09/22 12:36:06 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -223,7 +223,7 @@ ssl3_take_mac(SSL *s) int ssl3_get_finished(SSL *s, int a, int b) { - int al, i, ok; + int al, ok, md_len; long n; unsigned char *p; @@ -247,33 +247,31 @@ ssl3_get_finished(SSL *s, int a, int b) } s->s3->change_cipher_spec = 0; + md_len = s->method->ssl3_enc->finish_mac_length; p = (unsigned char *)s->init_msg; - i = s->s3->tmp.peer_finish_md_len; - if (i != n) { + if (s->s3->tmp.peer_finish_md_len != md_len || n != md_len) { al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_GET_FINISHED, SSL_R_BAD_DIGEST_LENGTH); goto f_err; } - if (timingsafe_memcmp(p, s->s3->tmp.peer_finish_md, i) != 0) { + if (timingsafe_memcmp(p, s->s3->tmp.peer_finish_md, md_len) != 0) { al = SSL_AD_DECRYPT_ERROR; SSLerr(SSL_F_SSL3_GET_FINISHED, SSL_R_DIGEST_CHECK_FAILED); goto f_err; } - /* Copy the finished so we can use it for - renegotiation checks */ + /* Copy finished so we can use it for renegotiation checks. */ + OPENSSL_assert(md_len <= EVP_MAX_MD_SIZE); if (s->type == SSL_ST_ACCEPT) { - OPENSSL_assert(i <= EVP_MAX_MD_SIZE); memcpy(s->s3->previous_client_finished, - s->s3->tmp.peer_finish_md, i); - s->s3->previous_client_finished_len = i; + s->s3->tmp.peer_finish_md, md_len); + s->s3->previous_client_finished_len = md_len; } else { - OPENSSL_assert(i <= EVP_MAX_MD_SIZE); memcpy(s->s3->previous_server_finished, - s->s3->tmp.peer_finish_md, i); - s->s3->previous_server_finished_len = i; + s->s3->tmp.peer_finish_md, md_len); + s->s3->previous_server_finished_len = md_len; } return (1); |