Follow on from CVE-2014-3571. This fixes the code that was the original source
[oweals/openssl.git] / ssl / d1_pkt.c
index 9fe21c3ea1445af08a03e52d6bbffb31f5936a5f..bc478c240cc3f72347aac80b851fcf5771137771 100644 (file)
@@ -139,7 +139,6 @@ static int dtls1_process_record(SSL *s);
 #if PQ_64BIT_IS_INTEGER
 static PQ_64BIT bytes_to_long_long(unsigned char *bytes, PQ_64BIT *num);
 #endif
-static void dtls1_clear_timeouts(SSL *s);
 
 /* copy buffered record into SSL structure */
 static int
@@ -256,9 +255,6 @@ dtls1_process_buffered_records(SSL *s)
     item = pqueue_peek(s->d1->unprocessed_rcds.q);
     if (item)
         {
-        DTLS1_RECORD_DATA *rdata;
-        rdata = (DTLS1_RECORD_DATA *)item->data;
-        
         /* Check if epoch is current. */
         if (s->d1->unprocessed_rcds.epoch != s->d1->r_epoch)
             return(1);  /* Nothing to do. */
@@ -331,15 +327,13 @@ dtls1_get_buffered_record(SSL *s)
 static int
 dtls1_process_record(SSL *s)
 {
-    int i,al;
-       int clear=0;
-    int enc_err;
+       int i,al;
+       int enc_err;
        SSL_SESSION *sess;
-    SSL3_RECORD *rr;
-       unsigned int mac_size;
+       SSL3_RECORD *rr;
+       unsigned int mac_size, orig_len;
        unsigned char md[EVP_MAX_MD_SIZE];
 
-
        rr= &(s->s3->rrec);
     sess = s->session;
 
@@ -370,13 +364,15 @@ dtls1_process_record(SSL *s)
        rr->data=rr->input;
 
        enc_err = s->method->ssl3_enc->enc(s,0);
-       if (enc_err <= 0)
+       /* enc_err is:
+        *    0: (in non-constant time) if the record is publically invalid.
+        *    1: if the padding is valid
+        *    -1: if the padding is invalid */
+       if (enc_err == 0)
                {
-               if (enc_err == 0)
-                       /* SSLerr() and ssl3_send_alert() have been called */
-                       goto err;
-
-               /* otherwise enc_err == -1 */
+               /* For DTLS we simply ignore bad packets. */
+               rr->length = 0;
+               s->packet_length = 0;
                goto err;
                }
 
@@ -387,42 +383,67 @@ printf("\n");
 #endif
 
        /* r->length is now the compressed data plus mac */
-if (   (sess == NULL) ||
-               (s->enc_read_ctx == NULL) ||
-               (s->read_hash == NULL))
-    clear=1;
-
-       if (!clear)
+       if ((sess != NULL) &&
+           (s->enc_read_ctx != NULL) &&
+           (s->read_hash != NULL))
                {
+               /* s->read_hash != NULL => mac_size != -1 */
+               unsigned char *mac = NULL;
+               unsigned char mac_tmp[EVP_MAX_MD_SIZE];
                mac_size=EVP_MD_size(s->read_hash);
+               OPENSSL_assert(mac_size <= EVP_MAX_MD_SIZE);
 
-               if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH+mac_size)
-                       {
-#if 0 /* OK only for stream ciphers (then rr->length is visible from ciphertext anyway) */
-                       al=SSL_AD_RECORD_OVERFLOW;
-                       SSLerr(SSL_F_DTLS1_PROCESS_RECORD,SSL_R_PRE_MAC_LENGTH_TOO_LONG);
-                       goto f_err;
-#else
-                       goto err;
-#endif                 
-                       }
-               /* check the MAC for rr->input (it's in mac_size bytes at the tail) */
-               if (rr->length < mac_size)
+               /* kludge: *_cbc_remove_padding passes padding length in rr->type */
+               orig_len = rr->length+((unsigned int)rr->type>>8);
+
+               /* 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 (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 &&
+                    orig_len < mac_size+1))
                        {
-#if 0 /* OK only for stream ciphers */
                        al=SSL_AD_DECODE_ERROR;
                        SSLerr(SSL_F_DTLS1_PROCESS_RECORD,SSL_R_LENGTH_TOO_SHORT);
                        goto f_err;
-#else
-                       goto err;
-#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 (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE)
                        {
-                       goto err;
+                       /* 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, orig_len);
+                       rr->length -= mac_size;
+                       }
+               else
+                       {
+                       /* In this case there's no padding, so |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 /* not send */);
+               if (i < 0 || mac == NULL || CRYPTO_memcmp(md, mac, (size_t)mac_size) != 0)
+                       enc_err = -1;
+               if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH+mac_size)
+                       enc_err = -1;
+               }
+
+       if (enc_err < 0)
+               {
+               /* decryption failed, silently discard message */
+               rr->length = 0;
+               s->packet_length = 0;
+               goto err;
                }
 
        /* r->length is now just compressed */
