Add a value barrier in p224_select_point
Clang seems to be undoing the constant-time code here. This is another
constant-time table lookup, so this is more of the usual problem. Found
with the valgrind-based tooling.
I didn't switch this to constant_time_conditional_memxor since we still
haven't figured out https://crbug.com/boringssl/655, but we definitely
need some kind of abstraction for this pattern.
Change-Id: Ic11873b23cde31375ac1a326ed09ac1ca53ec913
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64310
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/fipsmodule/ec/p224-64.c b/crypto/fipsmodule/ec/p224-64.c
index 9d0242c..d2b7f9f 100644
--- a/crypto/fipsmodule/ec/p224-64.c
+++ b/crypto/fipsmodule/ec/p224-64.c
@@ -24,6 +24,7 @@
#include <openssl/err.h>
#include <openssl/mem.h>
+#include <assert.h>
#include <string.h>
#include "internal.h"
@@ -836,12 +837,12 @@
for (size_t i = 0; i < size; i++) {
const p224_limb *inlimbs = &pre_comp[i][0][0];
- uint64_t mask = i ^ idx;
- mask |= mask >> 4;
- mask |= mask >> 2;
- mask |= mask >> 1;
- mask &= 1;
- mask--;
+ static_assert(sizeof(uint64_t) <= sizeof(crypto_word_t),
+ "crypto_word_t too small");
+ static_assert(sizeof(size_t) <= sizeof(crypto_word_t),
+ "crypto_word_t too small");
+ // Without a value barrier, Clang adds a branch here.
+ uint64_t mask = value_barrier_w(constant_time_eq_w(i, idx));
for (size_t j = 0; j < 4 * 3; j++) {
outlimbs[j] |= inlimbs[j] & mask;
}
