diff options
Diffstat (limited to 'ssl/s3_pkt.c')
-rw-r--r-- | ssl/s3_pkt.c | 80 |
1 files changed, 41 insertions, 39 deletions
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index 0e041c0ce0..dabcfd2464 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -246,11 +246,9 @@ static int ssl3_get_record(SSL *s) unsigned char *p; unsigned char md[EVP_MAX_MD_SIZE]; short version; - unsigned int mac_size; + unsigned 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; @@ -356,17 +354,10 @@ again: rr->data=rr->input; enc_err = s->method->ssl3_enc->enc(s,0); - if (enc_err <= 0) + if (enc_err == 0) { - if (enc_err == 0) - /* SSLerr() and ssl3_send_alert() have been called */ - goto err; - - /* 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; + /* SSLerr() and ssl3_send_alert() have been called */ + goto err; } #ifdef TLS_DEBUG @@ -383,44 +374,55 @@ printf("\n"); if (!clear) { - mac_size=EVP_MD_size(s->read_hash); - - if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH+extra+mac_size) + /* !clear => s->read_hash != NULL => mac_size != -1 */ + unsigned char *mac = NULL; + unsigned char mac_tmp[EVP_MAX_MD_SIZE]; + mac_size=EVP_MD_CTX_size(s->read_hash); + OPENSSL_assert(mac_size <= EVP_MAX_MD_SIZE); + + /* orig_len is the length of the record before any padding was + * removed. This is public information, as is the MAC in use, + * therefore we can safely process the record in a different + * amount of time if it's too short to possibly contain a MAC. + */ + if (rr->orig_len < mac_size || + /* CBC records must have a padding length byte too. */ + (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE && + rr->orig_len < mac_size+1)) { -#if 0 /* OK only for stream ciphers (then rr->length is visible from ciphertext anyway) */ - al=SSL_AD_RECORD_OVERFLOW; - SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_PRE_MAC_LENGTH_TOO_LONG); + al=SSL_AD_DECODE_ERROR; + SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_LENGTH_TOO_SHORT); goto f_err; -#else - 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 (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE) { + /* We update the length so that the TLS header bytes + * can be constructed correctly but we need to extract + * the MAC in constant time from within the record, + * without leaking the contents of the padding bytes. + * */ + mac = mac_tmp; + ssl3_cbc_copy_mac(mac_tmp, rr, 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 - decryption_failed_or_bad_record_mac = 1; - rr->length = 0; -#endif + /* In this case there's no padding, so |rec->orig_len| + * equals |rec->length| and we checked that there's + * enough bytes for |mac_size| above. */ + rr->length -= mac_size; + mac = &rr->data[rr->length]; } - i=s->method->ssl3_enc->mac(s,md,0); + + i=s->method->ssl3_enc->mac(s,md,0 /* not send */); if (i < 0 || mac == NULL || CRYPTO_memcmp(md, mac, (size_t)mac_size) != 0) - { - decryption_failed_or_bad_record_mac = 1; - } + enc_err = -1; + if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH+extra+mac_size) + enc_err = -1; } - if (decryption_failed_or_bad_record_mac) + if (enc_err < 0) { /* A separate 'decryption_failed' alert was introduced with TLS 1.0, * SSL 3.0 only has 'bad_record_mac'. But unless a decryption |