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 #455Signed-off-by: Nikos Mavrogiannopoulos's avatarNikos Mavrogiannopoulos <[email protected]>
parent 3f21d860
......@@ -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,
......
......@@ -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. */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment