Make NTRU's mod3 and modQ reductions division-free, restoring the reference implementation's fold-and-select and its MODQ mask so the secret operand is not exposed through division latency, with an exhaustive regression test and the Ant bctest include it needs; reported by the Robusta team.

Author: dghgit

ant/bc+-build.xml

@@ -988,6 +988,7 @@
              <fileset dir="${src.dir}" includes="**/crypto/*/*Test.java" />
              <fileset dir="${src.dir}" includes="**/crypto/agreement/owl/*Test.java" />
              <fileset dir="${src.dir}" includes="**/pqc/crypto/hqc/*Test.java" />
+             <fileset dir="${src.dir}" includes="**/pqc/math/ntru/*Test.java" />
              <fileset dir="${src.dir}" includes="**/tsp/*UnitTest.java" />
              <fileset dir="${src.dir}" includes="**/utiltest/*Test.java" />
              <fileset dir="${src.dir}" includes="**/util/io/pem/*Test.java" />

core/src/main/java/org/bouncycastle/pqc/crypto/ntru/NTRUSampling.java

@@ -191,8 +191,20 @@ public HRSSPolynomial sampleIidPlus(byte[] uniformBytes)
         return r;
     }
 
+    // NOTE: this reduces the bytes of SHAKE(rmSeed) into the ternary message polynomials r and m
+    // during encapsulation, and the secret key polynomials f and g during key generation, so it
+    // must not divide. Same division-free fold-and-select as the reference mod3 (see
+    // org.bouncycastle.pqc.math.ntru.Polynomial.mod3); do not replace it with a % 3.
     private static int mod3(int a)
     {
-        return a % 3;
+        int r = (a >>> 8) + (a & 0xff);
+        r = (r >>> 4) + (r & 0x0f);
+        r = (r >>> 2) + (r & 0x03);
+        r = (r >>> 2) + (r & 0x03);
+
+        int t = r - 3;
+        int c = t >> 31;
+
+        return (c & r) ^ (~c & t);
     }
 }

core/src/main/java/org/bouncycastle/pqc/math/ntru/Polynomial.java

@@ -36,21 +36,43 @@ static short bothNegativeMask(short x, short y)
     }
 
     // defined in poly_mod.c
+    //
+    // NOTE: this runs on secret data - the secret key polynomials f and g during key
+    // generation, the message polynomials r and m during encapsulation, and the recovered
+    // coefficients during decapsulation - so it must not divide. Folding by 255, then 15, then
+    // 3 preserves the residue mod 3 (3 divides both 255 and 15, and 4 = 1 mod 3) and leaves a
+    // value of at most 5, which one masked conditional subtraction reduces to 0..2. This is the
+    // reference implementation's division-free mod3; do not "simplify" it back to a % 3, which
+    // compiles to an integer division whose latency depends on the secret operand.
     static short mod3(short a)
     {
-        return (short)((a & 0xffff) % 3);
+        int r = a & 0xffff;
+        r = (r >>> 8) + (r & 0xff);
+        r = (r >>> 4) + (r & 0x0f);
+        r = (r >>> 2) + (r & 0x03);
+        r = (r >>> 2) + (r & 0x03);
+
+        int t = r - 3;
+        int c = t >> 31;
+
+        return (short)((c & r) ^ (~c & t));
     }
 
     // defined in poly_s3_inv.c
     static byte mod3(byte a)
     {
-        return (byte)((a & 0xff) % 3);
+        return (byte)mod3((short)(a & 0xff));
     }
 
     // defined in poly.h
+    //
+    // NOTE: q is always a power of two (NTRUParameterSet.q() returns 1 << logQ) and every call
+    // site passes a value already masked to 16 bits, so masking is exactly x % q for the inputs
+    // that occur here - and, unlike a division by a variable divisor, it cannot leak the secret
+    // dividend through its latency. This matches the reference MODQ(X) ((X) & (NTRU_Q-1)).
     static int modQ(int x, int q)
     {
-        return x % q;
+        return x & (q - 1);
     }
 
 

core/src/test/java/org/bouncycastle/pqc/math/ntru/AllTests.java

@@ -0,0 +1,42 @@
+package org.bouncycastle.pqc.math.ntru;
+
+import junit.extensions.TestSetup;
+import junit.framework.Test;
+import junit.framework.TestCase;
+import junit.framework.TestSuite;
+import org.bouncycastle.test.PrintTestResult;
+
+public class AllTests
+    extends TestCase
+{
+    public static void main(String[] args)
+    {
+        PrintTestResult.printResult(junit.textui.TestRunner.run(suite()));
+    }
+
+    public static Test suite()
+    {
+        TestSuite suite = new TestSuite("NTRU Internal Tests");
+
+        suite.addTestSuite(PolynomialModTest.class);
+
+        return new BCTestSetup(suite);
+    }
+
+    static class BCTestSetup
+        extends TestSetup
+    {
+        public BCTestSetup(Test test)
+        {
+            super(test);
+        }
+
+        protected void setUp()
+        {
+        }
+
+        protected void tearDown()
+        {
+        }
+    }
+}

