Prevent non-constant-time code in Kyber-R3 and ML-KEM implementation (#…

…1619)

### Issues:
Resolves V1399146249

### Description of changes: 
A branch can be introduced by the compiler from the `poly_frommsg` code.
This has been reported for Clang versions
15-18, x86_64, for the following compiler flags:
- `-Os`
- `-O1`
- `-O2 -fno-vectorize`
- `-O3 -fno-vectorize`

An upstream fix has been published
[here](pq-crystals/kyber@9b8d306)
which we are deciding not to take directly. Due to the way AWS-LC
compiles the Kyber reference code, it may not have the
desired effect. We instead opt to use our own constant-time
implementations for performing conditional bit-masks which
have much stronger guarantees around portability.

### Call-outs:
Point out areas that need special attention or support during the review
process. Discuss architecture or design changes.

- An interesting observation is that the old code would *always* produce
SIMD
instructions despite passing in `-fno-vectorize` and technically never
produced
the non-constant-time branching described.
- This new code for performing the conditional bit-masks seems to
completely
avoid SIMD now. The fact that this is more controlled/predicted is an
improvement in my book.

crypto/kyber/README.md

@@ -24,5 +24,6 @@ The following changes were made to the source code in `pqcrystals_kyber_ref_comm
 * `symmetric-shake.c`: unnecessary include of `fips202.h` is removed.
 * `api.h`, `fips202.h`, `params.h`: modified [in this PR](https://github.com/aws/aws-lc/pull/655) to support our [prefixed symbols build](https://github.com/aws/aws-lc/blob/main/BUILDING.md#building-with-prefixed-symbols).
 * `poly.c` and 'polyvec.c' were modified to remove 6 lines of comment from these two reference commits ([dda29cc](https://github.com/pq-crystals/kyber/commit/dda29cc63af721981ee2c831cf00822e69be3220), [272125f](https://github.com/pq-crystals/kyber/commit/272125f6acc8e8b6850fd68ceb901a660ff48196))
+* `poly.c`: the `poly_frommsg` function was modified to address the constant-time issue described [here](https://github.com/pq-crystals/kyber/commit/9b8d30698a3e7449aeb34e62339d4176f11e3c6c).
 
 **Usage.** The KEM API is defined and documented in `include/openssl/evp.h`. To see examples of how to use any KEM, including Kyber, see `crypto/kem/README.md`.

crypto/kyber/pqcrystals_kyber_ref_common/poly.c

@@ -1,11 +1,16 @@
 #include <stdint.h>
+
+#include <openssl/type_check.h>
+
 #include "params.h"
 #include "poly.h"
 #include "ntt.h"
 #include "reduce.h"
 #include "cbd.h"
 #include "symmetric.h"
 
+#include "../../internal.h"
+
 /*************************************************
 * Name:        poly_compress
 *
@@ -164,16 +169,21 @@ void poly_frombytes(poly *r, const uint8_t a[KYBER_POLYBYTES])
 void poly_frommsg(poly *r, const uint8_t msg[KYBER_INDCPA_MSGBYTES])
 {
   unsigned int i,j;
-  int16_t mask;
+  crypto_word_t mask;
 
 #if (KYBER_INDCPA_MSGBYTES != KYBER_N/8)
 #error "KYBER_INDCPA_MSGBYTES must be equal to KYBER_N/8 bytes!"
 #endif
 
   for(i=0;i<KYBER_N/8;i++) {
     for(j=0;j<8;j++) {
-      mask = -(int16_t)((msg[i] >> j)&1);
-      r->coeffs[8*i+j] = mask & ((KYBER_Q+1)/2);
+      mask = constant_time_is_zero_w((msg[i] >> j) & 1);
+      // We cast the result of constant_time_select_w, which is a crypto_word_t,
+      // to int16_t. The constants must be within the range of int16_t.
+      OPENSSL_STATIC_ASSERT(((KYBER_Q+1)/2) <= INT16_MAX,
+                            value_exceeds_int16_max);
+      r->coeffs[8*i+j] = (int16_t) constant_time_select_w(mask,
+                                                          0, ((KYBER_Q+1)/2));
     }
   }
 }

crypto/ml_kem/README.md

@@ -12,5 +12,6 @@ The following changes were made to the source code in `ml_kem_ipd_ref_common` di
 - `fips202.{h|c}` are deleted and the ones from `crypto/kyber/pqcrystals_kyber_ref_common` directory are used.
 - `symmetric-shake.c`: unnecessary include of fips202.h is removed.
 - `api.h`: `pqcrystals` prefix substituted with `ml_kem` (to be able build alongside `crypto/kyber`).
+* `poly.c`: the `poly_frommsg` function was modified to address the constant-time issue described [here](https://github.com/pq-crystals/kyber/commit/9b8d30698a3e7449aeb34e62339d4176f11e3c6c).
 
 The KATs were generated by compiling and running the KAT generator tests from the official repository. Specifically, running `make` in the `ref` folder produces `nistkat/PQCgenKAT_kem512` binary that can generates the test vectors.

crypto/ml_kem/ml_kem_ipd_ref_common/poly.c

@@ -1,11 +1,16 @@
 #include <stdint.h>
+
+#include <openssl/type_check.h>
+
 #include "params.h"
 #include "poly.h"
 #include "ntt.h"
 #include "reduce.h"
 #include "cbd.h"
 #include "symmetric.h"
 
+#include "../../internal.h"
+
 /*************************************************
 * Name:        poly_compress
 *
@@ -167,16 +172,21 @@ void poly_frombytes(poly *r, const uint8_t a[KYBER_POLYBYTES])
 void poly_frommsg(poly *r, const uint8_t msg[KYBER_INDCPA_MSGBYTES])
 {
   unsigned int i,j;
-  int16_t mask;
+  crypto_word_t mask;
 
 #if (KYBER_INDCPA_MSGBYTES != KYBER_N/8)
 #error "KYBER_INDCPA_MSGBYTES must be equal to KYBER_N/8 bytes!"
 #endif
 
   for(i=0;i<KYBER_N/8;i++) {
     for(j=0;j<8;j++) {
-      mask = -(int16_t)((msg[i] >> j)&1);
-      r->coeffs[8*i+j] = mask & ((KYBER_Q+1)/2);
+      mask = constant_time_is_zero_w((msg[i] >> j) & 1);
+      // We cast the result of constant_time_select_w, which is a crypto_word_t,
+      // to int16_t. The constants must be within the range of int16_t.
+      OPENSSL_STATIC_ASSERT(((KYBER_Q+1)/2) <= INT16_MAX,
+                            value_exceeds_int16_max);
+      r->coeffs[8*i+j] = (int16_t) constant_time_select_w(mask,
+                                                          0, ((KYBER_Q+1)/2));
     }
   }
 }