Skip to content

Commit f399c81

Browse files
mkannwischerhanno-becker
authored andcommitted
verify: Switch to constant-time memcmp
At the end of verify one has to compare the input challenge to the re-computed challenge. If they are equal (and some previous checks on h and z passed), the signature is valid. Currently, our constant-time tests do not declassify the message and we, hence, need to declassify in this final step. Before this commit, the declassification would happen on the recomputed challenge just before the memcmp. Now that a constant-time memcmp was added in #714, we might as well use that; that, plus a constant-time selection, allows us to not use message-dependent branches. For now, we still declassify the result of the verification itself to allow branching on it, e.g. in crypto_sign_open. This may be further improved in subsequent work, making crypto_sign_open constant flow and leaving a potential declassification of the verification result to the call-sites. Signed-off-by: Matthias J. Kannwischer <[email protected]> Signed-off-by: Hanno Becker <[email protected]>
1 parent f519792 commit f399c81

File tree

2 files changed

+6
-22
lines changed

2 files changed

+6
-22
lines changed

mldsa/src/sign.c

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,6 @@ int crypto_sign_verify_internal(const uint8_t *sig, size_t siglen,
874874
const uint8_t pk[MLDSA_CRYPTO_PUBLICKEYBYTES],
875875
int externalmu)
876876
{
877-
unsigned int i;
878877
int ret;
879878
MLD_ALLOC(buf, uint8_t, (MLDSA_K * MLDSA_POLYW1_PACKEDBYTES));
880879
MLD_ALLOC(rho, uint8_t, MLDSA_SEEDBYTES);
@@ -961,28 +960,11 @@ int crypto_sign_verify_internal(const uint8_t *sig, size_t siglen,
961960
mld_H(c2, MLDSA_CTILDEBYTES, mu, MLDSA_CRHBYTES, buf,
962961
MLDSA_K * MLDSA_POLYW1_PACKEDBYTES, NULL, 0);
963962

964-
/* Constant time: All data in verification is usually considered public.
965-
* However, in our constant-time tests we do not declassify the message and
966-
* context string.
967-
* The following conditional is the only place in verification whose run-time
968-
* depends on the message. As all that can be leakaged here is the output of
969-
* a hash call (that should behave like a random oracle), it is safe to
970-
* declassify here even with a secret message.
971-
*/
972-
MLD_CT_TESTING_DECLASSIFY(c2, MLDSA_CTILDEBYTES);
973-
ret = MLD_ERR_FAIL;
974-
for (i = 0; i < MLDSA_CTILDEBYTES; ++i)
975-
__loop__(
976-
invariant(i <= MLDSA_CTILDEBYTES)
977-
)
978-
{
979-
if (c[i] != c2[i])
980-
{
981-
goto cleanup;
982-
}
983-
}
963+
ret = mld_ct_sel_int32(MLD_ERR_FAIL, 0,
964+
mld_ct_memcmp(c, c2, MLDSA_CTILDEBYTES));
984965

985-
ret = 0;
966+
/* Declassify the final result of the verification. */
967+
MLD_CT_TESTING_DECLASSIFY(&ret, sizeof(ret));
986968

987969
cleanup:
988970
/* @[FIPS204, Section 3.6.3] Destruction of intermediate values. */

proofs/cbmc/crypto_sign_verify_internal/Makefile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ USE_FUNCTION_CONTRACTS+=$(MLD_NAMESPACE)polyveck_caddq
3939
USE_FUNCTION_CONTRACTS+=$(MLD_NAMESPACE)polyveck_use_hint
4040
USE_FUNCTION_CONTRACTS+=$(MLD_NAMESPACE)polyveck_pack_w1
4141
USE_FUNCTION_CONTRACTS+=mld_zeroize
42+
USE_FUNCTION_CONTRACTS+=mld_ct_memcmp
43+
USE_FUNCTION_CONTRACTS+=mld_ct_sel_int32
4244

4345
APPLY_LOOP_CONTRACTS=on
4446
USE_DYNAMIC_FRAMES=1

0 commit comments

Comments
 (0)