core/src/test/java/org/bouncycastle/pqc/math/ntru/PolynomialModTest.java

@@ -0,0 +1,95 @@
+package org.bouncycastle.pqc.math.ntru;
+
+import junit.framework.TestCase;
+
+/**
+ * Regression tests for NTRU's modular reductions.
+ * <p>
+ * {@link Polynomial#mod3(short)}, {@link Polynomial#mod3(byte)} and
+ * {@link Polynomial#modQ(int, int)} used to be written with the {@code %} operator, which
+ * compiles to an integer division whose latency depends on its operands - and all three run on
+ * secret data (the secret key polynomials f and g, the message polynomials r and m, and the
+ * coefficients recovered during decapsulation). They are now division-free, matching the
+ * reference implementation, and these tests pin them to the values {@code %} produced across the
+ * entire input domain that reaches them.
+ * <p>
+ * This test has to live in {@code org.bouncycastle.pqc.math.ntru} rather than the
+ * {@code ...ntru.test} package alongside {@link org.bouncycastle.pqc.math.ntru.test.PolynomialTest},
+ * because the methods are package-private.
+ */
+public class PolynomialModTest
+    extends TestCase
+{
+    /**
+     * Every logQ appearing in a parameter set: NTRUHPS2048509/677 use 11, NTRUHPS4096821/1229
+     * use 12, NTRUHRSS701 uses 13 and NTRUHRSS1373 uses 14. NTRUParameterSet.q() is 1 << logQ,
+     * which is what makes masking a valid reduction.
+     */
+    private static final int[] LOG_QS = new int[]{11, 12, 13, 14};
+
+    public void testMod3ShortOverEveryInput()
+    {
+        for (int a = Short.MIN_VALUE; a <= Short.MAX_VALUE; ++a)
+        {
+            short s = (short)a;
+            assertEquals("mod3(short " + s + ")", (s & 0xffff) % 3, Polynomial.mod3(s));
+        }
+    }
+
+    public void testMod3ByteOverEveryInput()
+    {
+        for (int a = Byte.MIN_VALUE; a <= Byte.MAX_VALUE; ++a)
+        {
+            byte b = (byte)a;
+            assertEquals("mod3(byte " + b + ")", (byte)((b & 0xff) % 3), Polynomial.mod3(b));
+        }
+    }
+
+    /**
+     * The callers treat the result as a ternary coefficient, so nothing may escape 0..2 - the
+     * masked conditional subtraction has to leave the folded value fully reduced.
+     */
+    public void testMod3IsAlwaysTernary()
+    {
+        for (int a = Short.MIN_VALUE; a <= Short.MAX_VALUE; ++a)
+        {
+            short r = Polynomial.mod3((short)a);
+            assertTrue("mod3(short " + a + ") = " + r, r >= 0 && r <= 2);
+        }
+
+        for (int a = Byte.MIN_VALUE; a <= Byte.MAX_VALUE; ++a)
+        {
+            byte r = Polynomial.mod3((byte)a);
+            assertTrue("mod3(byte " + a + ") = " + r, r >= 0 && r <= 2);
+        }
+    }
+
+    /**
+     * Every call site passes a dividend already masked to 16 bits, so that is the domain that
+     * has to agree with {@code %}; q is a power of two for every parameter set.
+     */
+    public void testModQOverEveryInputAndParameterSet()
+    {
+        for (int i = 0; i < LOG_QS.length; ++i)
+        {
+            int q = 1 << LOG_QS[i];
+            for (int x = 0; x <= 0xffff; ++x)
+            {
+                assertEquals("modQ(" + x + ", " + q + ")", x % q, Polynomial.modQ(x, q));
+            }
+        }
+    }
+
+    public void testModQIsInRange()
+    {
+        for (int i = 0; i < LOG_QS.length; ++i)
+        {
+            int q = 1 << LOG_QS[i];
+            for (int x = 0; x <= 0xffff; ++x)
+            {
+                int r = Polynomial.modQ(x, q);
+                assertTrue("modQ(" + x + ", " + q + ") = " + r, r >= 0 && r < q);
+            }
+        }
+    }
+}