From 66c536588583ace70e5e7de7b32c029dcd951aa6 Mon Sep 17 00:00:00 2001 From: Kris Kwiatkowski Date: Tue, 29 Jun 2021 09:47:50 +0100 Subject: [PATCH] CT checks for Frodo --- src/common/utils.c | 18 ++++++ src/kem/frodo/frodokem640shake/clean/kem.c | 20 +++++- src/kem/frodo/frodokem640shake/clean/util.c | 1 - test/ut.cpp | 71 ++++++++++++++++++++- 4 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/common/utils.c b/src/common/utils.c index 27fe5fa5..c8789ff4 100644 --- a/src/common/utils.c +++ b/src/common/utils.c @@ -1,6 +1,23 @@ #include #include +#include +// EXAMPLE how memcheck won't recognize this as a bug, but valgrind will do +#define ENABLE_EXAMPLE_MEMCHECK_VS_VALGRIND 0 + +#if ENABLE_EXAMPLE_MEMCHECK_VS_VALGRIND +// Constant time memcmp. Returns 0 if p==q, otherwise 1 +uint8_t ct_memcmp(const void *a, const void *b, size_t n) { + const uint8_t *pa = (uint8_t *) a, *pb = (uint8_t *) b; + uint64_t r = 0; + + ct_poison(&r, 8); // -- this would trigger UUM in the ConstantTime.CtCheck_memcmp_chained testg + + while (n--) { r |= *pa++ ^ *pb++; } + r = (r >> 1) - r; // MSB == 1 iff r!=0 + return (r>>63)&1; // CHECK: propagation rules make a difference +} +#else // Constant time memcmp. Returns 0 if p==q, otherwise 1 uint8_t ct_memcmp(const void *a, const void *b, size_t n) { const uint8_t *pa = (uint8_t *) a, *pb = (uint8_t *) b; @@ -11,3 +28,4 @@ uint8_t ct_memcmp(const void *a, const void *b, size_t n) { r >>= 7; return r; } +#endif diff --git a/src/kem/frodo/frodokem640shake/clean/kem.c b/src/kem/frodo/frodokem640shake/clean/kem.c index 40d5dd76..9c6d6c0c 100644 --- a/src/kem/frodo/frodokem640shake/clean/kem.c +++ b/src/kem/frodo/frodokem640shake/clean/kem.c @@ -14,6 +14,8 @@ #include "common.h" #include "params.h" +#include "common/utils.h" + int PQCLEAN_FRODOKEM640SHAKE_CLEAN_crypto_kem_keypair(uint8_t *pk, uint8_t *sk) { // FrodoKEM's key generation // Outputs: public key pk ( BYTES_SEED_A + (PARAMS_LOGQ*PARAMS_N*PARAMS_NBAR)/8 bytes) @@ -139,7 +141,6 @@ int PQCLEAN_FRODOKEM640SHAKE_CLEAN_crypto_kem_enc(uint8_t *ct, uint8_t *ss, cons return 0; } - int PQCLEAN_FRODOKEM640SHAKE_CLEAN_crypto_kem_dec(uint8_t *ss, const uint8_t *ct, const uint8_t *sk) { // FrodoKEM's key decapsulation uint16_t B[PARAMS_N * PARAMS_NBAR] = {0}; @@ -193,6 +194,7 @@ int PQCLEAN_FRODOKEM640SHAKE_CLEAN_crypto_kem_dec(uint8_t *ss, const uint8_t *ct for (size_t i = 0; i < (2 * PARAMS_N + PARAMS_NBAR) * PARAMS_NBAR; i++) { Sp[i] = PQCLEAN_FRODOKEM640SHAKE_CLEAN_LE_TO_UINT16(Sp[i]); } + PQCLEAN_FRODOKEM640SHAKE_CLEAN_sample_n(Sp, PARAMS_N * PARAMS_NBAR); PQCLEAN_FRODOKEM640SHAKE_CLEAN_sample_n(Ep, PARAMS_N * PARAMS_NBAR); PQCLEAN_FRODOKEM640SHAKE_CLEAN_mul_add_sa_plus_e(BBp, Sp, Ep, pk_seedA); @@ -214,13 +216,27 @@ int PQCLEAN_FRODOKEM640SHAKE_CLEAN_crypto_kem_dec(uint8_t *ss, const uint8_t *ct BBp[i] = BBp[i] & ((1 << PARAMS_LOGQ) - 1); } +// Enable constant time compare +#if 0 // If (Bp == BBp & C == CC) then ss = F(ct || k'), else ss = F(ct || s) // Needs to avoid branching on secret data as per: // Qian Guo, Thomas Johansson, Alexander Nilsson. A key-recovery timing attack on post-quantum // primitives using the Fujisaki-Okamoto transformation and its application on FrodoKEM. In CRYPTO 2020. - int8_t selector = PQCLEAN_FRODOKEM640SHAKE_CLEAN_ct_verify(Bp, BBp, PARAMS_N * PARAMS_NBAR) | PQCLEAN_FRODOKEM640SHAKE_CLEAN_ct_verify(C, CC, PARAMS_NBAR * PARAMS_NBAR); + int8_t selector = ct_memcmp(Bp, BBp, PARAMS_N * PARAMS_NBAR) | ct_memcmp(C, CC, PARAMS_NBAR * PARAMS_NBAR); // If (selector == 0) then load k' to do ss = F(ct || k'), else if (selector == -1) load s to do ss = F(ct || s) PQCLEAN_FRODOKEM640SHAKE_CLEAN_ct_select((uint8_t *)Fin_k, (uint8_t *)kprime, (uint8_t *)sk_s, CRYPTO_BYTES, selector); +#else + if (memcmp(Bp, BBp, 2*PARAMS_N*PARAMS_NBAR) == 0 && memcmp(C, CC, 2*PARAMS_NBAR*PARAMS_NBAR) == 0) { + // Load k' to do ss = F(ct || k') + memcpy(Fin_k, kprime, CRYPTO_BYTES); + } else { + // Load s to do ss = F(ct || s) + // This branch is executed when a malicious ciphertext is decapsulated + // and is necessary for security. Note that the known answer tests + // will not exercise this line of code but it should not be removed. + memcpy(Fin_k, sk_s, CRYPTO_BYTES); + } +#endif shake(ss, CRYPTO_BYTES, Fin, CRYPTO_CIPHERTEXTBYTES + CRYPTO_BYTES); // Cleanup: diff --git a/src/kem/frodo/frodokem640shake/clean/util.c b/src/kem/frodo/frodokem640shake/clean/util.c index 67be6b76..60a0bc75 100644 --- a/src/kem/frodo/frodokem640shake/clean/util.c +++ b/src/kem/frodo/frodokem640shake/clean/util.c @@ -246,7 +246,6 @@ int8_t PQCLEAN_FRODOKEM640SHAKE_CLEAN_ct_verify(const uint16_t *a, const uint16_ void PQCLEAN_FRODOKEM640SHAKE_CLEAN_ct_select(uint8_t *r, const uint8_t *a, const uint8_t *b, size_t len, int8_t selector) { // Select one of the two input arrays to be moved to r // If (selector == 0) then load r with a, else if (selector == -1) load r with b - for (size_t i = 0; i < len; i++) { r[i] = (~selector & a[i]) | (selector & b[i]); } diff --git a/test/ut.cpp b/test/ut.cpp index 701c11bf..e65960d0 100644 --- a/test/ut.cpp +++ b/test/ut.cpp @@ -1,8 +1,10 @@ #include +#include #include + #include #include -#include +#include TEST(KEM,OneOff) { @@ -50,3 +52,70 @@ TEST(SIGN,OneOff) { pqc_sig_verify(p, sig.data(), sigsz, msg, 1234, pk.data())); } } + +TEST(Frodo, Decaps) { + const pqc_ctx_t *p = pqc_kem_alg_by_id(PQC_ALG_KEM_FRODOKEM640SHAKE); + + std::vector ct(pqc_ciphertext_bsz(p)); + std::vector ss1(pqc_shared_secret_bsz(p)); + std::vector ss2(pqc_shared_secret_bsz(p)); + std::vector sk(pqc_private_key_bsz(p)); + std::vector pk(pqc_public_key_bsz(p)); + bool res; + + ASSERT_TRUE( + pqc_keygen(p, pk.data(), sk.data())); + + ct_poison(sk.data(), 16); + ct_poison((unsigned char*)sk.data()+16+9616, 2*640*8 /*CRYPTO_SECRETBYTES*/); + ASSERT_TRUE( + pqc_kem_encapsulate(p, ct.data(), ss1.data(), pk.data())); + + // Decapsulate + ct_expect_umr(); + res = pqc_kem_decapsulate(p, ss2.data(), ct.data(), sk.data()); + ct_require_umr(); + + // Purify res to allow non-ct check by ASSERT_TRUE + ct_purify(&res, 1); + ASSERT_TRUE(res); + + // ss2 needs to be purified as it originates from poisoned data + ct_purify(ss2.data(), ss2.size()); + ASSERT_EQ(ss2, ss1); +} + +TEST(Frodo, Decaps_Negative) { + const pqc_ctx_t *p = pqc_kem_alg_by_id(PQC_ALG_KEM_FRODOKEM640SHAKE); + + std::vector ct(pqc_ciphertext_bsz(p)); + std::vector ss1(pqc_shared_secret_bsz(p)); + std::vector ss2(pqc_shared_secret_bsz(p)); + std::vector sk(pqc_private_key_bsz(p)); + std::vector pk(pqc_public_key_bsz(p)); + bool res; + + // Setup + ASSERT_TRUE( + pqc_keygen(p, pk.data(), sk.data())); + ct_poison(sk.data(), 16); + ct_poison(((unsigned char*)sk.data())+16+9616, 2*640*8); + + ASSERT_TRUE( + pqc_kem_encapsulate(p, ct.data(), ss1.data(), pk.data())); + + // Alter C1 of the ciphertext + ct[2] ^= 1; + + ct_expect_umr(); + res = pqc_kem_decapsulate(p, ss2.data(), ct.data(), sk.data()); + ct_require_umr(); + + // Purify res to allow non-ct check by ASSERT_TRUE + ct_purify(&res, 1); + ASSERT_TRUE(res); + + // ss2 needs to be purified as it originates from poisoned data + ct_purify(ss2.data(), ss2.size()); + ASSERT_NE(ss2, ss1); +}