Fix and test correctness of constant_time_conditional_memxor on GCC

[ctz]

This fixes https://issues.chromium.org/issues/455606200 and tests the fix. If you drop the fix commit, the tests look like:

$ cargo test --lib memxor
   Compiling ring v0.17.14 (/home/jbp/src/ring)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 2.01s
     Running unittests src/lib.rs (target/debug/deps/ring-b77c8886ded000b2)

running 2 tests
test tests::bb_bytes_tests::constant_time_conditional_memxor ... FAILED
test tests::c_constant_time_tests::constant_time_conditional_memxor ... FAILED

failures:

---- tests::bb_bytes_tests::constant_time_conditional_memxor stdout ----

thread 'tests::bb_bytes_tests::constant_time_conditional_memxor' panicked at src/tests/bb_bytes_tests.rs:80:9:
assertion `left == right` failed
  left: [25, 36, 96, 73, 237, 207, 229, 196, 193, 236, 141, 234, 111, 188, 79, 84, 151, 103, 148, 220, 140, 42, 63, 21, 117, 238, 23, 130, 84, 238, 108, 222, 53, 172, 125, 76, 124, 195, 41, 53, 147, 91, 25, 16, 106, 211, 76, 165, 218, 78, 225, 199, 151, 228, 42, 168, 18, 52, 197, 5, 255, 179, 151, 238, 53, 19, 123, 54, 66, 115, 137, 141, 67, 219, 188, 58, 209, 194, 64, 201, 64, 232, 35, 67, 200, 168, 24, 57, 251, 233, 55, 220, 43, 144, 94, 124, 89, 164, 77, 184, 171, 237, 7, 36, 93, 163, 220, 63, 2, 127, 249, 128, 63, 216, 191, 84, 20, 47, 115, 137, 18, 171, 86, 233, 25, 172, 181, 111, 69, 92, 11, 139, 102, 114, 239, 35, 69, 102, 138, 87, 82, 70, 19, 63, 15, 43, 238, 153, 30, 143, 243, 233, 114, 140, 45, 140, 64, 123, 7, 29, 237, 105, 20, 111, 176, 131, 92, 93, 81, 200, 227, 212, 195, 6, 87, 36, 38, 231, 42, 221, 92, 183, 241, 46, 116, 247, 223, 65, 36, 123, 67, 23, 232, 51, 48, 4, 39, 87, 11, 13, 43, 61, 63, 214, 134, 232, 55, 156, 96, 117, 61, 158, 100, 79, 6, 135, 207, 172, 31, 86, 194, 230, 90, 225, 67, 185, 108, 121, 95, 64, 43, 119, 35, 115, 12, 206, 61, 158, 137, 213, 229, 76, 152, 25, 128, 171, 26, 221, 39, 74, 176, 23, 117, 91, 143]
 right: [25, 36, 96, 73, 237, 207, 229, 196, 193, 236, 141, 234, 111, 188, 79, 84, 151, 103, 148, 220, 140, 42, 63, 21, 117, 238, 23, 130, 84, 238, 108, 222, 53, 172, 125, 76, 124, 195, 41, 53, 147, 91, 25, 16, 106, 211, 76, 165, 218, 78, 225, 199, 151, 228, 42, 168, 18, 52, 197, 5, 255, 179, 151, 238, 53, 19, 123, 54, 66, 115, 137, 141, 67, 219, 188, 58, 209, 194, 64, 201, 64, 232, 35, 67, 200, 168, 24, 57, 251, 233, 55, 220, 43, 144, 94, 124, 89, 164, 77, 184, 171, 237, 7, 36, 93, 163, 220, 63, 2, 127, 249, 128, 63, 216, 191, 84, 20, 47, 115, 137, 18, 171, 86, 233, 25, 172, 181, 111, 69, 92, 11, 139, 102, 114, 239, 35, 69, 102, 138, 87, 82, 70, 19, 63, 15, 43, 238, 153, 30, 143, 243, 233, 114, 140, 45, 140, 64, 123, 7, 29, 237, 105, 20, 111, 176, 131, 92, 93, 81, 200, 227, 212, 195, 6, 87, 36, 38, 231, 42, 221, 92, 183, 241, 46, 116, 247, 223, 65, 36, 123, 67, 23, 232, 51, 48, 4, 39, 87, 11, 13, 43, 61, 63, 214, 134, 232, 55, 156, 96, 117, 61, 158, 100, 79, 6, 135, 207, 172, 31, 86, 194, 230, 90, 225, 243, 42, 101, 84, 83, 26, 150, 153, 193, 76, 73, 201, 51, 166, 129, 227, 232, 190, 198, 221, 48, 24, 200, 246, 24, 26, 185, 79, 227, 19, 158]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- tests::c_constant_time_tests::constant_time_conditional_memxor stdout ----

