X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=blobdiff_plain;f=ssl%2Fs3_pkt.c;h=1f89e8ccef2f67c91a4a9204223358dd5af11ee0;hp=5e60e18972734d059f295f72517e84bb7f18cd78;hb=dc0ed30cfeb37d64fc2bd26887b19e0898a96bde;hpb=82b0bf0b8792bdc113cadc04a1f9d40f0e0cfbfc diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index 5e60e18972..1f89e8ccef 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -112,9 +112,9 @@ #include #include #define USE_SOCKETS +#include "ssl_locl.h" #include #include -#include "ssl_locl.h" static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, unsigned int len, int create_empty_fragment); @@ -236,7 +236,10 @@ static int ssl3_get_record(SSL *s) unsigned char md[EVP_MAX_MD_SIZE]; short version; unsigned int mac_size; - int clear=0,extra; + 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; @@ -245,7 +248,7 @@ static int ssl3_get_record(SSL *s) extra=SSL3_RT_MAX_EXTRA; else extra=0; - if (extra != (s->s3->rbuf.len - SSL3_RT_MAX_PACKET_SIZE)) + if (extra != s->s3->rbuf.len - SSL3_RT_MAX_PACKET_SIZE) { /* actually likely an application error: SLS_OP_MICROSOFT_BIG_SSLV3_BUFFER * set after ssl3_setup_buffers() was done */ @@ -295,8 +298,7 @@ again: goto err; } - if (rr->length > - (unsigned int)SSL3_RT_MAX_ENCRYPTED_LENGTH+extra) + if (rr->length > SSL3_RT_MAX_ENCRYPTED_LENGTH+extra) { al=SSL_AD_RECORD_OVERFLOW; SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_PACKET_LENGTH_TOO_LONG); @@ -308,7 +310,7 @@ again: /* s->rstate == SSL_ST_READ_BODY, get and decode the data */ - if (rr->length > (s->packet_length-SSL3_RT_HEADER_LENGTH)) + if (rr->length > s->packet_length-SSL3_RT_HEADER_LENGTH) { /* now s->packet_length == SSL3_RT_HEADER_LENGTH */ i=rr->length; @@ -336,7 +338,7 @@ again: * rr->length bytes of encrypted compressed stuff. */ /* check is not needed I believe */ - if (rr->length > (unsigned int)SSL3_RT_MAX_ENCRYPTED_LENGTH+extra) + if (rr->length > SSL3_RT_MAX_ENCRYPTED_LENGTH+extra) { al=SSL_AD_RECORD_OVERFLOW; SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_ENCRYPTED_LENGTH_TOO_LONG); @@ -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,33 +385,50 @@ 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) { - if (rr->length > - (unsigned int)SSL3_RT_MAX_COMPRESSED_LENGTH+extra) + if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH+extra) { al=SSL_AD_RECORD_OVERFLOW; SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_COMPRESSED_LENGTH_TOO_LONG); @@ -420,7 +442,7 @@ printf("\n"); } } - if (rr->length > (unsigned int)SSL3_RT_MAX_PLAIN_LENGTH+extra) + if (rr->length > SSL3_RT_MAX_PLAIN_LENGTH+extra) { al=SSL_AD_RECORD_OVERFLOW; SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_DATA_LENGTH_TOO_LONG); @@ -444,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: @@ -605,7 +619,7 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, if (prefix_len <= 0) goto err; - if (s->s3->wbuf.len < prefix_len + SSL3_RT_MAX_PACKET_SIZE) + if (s->s3->wbuf.len < (size_t)prefix_len + SSL3_RT_MAX_PACKET_SIZE) { /* insufficient space */ SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); @@ -848,7 +862,7 @@ start: { al=SSL_AD_UNEXPECTED_MESSAGE; SSLerr(SSL_F_SSL3_READ_BYTES,SSL_R_DATA_BETWEEN_CCS_AND_FINISHED); - goto err; + goto f_err; } /* If the other end has shut down, throw anything we read away @@ -955,7 +969,7 @@ start: { al=SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_READ_BYTES,SSL_R_BAD_HELLO_REQUEST); - goto err; + goto f_err; } if (s->msg_callback) @@ -1066,9 +1080,9 @@ start: if ( (rr->length != 1) || (rr->off != 0) || (rr->data[0] != SSL3_MT_CCS)) { - i=SSL_AD_ILLEGAL_PARAMETER; + al=SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_SSL3_READ_BYTES,SSL_R_BAD_CHANGE_CIPHER_SPEC); - goto err; + goto f_err; } rr->length=0;