Merge pull request #4 from soatok/pornin-feedback

Implement Thomas Pornin's Feedback

lib/compare.ts

@@ -1,4 +1,5 @@
 import { int32 } from './int32';
+import {select_int32} from "./select";
 
 export function compare(left: Uint8Array, right: Uint8Array): number {
     if (left.length !== right.length) {
@@ -28,8 +29,21 @@ export function compare_ints(left: number, right: number): number {
     */
     const L: int32 = int32.fromInt(left);
     const R: int32 = int32.fromInt(right);
-    const gt: number = R.sub(L).msb();
-    const eq: number = R.xor(L).sub(1).msb();
+    /*
+    This borrows a trick from Thomas Pornin's CTTK library:
+
+    https://github.com/pornin/CTTK/blob/1d592024398f06c8eda1d325bdbd105ac32d92b3/inc/cttk.h#L552-L581
+
+	> If both x >= 2^31 and y >= 2^31, then we can virtually
+	> subtract 2^31 from both, and we are back to the first
+	> case. Since (y-2^31)-(x-2^31) = y-x, the direct subtraction
+	> is already fine.
+
+	Except (L, R, diff) := (x, y, z), respectively
+    */
+    const diff: int32 = R.sub(L);
+    const gt: number = diff.xor(L.xor(R).and(L.xor(diff))).msb();
+    const eq: number = R.xor(L).isZero();
     L.wipe();
     R.wipe();
     return (gt + gt + eq) - 1;

lib/int32.ts

@@ -54,15 +54,19 @@ export class int32 {
         return int32_compare(this, arg);
     }
 
+    isZero(): number {
+        return int32_is_zero(this);
+    }
+
     lshift(amount: number): int32 {
         if (amount > 31) {
             return int32.zero();
         }
         return int32_lshift(this, amount);
     }
 
-    negate(): int32 {
-        return int32_negate(this);
+    not(): int32 {
+        return int32_not(this);
     }
 
     or(arg: number|int32): int32 {
@@ -138,18 +142,44 @@ function int32_and_number(a: int32, b: number): int32 {
 }
 
 function int32_compare(left, right): int32 {
-    let gt: number = right.sub(left).msb();
-    let eq: number = right.xor(left).sub(1).msb();
+    const diff: int32 = right.sub(left);
+    /*
+    This borrows a trick from Thomas Pornin's CTTK library:
+
+    https://github.com/pornin/CTTK/blob/1d592024398f06c8eda1d325bdbd105ac32d92b3/inc/cttk.h#L552-L581
+
+	> If both x >= 2^31 and y >= 2^31, then we can virtually
+	> subtract 2^31 from both, and we are back to the first
+	> case. Since (y-2^31)-(x-2^31) = y-x, the direct subtraction
+	> is already fine.
+
+	Except (left, right, diff) := (x, y, z), respectively
+    */
+    let gt: number = diff.xor(left.xor(right).and(left.xor(diff))).msb();
+    let eq: number = right.xor(left).isZero();
     return int32.fromInt((gt + gt + eq) - 1);
 }
 
+function int32_is_zero(a: int32): number {
+    /*
+     Bitiwse OR the bits of the limbs together
+     then subtract 1 and grab the most significant bit.
+
+     [0] - 1 >>> 31
+      -> 1
+     [1..0xffff] - 1 >>> 31
+      -> 0
+     */
+    return ((a.low() | a.high()) - 1) >>> 31;
+}
+
 function int32_lshift(a: int32, x: number) {
     let l: number = (a.low() << x);
     let h: number = (l >>> 16) | (a.high() << x);
     return new int32(l & 0xffff, h & 0xffff);
 }
 
-function int32_negate(a: int32): int32 {
+function int32_not(a: int32): int32 {
     let l: number = a.low() ^ 0xffff;
     let h: number = a.high() ^ 0xffff;
     return new int32(l & 0xffff, h & 0xffff);

lib/trim.ts

@@ -14,10 +14,10 @@ export function trim_zeroes_right(buf: Uint8Array): Uint8Array {
     for (i = buf.length - 1; i >= 0; i--) {
         /* if foundNonZero === 0 && buf[i] !== 0, index := i */
         isNonZero = int32.fromInt(buf[i]).sub(1).rshift(8).and(m);
-        index = (foundNonZero.negate().and(i))
+        index = (foundNonZero.not().and(i))
             .xor(foundNonZero.and(index))
             .toNumber();
-        foundNonZero = foundNonZero.or(isNonZero.negate()).and(m);
+        foundNonZero = foundNonZero.or(isNonZero.not()).and(m);
     }
     foundNonZero.wipe();
     isNonZero.wipe();

tests/compare.test.ts

@@ -37,5 +37,23 @@ describe('Constant-Time Comparison', () => {
         expect(compare_ints(A, C)).to.be.equal(-1);
         expect(compare_ints(B, C)).to.be.equal(-1);
         expect(compare_ints(C, C)).to.be.equal(0);
+    });
+
+    it('compare_ints() big', () => {
+        const SMOL =    0x12345678;
+        const CHUNGUS = 0x7fffffff;
+        const MAX =     0xffffffff;
+
+        expect(compare_ints(MAX, MAX)).to.be.equal(0);
+        expect(compare_ints(CHUNGUS, MAX)).to.be.equal(-1);
+        expect(compare_ints(SMOL, MAX)).to.be.equal(-1);
+
+        expect(compare_ints(MAX, CHUNGUS)).to.be.equal(1);
+        expect(compare_ints(CHUNGUS, CHUNGUS)).to.be.equal(0);
+        expect(compare_ints(SMOL, CHUNGUS)).to.be.equal(-1);
+
+        expect(compare_ints(MAX, SMOL)).to.be.equal(1);
+        expect(compare_ints(CHUNGUS, SMOL)).to.be.equal(1);
+        expect(compare_ints(SMOL, SMOL)).to.be.equal(0);
     })
 });

tests/int32.test.ts

@@ -68,9 +68,9 @@ describe('Int32', () => {
             .to.be.equal(0);
     });
 
-    it('negate()', () => {;
+    it('not()', () => {
         const z: int32 = int32.fromInt(0x12345678);
-        expect(z.negate().toHex()).to.be.equal('edcba987');
+        expect(z.not().toHex()).to.be.equal('edcba987');
         expect(z.toHex()).to.be.equal('12345678');
     });