thread 'tests::c_constant_time_tests::constant_time_conditional_memxor' panicked at src/tests/c_constant_time_tests.rs:92:9:
assertion `left == right` failed
  left: [78, 61, 92, 211, 185, 83, 208, 248, 78, 66, 191, 79, 146, 11, 138, 78, 124, 172, 52, 243, 163, 41, 64, 221, 39, 157, 139, 169, 184, 64, 146, 246, 51, 152, 191, 222, 251, 91, 163, 212, 213, 94, 242, 117, 92, 129, 93, 130, 73, 132, 246, 112, 218, 117, 144, 154, 217, 9, 220, 161, 230, 126, 59, 137, 66, 66, 225, 196, 216, 198, 40, 11, 148, 239, 170, 113, 179, 148, 230, 74, 60, 153, 95, 157, 251, 241, 33, 19, 78, 174, 46, 132, 98, 255, 166, 15, 11, 97, 22, 149, 70, 202, 152, 171, 57, 178, 199, 84, 195, 163, 70, 249, 80, 28, 7, 132, 160, 140, 13, 210, 106, 42, 24, 14, 105, 82, 95, 84, 248, 136, 186, 83, 142, 212, 245, 191, 253, 23, 56, 226, 65, 137, 97, 27, 219, 2, 30, 114, 68, 147, 241, 218, 58, 39, 68, 57, 223, 106, 155, 18, 134, 60, 218, 38, 193, 107, 81, 18, 149, 39, 243, 95, 222, 234, 254, 101, 114, 142, 132, 247, 73, 215, 163, 153, 168, 73, 158, 60, 3, 132, 66, 245, 248, 90, 84, 144, 117, 150, 161, 213, 215, 168, 249, 96, 227, 89, 23, 104, 11, 48, 202, 39, 253, 99, 239, 78, 159, 199, 174, 37, 237, 33, 154, 60, 165, 152, 218, 44, 168, 136, 233, 221, 226, 86, 194, 216, 43, 21, 172, 179, 232, 140, 198, 252, 76, 124, 6, 202, 9, 229, 236, 170, 81, 1, 126]
 right: [78, 61, 92, 211, 185, 83, 208, 248, 78, 66, 191, 79, 146, 11, 138, 78, 124, 172, 52, 243, 163, 41, 64, 221, 39, 157, 139, 169, 184, 64, 146, 246, 51, 152, 191, 222, 251, 91, 163, 212, 213, 94, 242, 117, 92, 129, 93, 130, 73, 132, 246, 112, 218, 117, 144, 154, 217, 9, 220, 161, 230, 126, 59, 137, 66, 66, 225, 196, 216, 198, 40, 11, 148, 239, 170, 113, 179, 148, 230, 74, 60, 153, 95, 157, 251, 241, 33, 19, 78, 174, 46, 132, 98, 255, 166, 15, 11, 97, 22, 149, 70, 202, 152, 171, 57, 178, 199, 84, 195, 163, 70, 249, 80, 28, 7, 132, 160, 140, 13, 210, 106, 42, 24, 14, 105, 82, 95, 84, 248, 136, 186, 83, 142, 212, 245, 191, 253, 23, 56, 226, 65, 137, 97, 27, 219, 2, 30, 114, 68, 147, 241, 218, 58, 39, 68, 57, 223, 106, 155, 18, 134, 60, 218, 38, 193, 107, 81, 18, 149, 39, 243, 95, 222, 234, 254, 101, 114, 142, 132, 247, 73, 215, 163, 153, 168, 73, 158, 60, 3, 132, 66, 245, 248, 90, 84, 144, 117, 150, 161, 213, 215, 168, 249, 96, 227, 89, 23, 104, 11, 48, 202, 39, 253, 99, 239, 78, 159, 199, 174, 37, 237, 33, 154, 60, 218, 78, 203, 105, 191, 19, 1, 22, 76, 122, 2, 25, 150, 84, 14, 109, 200, 10, 140, 255, 75, 13, 222, 25, 81, 195, 222, 69, 76, 206, 225]


failures:
    tests::bb_bytes_tests::constant_time_conditional_memxor
    tests::c_constant_time_tests::constant_time_conditional_memxor

test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 102 filtered out; finished in 0.00s

$ CC=clang cargo test --lib memxor
   Compiling ring v0.17.14 (/home/jbp/src/ring)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 2.73s
     Running unittests src/lib.rs (target/debug/deps/ring-b77c8886ded000b2)

running 2 tests
test tests::c_constant_time_tests::constant_time_conditional_memxor ... ok
test tests::bb_bytes_tests::constant_time_conditional_memxor ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 102 filtered out; finished in 0.00s

Which provides a demonstration of the upstream bug.

[briansmith]

Good catch. Normally what we do in ring is simplify the code to match what is actually needed. In the case of the single caller of constant_time_conditional_memxor, we have [edited: originally had upstream's C++ code here, instead of ring's C code]:

  uint8_t t_bytes[3][32] = {
      {constant_time_is_zero_w(b) & 1}, {constant_time_is_zero_w(b) & 1}, {0}};
#if defined(__clang__) // materialize for vectorization, 6% speedup
  __asm__("" : "+m" (t_bytes) : /*no inputs*/);
#endif
  OPENSSL_STATIC_ASSERT(sizeof(t_bytes) == sizeof(k25519Precomp[pos][0]), "");
  for (int i = 0; i < 8; i++) {
    constant_time_conditional_memxor(t_bytes, k25519Precomp[pos][i],
                                     sizeof(t_bytes),
                                     constant_time_eq_w(babs, 1 + i));
  }

size_of(t_bytes) == 96, 96 % 32 == 0, so we could instead restrict n to be a multiple of 32 and remove the tail case, e.g. with

- #endif
+ #else
    for (size_t i = 0; i < n; i++) {
      out[i] ^= value_barrier_w(mask) & in[i];
    }
+ #endif

I guess since the production use is with 96 bytes then we should be testing the 96 byte case specifically, at least.

[briansmith]

#2735 removes the offending code completely.