Make poly_chknorm constant flow (#2788)

### Issues:
Resolves #V1982530715 and #V1982532566

### Description of changes: 
The reference implementation implements `poly_chknorm` in variables
time. It argues that while the input coefficients itself are secret in
some call sites, it is okay to leak which coefficient lead to rejection.
It, hence, does absolute value computation in constant-time and then
checks the bound using a conditional.

This approach appears safe, but somewhat unclean as it is still
operating on secret data. When performing constant-time testing it also
requires a number of declassifications.

This commit takes a more conservative approach and changes
`poly_chknorm` to a constant-time implementation in the hope that the
performance penalty is acceptable.

A minor change is that the API of `poly_chknorm` is changed to returning
`0xFFFFFFFF` in the case of failure to be able to re-use existing
constant-time primitives.

### Call-outs:
PR source adapted from: Upstream PR in
pq-code-package/mldsa-native#392.


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.

Author: jakemas

crypto/fipsmodule/ml_dsa/ml_dsa_ref/poly.c

@@ -228,29 +228,32 @@ void ml_dsa_poly_use_hint(ml_dsa_params *params,
 * Arguments:   - const poly *a: pointer to polynomial
 *              - int32_t B: norm bound
 *
-* Returns 0 if norm is strictly smaller than B <= (Q-1)/8 and 1 otherwise.
+* Returns 0 if norm is strictly smaller than B <= (Q-1)/8 and 0xFFFFFFFF otherwise.
 **************************************************/
-int ml_dsa_poly_chknorm(const ml_dsa_poly *a, int32_t B) {
+uint32_t ml_dsa_poly_chknorm(const ml_dsa_poly *a, int32_t B) {
   unsigned int i;
   int32_t t;
+  uint32_t r = 0;
 
   if(B > (ML_DSA_Q-1)/8) {
-    return 1;
+    return 0xFFFFFFFF;
   }
 
-  /* It is ok to leak which coefficient violates the bound since
-     the probability for each coefficient is independent of secret
-     data but we must not leak the sign of the centralized representative. */
+  /* Constant-time implementation as defense-in-depth. According to Section 5.5
+     of the Dilithium specification, it is safe to leak which coefficient violates
+     the bound, but we implement this in constant-time as additional hardening.
+     We accumulate violations using bitwise OR instead of early exit. See 5.5 in
+     https://pq-crystals.org/dilithium/data/dilithium-specification-round3-20210208.pdf
+     */
   for(i = 0; i < ML_DSA_N; ++i) {
     /* Absolute value */
     t = constant_time_select_int(constant_time_msb_w(a->coeffs[i]),
                                  -a->coeffs[i], a->coeffs[i]);
 
-    if(t >= B) {
-      return 1;
-    }
+    /* Check if t >= B and accumulate result */
+    r |= constant_time_ge_w((uint32_t)t, (uint32_t)B);
   }
-  return 0;
+  return r;
 }
 
 /*************************************************

crypto/fipsmodule/ml_dsa/ml_dsa_ref/poly.h

@@ -43,7 +43,7 @@ void ml_dsa_poly_use_hint(ml_dsa_params *params,
                           const ml_dsa_poly *a,
                           const ml_dsa_poly *h);
 
-int ml_dsa_poly_chknorm(const ml_dsa_poly *a, int32_t B);
+uint32_t ml_dsa_poly_chknorm(const ml_dsa_poly *a, int32_t B);
 
 void ml_dsa_poly_uniform(ml_dsa_poly *a,
                          const uint8_t seed[ML_DSA_SEEDBYTES],

crypto/fipsmodule/ml_dsa/ml_dsa_ref/polyvec.c

@@ -223,16 +223,16 @@ void ml_dsa_polyvecl_pointwise_acc_montgomery(ml_dsa_params *params,
 *              - int32_t B: norm bound
 *
 * Returns 0 if norm of all polynomials is strictly smaller than B <= (Q-1)/8
-* and 1 otherwise.
+* and 0xFFFFFFFF otherwise.
 **************************************************/
-int ml_dsa_polyvecl_chknorm(ml_dsa_params *params, const polyvecl *v, int32_t bound)  {
+uint32_t ml_dsa_polyvecl_chknorm(ml_dsa_params *params, const polyvecl *v, int32_t bound)  {
   unsigned int i;
+  uint32_t r = 0;
+  
   for(i = 0; i < params->l; ++i) {
-    if(ml_dsa_poly_chknorm(&v->vec[i], bound)) {
-      return 1;
-    }
+    r |= ml_dsa_poly_chknorm(&v->vec[i], bound);
   }
-  return 0;
+  return r;
 }
 
 /**************************************************************/
@@ -418,16 +418,20 @@ void ml_dsa_polyveck_pointwise_poly_montgomery(ml_dsa_params *params,
 *              - int32_t B: norm bound
 *
 * Returns 0 if norm of all polynomials are strictly smaller than B <= (Q-1)/8
-* and 1 otherwise.
+* and 0xFFFFFFFF otherwise.
 **************************************************/
-int ml_dsa_polyveck_chknorm(ml_dsa_params *params, const polyveck *v, int32_t bound) {
+uint32_t ml_dsa_polyveck_chknorm(ml_dsa_params *params, const polyveck *v, int32_t bound) {
   unsigned int i;
+  uint32_t r = 0;
+  
+    /* Reference: Leaks which polynomial violates the bound via a conditional.
+     * We are more conservative to reduce the number of declassifications in
+     * constant-time testing.
+     */
   for(i = 0; i < params->k; ++i) {
-    if(ml_dsa_poly_chknorm(&v->vec[i], bound)) {
-      return 1;
-    }
+    r |= ml_dsa_poly_chknorm(&v->vec[i], bound);
   }
-  return 0;
+  return r;
 }
 
 /*************************************************

crypto/fipsmodule/ml_dsa/ml_dsa_ref/polyvec.h

@@ -41,7 +41,7 @@ void ml_dsa_polyvecl_pointwise_acc_montgomery(ml_dsa_params *params,
                                               const polyvecl *u,
                                               const polyvecl *v);
 
-int ml_dsa_polyvecl_chknorm(ml_dsa_params *params, const polyvecl *v, int32_t B);
+uint32_t ml_dsa_polyvecl_chknorm(ml_dsa_params *params, const polyvecl *v, int32_t B);
 
 typedef struct {
   ml_dsa_poly vec[ML_DSA_K_MAX];
@@ -77,7 +77,7 @@ void ml_dsa_polyveck_pointwise_poly_montgomery(ml_dsa_params *params,
                                                const ml_dsa_poly *a,
                                                const polyveck *v);
 
-int ml_dsa_polyveck_chknorm(ml_dsa_params *params, const polyveck *v, int32_t B);
+uint32_t ml_dsa_polyveck_chknorm(ml_dsa_params *params, const polyveck *v, int32_t B);
 
 void ml_dsa_polyveck_power2round(ml_dsa_params *params,
                                  polyveck *v1,

crypto/fipsmodule/ml_dsa/ml_dsa_ref/sign.c

@@ -178,6 +178,7 @@ int ml_dsa_sign_internal(ml_dsa_params *params,
   uint8_t seedbuf[2*ML_DSA_SEEDBYTES + ML_DSA_TRBYTES + 2*ML_DSA_CRHBYTES];
   uint8_t *rho, *tr, *key, *mu, *rhoprime;
   uint16_t nonce = 0;
+  uint32_t z_invalid, w0_invalid, h_invalid;
   polyvecl mat[ML_DSA_K_MAX], s1, y, z;
   polyveck t0, s2, w1, w0, h;
   ml_dsa_poly cp;
@@ -248,7 +249,8 @@ int ml_dsa_sign_internal(ml_dsa_params *params,
   ml_dsa_polyvecl_invntt_tomont(params, &z);
   ml_dsa_polyvecl_add(params, &z, &z, &y);
   ml_dsa_polyvecl_reduce(params, &z);
-  if(ml_dsa_polyvecl_chknorm(params, &z, params->gamma1 - params->beta)) {
+  z_invalid = ml_dsa_polyvecl_chknorm(params, &z, params->gamma1 - params->beta);
+  if(z_invalid) {
     goto rej;
   }
 
@@ -258,15 +260,22 @@ int ml_dsa_sign_internal(ml_dsa_params *params,
   ml_dsa_polyveck_invntt_tomont(params, &h);
   ml_dsa_polyveck_sub(params, &w0, &w0, &h);
   ml_dsa_polyveck_reduce(params, &w0);
-  if(ml_dsa_polyveck_chknorm(params, &w0, params->gamma2 - params->beta)) {
+  w0_invalid = ml_dsa_polyveck_chknorm(params, &w0, params->gamma2 - params->beta);
+  /* Leaking the fact that a signature was rejected at this stage is acceptable as
+   * the next attempt at a signature will be (indistinguishable from) independent of
+   * this one. See 5.5 in
+   * https://pq-crystals.org/dilithium/data/dilithium-specification-round3-20210208.pdf
+   */
+  if(w0_invalid) {
     goto rej;
   }
 
   /* FIPS 204: line 25 */
   ml_dsa_polyveck_pointwise_poly_montgomery(params, &h, &cp, &t0);
   ml_dsa_polyveck_invntt_tomont(params, &h);
   ml_dsa_polyveck_reduce(params, &h);
-  if(ml_dsa_polyveck_chknorm(params, &h, params->gamma2)) {
+  h_invalid = ml_dsa_polyveck_chknorm(params, &h, params->gamma2);
+  if(h_invalid) {
     goto rej;
   }