diff options
author | Richard Levitte <levitte@openssl.org> | 2003-02-19 12:04:07 +0000 |
---|---|---|
committer | Richard Levitte <levitte@openssl.org> | 2003-02-19 12:04:07 +0000 |
commit | 39c06a8b7349fa8fab06f7ebb4409fbeaa18c0d6 (patch) | |
tree | feb382efd93fe9c4ba261d2c4124eb7db9623c37 | |
parent | c0a48f4cfda9749e7bba04161a3523da61f37447 (diff) |
Security fix: Vaudenay timing attack on CBC.
An advisory will be posted to the web. Expect a release within the hour.
-rw-r--r-- | CHANGES | 12 | ||||
-rw-r--r-- | ssl/s3_pkt.c | 47 |
2 files changed, 41 insertions, 18 deletions
@@ -2,9 +2,17 @@ OpenSSL CHANGES _______________ - Changes between 0.9.6h and 0.9.6i [xx XXX xxxx] + Changes between 0.9.6h and 0.9.6i [19 Feb 2003] - *) + *) In ssl3_get_record (ssl/s3_pkt.c), minimize information leaked + via timing by performing a MAC computation even if incorrrect + block cipher padding has been found. This is a countermeasure + against active attacks where the attacker has to distinguish + between bad padding and a MAC verification error. (CAN-2003-0078) + + [Bodo Moeller; problem pointed out by Brice Canvel (EPFL), + Alain Hiltgen (UBS), Serge Vaudenay (EPFL), and + Martin Vuagnoux (EPFL, Ilion)] Changes between 0.9.6g and 0.9.6h [5 Dec 2002] diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index f52303c7c3..caf975d688 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -238,6 +238,8 @@ static int ssl3_get_record(SSL *s) unsigned int mac_size; int clear=0; size_t extra; + int decryption_failed_or_bad_record_mac = 0; + unsigned char *mac = NULL; rr= &(s->s3->rrec); sess=s->session; @@ -353,8 +355,11 @@ again: /* SSLerr() and ssl3_send_alert() have been called */ goto err; - /* otherwise enc_err == -1 */ - goto decryption_failed_or_bad_record_mac; + /* Otherwise enc_err == -1, which indicates bad padding + * (rec->length has not been changed in this case). + * To minimize information leaked via timing, we will perform + * the MAC computation anyway. */ + decryption_failed_or_bad_record_mac = 1; } #ifdef TLS_DEBUG @@ -380,28 +385,46 @@ printf("\n"); SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_PRE_MAC_LENGTH_TOO_LONG); goto f_err; #else - goto decryption_failed_or_bad_record_mac; + decryption_failed_or_bad_record_mac = 1; #endif } /* check the MAC for rr->input (it's in mac_size bytes at the tail) */ - if (rr->length < mac_size) + if (rr->length >= mac_size) { + rr->length -= mac_size; + mac = &rr->data[rr->length]; + } + else + { + /* record (minus padding) is too short to contain a MAC */ #if 0 /* OK only for stream ciphers */ al=SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_LENGTH_TOO_SHORT); goto f_err; #else - goto decryption_failed_or_bad_record_mac; + decryption_failed_or_bad_record_mac = 1; + rr->length = 0; #endif } - rr->length-=mac_size; i=s->method->ssl3_enc->mac(s,md,0); - if (memcmp(md,&(rr->data[rr->length]),mac_size) != 0) + if (mac == NULL || memcmp(md, mac, mac_size) != 0) { - goto decryption_failed_or_bad_record_mac; + decryption_failed_or_bad_record_mac = 1; } } + if (decryption_failed_or_bad_record_mac) + { + /* A separate 'decryption_failed' alert was introduced with TLS 1.0, + * SSL 3.0 only has 'bad_record_mac'. But unless a decryption + * failure is directly visible from the ciphertext anyway, + * we should not reveal which kind of error occured -- this + * might become visible to an attacker (e.g. via a logfile) */ + al=SSL_AD_BAD_RECORD_MAC; + SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); + goto f_err; + } + /* r->length is now just compressed */ if (s->expand != NULL) { @@ -443,14 +466,6 @@ printf("\n"); return(1); -decryption_failed_or_bad_record_mac: - /* Separate 'decryption_failed' alert was introduced with TLS 1.0, - * SSL 3.0 only has 'bad_record_mac'. But unless a decryption - * failure is directly visible from the ciphertext anyway, - * we should not reveal which kind of error occured -- this - * might become visible to an attacker (e.g. via logfile) */ - al=SSL_AD_BAD_RECORD_MAC; - SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); f_err: ssl3_send_alert(s,SSL3_AL_FATAL,al); err: |