fix(kyber): Close potential side-channel vulnerability (#534)

* fix(kyber): Close potential side-channel vulnerability Address the Kyber side-channel vulnerabilities that are potentially introduced by the division in certain poly operations. Closes #533 * Add note to SECURITY.md * Update security note
PQClean · Jan 25, 2024 · 3b43bc6 · 3b43bc6
1 parent a6c205a
commit 3b43bc6
Show file tree

Hide file tree

Showing 16 changed files with 115 additions and 37 deletions.
diff --git a/SECURITY.md b/SECURITY.md
@@ -20,13 +20,18 @@ Use at your own risk.
 ### Historic issues
 * LEDAcryptKEM `leaktime` implementations are known to not be constant-time and expected to have timing side channel vulnerabilities.
   * LEDA support has since been dropped from PQClean as it got eliminated from the NIST PQC standardization project in round 3.
-* Rainbow level I has been shown to be insecure by Ward Beullens. 
+* Rainbow level I has been shown to be insecure by Ward Beullens.
   * Rainbow was eliminated from the NIST PQC standardization project and removed from PQClean in round 4.
 
 <!-- new date line
 ### 2019-XX-XX
 -->
 
+### 2024-01-25
+* Kyber used division operations that might leak side-channel information.
+  [PR #534](https://github.com/PQClean/PQClean/pull/534) addressed this for the `clean` and `avx2` implementations.
+  Note that the `aarch64` implementation is waiting for [PR#527](https://github.com/PQClean/PQClean/pull/527) to get fixed.
+
 ### 2020-12-11
 * The fix of the timing leak in the CCA transform of FrodoKEM in [PR #303](https://github.com/PQClean/PQClean/pull/303) was ineffective. The FrodoKEM team released another [fix](https://github.com/microsoft/PQCrypto-LWEKE/commit/669522db63850fa64d1a24a47e138e80a59349db) which was ported to PQClean in [PR #367](https://github.com/PQClean/PQClean/pull/367).
 

diff --git a/crypto_kem/kyber1024/META.yml b/crypto_kem/kyber1024/META.yml
@@ -22,9 +22,9 @@ auxiliary-submitters:
   - Jintai Ding
 implementations:
     - name: clean
-      version: https://github.com/pq-crystals/kyber/commit/4ecce06d584a4e7cecc42f141aadae39b8431b06 via https://github.com/mkannwischer/package-pqclean/tree/146bcfac/kyber
+      version: https://github.com/pq-crystals/kyber/commit/11d00ff1f20cfca1f72d819e5a45165c1e0a2816 via https://github.com/mkannwischer/package-pqclean/tree/0e93c032/kyber
     - name: avx2
-      version: https://github.com/pq-crystals/kyber/commit/4ecce06d584a4e7cecc42f141aadae39b8431b06 via https://github.com/mkannwischer/package-pqclean/tree/146bcfac/kyber
+      version: https://github.com/pq-crystals/kyber/commit/11d00ff1f20cfca1f72d819e5a45165c1e0a2816 via https://github.com/mkannwischer/package-pqclean/tree/0e93c032/kyber
       supported_platforms:
         - architecture: x86_64
           operating_systems:

diff --git a/crypto_kem/kyber1024/clean/poly.c b/crypto_kem/kyber1024/clean/poly.c
@@ -16,16 +16,22 @@
 *              - const poly *a: pointer to input polynomial
 **************************************************/
 void PQCLEAN_KYBER1024_CLEAN_poly_compress(uint8_t r[KYBER_POLYCOMPRESSEDBYTES], const poly *a) {
-    size_t i, j;
-    int16_t u;
+    unsigned int i, j;
+    int32_t u;
+    uint32_t d0;
     uint8_t t[8];
 
     for (i = 0; i < KYBER_N / 8; i++) {
         for (j = 0; j < 8; j++) {
             // map to positive standard representatives
             u  = a->coeffs[8 * i + j];
             u += (u >> 15) & KYBER_Q;
-            t[j] = ((((uint32_t)u << 5) + KYBER_Q / 2) / KYBER_Q) & 31;
+            /*    t[j] = ((((uint32_t)u << 5) + KYBER_Q/2)/KYBER_Q) & 31; */
+            d0 = u << 5;
+            d0 += 1664;
+            d0 *= 40318;
+            d0 >>= 27;
+            t[j] = d0 & 0x1f;
         }
 
         r[0] = (t[0] >> 0) | (t[1] << 5);
@@ -141,15 +147,20 @@ void PQCLEAN_KYBER1024_CLEAN_poly_frommsg(poly *r, const uint8_t msg[KYBER_INDCP
 *              - const poly *a: pointer to input polynomial
 **************************************************/
 void PQCLEAN_KYBER1024_CLEAN_poly_tomsg(uint8_t msg[KYBER_INDCPA_MSGBYTES], const poly *a) {
-    size_t i, j;
-    uint16_t t;
+    unsigned int i, j;
+    uint32_t t;
 
     for (i = 0; i < KYBER_N / 8; i++) {
         msg[i] = 0;
         for (j = 0; j < 8; j++) {
             t  = a->coeffs[8 * i + j];
-            t += ((int16_t)t >> 15) & KYBER_Q;
-            t  = (((t << 1) + KYBER_Q / 2) / KYBER_Q) & 1;
+            // t += ((int16_t)t >> 15) & KYBER_Q;
+            // t  = (((t << 1) + KYBER_Q/2)/KYBER_Q) & 1;
+            t <<= 1;
+            t += 1665;
+            t *= 80635;
+            t >>= 28;
+            t &= 1;
             msg[i] |= t << j;
         }
     }

diff --git a/crypto_kem/kyber1024/clean/polyvec.c b/crypto_kem/kyber1024/clean/polyvec.c
@@ -14,14 +14,22 @@
 **************************************************/
 void PQCLEAN_KYBER1024_CLEAN_polyvec_compress(uint8_t r[KYBER_POLYVECCOMPRESSEDBYTES], const polyvec *a) {
     unsigned int i, j, k;
+    uint64_t d0;
 
     uint16_t t[8];
     for (i = 0; i < KYBER_K; i++) {
         for (j = 0; j < KYBER_N / 8; j++) {
             for (k = 0; k < 8; k++) {
                 t[k]  = a->vec[i].coeffs[8 * j + k];
                 t[k] += ((int16_t)t[k] >> 15) & KYBER_Q;
-                t[k]  = ((((uint32_t)t[k] << 11) + KYBER_Q / 2) / KYBER_Q) & 0x7ff;
+                /*      t[k]  = ((((uint32_t)t[k] << 11) + KYBER_Q/2)/KYBER_Q) & 0x7ff; */
+                d0 = t[k];
+                d0 <<= 11;
+                d0 += 1664;
+                d0 *= 645084;
+                d0 >>= 31;
+                t[k] = d0 & 0x7ff;
+
             }
 
             r[ 0] = (uint8_t)(t[0] >>  0);

diff --git a/crypto_kem/kyber512/META.yml b/crypto_kem/kyber512/META.yml
@@ -22,9 +22,9 @@ auxiliary-submitters:
   - Jintai Ding
 implementations:
     - name: clean
-      version: https://github.com/pq-crystals/kyber/commit/4ecce06d584a4e7cecc42f141aadae39b8431b06 via https://github.com/mkannwischer/package-pqclean/tree/146bcfac/kyber
+      version: https://github.com/pq-crystals/kyber/commit/11d00ff1f20cfca1f72d819e5a45165c1e0a2816 via https://github.com/mkannwischer/package-pqclean/tree/0e93c032/kyber
     - name: avx2
-      version: https://github.com/pq-crystals/kyber/commit/4ecce06d584a4e7cecc42f141aadae39b8431b06 via https://github.com/mkannwischer/package-pqclean/tree/146bcfac/kyber
+      version: https://github.com/pq-crystals/kyber/commit/11d00ff1f20cfca1f72d819e5a45165c1e0a2816 via https://github.com/mkannwischer/package-pqclean/tree/0e93c032/kyber
       supported_platforms:
         - architecture: x86_64
           operating_systems:

diff --git a/crypto_kem/kyber512/clean/poly.c b/crypto_kem/kyber512/clean/poly.c
@@ -16,16 +16,22 @@
 *              - const poly *a: pointer to input polynomial
 **************************************************/
 void PQCLEAN_KYBER512_CLEAN_poly_compress(uint8_t r[KYBER_POLYCOMPRESSEDBYTES], const poly *a) {
-    size_t i, j;
-    int16_t u;
+    unsigned int i, j;
+    int32_t u;
+    uint32_t d0;
     uint8_t t[8];
 
     for (i = 0; i < KYBER_N / 8; i++) {
         for (j = 0; j < 8; j++) {
             // map to positive standard representatives
             u  = a->coeffs[8 * i + j];
             u += (u >> 15) & KYBER_Q;
-            t[j] = ((((uint16_t)u << 4) + KYBER_Q / 2) / KYBER_Q) & 15;
+            /*    t[j] = ((((uint16_t)u << 4) + KYBER_Q/2)/KYBER_Q) & 15; */
+            d0 = u << 4;
+            d0 += 1665;
+            d0 *= 80635;
+            d0 >>= 28;
+            t[j] = d0 & 0xf;
         }
 
         r[0] = t[0] | (t[1] << 4);
@@ -128,15 +134,20 @@ void PQCLEAN_KYBER512_CLEAN_poly_frommsg(poly *r, const uint8_t msg[KYBER_INDCPA
 *              - const poly *a: pointer to input polynomial
 **************************************************/
 void PQCLEAN_KYBER512_CLEAN_poly_tomsg(uint8_t msg[KYBER_INDCPA_MSGBYTES], const poly *a) {
-    size_t i, j;
-    uint16_t t;
+    unsigned int i, j;
+    uint32_t t;
 
     for (i = 0; i < KYBER_N / 8; i++) {
         msg[i] = 0;
         for (j = 0; j < 8; j++) {
             t  = a->coeffs[8 * i + j];
-            t += ((int16_t)t >> 15) & KYBER_Q;
-            t  = (((t << 1) + KYBER_Q / 2) / KYBER_Q) & 1;
+            // t += ((int16_t)t >> 15) & KYBER_Q;
+            // t  = (((t << 1) + KYBER_Q/2)/KYBER_Q) & 1;
+            t <<= 1;
+            t += 1665;
+            t *= 80635;
+            t >>= 28;
+            t &= 1;
             msg[i] |= t << j;
         }
     }

diff --git a/crypto_kem/kyber512/clean/polyvec.c b/crypto_kem/kyber512/clean/polyvec.c
@@ -14,14 +14,21 @@
 **************************************************/
 void PQCLEAN_KYBER512_CLEAN_polyvec_compress(uint8_t r[KYBER_POLYVECCOMPRESSEDBYTES], const polyvec *a) {
     unsigned int i, j, k;
+    uint64_t d0;
 
     uint16_t t[4];
     for (i = 0; i < KYBER_K; i++) {
         for (j = 0; j < KYBER_N / 4; j++) {
             for (k = 0; k < 4; k++) {
                 t[k]  = a->vec[i].coeffs[4 * j + k];
                 t[k] += ((int16_t)t[k] >> 15) & KYBER_Q;
-                t[k]  = ((((uint32_t)t[k] << 10) + KYBER_Q / 2) / KYBER_Q) & 0x3ff;
+                /*      t[k]  = ((((uint32_t)t[k] << 10) + KYBER_Q/2)/ KYBER_Q) & 0x3ff; */
+                d0 = t[k];
+                d0 <<= 10;
+                d0 += 1665;
+                d0 *= 1290167;
+                d0 >>= 32;
+                t[k] = d0 & 0x3ff;
             }
 
             r[0] = (uint8_t)(t[0] >> 0);

diff --git a/crypto_kem/kyber768/META.yml b/crypto_kem/kyber768/META.yml
@@ -22,9 +22,9 @@ auxiliary-submitters:
   - Jintai Ding
 implementations:
     - name: clean
-      version: https://github.com/pq-crystals/kyber/commit/4ecce06d584a4e7cecc42f141aadae39b8431b06 via https://github.com/mkannwischer/package-pqclean/tree/146bcfac/kyber
+      version: https://github.com/pq-crystals/kyber/commit/11d00ff1f20cfca1f72d819e5a45165c1e0a2816 via https://github.com/mkannwischer/package-pqclean/tree/0e93c032/kyber
     - name: avx2
-      version: https://github.com/pq-crystals/kyber/commit/4ecce06d584a4e7cecc42f141aadae39b8431b06 via https://github.com/mkannwischer/package-pqclean/tree/146bcfac/kyber
+      version: https://github.com/pq-crystals/kyber/commit/11d00ff1f20cfca1f72d819e5a45165c1e0a2816 via https://github.com/mkannwischer/package-pqclean/tree/0e93c032/kyber
       supported_platforms:
         - architecture: x86_64
           operating_systems:

diff --git a/crypto_kem/kyber768/clean/poly.c b/crypto_kem/kyber768/clean/poly.c
@@ -16,16 +16,22 @@
 *              - const poly *a: pointer to input polynomial
 **************************************************/
 void PQCLEAN_KYBER768_CLEAN_poly_compress(uint8_t r[KYBER_POLYCOMPRESSEDBYTES], const poly *a) {
-    size_t i, j;
-    int16_t u;
+    unsigned int i, j;
+    int32_t u;
+    uint32_t d0;
     uint8_t t[8];
 
     for (i = 0; i < KYBER_N / 8; i++) {
         for (j = 0; j < 8; j++) {
             // map to positive standard representatives
             u  = a->coeffs[8 * i + j];
             u += (u >> 15) & KYBER_Q;
-            t[j] = ((((uint16_t)u << 4) + KYBER_Q / 2) / KYBER_Q) & 15;
+            /*    t[j] = ((((uint16_t)u << 4) + KYBER_Q/2)/KYBER_Q) & 15; */
+            d0 = u << 4;
+            d0 += 1665;
+            d0 *= 80635;
+            d0 >>= 28;
+            t[j] = d0 & 0xf;
         }
 
         r[0] = t[0] | (t[1] << 4);
@@ -128,15 +134,20 @@ void PQCLEAN_KYBER768_CLEAN_poly_frommsg(poly *r, const uint8_t msg[KYBER_INDCPA
 *              - const poly *a: pointer to input polynomial
 **************************************************/
 void PQCLEAN_KYBER768_CLEAN_poly_tomsg(uint8_t msg[KYBER_INDCPA_MSGBYTES], const poly *a) {
-    size_t i, j;
-    uint16_t t;
+    unsigned int i, j;
+    uint32_t t;
 
     for (i = 0; i < KYBER_N / 8; i++) {
         msg[i] = 0;
         for (j = 0; j < 8; j++) {
             t  = a->coeffs[8 * i + j];
-            t += ((int16_t)t >> 15) & KYBER_Q;
-            t  = (((t << 1) + KYBER_Q / 2) / KYBER_Q) & 1;
+            // t += ((int16_t)t >> 15) & KYBER_Q;
+            // t  = (((t << 1) + KYBER_Q/2)/KYBER_Q) & 1;
+            t <<= 1;
+            t += 1665;
+            t *= 80635;
+            t >>= 28;
+            t &= 1;
             msg[i] |= t << j;
         }
     }

diff --git a/crypto_kem/kyber768/clean/polyvec.c b/crypto_kem/kyber768/clean/polyvec.c
@@ -14,14 +14,21 @@
 **************************************************/
 void PQCLEAN_KYBER768_CLEAN_polyvec_compress(uint8_t r[KYBER_POLYVECCOMPRESSEDBYTES], const polyvec *a) {
     unsigned int i, j, k;
+    uint64_t d0;
 
     uint16_t t[4];
     for (i = 0; i < KYBER_K; i++) {
         for (j = 0; j < KYBER_N / 4; j++) {
             for (k = 0; k < 4; k++) {
                 t[k]  = a->vec[i].coeffs[4 * j + k];
                 t[k] += ((int16_t)t[k] >> 15) & KYBER_Q;
-                t[k]  = ((((uint32_t)t[k] << 10) + KYBER_Q / 2) / KYBER_Q) & 0x3ff;
+                /*      t[k]  = ((((uint32_t)t[k] << 10) + KYBER_Q/2)/ KYBER_Q) & 0x3ff; */
+                d0 = t[k];
+                d0 <<= 10;
+                d0 += 1665;
+                d0 *= 1290167;
+                d0 >>= 32;
+                t[k] = d0 & 0x3ff;
             }
 
             r[0] = (uint8_t)(t[0] >> 0);

diff --git a/test/duplicate_consistency/kyber1024_avx2.yml b/test/duplicate_consistency/kyber1024_avx2.yml
@@ -5,6 +5,7 @@ consistency_checks:
       files:
       - indcpa.h
       - verify.h
+      - kem.c
       - symmetric-shake.c
     - source:
         scheme: kyber512
@@ -34,6 +35,7 @@ consistency_checks:
       files:
       - indcpa.h
       - verify.h
+      - kem.c
       - symmetric-shake.c
     - source:
         scheme: kyber768
@@ -67,4 +69,5 @@ consistency_checks:
       - indcpa.h
       - kem.h
       - verify.h
-      - symmetric-shake.c
+      - kem.c
+      - symmetric-shake.c
diff --git a/test/duplicate_consistency/kyber1024_clean.yml b/test/duplicate_consistency/kyber1024_clean.yml
@@ -23,6 +23,7 @@ consistency_checks:
       files:
       - indcpa.h
       - verify.h
+      - kem.c
       - symmetric-shake.c
     - source:
         scheme: kyber768
@@ -49,6 +50,7 @@ consistency_checks:
       files:
       - indcpa.h
       - verify.h
+      - kem.c
       - symmetric-shake.c
     - source:
         scheme: kyber1024
@@ -58,4 +60,5 @@ consistency_checks:
       - indcpa.h
       - kem.h
       - verify.h
-      - symmetric-shake.c
+      - kem.c
+      - symmetric-shake.c
diff --git a/test/duplicate_consistency/kyber512_avx2.yml b/test/duplicate_consistency/kyber512_avx2.yml
@@ -7,13 +7,15 @@ consistency_checks:
       - indcpa.h
       - kem.h
       - verify.h
+      - kem.c
       - symmetric-shake.c
     - source:
         scheme: kyber768
         implementation: clean
       files:
       - indcpa.h
       - verify.h
+      - kem.c
       - symmetric-shake.c
     - source:
         scheme: kyber768
@@ -44,6 +46,7 @@ consistency_checks:
       files:
       - indcpa.h
       - verify.h
+      - kem.c
       - symmetric-shake.c
     - source:
         scheme: kyber1024
@@ -66,4 +69,4 @@ consistency_checks:
       - kem.c
       - rejsample.c
       - symmetric-shake.c
-      - verify.c
+      - verify.c
diff --git a/test/duplicate_consistency/kyber512_clean.yml b/test/duplicate_consistency/kyber512_clean.yml
@@ -7,6 +7,7 @@ consistency_checks:
       - indcpa.h
       - kem.h
       - verify.h
+      - kem.c
       - symmetric-shake.c
     - source:
         scheme: kyber768
@@ -34,6 +35,7 @@ consistency_checks:
       files:
       - indcpa.h
       - verify.h
+      - kem.c
       - symmetric-shake.c
     - source:
         scheme: kyber1024
@@ -59,4 +61,5 @@ consistency_checks:
       files:
       - indcpa.h
       - verify.h
-      - symmetric-shake.c
+      - kem.c
+      - symmetric-shake.c