Merge pull request #369 from pq-code-package/mldsa-zeroize

Destruct intermediate buffers on the stack

Author: mkannwischer

mldsa/common.h

@@ -6,6 +6,7 @@
 #ifndef MLD_COMMON_H
 #define MLD_COMMON_H
 
+#include "cbmc.h"
 #include "params.h"
 #include "sys.h"
 
@@ -51,4 +52,54 @@
 #if defined(MLD_CONFIG_USE_NATIVE_BACKEND_FIPS202)
 #include MLD_CONFIG_FIPS202_BACKEND_FILE
 #endif
+
+#if !defined(__ASSEMBLER__)
+#include <string.h>
+
+/*************************************************
+ * Name:        mld_zeroize
+ *
+ * Description: Force-zeroize a buffer.
+ *              FIPS 204. Section 3.6.3 Destruction of intermediate values.
+ *
+ * Arguments:   void *ptr: pointer to buffer to be zeroed
+ *              size_t len: Amount of bytes to be zeroed
+ **************************************************/
+static MLD_INLINE void mld_zeroize(void *ptr, size_t len)
+__contract__(
+  requires(memory_no_alias(ptr, len))
+  assigns(memory_slice(ptr, len))
+);
+
+#if defined(MLD_CONFIG_CUSTOM_ZEROIZE)
+static MLD_INLINE void mld_zeroize(void *ptr, size_t len)
+{
+  mld_zeroize_native(ptr, len);
+}
+#elif defined(MLD_SYS_WINDOWS)
+#include <windows.h>
+static MLD_INLINE void mld_zeroize(void *ptr, size_t len)
+{
+  SecureZeroMemory(ptr, len);
+}
+#elif defined(MLD_HAVE_INLINE_ASM)
+static MLD_INLINE void mld_zeroize(void *ptr, size_t len)
+{
+  memset(ptr, 0, len);
+  /* This follows OpenSSL and seems sufficient to prevent the compiler
+   * from optimizing away the memset.
+   *
+   * If there was a reliable way to detect availability of memset_s(),
+   * that would be preferred. */
+  __asm__ __volatile__("" : : "r"(ptr) : "memory");
+}
+#else /* !MLD_CONFIG_CUSTOM_ZEROIZE && !MLD_SYS_WINDOWS && MLD_HAVE_INLINE_ASM \
+       */
+#error No plausibly-secure implementation of mld_zeroize available. Please provide your own using MLD_CONFIG_CUSTOM_ZEROIZE.
+#endif /* !MLD_CONFIG_CUSTOM_ZEROIZE && !MLD_SYS_WINDOWS && \
+          !MLD_HAVE_INLINE_ASM */
+
+#endif /* !__ASSEMBLER__ */
+
+
 #endif /* !MLD_COMMON_H */

mldsa/config.h

@@ -59,6 +59,46 @@
 #define MLD_CONFIG_FIPS202_BACKEND_FILE "fips202/native/auto.h"
 #endif
 
+/******************************************************************************
+ * Name:        MLD_CONFIG_CUSTOM_ZEROIZE
+ *
+ * Description: In compliance with FIPS 204 Section 3.6.3, mldsa-native zeroizes
+ *              intermediate stack buffers before returning from function calls.
+ *
+ *              Set this option and define `mld_zeroize_native` if you want to
+ *              use a custom method to zeroize intermediate stack buffers.
+ *              The default implementation uses SecureZeroMemory on Windows
+ *              and a memset + compiler barrier otherwise. If neither of those
+ *              is available on the target platform, compilation will fail,
+ *              and you will need to use MLD_CONFIG_CUSTOM_ZEROIZE to provide
+ *              a custom implementation of `mld_zeroize_native()`.
+ *
+ *              WARNING:
+ *              The explicit stack zeroization conducted by mldsa-native
+ *              reduces the likelihood of data leaking on the stack, but
+ *              does not eliminate it! The C standard makes no guarantee about
+ *              where a compiler allocates structures and whether/where it makes
+ *              copies of them. Also, in addition to entire structures, there
+ *              may also be potentially exploitable leakage of individual values
+ *              on the stack.
+ *
+ *              If you need bullet-proof zeroization of the stack, you need to
+ *              consider additional measures instead of what this feature
+ *              provides. In this case, you can set mld_zeroize_native to a
+ *              no-op.
+ *
+ *****************************************************************************/
+/* #define MLD_CONFIG_CUSTOM_ZEROIZE
+   #if !defined(__ASSEMBLER__)
+   #include <stdint.h>
+   #include "sys.h"
+   static MLD_INLINE void mld_zeroize_native(void *ptr, size_t len)
+   {
+       ... your implementation ...
+   }
+   #endif
+*/
+
 /******************************************************************************
  * Name:        MLD_CONFIG_KEYGEN_PCT
  *

mldsa/fips202/fips202.c

@@ -437,8 +437,8 @@ void shake128_squeezeblocks(uint8_t *out, size_t nblocks, keccak_state *state)
 
 void shake128_release(keccak_state *state)
 {
-  (void)state;
-  /* TODO: add secure zeroization*/
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(state, sizeof(keccak_state));
 }
 
 /*************************************************
@@ -535,8 +535,8 @@ void shake256_squeezeblocks(uint8_t *out, size_t nblocks, keccak_state *state)
 
 void shake256_release(keccak_state *state)
 {
-  (void)state;
-  /* TODO: add secure zeroization*/
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(state, sizeof(keccak_state));
 }
 
 /*************************************************
@@ -560,6 +560,9 @@ void shake128(uint8_t *out, size_t outlen, const uint8_t *in, size_t inlen)
   outlen -= nblocks * SHAKE128_RATE;
   out += nblocks * SHAKE128_RATE;
   shake128_squeeze(out, outlen, &state);
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(&state, sizeof(state));
 }
 
 /*************************************************
@@ -583,6 +586,9 @@ void shake256(uint8_t *out, size_t outlen, const uint8_t *in, size_t inlen)
   outlen -= nblocks * SHAKE256_RATE;
   out += nblocks * SHAKE256_RATE;
   shake256_squeeze(out, outlen, &state);
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(&state, sizeof(state));
 }
 
 /*************************************************
@@ -606,6 +612,9 @@ void sha3_256(uint8_t h[SHA3_256_HASHBYTES], const uint8_t *in, size_t inlen)
   {
     store64(h + 8 * i, s[i]);
   }
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(s, sizeof(s));
 }
 
 /*************************************************
@@ -629,4 +638,7 @@ void sha3_512(uint8_t h[SHA3_512_HASHBYTES], const uint8_t *in, size_t inlen)
   {
     store64(h + 8 * i, s[i]);
   }
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(s, sizeof(s));
 }

mldsa/fips202/fips202x4.c

@@ -123,10 +123,8 @@ void mld_shake128x4_squeezeblocks(uint8_t *out0, uint8_t *out1, uint8_t *out2,
 void mld_shake128x4_init(mld_shake128x4ctx *state) { (void)state; }
 void mld_shake128x4_release(mld_shake128x4ctx *state)
 {
-  /* Specification: Partially implements
-   * @[FIPS203, Section 3.3, Destruction of intermediate values] */
-  (void)state;
-  /*mld_zeroize(state, sizeof(mld_shake128x4ctx));*/
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(state, sizeof(mld_shake128x4ctx));
 }
 
 
