Fix IV check and padding removal.
[openssl.git] / ssl / s3_cbc.c
index 2ab7bfb60a5632de169ee47b87702d3bd130e19a..8dfc11e713204fe82014e810e431daf2c7c465c5 100644 (file)
@@ -116,7 +116,9 @@ int ssl3_cbc_remove_padding(const SSL* s,
        good = constant_time_ge(rec->length, padding_length+overhead);
        /* SSLv3 requires that the padding is minimal. */
        good &= constant_time_ge(block_size, padding_length+1);
-       rec->length -= good & (padding_length+1);
+       padding_length = good & (padding_length+1);
+       rec->length -= padding_length;
+       rec->type |= padding_length<<8; /* kludge: pass padding length */
        return (int)((good & 1) | (~good & -1));
 }
 
@@ -137,14 +139,21 @@ int tls1_cbc_remove_padding(const SSL* s,
                            unsigned mac_size)
        {
        unsigned padding_length, good, to_check, i;
-       const char has_explicit_iv = s->version == DTLS1_VERSION;
-       const unsigned overhead = 1 /* padding length byte */ +
-                                 mac_size +
-                                 (has_explicit_iv ? block_size : 0);
-
-       /* These lengths are all public so we can test them in non-constant
-        * time. */
-       if (overhead > rec->length)
+       const unsigned overhead = 1 /* padding length byte */ + mac_size;
+       /* Check if version requires explicit IV */
+       if (s->version == DTLS1_VERSION)
+               {
+               /* These lengths are all public so we can test them in
+                * non-constant time.
+                */
+               if (overhead + block_size > rec->length)
+                       return 0;
+               /* We can now safely skip explicit IV */
+               rec->data += block_size;
+               rec->input += block_size;
+               rec->length -= block_size;
+               }
+       else if (overhead > rec->length)
                return 0;
 
        padding_length = rec->data[rec->length-1];
@@ -202,23 +211,9 @@ int tls1_cbc_remove_padding(const SSL* s,
        good <<= sizeof(good)*8-1;
        good = DUPLICATE_MSB_TO_ALL(good);
 
-       rec->length -= good & (padding_length+1);
-
-       /* We can always safely skip the explicit IV. We check at the beginning
-        * of this function that the record has at least enough space for the
-        * IV, MAC and padding length byte. (These can be checked in
-        * non-constant time because it's all public information.) So, if the
-        * padding was invalid, then we didn't change |rec->length| and this is
-        * safe. If the padding was valid then we know that we have at least
-        * overhead+padding_length bytes of space and so this is still safe
-        * because overhead accounts for the explicit IV. */
-       if (has_explicit_iv)
-               {
-               rec->data += block_size;
-               rec->input += block_size;
-               rec->length -= block_size;
-               rec->orig_len -= block_size;
-               }
+       padding_length = good & (padding_length+1);
+       rec->length -= padding_length;
+       rec->type |= padding_length<<8; /* kludge: pass padding length */
 
        return (int)((good & 1) | (~good & -1));
        }
@@ -245,7 +240,7 @@ int tls1_cbc_remove_padding(const SSL* s,
  */
 void ssl3_cbc_copy_mac(unsigned char* out,
                       const SSL3_RECORD *rec,
-                      unsigned md_size)
+                      unsigned md_size,unsigned orig_len)
        {
 #if defined(CBC_MAC_ROTATE_IN_PLACE)
        unsigned char rotated_mac_buf[EVP_MAX_MD_SIZE*2];
@@ -264,7 +259,7 @@ void ssl3_cbc_copy_mac(unsigned char* out,
        unsigned div_spoiler;
        unsigned rotate_offset;
 
-       OPENSSL_assert(rec->orig_len >= md_size);
+       OPENSSL_assert(orig_len >= md_size);
        OPENSSL_assert(md_size <= EVP_MAX_MD_SIZE);
 
 #if defined(CBC_MAC_ROTATE_IN_PLACE)
@@ -272,8 +267,8 @@ void ssl3_cbc_copy_mac(unsigned char* out,
 #endif
 
        /* This information is public so it's safe to branch based on it. */
-       if (rec->orig_len > md_size + 255 + 1)
-               scan_start = rec->orig_len - (md_size + 255 + 1);
+       if (orig_len > md_size + 255 + 1)
+               scan_start = orig_len - (md_size + 255 + 1);
        /* div_spoiler contains a multiple of md_size that is used to cause the
         * modulo operation to be constant time. Without this, the time varies
         * based on the amount of padding when running on Intel chips at least.
@@ -286,9 +281,9 @@ void ssl3_cbc_copy_mac(unsigned char* out,
        rotate_offset = (div_spoiler + mac_start - scan_start) % md_size;
 
        memset(rotated_mac, 0, md_size);
-       for (i = scan_start; i < rec->orig_len;)
+       for (i = scan_start; i < orig_len;)
                {
-               for (j = 0; j < md_size && i < rec->orig_len; i++, j++)
+               for (j = 0; j < md_size && i < orig_len; i++, j++)
                        {
                        unsigned char mac_started = constant_time_ge(i, mac_start);
                        unsigned char mac_ended = constant_time_ge(i, mac_end);
@@ -317,16 +312,24 @@ void ssl3_cbc_copy_mac(unsigned char* out,
 #endif
        }
 
+/* u32toLE serialises an unsigned, 32-bit number (n) as four bytes at (p) in
+ * little-endian order. The value of p is advanced by four. */
+#define u32toLE(n, p) \
+       (*((p)++)=(unsigned char)(n), \
+        *((p)++)=(unsigned char)(n>>8), \
+        *((p)++)=(unsigned char)(n>>16), \
+        *((p)++)=(unsigned char)(n>>24))
+
 /* These functions serialize the state of a hash and thus perform the standard
  * "final" operation without adding the padding and length that such a function
  * typically does. */
 static void tls1_md5_final_raw(void* ctx, unsigned char *md_out)
        {
        MD5_CTX *md5 = ctx;
-       l2n(md5->A, md_out);
-       l2n(md5->B, md_out);
-       l2n(md5->C, md_out);
-       l2n(md5->D, md_out);
+       u32toLE(md5->A, md_out);
+       u32toLE(md5->B, md_out);
+       u32toLE(md5->C, md_out);
+       u32toLE(md5->D, md_out);
        }
 
 static void tls1_sha1_final_raw(void* ctx, unsigned char *md_out)
@@ -446,6 +449,7 @@ void ssl3_cbc_digest_record(
        /* mdLengthSize is the number of bytes in the length field that terminates
        * the hash. */
        unsigned md_length_size = 8;
+       char length_is_big_endian = 1;
 
        /* This is a, hopefully redundant, check that allows us to forget about
         * many possible overflows later in this function. */
@@ -459,6 +463,7 @@ void ssl3_cbc_digest_record(
                        md_transform = (void(*)(void *ctx, const unsigned char *block)) MD5_Transform;
                        md_size = 16;
                        sslv3_pad_length = 48;
+                       length_is_big_endian = 0;
                        break;
                case NID_sha1:
                        SHA1_Init((SHA_CTX*)md_state.c);
@@ -599,11 +604,22 @@ void ssl3_cbc_digest_record(
                md_transform(md_state.c, hmac_pad);
                }
 
-       memset(length_bytes,0,md_length_size-4);
-       length_bytes[md_length_size-4] = (unsigned char)(bits>>24);
-       length_bytes[md_length_size-3] = (unsigned char)(bits>>16);
-       length_bytes[md_length_size-2] = (unsigned char)(bits>>8);
-       length_bytes[md_length_size-1] = (unsigned char)bits;
+       if (length_is_big_endian)
+               {
+               memset(length_bytes,0,md_length_size-4);
+               length_bytes[md_length_size-4] = (unsigned char)(bits>>24);
+               length_bytes[md_length_size-3] = (unsigned char)(bits>>16);
+               length_bytes[md_length_size-2] = (unsigned char)(bits>>8);
+               length_bytes[md_length_size-1] = (unsigned char)bits;
+               }
+       else
+               {
+               memset(length_bytes,0,md_length_size);
+               length_bytes[md_length_size-5] = (unsigned char)(bits>>24);
+               length_bytes[md_length_size-6] = (unsigned char)(bits>>16);
+               length_bytes[md_length_size-7] = (unsigned char)(bits>>8);
+               length_bytes[md_length_size-8] = (unsigned char)bits;
+               }
 
        if (k > 0)
                {