dummy_wait: correctly account the length field in SHA384 HMAC

The existing lucky13 attack count-measures did not work correctly for
SHA384 HMAC.

The overall impact of that should not be significant as SHA384 is prioritized
lower than SHA256 or SHA1 and thus it is not typically negotiated, unless a
client prioritizes a SHA384 MAC, or a server only supports SHA384, and in both
cases the vulnerability is only present if Encrypt-then-MAC (RFC7366) is unsupported
by the peer.

Resolves #455

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

2 files changed, 13 insertions, 15 deletions

diff --git a/lib/algorithms/mac.c b/lib/algorithms/mac.c
index ba982b72fc..f6f9cb3ecf 100644
--- a/lib/algorithms/mac.c
+++ b/lib/algorithms/mac.c
@@ -59,14 +59,14 @@ static const mac_entry_st hash_algorithms[] = {
 	 .id = GNUTLS_MAC_SHA384,
 	 .output_size = 48,
 	 .key_size = 48,
-	 .block_size = 64},
+	 .block_size = 128},
 	{.name = "SHA512",
 	 .oid = HASH_OID_SHA512,
 	 .mac_oid = MAC_OID_SHA512,
 	 .id = GNUTLS_MAC_SHA512,
 	 .output_size = 64,
 	 .key_size = 64,
-	 .block_size = 64},
+	 .block_size = 128},
 	{.name = "SHA224",
 	 .oid = HASH_OID_SHA224,
 	 .mac_oid = MAC_OID_SHA224,
diff --git a/lib/cipher.c b/lib/cipher.c
index a2215bc1ee..792f4e8c71 100644
--- a/lib/cipher.c
+++ b/lib/cipher.c
@@ -499,9 +499,10 @@ static void dummy_wait(record_parameters_st * params,
 		       gnutls_datum_t * plaintext, unsigned pad_failed,
 		       unsigned int pad, unsigned total)
 {
-	/* this hack is only needed on CBC ciphers */
+	/* this hack is only needed on CBC ciphers when Encrypt-then-MAC mode
+	 * is not supported by the peer. */
 	if (_gnutls_cipher_type(params->cipher) == CIPHER_BLOCK) {
-		unsigned len;
+		unsigned len, v;
 
 		/* force an additional hash compression function evaluation to prevent timing 
 		 * attacks that distinguish between wrong-mac + correct pad, from wrong-mac + incorrect pad.
@@ -509,11 +510,14 @@ static void dummy_wait(record_parameters_st * params,
 		if (pad_failed == 0 && pad > 0) {
 			len = _gnutls_mac_block_size(params->mac);
 			if (len > 0) {
-				/* This is really specific to the current hash functions.
-				 * It should be removed once a protocol fix is in place.
-				 */
-				if ((pad + total) % len > len - 9
-				    && total % len <= len - 9) {
+				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.
@@ -852,12 +856,6 @@ decrypt_packet(gnutls_session_t session,
 		if (unlikely(ret < 0))
 			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. 
-		 * 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.
-		 */
 		if (unlikely
 		    (gnutls_memcmp(tag, tag_ptr, tag_size) != 0 || pad_failed != 0)) {
 			/* HMAC was not the same. */