CT checks for Frodo

src/common/utils.c

@@ -1,6 +1,23 @@
 #include <stdint.h>
 #include <stddef.h>
+#include <common/ct_check.h>
 
+// 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

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:

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]);
     }

test/ut.cpp

@@ -1,8 +1,10 @@
 #include <algorithm>
+#include <random>
 #include <vector>
+
 #include <gtest/gtest.h>
 #include <pqc/pqc.h>
-#include <random>
+#include <common/ct_check.h>
 
 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<uint8_t> ct(pqc_ciphertext_bsz(p));
+    std::vector<uint8_t> ss1(pqc_shared_secret_bsz(p));
+    std::vector<uint8_t> ss2(pqc_shared_secret_bsz(p));
+    std::vector<uint8_t> sk(pqc_private_key_bsz(p));
+    std::vector<uint8_t> 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<uint8_t> ct(pqc_ciphertext_bsz(p));
+    std::vector<uint8_t> ss1(pqc_shared_secret_bsz(p));
+    std::vector<uint8_t> ss2(pqc_shared_secret_bsz(p));
+    std::vector<uint8_t> sk(pqc_private_key_bsz(p));
+    std::vector<uint8_t> 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);
+}