Fix EVP_tls_cbc_digest_record is slow using SHA-384 and short messages
Symptom: When using larger hash functions and short messages,
these six blocks take too much time to be conditionally copied.
Observations:
- SHA-384 consumes more data per iteration, unlike SHA-256.
- The value of `kVarianceBlocks` must depend on the parameters
of the selected hash algorithm.
- Avoid magic constants.
Changes:
- A new formula for the kVarianceBlocks value.
- Stronger test vectors were created in change: 32724.
- The new formula passes these tests.
Discussion:
OpenSSL team: https://github.com/openssl/openssl/pull/7342
Quoting mattcaswell:
> The "real" data that needs to be hashed has to be padded for the
> hashing algorithm. For SHA1 the smallest amount of padding that
> can be added is the "0x80" byte plus 8 bytes containing the message
> length, i.e. 9 bytes. If the data length is within 9 bytes of the
> end of the hash block boundary then the padding will push it into
> an extra block to be hashed.
Change-Id: Id1ad2389927014316eed2b453aac6e4c2a585c5c
Reviewed-on: https://boringssl-review.googlesource.com/c/32624
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/crypto/cipher_extra/tls_cbc.c b/crypto/cipher_extra/tls_cbc.c
index a24602b..52353f2 100644
--- a/crypto/cipher_extra/tls_cbc.c
+++ b/crypto/cipher_extra/tls_cbc.c
@@ -329,9 +329,18 @@
// padding value.
//
// TLSv1 has MACs up to 48 bytes long (SHA-384) and the padding is not
- // required to be minimal. Therefore we say that the final six blocks
- // can vary based on the padding.
- static const size_t kVarianceBlocks = 6;
+ // required to be minimal. Therefore we say that the final |kVarianceBlocks|
+ // blocks can vary based on the padding and on the hash used. This value
+ // must be derived from public information.
+ const size_t kVarianceBlocks =
+ ( 255 + 1 + // maximum padding bytes + padding length
+ md_size + // length of hash's output
+ md_block_size - 1 // ceiling
+ ) / md_block_size
+ + 1; // the 0x80 marker and the encoded message length could or not
+ // require an extra block; since the exact value depends on the
+ // message length; thus, one extra block is always added to run
+ // in constant time.
// From now on we're dealing with the MAC, which conceptually has 13
// bytes of `header' before the start of the data.