Prevent false positive in constant time checks
An optimization around the array access of in made valgrind think this
loop was non-constant time, while in fact it is. Prevent this
optimization by adding a value barrier.
See bug for more details.
Test: cmake -DCONSTANT_TIME_VALIDATION=1 -DCMAKE_BUILD_TYPE=Release \
-GNinja -B build && \
ninja -C build && \
valgrind ./build/crypto_test \
--gtest_filter=*PerAEADTest.UnalignedInput/AES_128_CBC_SHA1_TLS*
Test: cmake -DCMAKE_BUILD_TYPE=Release -GNinja -B build && \
ninja -C build && ./build/crypto_test
Bug: 436788086
Change-Id: I3f00d06c71cb3b30146dde890f157078915ea1f4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81867
Reviewed-by: David Benjamin <davidben@google.com>
Auto-Submit: Miriam Polzer <mpolzer@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/cipher/tls_cbc.cc b/crypto/cipher/tls_cbc.cc
index d34d2d6..31d3218 100644
--- a/crypto/cipher/tls_cbc.cc
+++ b/crypto/cipher/tls_cbc.cc
@@ -53,7 +53,10 @@
for (size_t i = 0; i < to_check; i++) {
uint8_t mask = constant_time_ge_8(padding_length, i);
- uint8_t b = in[in_len - 1 - i];
+ // The value barrier on |(in_len - 1 - i)| isn't needed to enforce
+ // constant-time. It is just there to prevent a false positive in
+ // constant-time checks by valgrind.
+ uint8_t b = in[value_barrier_w(in_len - 1 - i)];
// The final |padding_length+1| bytes should all have the value
// |padding_length|. Therefore the XOR should be zero.
good &= ~(mask & (padding_length ^ b));
