From f3935c3429597dc6fe9e51fbb176ae0aa747eb4f Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Date: Wed, 20 Jun 2018 10:58:36 +0200
Subject: [PATCH 2/4] dummy_wait: always hash the same amount of blocks that
 would have been on minimum pad

This improves protection against lucky13-type of attacks when
encrypt-then-mac is not in use.

Resolves #456
---
 lib/gnutls_cipher.c | 63 ++++++++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

Index: gnutls-3.4.17/lib/gnutls_cipher.c
===================================================================
--- gnutls-3.4.17.orig/lib/gnutls_cipher.c
+++ gnutls-3.4.17/lib/gnutls_cipher.c
@@ -453,42 +453,42 @@ compressed_to_ciphertext(gnutls_session_
 	return length;
 }
 
-static void dummy_wait(record_parameters_st * params,
-		       gnutls_datum_t * plaintext, unsigned pad_failed,
-		       unsigned int pad, unsigned total)
+static void dummy_wait(record_parameters_st *params,
+		       gnutls_datum_t *plaintext,
+		       unsigned int mac_data, unsigned max_mac_data)
 {
 	/* this hack is only needed on CBC ciphers */
 	if (_gnutls_cipher_type(params->cipher) == CIPHER_BLOCK) {
-		unsigned len, v;
-
-		/* force an additional hash compression function evaluation to prevent timing 
+		unsigned v;
+		unsigned int tag_size =
+		  _gnutls_auth_cipher_tag_len(&params->read.cipher_state);
+		unsigned hash_block = _gnutls_mac_block_size(params->mac);
+		/* force additional hash compression function evaluations to prevent timing
 		 * attacks that distinguish between wrong-mac + correct pad, from wrong-mac + incorrect pad.
 		 */
-		if (pad_failed == 0 && pad > 0) {
-			len = _gnutls_mac_block_size(params->mac);
-			if (len > 0) {
-				if (params->mac && params->mac->id == GNUTLS_MAC_SHA384)
-					/* v = 1 for the hash function padding + 16 for message length */
-					v = 17;
-				else /* v = 1 for the hash function padding + 8 for message length */
-					v = 9;
-
-				if ((pad + total) % len > len - v
-				    && total % len <= len - v) {
-					if (len < plaintext->size)
-						_gnutls_auth_cipher_add_auth
-						    (&params->read.
-						     cipher_state,
-						     plaintext->data, len);
-					else
-						_gnutls_auth_cipher_add_auth
-						    (&params->read.
-						     cipher_state,
-						     plaintext->data,
-						     plaintext->size);
-				}
-			}
-		}
+		if (params->mac && params->mac->id == GNUTLS_MAC_SHA384)
+                       /* v = 1 for the hash function padding + 16 for message length */
+                       v = 17;
+               else /* v = 1 for the hash function padding + 8 for message length */
+                       v = 9;
+
+               if (hash_block > 0) {
+                       int max_blocks = (max_mac_data+v+hash_block-1)/hash_block;
+                       int hashed_blocks = (mac_data+v+hash_block-1)/hash_block;
+                       unsigned to_hash;
+
+                       max_blocks -= hashed_blocks;
+                       if (max_blocks < 1)
+                               return;
+
+                       to_hash = max_blocks * hash_block;
+                       if ((unsigned)to_hash+1+tag_size < plaintext->size) {
+                               _gnutls_auth_cipher_add_auth
+                                            (&params->read.cipher_state,
+                                            plaintext->data+plaintext->size-tag_size-to_hash-1,
+                                            to_hash);
+		       }
+	       }
 	}
 }
 
@@ -814,7 +815,7 @@ ciphertext_to_compressed(gnutls_session_
 			return gnutls_assert_val(ret);
 
 		/* Here there could be a timing leakage in CBC ciphersuites that
-		 * could be exploited if the cost of a successful memcmp is high. 
+		 * could be exploited if the cost of a successful memcmp is high.
 		 * A constant time memcmp would help there, but it is not easy to maintain
 		 * against compiler optimizations. Currently we rely on the fact that
 		 * a memcmp comparison is negligible over the crypto operations.
@@ -822,8 +823,10 @@ ciphertext_to_compressed(gnutls_session_
 		if (unlikely
 		    (gnutls_memcmp(tag, tag_ptr, tag_size) != 0 || pad_failed != 0)) {
 			/* HMAC was not the same. */
-			dummy_wait(params, compressed, pad_failed, pad,
-				   length + preamble_size);
+			gnutls_datum_t data = {compressed->data, ciphertext->size};
+
+			dummy_wait(params, &data, length + preamble_size,
+				   preamble_size + ciphertext->size - tag_size - 1);
 
 			return gnutls_assert_val(GNUTLS_E_DECRYPTION_FAILED);
 		}
Index: gnutls-3.4.17/tests/pkcs12_encode.c
===================================================================
--- gnutls-3.4.17.orig/tests/pkcs12_encode.c
+++ gnutls-3.4.17/tests/pkcs12_encode.c
@@ -220,18 +220,6 @@ void doit(void)
 		exit(1);
 	}
 
-	ret = gnutls_pkcs12_generate_mac2(pkcs12, GNUTLS_MAC_SHA512, "passwd1");
-	if (ret < 0) {
-		fprintf(stderr, "generate_mac2: %s (%d)\n", gnutls_strerror(ret), ret);
-		exit(1);
-	}
-
-	ret = gnutls_pkcs12_verify_mac(pkcs12, "passwd1");
-	if (ret < 0) {
-		fprintf(stderr, "verify_mac2: %s (%d)\n", gnutls_strerror(ret), ret);
-		exit(1);
-	}
-
 	size = sizeof(outbuf);
 	ret =
 	    gnutls_pkcs12_export(pkcs12, GNUTLS_X509_FMT_PEM, outbuf,