@@ -150,8 +148,6 @@ void mld_shake256x4_squeezeblocks(uint8_t *out0, uint8_t *out1, uint8_t *out2,
 void mld_shake256x4_init(mld_shake256x4ctx *state) { (void)state; }
 void mld_shake256x4_release(mld_shake256x4ctx *state)
 {
-  /* Specification: Partially implements
-   * @[FIPS203, Section 3.3, Destruction of intermediate values] */
-  (void)state;
-  /*mld_zeroize(state, sizeof(mld_shake256x4ctx));*/
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(state, sizeof(mld_shake256x4ctx));
 }

mldsa/poly.c

@@ -376,6 +376,9 @@ void mld_poly_uniform(mld_poly *a, const uint8_t seed[MLDSA_SEEDBYTES + 2])
     ctr = mld_rej_uniform(a->coeffs, MLDSA_N, ctr, buf, buflen);
   }
   mld_xof128_release(&state);
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(buf, sizeof(buf));
 }
 
 void mld_poly_uniform_4x(mld_poly *vec0, mld_poly *vec1, mld_poly *vec2,
@@ -431,6 +434,9 @@ void mld_poly_uniform_4x(mld_poly *vec0, mld_poly *vec1, mld_poly *vec2,
     ctr[3] = mld_rej_uniform(vec3->coeffs, MLDSA_N, ctr[3], buf[3], buflen);
   }
   mld_xof128_x4_release(&state);
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(buf, sizeof(buf));
 }
 
 /*************************************************
@@ -628,6 +634,10 @@ void mld_poly_uniform_eta_4x(mld_poly *r0, mld_poly *r1, mld_poly *r2,
   }
 
   mld_xof256_x4_release(&state);
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(buf, sizeof(buf));
+  mld_zeroize(extseed, sizeof(extseed));
 }
 
 
@@ -651,6 +661,10 @@ void mld_poly_uniform_gamma1(mld_poly *a, const uint8_t seed[MLDSA_CRHBYTES],
   mld_polyz_unpack(a, buf);
 
   mld_xof256_release(&state);
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(buf, sizeof(buf));
+  mld_zeroize(extseed, sizeof(extseed));
 }
 
 void mld_poly_uniform_gamma1_4x(mld_poly *r0, mld_poly *r1, mld_poly *r2,
@@ -690,6 +704,10 @@ void mld_poly_uniform_gamma1_4x(mld_poly *r0, mld_poly *r1, mld_poly *r2,
   mld_polyz_unpack(r2, buf[2]);
   mld_polyz_unpack(r3, buf[3]);
   mld_xof256_x4_release(&state);
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(buf, sizeof(buf));
+  mld_zeroize(extseed, sizeof(extseed));
 }
 
 
@@ -764,6 +782,10 @@ void mld_poly_challenge(mld_poly *c, const uint8_t seed[MLDSA_CTILDEBYTES])
     signs >>= 1;
   }
 
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(buf, sizeof(buf));
+  mld_zeroize(&signs, sizeof(signs));
+
   mld_assert_bound(c->coeffs, MLDSA_N, -1, 2);
 }
 

mldsa/polyvec.c

@@ -110,6 +110,9 @@ void mld_polyvec_matrix_expand(mld_polyvecl mat[MLDSA_K],
       mld_poly_permute_bitrev_to_custom(mat[i].vec[j].coeffs);
     }
   }
+
+  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
+  mld_zeroize(seed_ext, sizeof(seed_ext));
 }
 
 void mld_polyvec_matrix_pointwise_montgomery(mld_polyveck *t,