From be7a0bbdb818f0f4ee40fcd02322d45512770efe 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 --- CMakeLists.txt | 1 + src/common/utils.h | 11 ++ src/kem/frodo/frodokem640shake/clean/kem.c | 22 +++- src/kem/frodo/frodokem640shake/clean/util.c | 6 +- test/ct.cpp | 28 +++- test/ut.cpp | 134 ++++++++++++++------ 6 files changed, 158 insertions(+), 44 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 56cfeb37..2c8f974b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -328,6 +328,7 @@ add_library( src/common/randombytes.c src/common/sha2.c src/common/nistseedexpander.c + src/common/utils.c src/capi/pqapi.c ${COMMON_EXTRA_SRC}) diff --git a/src/common/utils.h b/src/common/utils.h index 45142dab..5eca5acb 100644 --- a/src/common/utils.h +++ b/src/common/utils.h @@ -2,6 +2,8 @@ #define PQC_COMMON_UTILS_ #include +#include +#include // Helper to stringify constants #define STR(x) STR_(x) @@ -32,6 +34,15 @@ (((uint16_t)(x)[0])<<8 | \ ((uint16_t)(x)[1])<<0) \ +/** + * \brief Compares two arrays in constant time. + * \param [in] a first array + * \param [in] b second arrray + * \param [in] sz number of bytes to compare + * \returns 0 if arrays are equal, otherwise 1. + */ +uint8_t ct_memcmp(const void *a, const void *b, size_t sz); + const X86Features * get_cpu_caps(void); #endif diff --git a/src/kem/frodo/frodokem640shake/clean/kem.c b/src/kem/frodo/frodokem640shake/clean/kem.c index 40d5dd76..2d505728 100644 --- a/src/kem/frodo/frodokem640shake/clean/kem.c +++ b/src/kem/frodo/frodokem640shake/clean/kem.c @@ -14,6 +14,9 @@ #include "common.h" #include "params.h" +#include "common/ct_check.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 +142,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}; @@ -218,9 +220,25 @@ int PQCLEAN_FRODOKEM640SHAKE_CLEAN_crypto_kem_dec(uint8_t *ss, const uint8_t *ct // 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); +#if 0 + 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 + // Is (Bp == BBp & C == CC) = true + //ct_poison(Bp, sizeof(Bp)); + //ct_poison(BBp, sizeof(BBp)); + if (ct_memcmp(Bp, BBp, 2*PARAMS_N*PARAMS_NBAR) == 0 && ct_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..9dafd83d 100644 --- a/src/kem/frodo/frodokem640shake/clean/util.c +++ b/src/kem/frodo/frodokem640shake/clean/util.c @@ -11,6 +11,8 @@ #include "common.h" #include "params.h" +#include "common/ct_check.h" + static inline uint8_t min(uint8_t x, uint8_t y) { if (x < y) { return x; @@ -246,9 +248,9 @@ 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 - + uint8_t mask = 0 - selector; for (size_t i = 0; i < len; i++) { - r[i] = (~selector & a[i]) | (selector & b[i]); + r[i] = (~mask & a[i]) | (mask & b[i]); } } diff --git a/test/ct.cpp b/test/ct.cpp index 6209215e..02ce9d13 100644 --- a/test/ct.cpp +++ b/test/ct.cpp @@ -2,9 +2,12 @@ #include #include -#include -TEST(ConstantTime, CtGrind_Negative) { +extern "C" { +uint8_t ct_memcmp(const void *a, const void *b, size_t sz); +} + +TEST(ConstantTime, CtCheck_Negative) { unsigned char a[16], b[16]; unsigned i; memset(a, 42, 16); @@ -24,7 +27,7 @@ TEST(ConstantTime, CtGrind_Negative) { ASSERT_EQ(a[0], b[0]); } -TEST(ConstantTime, CtGrind_Positive_NoAccess) { +TEST(ConstantTime, CtCheck_Positive_NoAccess) { unsigned i; char result = 0; unsigned char a[16], b[16]; @@ -45,7 +48,7 @@ TEST(ConstantTime, CtGrind_Positive_NoAccess) { } -TEST(ConstantTime, CtGrind_Negative_UseSecretAsIndex) { +TEST(ConstantTime, CtCheck_Negative_UseSecretAsIndex) { static const unsigned char tab[2] = {1, 0}; unsigned char a[16]; unsigned char result; @@ -63,3 +66,20 @@ TEST(ConstantTime, CtGrind_Negative_UseSecretAsIndex) { ct_purify(&result, 1); ASSERT_EQ(result, 1); } + +TEST(ConstantTime, CtCheck_memcmp) { + unsigned char a[16], b[16]; + memset(a, 42, sizeof(a)); + memset(b, 42, sizeof(b)); + uint8_t ret; + + ct_poison(a, 16); + ret = ct_memcmp(a,b,16); + ct_purify(&ret, 1); + ASSERT_EQ(ret,0); + + b[1] = 0; + ret = ct_memcmp(a,b,16); + ct_purify(&ret, 1); + ASSERT_EQ(ret,1); +} diff --git a/test/ut.cpp b/test/ut.cpp index 8ea7a2c8..14ae0e4a 100644 --- a/test/ut.cpp +++ b/test/ut.cpp @@ -1,29 +1,31 @@ #include +#include #include + #include #include -#include +#include TEST(KEM,OneOff) { - for (int i=0; i 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)); - - ASSERT_TRUE( - pqc_keygen(p, pk.data(), sk.data())); - ASSERT_TRUE( - pqc_kem_encapsulate(p, ct.data(), ss1.data(), pk.data())); - ASSERT_TRUE( - pqc_kem_decapsulate(p, ss2.data(), ct.data(), sk.data())); - ASSERT_TRUE( - std::equal(ss1.begin(), ss1.end(), ss2.begin())); - } + for (int i=0; i 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)); + + ASSERT_TRUE( + pqc_keygen(p, pk.data(), sk.data())); + ASSERT_TRUE( + pqc_kem_encapsulate(p, ct.data(), ss1.data(), pk.data())); + ASSERT_TRUE( + pqc_kem_decapsulate(p, ss2.data(), ct.data(), sk.data())); + ASSERT_TRUE( + std::equal(ss1.begin(), ss1.end(), ss2.begin())); + } } TEST(SIGN,OneOff) { @@ -32,21 +34,81 @@ TEST(SIGN,OneOff) { std::uniform_int_distribution dist(0, 0xFF); uint8_t msg[1234] = {0}; - for (int i=0; i sig(pqc_signature_bsz(p)); - std::vector sk(pqc_private_key_bsz(p)); - std::vector pk(pqc_public_key_bsz(p)); - - ASSERT_TRUE( - pqc_keygen(p, pk.data(), sk.data())); - uint64_t sigsz = sig.size(); - ASSERT_TRUE( - pqc_sig_create(p, sig.data(), &sigsz, msg, 1234, sk.data())); - ASSERT_TRUE( - pqc_sig_verify(p, sig.data(), sigsz, msg, 1234, pk.data())); - } + for (int i=0; i sig(pqc_signature_bsz(p)); + std::vector sk(pqc_private_key_bsz(p)); + std::vector pk(pqc_public_key_bsz(p)); + + ASSERT_TRUE( + pqc_keygen(p, pk.data(), sk.data())); + uint64_t sigsz = sig.size(); + ASSERT_TRUE( + pqc_sig_create(p, sig.data(), &sigsz, msg, 1234, sk.data())); + ASSERT_TRUE( + 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 /*CRYPTO_BYTES*/); + ASSERT_TRUE( + pqc_kem_encapsulate(p, ct.data(), ss1.data(), pk.data())); + + // Decapsulate + res = pqc_kem_decapsulate(p, ss2.data(), ct.data(), sk.data()); + + // 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); + + ASSERT_TRUE( + pqc_kem_encapsulate(p, ct.data(), ss1.data(), pk.data())); + + // Ensure C2 of ciphertext is altered + ct[ct.size() - 1] ^= 1; + res = pqc_kem_decapsulate(p, ss2.data(), ct.data(), sk.data()); + + // 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); }