Fix constant_time_conditional_memxor on some input lengths
This wasn't called this way within the library, but fix it before it
ever is.
Thanks to Joe Birr-Pixton for catching this.
Bug: 455606200
Change-Id: Ia6f4a63c595ca27822073188851f0bd62a6196ba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/83367
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Lily Chen <chlily@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/crypto/constant_time_test.cc b/crypto/constant_time_test.cc
index 77b6c7d..cc0f97f 100644
--- a/crypto/constant_time_test.cc
+++ b/crypto/constant_time_test.cc
@@ -141,69 +141,77 @@
}
TEST(ConstantTimeTest, MemCmov) {
- for (int i = 0; i < 100; i++) {
- uint8_t out[256], in[256];
- RAND_bytes(out, sizeof(out));
- RAND_bytes(in, sizeof(in));
+ static constexpr size_t kMaxLen = 256;
+ uint8_t out[kMaxLen], in[kMaxLen];
+ for (size_t len = 0; len <= kMaxLen; len++) {
+ SCOPED_TRACE(len);
+ for (int i = 0; i < 100; i++) {
+ RAND_bytes(out, len);
+ RAND_bytes(in, len);
- uint8_t b = 0;
- RAND_bytes(&b, 1);
- b = constant_time_is_zero_8(b & 0xf);
+ uint8_t b = 0;
+ RAND_bytes(&b, 1);
+ b = constant_time_is_zero_8(b & 0xf);
- uint8_t ref_in[256];
- OPENSSL_memcpy(ref_in, in, sizeof(in));
+ uint8_t ref_in[256];
+ OPENSSL_memcpy(ref_in, in, len);
- uint8_t ref_out[256];
- OPENSSL_memcpy(ref_out, out, sizeof(out));
- if (b) {
- OPENSSL_memcpy(ref_out, in, sizeof(in));
+ uint8_t ref_out[256];
+ OPENSSL_memcpy(ref_out, out, len);
+ if (b) {
+ OPENSSL_memcpy(ref_out, in, len);
+ }
+
+ CONSTTIME_SECRET(out, len);
+ CONSTTIME_SECRET(in, len);
+ CONSTTIME_SECRET(&b, 1);
+
+ constant_time_conditional_memcpy(out, in, len, b);
+
+ CONSTTIME_DECLASSIFY(&in, len);
+ CONSTTIME_DECLASSIFY(&out, len);
+
+ EXPECT_EQ(Bytes(in, len), Bytes(ref_in, len));
+ EXPECT_EQ(Bytes(out, len), Bytes(ref_out, len));
}
-
- CONSTTIME_SECRET(out, sizeof(out));
- CONSTTIME_SECRET(in, sizeof(in));
- CONSTTIME_SECRET(&b, 1);
-
- constant_time_conditional_memcpy(out, in, sizeof(out), b);
-
- CONSTTIME_DECLASSIFY(&in, sizeof(in));
- CONSTTIME_DECLASSIFY(&out, sizeof(out));
-
- EXPECT_EQ(Bytes(in), Bytes(ref_in));
- EXPECT_EQ(Bytes(out), Bytes(ref_out));
}
}
TEST(ConstantTimeTest, MemCxor) {
- for (int i = 0; i < 100; i++) {
- uint8_t out[256], in[256];
- RAND_bytes(out, sizeof(out));
- RAND_bytes(in, sizeof(in));
+ static constexpr size_t kMaxLen = 256;
+ uint8_t out[kMaxLen], in[kMaxLen];
+ for (size_t len = 0; len <= kMaxLen; len++) {
+ SCOPED_TRACE(len);
+ for (int i = 0; i < 100; i++) {
+ RAND_bytes(out, len);
+ RAND_bytes(in, len);
- uint8_t b = 0;
- RAND_bytes(&b, 1);
- b = constant_time_is_zero_8(b & 0xf);
+ uint8_t b = 0;
+ RAND_bytes(&b, 1);
+ b = constant_time_is_zero_8(b & 0xf);
- uint8_t ref_in[256];
- OPENSSL_memcpy(ref_in, in, sizeof(in));
+ uint8_t ref_in[kMaxLen];
+ OPENSSL_memcpy(ref_in, in, len);
- uint8_t ref_out[256];
- OPENSSL_memcpy(ref_out, out, sizeof(out));
- if (b) {
- for (size_t j = 0; j < sizeof(ref_out); ++j) {
- ref_out[j] ^= in[j];
+ uint8_t ref_out[kMaxLen];
+ OPENSSL_memcpy(ref_out, out, len);
+ if (b) {
+ for (size_t j = 0; j < len; ++j) {
+ ref_out[j] ^= in[j];
+ }
}
+
+ CONSTTIME_SECRET(out, len);
+ CONSTTIME_SECRET(in, len);
+ CONSTTIME_SECRET(&b, 1);
+
+ constant_time_conditional_memxor(out, in, len, b);
+
+ CONSTTIME_DECLASSIFY(&in, len);
+ CONSTTIME_DECLASSIFY(&out, len);
+
+ EXPECT_EQ(Bytes(in, len), Bytes(ref_in, len));
+ EXPECT_EQ(Bytes(out, len), Bytes(ref_out, len));
}
-
- CONSTTIME_SECRET(out, sizeof(out));
- CONSTTIME_SECRET(in, sizeof(in));
- CONSTTIME_SECRET(&b, 1);
-
- constant_time_conditional_memxor(out, in, sizeof(out), b);
-
- CONSTTIME_DECLASSIFY(&in, sizeof(in));
- CONSTTIME_DECLASSIFY(&out, sizeof(out));
-
- EXPECT_EQ(Bytes(in), Bytes(ref_in));
- EXPECT_EQ(Bytes(out), Bytes(ref_out));
}
}
diff --git a/crypto/internal.h b/crypto/internal.h
index 633e4fa..1a22a94 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -448,6 +448,7 @@
for (size_t i = 0; i < n_vec; i += 32) {
*(v32u8 *)&out[i] ^= masks & *(v32u8 *)&in[i];
}
+ in += n_vec;
out += n_vec;
n -= n_vec;
#endif
