Fix invsqrt() boolean return

The invsqrt() function used to return true when its operand was a
non-zero square.  Thing is, it was used to determine whether it was a
square, *period*.  If the operand was zero, we had the wrong answer.

This introduced two "errors" visible by the end user:

* The all-zero X25519 point could not be mapped back to an Elligator 2
  representative.  In practice that point is never used, but that does
  not comply with the original paper.

* The (0, 1) EdDSa public key could not be serialised, causing any
  signature verification associated with it to fail. This one we really
  don't care, because even though it is on the curve, that point has low
  order, and legitimate public keys all have prime order.

Fixes #231

diff --git a/src/monocypher.c b/src/monocypher.c
index 31f1f59c4d7cc21c25750e3001d07364453352b4..a58a691e0b3102e9b7e675ed03fc14095f1021df 100644 (file)
--- a/src/monocypher.c
+++ b/src/monocypher.c
@@ -1375,7 +1375,7 @@ static int fe_isequal(const fe f, const fe g)
 }
 
 // Inverse square root.
-// Returns true if x is a non zero square, false otherwise.
+// Returns true if x is a square, false otherwise.
 // After the call:
 //   isr = sqrt(1/x)        if x is a non-zero square.
 //   isr = sqrt(sqrt(-1)/x) if x is not a square.
@@ -1457,6 +1457,7 @@ static int invsqrt(fe isr, const fe x)
     fe_mul(quartic, quartic, x);
 
     i32 *check = t2;
+    fe_0  (check);          int z0 = fe_isequal(x      , check);
     fe_1  (check);          int p1 = fe_isequal(quartic, check);
     fe_neg(check, check );  int m1 = fe_isequal(quartic, check);
     fe_neg(check, sqrtm1);  int ms = fe_isequal(quartic, check);
@@ -1470,7 +1471,7 @@ static int invsqrt(fe isr, const fe x)
     WIPE_BUFFER(t0);
     WIPE_BUFFER(t1);
     WIPE_BUFFER(t2);
-    return p1 | m1;
+    return p1 | m1 | z0;
 }
 
 // Inverse in terms of inverse square root.
@@ -1760,12 +1761,6 @@ static void ge_tobytes(u8 s[32], const ge *h)
 //   isr = invsqrt(num * den)  // abort if not square
 //   x   = num * isr
 // Finally, negate x if its sign is not as specified.
-//
-// Note that using invsqrt causes this function to fail when y = 1.
-// The point (0, 1) *is* on the curve, so in principle we should not
-// reject it.  However, we are only using it to read EdDSA public keys,
-// And the legitimate ones never have low order. Indeed, some libraries
-// reject *all* low order points, on purpose.
 static int ge_frombytes_neg_vartime(ge *h, const u8 s[32])
 {
     fe_frombytes(h->Y, s, 1);

diff --git a/tests/gen/elligator-inverse.py b/tests/gen/elligator-inverse.py
index 8a7245c516c7cb02858c82465004591785dc53a2..d8c0cd05641a02537adfe5f6652851e695a1fb0c 100755 (executable)
--- a/tests/gen/elligator-inverse.py
+++ b/tests/gen/elligator-inverse.py
@@ -94,7 +94,7 @@ for cofactor in range(8):
             u.print()
             print(format(tweak, '02x') + ":")
             print('ff:') # Failure
-            print('00:') # dummy value for the hash
+            print(':') # dummy value for the hash
             print()
             break
 

diff --git a/tests/gen/vectors/elligator_inv b/tests/gen/vectors/elligator_inv
index cc53d6b7673637900b33a171d5e9261904c36ead..1948f9742c98a775998bff658b0bbedc83690b1b 100644 (file)
--- a/tests/gen/vectors/elligator_inv
+++ b/tests/gen/vectors/elligator_inv
@@ -1,7 +1,7 @@
 e6f66fdf6e230c603c5e6e59a254ea1476a13eb9511b9549846781e12e52230a:
 00:
 ff:
-00:
+:
 
 46951964003c940878063ccfd0348af42150ca16d2646f2c5856e8338377d800:
 00:
@@ -15,5 +15,5 @@ ff:
 
 0000000000000000000000000000000000000000000000000000000000000000:
 00:
-ff:
 00:
+0000000000000000000000000000000000000000000000000000000000000000:

diff --git a/tests/test.c b/tests/test.c
index 29beb3e9b851b836cc3133c2ec349f8b1da58a61..4eb1211b658b53f750e9d494e3ffed7c5d38cc60 100644 (file)
--- a/tests/test.c
+++ b/tests/test.c
@@ -328,9 +328,6 @@ static void elligator_inv(vector_reader *reader)
         printf("Elligator inverse map: failure mismatch\n");
         exit(1);
     }
-    if (check) {
-        out.buf[0] = 0;
-    }
 }
 
 //////////////////////////////