Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions mldsa/src/sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -616,10 +616,6 @@ __contract__(
}

/* All is well - write signature */
/* Constant time: At this point it is clear that the signature is valid - it
* can, hence, be considered public. */
MLD_CT_TESTING_DECLASSIFY(h, sizeof(*h));
MLD_CT_TESTING_DECLASSIFY(z, sizeof(*z));
mld_pack_sig(sig, challenge_bytes, z, h, n);

ret = 0; /* success */
Expand Down Expand Up @@ -874,6 +870,7 @@ int crypto_sign_verify_internal(const uint8_t *sig, size_t siglen,
const uint8_t pk[MLDSA_CRYPTO_PUBLICKEYBYTES],
int externalmu)
{
uint32_t z_invalid;
unsigned int i;
int ret;
MLD_ALLOC(buf, uint8_t, (MLDSA_K * MLDSA_POLYW1_PACKEDBYTES));
Expand Down Expand Up @@ -908,12 +905,24 @@ int crypto_sign_verify_internal(const uint8_t *sig, size_t siglen,
/* mld_unpack_sig and mld_polyvecl_chknorm signal failure through a
* single non-zero error code that's not yet aligned with MLD_ERR_XXX.
* Map it to MLD_ERR_FAIL explicitly. */

/* Constant-time: The signature is commonly considered public. However, we
* mark it as secret to make explicit which parts of the code take time
* depending on the signature.
Comment on lines +909 to +911
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems outdated?

*
* mld_unpack_sig uses conditional branches to unpack the hint vector h.
* The h-component of the signature, hence, needs to be declassified.
Comment on lines +913 to +914
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* mld_unpack_sig uses conditional branches to unpack the hint vector h.
* The h-component of the signature, hence, needs to be declassified.
* mld_unpack_sig uses conditional branches to unpack the hint vector h.
* It is the caller's responsibility to declassify the h-component of the signature.

*/
if (mld_unpack_sig(c, z, h, sig))
{
ret = MLD_ERR_FAIL;
goto cleanup;
}
if (mld_polyvecl_chknorm(z, MLDSA_GAMMA1 - MLDSA_BETA))

z_invalid = mld_polyvecl_chknorm(z, MLDSA_GAMMA1 - MLDSA_BETA);
/* Constant-time: We only leak if z is invalid or not. */
MLD_CT_TESTING_DECLASSIFY(&z_invalid, sizeof(uint32_t));
if (z_invalid)
{
ret = MLD_ERR_FAIL;
goto cleanup;
Expand All @@ -937,6 +946,8 @@ int crypto_sign_verify_internal(const uint8_t *sig, size_t siglen,
}

/* Matrix-vector multiplication; compute Az - c2^dt1 */
/* Constant-time: poly_challenge uses conditional branches depending on c. */
MLD_CT_TESTING_DECLASSIFY(c, MLDSA_CTILDEBYTES);
Comment on lines +949 to +950
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current version, the implementation is inconsistent regarding what is declassified by the function vs. expected to be declassified by the call-site: The h-component is expected to be declassified by the call-site, but the c-component is declassified here.

mld_poly_challenge(cp, c);
mld_polyvec_matrix_expand(mat, rho);

Expand All @@ -955,6 +966,9 @@ int crypto_sign_verify_internal(const uint8_t *sig, size_t siglen,

/* Reconstruct w1 */
mld_polyveck_caddq(w1);

/* Constant-time: use_hint uses conditional branches depending on w1. */
MLD_CT_TESTING_DECLASSIFY(w1, sizeof(mld_polyveck));
mld_polyveck_use_hint(tmp, w1, h);
mld_polyveck_pack_w1(buf, tmp);
/* Call random oracle and verify challenge */
Expand Down
51 changes: 48 additions & 3 deletions test/test_mldsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
} while (0)


#define MLD_CT_TESTING_DECLASSIFY_H(s) \
MLD_CT_TESTING_DECLASSIFY( \
(s) + MLDSA_CTILDEBYTES + MLDSA_L * MLDSA_POLYZ_PACKEDBYTES, \
MLDSA_POLYVECH_PACKEDBYTES);

static int test_sign_core(uint8_t pk[MLDSA_CRYPTO_PUBLICKEYBYTES],
uint8_t sk[MLDSA_CRYPTO_SECRETKEYBYTES],
uint8_t sm[MLEN + MLDSA_CRYPTO_BYTES],
Expand All @@ -49,10 +54,17 @@ static int test_sign_core(uint8_t pk[MLDSA_CRYPTO_PUBLICKEYBYTES],

CHECK(crypto_sign(sm, &smlen, m, MLEN, ctx, CTXLEN, sk) == 0);

/* Mark signature as secret to force explcit declassification in verification
*/
MLD_CT_TESTING_SECRET(sm, MLEN + MLDSA_CRYPTO_BYTES);
/* Constant-time: The unpacking of the h-component of the signature requires
* conditional branches.*/
MLD_CT_TESTING_DECLASSIFY_H(sm);

rc = crypto_sign_open(m2, &mlen, sm, smlen, ctx, CTXLEN, pk);

/* Constant time: Declassify outputs to check them. */
MLD_CT_TESTING_DECLASSIFY(rc, sizeof(int));
MLD_CT_TESTING_DECLASSIFY(&rc, sizeof(int));
MLD_CT_TESTING_DECLASSIFY(m, MLEN);
MLD_CT_TESTING_DECLASSIFY(m2, (MLEN + MLDSA_CRYPTO_BYTES));

Expand Down Expand Up @@ -120,6 +132,12 @@ static int test_sign_extmu(void)
MLD_CT_TESTING_SECRET(mu, sizeof(mu));

CHECK(crypto_sign_signature_extmu(sig, &siglen, mu, sk) == 0);
/* Mark signature as secret to force explcit declassification in verification
*/
MLD_CT_TESTING_SECRET(sig, sizeof(sig));
/* Constant-time: The unpacking of the h-component of the signature requires
* conditional branches.*/
MLD_CT_TESTING_DECLASSIFY_H(sig);
CHECK(crypto_sign_verify_extmu(sig, siglen, mu, pk) == 0);

return 0;
Expand Down Expand Up @@ -147,6 +165,12 @@ static int test_sign_pre_hash(void)

CHECK(crypto_sign_signature_pre_hash_shake256(sig, &siglen, m, MLEN, ctx,
CTXLEN, rnd, sk) == 0);
/* Mark signature as secret to force explcit declassification in verification
*/
MLD_CT_TESTING_SECRET(sig, sizeof(sig));
/* Constant-time: The unpacking of the h-component of the signature requires
* conditional branches.*/
MLD_CT_TESTING_DECLASSIFY_H(sig);
CHECK(crypto_sign_verify_pre_hash_shake256(sig, siglen, m, MLEN, ctx, CTXLEN,
pk) == 0);

Expand Down Expand Up @@ -234,6 +258,13 @@ static int test_wrong_pk(void)

CHECK(crypto_sign(sm, &smlen, m, MLEN, ctx, CTXLEN, sk) == 0);

/* Mark signature as secret to force explcit declassification in verification
*/
MLD_CT_TESTING_SECRET(sm, sizeof(sm));
/* Constant-time: The unpacking of the h-component of the signature requires
* conditional branches.*/
MLD_CT_TESTING_DECLASSIFY_H(sm);

/* flip bit in public key */
randombytes((uint8_t *)&idx, sizeof(size_t));
idx %= MLDSA_CRYPTO_PUBLICKEYBYTES;
Expand Down Expand Up @@ -285,6 +316,13 @@ static int test_wrong_sig(void)

CHECK(crypto_sign(sm, &smlen, m, MLEN, ctx, CTXLEN, sk) == 0);

/* Mark signature as secret to force explcit declassification in verification
*/
MLD_CT_TESTING_SECRET(sm, sizeof(sm));
/* Constant-time: The unpacking of the h-component of the signature requires
* conditional branches.*/
MLD_CT_TESTING_DECLASSIFY_H(sm);

/* flip bit in signed message */
randombytes((uint8_t *)&idx, sizeof(size_t));
idx %= MLEN + MLDSA_CRYPTO_BYTES;
Expand All @@ -294,7 +332,7 @@ static int test_wrong_sig(void)
rc = crypto_sign_open(m2, &mlen, sm, smlen, ctx, CTXLEN, pk);

/* Constant time: Declassify outputs to check them. */
MLD_CT_TESTING_DECLASSIFY(rc, sizeof(int));
MLD_CT_TESTING_DECLASSIFY(&rc, sizeof(int));
MLD_CT_TESTING_DECLASSIFY(m2, sizeof(m2));

if (!rc)
Expand Down Expand Up @@ -337,6 +375,13 @@ static int test_wrong_ctx(void)

CHECK(crypto_sign(sm, &smlen, m, MLEN, ctx, CTXLEN, sk) == 0);

/* Mark signature as secret to force explcit declassification in verification
*/
MLD_CT_TESTING_SECRET(sm, sizeof(sm));
/* Constant-time: The unpacking of the h-component of the signature requires
* conditional branches.*/
MLD_CT_TESTING_DECLASSIFY_H(sm);

/* flip bit in ctx */
randombytes((uint8_t *)&idx, sizeof(size_t));
idx %= CTXLEN;
Expand All @@ -346,7 +391,7 @@ static int test_wrong_ctx(void)
rc = crypto_sign_open(m2, &mlen, sm, smlen, ctx, CTXLEN, pk);

/* Constant time: Declassify outputs to check them. */
MLD_CT_TESTING_DECLASSIFY(rc, sizeof(int));
MLD_CT_TESTING_DECLASSIFY(&rc, sizeof(int));
MLD_CT_TESTING_DECLASSIFY(m2, sizeof(m2));

if (!rc)
Expand Down