@@ -484,14 +505,12 @@ int dtls1_get_record(SSL *s)
        int ssl_major,ssl_minor;
        int i,n;
        SSL3_RECORD *rr;
-       SSL_SESSION *sess;
        unsigned char *p = NULL;
        unsigned short version;
        DTLS1_BITMAP *bitmap;
        unsigned int is_next_epoch;
 
        rr= &(s->s3->rrec);
-       sess=s->session;
 
     /* The epoch may have changed.  If so, process all the
      * pending records.  This is a non-blocking operation. */
@@ -576,8 +595,6 @@ again:
                /* now s->packet_length == DTLS1_RT_HEADER_LENGTH */
                i=rr->length;
                n=ssl3_read_n(s,i,i,1);
-               if (n <= 0) return(n); /* error or non-blocking io */
-
                /* this packet contained a partial record, dump it */
                if ( n != i)
                        {
@@ -607,7 +624,8 @@ again:
         * would be dropped unnecessarily.
         */
        if (!(s->d1->listen && rr->type == SSL3_RT_HANDSHAKE &&
-               *p == SSL3_MT_CLIENT_HELLO) &&
+               s->packet_length > DTLS1_RT_HEADER_LENGTH &&
+               s->packet[DTLS1_RT_HEADER_LENGTH] == SSL3_MT_CLIENT_HELLO) &&
                ! dtls1_record_replay_check(s, bitmap, &(rr->seq_num)))
                {
                rr->length = 0;
@@ -620,10 +638,12 @@ again:
 
        /* If this record is from the next epoch (either HM or ALERT),
         * and a handshake is currently in progress, buffer it since it
-        * cannot be processed at this time. */
+        * cannot be processed at this time. However, do not buffer
+        * anything while listening.
+        */
        if (is_next_epoch)
                {
-               if (SSL_in_init(s) || s->in_handshake)
+               if ((SSL_in_init(s) || s->in_handshake) && !s->d1->listen)
                        {
                        dtls1_buffer_record(s, &(s->d1->unprocessed_rcds), &rr->seq_num);
                        }
@@ -639,7 +659,6 @@ again:
                goto again;     /* get another record */
                }
 
-       dtls1_clear_timeouts(s);  /* done waiting */
        return(1);
 
        }
@@ -754,6 +773,12 @@ start:
                        }
                }
 
+       if (s->d1->listen && rr->type != SSL3_RT_HANDSHAKE)
+               {
+               rr->length = 0;
+               goto start;
+               }
+
        /* we now have a packet which can be read and processed */
 
        if (s->s3->change_cipher_spec /* set when we receive ChangeCipherSpec,
@@ -920,6 +945,7 @@ start:
                        !(s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) &&
                        !s->s3->renegotiate)
                        {
+                       s->d1->handshake_read_seq++;
                        ssl3_renegotiate(s);
                        if (ssl3_renegotiate_check(s))
                                {
@@ -1108,6 +1134,9 @@ start:
                 */
                if (msg_hdr.type == SSL3_MT_FINISHED)
                        {
+                       if (dtls1_check_timeout_num(s) < 0)
+                               return -1;
+
                        dtls1_retransmit_buffered_messages(s);
                        rr->length = 0;
                        goto start;
@@ -1811,10 +1840,3 @@ bytes_to_long_long(unsigned char *bytes, PQ_64BIT *num)
        return _num;
        }
 #endif
-
-
-static void
-dtls1_clear_timeouts(SSL *s)
-       {
-       memset(&(s->d1->timeout), 0x00, sizeof(struct dtls1_timeout_st));
-       }