CT checks for Frodo

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

src/common/utils.h

@@ -2,6 +2,8 @@
 #define PQC_COMMON_UTILS_
 
 #include <cpuinfo_x86.h>
+#include <stdint.h>
+#include <stddef.h>
 
 // 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

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:

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

test/ct.cpp

@@ -2,9 +2,12 @@
 
 #include <gtest/gtest.h>
 #include <common/ct_check.h>
-#include <stdio.h>
 
-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);
+}

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,63 @@ 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 /*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<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);
+
+    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);
+}