Destruct intermediate buffers on the stack

FIPS 204, Section 3.6.3 demands the destruction of intermediate values
before returning to the caller. This commit implements this.

A new function mld_zeroize() is introduced which is used to wipe
intermediate stack buffers prior to function return. By default,
mld_zeroize() is implemented via SecureZeroMemory on Windows, and a
plain memset + compiler barrier otherwise. If neither makes sense,
compilation fails. Using memset_s would be preferred, but there is no
portable way of detecting whether it is available.

If users need to register a custom mld_zeroize, they can do so by
defining MLD_CONFIG_CUSTOM_ZEROIZE and defining mld_zeroize_native,
similar to how the arithmetic and FIPS-202 backends work. This includes
the case where the user does not wish to do stack zeroization, or has
an entirely separate (and more robust) method for it, such as compiler
instrumentation. In this case, mld_zeroize_native() can be set to a
no-op.

Signed-off-by: Jake Massimo <[email protected]>

Author: jakemas

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,