working on decaffeinating ed25519

aux/decaffeinate_ed25519_too.sage

@@ -0,0 +1,87 @@
+F = GF(2^255-19)
+dM = F(-121665)
+d = F(-121665/121666)
+ii = sqrt(F(-1))
+def lobit(x): return int(x) & 1
+def hibit(x): return lobit(2*x)
+
+magic = sqrt(F(-121666))
+if lobit(magic): magic = -magic
+
+def eddsa_to_decaf(x,y):
+    """
+    Converts an EdDSA point to a Decaf representation, in a manner compatible
+    with libdecaf.
+    
+    The input point must be even.
+    
+    Note well!  Decaf does not represent the cofactor information of a point.
+    So e2d(d2e(s)) = s, but d2e(e2d(x,y)) might not be (x,y).
+    """
+    if x*y == 0: return 0 # This will happen anyway with straightforward square root trick
+    if not is_square((1-y)/(1+y)): raise Exception("Unimplemented: odd point in eddsa_to_decaf")
+    if hibit(magic/(x*y)): (x,y) = (ii*y,ii*x)
+    if hibit(2*magic/x): y = -y
+    s = sqrt((1-y)/(1+y))
+    if hibit(s): s = -s
+    return s
+
+def isqrt_trick(to_isr,to_inv):
+    to_sqrt = to_isr*to_inv^2
+    if to_sqrt == 0: return 0,0 # This happens automatically in C; just to avoid problems in SAGE
+    if not is_square(to_sqrt): raise Exception("Not square in isqrt_trick!")
+    tmp = 1/sqrt(to_sqrt)
+    isr = tmp * to_inv
+    inv = tmp * isr * to_isr
+    assert isr^2 == 1/to_isr
+    assert inv == 1/to_inv
+    return isr, inv
+    
+
+def eddsa_to_decaf_opt(x,y,z=None):
+    """
+    Optimized version of eddsa_to_decaf.
+    
+    Uses only one isqrt.
+    """
+    if z is None:
+        # Pretend that we're in projective
+        z = F.random_element()
+        x *= z
+        y *= z
+    
+    isr,inv = isqrt_trick(z^2-y^2,x*y)
+    inv *= magic
+    
+    rotate = hibit(inv*z^2)
+    if rotate:
+        isr *= (z^2-y^2)*inv
+        y = ii*x
+    
+    if hibit(2*inv*y*z) != rotate: y = -y
+    s = (z-y) * isr
+    
+    if hibit(s): s = -s
+    return s
+
+print [eddsa_to_decaf_opt(x,y) == eddsa_to_decaf(x,y) for _,_,_,_,y1,y2 in points for x,y in [decode(y1,y2)]]
+
+def decaf_to_eddsa(s):
+    """
+    Convert a Decaf representation to an EdDSA point, in a manner compatible
+    with libdecaf.
+    
+    Note well! The Decaf representation of a point is canonical, but the EdDSA one
+    is not, in that  
+    """
+    if s == 0: return (0,1)
+    if hibit(s): raise Exception("invalid: s has high bit")
+    if not is_square(s^4 + (2-4*dM)*s^2 + 1): raise Exception("invalid: not on curve")
+    
+    t = sqrt(s^4 + (2-4*dM)*s^2 + 1)/s
+    if hibit(t): t = -t
+    y = (1-s^2)/(1+s^2)
+    x = 2*magic/t
+    if y == 0 or lobit(t/y): raise Exception("invalid: t/y has high bit")
+    assert y^2 - x^2 == 1+d*x^2*y^2
+    return (x,y)

src/GENERATED/c/curve25519/decaf.c

@@ -57,9 +57,13 @@ const uint8_t decaf_x25519_base_point[DECAF_X25519_PUBLIC_BYTES] = { 0x09 };
 /* End of template stuff */
 
 /* Sanity */
-#if (COFACTOR == 8) && !IMAGINE_TWIST
+#if (COFACTOR == 8) && !IMAGINE_TWIST && !UNSAFE_CURVE_HAS_POINTS_AT_INFINITY
 /* FUTURE MAGIC: Curve41417 doesn't have these properties. */
-    #error "Currently require IMAGINE_TWIST (and thus p=5 mod 8) for cofactor 8"
+#error "Currently require IMAGINE_TWIST (and thus p=5 mod 8) for cofactor 8"
+        /* OK, but why?
+         * Two reasons: #1: There are bugs when COFACTOR == && IMAGINE_TWIST
+         # #2: 
+         */
 #endif
 
 #if IMAGINE_TWIST && (P_MOD_8 != 5)
@@ -115,15 +119,6 @@ static mask_t gf_lobit(const gf x) {
 /** identity = (0,1) */
 const point_t API_NS(point_identity) = {{{{{0}}},{{{1}}},{{{1}}},{{{0}}}}};
 
-void API_NS(deisogenize) (
-    gf_s *__restrict__ s,
-    gf_s *__restrict__ minus_t_over_s,
-    const point_t p,
-    mask_t toggle_hibit_s,
-    mask_t toggle_hibit_t_over_s,
-    mask_t toggle_rotation
-);
-    
 void API_NS(deisogenize) (
     gf_s *__restrict__ s,
     gf_s *__restrict__ minus_t_over_s,
@@ -163,7 +158,7 @@ void API_NS(deisogenize) (
 #else
     /* More complicated because of rotation */
     /* MAGIC This code is wrong for certain non-Curve25519 curves;
-     * check if it's because of Cofactor==8 or IMAGINE_ROTATION */
+     * check if it's because of Cofactor==8 or IMAGINE_TWIST */
     
     gf c, d;
     gf_s *b = s, *a = minus_t_over_s;
@@ -177,11 +172,11 @@ void API_NS(deisogenize) (
         gf_mul ( c, a, b ); /* "zx" = Z^2 - aX^2 = Z^2 - X^2 */
     #else
         const gf_s *x = p->x, *t = p->t;
-        /* Won't hit the gf_cond_sel below because COFACTOR==8 requires IMAGINE_TWIST for now. */
         gf_sqr ( a, p->z );
         gf_sqr ( b, p->x );
         gf_add ( c, a, b ); /* "zx" = Z^2 - aX^2 = Z^2 + X^2 */
     #endif
+    /* Here: c = "zx" in the SAGE code = Z^2 - aX^2 */
     
     gf_mul ( a, p->z, t ); /* "tz" = T*Z */
     gf_sqr ( b, a );
@@ -201,8 +196,9 @@ void API_NS(deisogenize) (
         /* Curve25519: cond select between zx * 1/tz or sqrt(1-d); y=-x */
         gf_mul ( a, b, c ); 
         gf_cond_sel ( a, a, SQRT_ONE_MINUS_D, rotate );
-        gf_cond_sel ( x, p->y, x, rotate );
+        gf_cond_sel ( e, p->y, x, rotate );
     #else
+        const gf_s *e = x;
         (void)toggle_rotation;
         rotate = 0;
     #endif
@@ -216,7 +212,7 @@ void API_NS(deisogenize) (
     gf_cond_neg ( minus_t_over_s, tg );
     gf_cond_neg ( c, rotate ^ tg );
     gf_add ( d, d, c );
-    gf_mul ( s, d, x ); /* here "x" = y unless rotate */
+    gf_mul ( s, d, e ); /* here "x" = y unless rotate */
     gf_cond_neg ( s, toggle_hibit_s ^ gf_hibit(s) );
 #endif
 }
@@ -236,24 +232,24 @@ decaf_error_t API_NS(point_decode) (
     mask_t succ = gf_deserialize(s, ser, 0);
     mask_t zero = gf_eq(s, ZERO);
     succ &= bool_to_mask(allow_identity) | ~zero;
-    gf_sqr ( a, s );
+    gf_sqr ( a, s ); /* s^2 */
 #if IMAGINE_TWIST
     gf_sub ( f, ONE, a ); /* f = 1-as^2 = 1-s^2*/
 #else
     gf_add ( f, ONE, a ); /* f = 1-as^2 = 1+s^2 */
 #endif
     succ &= ~ gf_eq( f, ZERO );
-    gf_sqr ( b, f ); 
+    gf_sqr ( b, f );  /* (1-as^2)^2 = 1 - 2as^2 + a^2 s^4 */
     gf_mulw ( c, a, 4*IMAGINE_TWIST-4*EDWARDS_D ); 
-    gf_add ( c, c, b ); /* t^2 */
-    gf_mul ( d, f, s ); /* s(1-as^2) for denoms */
-    gf_sqr ( e, d );
-    gf_mul ( b, c, e );
+    gf_add ( c, c, b ); /* t^2 = 1 + (2a-4d) s^2 + s^4 */
+    gf_mul ( d, f, s ); /* s * (1-as^2) for denoms */
+    gf_sqr ( e, d );    /* s^2 * (1-as^2)^2 */
+    gf_mul ( b, c, e ); /* t^2 * s^2 * (1-as^2)^2 */
     
     succ &= gf_isr(e,b) | gf_eq(b,ZERO); /* e = 1/(t s (1-as^2)) */
-    gf_mul ( b, e, d ); /* 1/t */
-    gf_mul ( d, e, c ); /* d = t / (s(1-as^2)) */
-    gf_mul ( e, d, f ); /* t/s */
+    gf_mul ( b, e, d ); /* 1 / t */
+    gf_mul ( d, e, c ); /* t / (s(1-as^2)) */
+    gf_mul ( e, d, f ); /* t / s */
     mask_t negtos = gf_hibit(e);
     gf_cond_neg(b, negtos);
     gf_cond_neg(d, negtos);
@@ -279,8 +275,23 @@ decaf_error_t API_NS(point_decode) (
 #endif
     gf_mul ( p->t, p->x, a ); /* T = 2s (1-as^2)/t */
     
-    p->y->limb[0] -= zero;
+#if UNSAFE_CURVE_HAS_POINTS_AT_INFINITY
+    /* This can't happen for any of the supported configurations.
+     *
+     * If it can happen (because s=1), it's because the curve has points
+     * at infinity, which means that there may be critical security bugs
+     * elsewhere in the library.  In that case, it's better that you hit
+     * the assertion in point_valid, which will happen in the test suite
+     * since it tests s=1.
+     *
+     * This debugging option is to allow testing of IMAGINE_TWIST = 0 on
+     * Ed25519, without hitting that assertion.  Don't use it in
+     * production.
+     */
+    succ &= ~gf_eq(p->z,ZERO);
+#endif
     
+    p->y->limb[0] -= zero;
     assert(API_NS(point_valid)(p) | ~succ);
     
     return decaf_succeed_if(mask_to_bool(succ));
@@ -822,7 +833,7 @@ void API_NS(point_debugging_torque) (
     point_t q,
     const point_t p
 ) {
-#if COFACTOR == 8
+#if COFACTOR == 8 && IMAGINE_TWIST
     gf tmp;
     gf_mul(tmp,p->x,SQRT_MINUS_ONE);
     gf_mul(q->x,p->y,SQRT_MINUS_ONE);

src/GENERATED/c/curve25519/elligator.c

@@ -61,7 +61,7 @@ void API_NS(point_from_hash_nonuniform) (
     
     /* s@a = +-|N.e| */
     gf_mul(a,N,e);
-    gf_cond_neg(a,gf_hibit(a)^square); /* NB this is - what is listen in the paper */
+    gf_cond_neg(a,gf_hibit(a)^square); /* NB this is - what is listed in the paper */
     
     /* t@b = -+ cN(r-1)((a-2d)e)^2 - 1 */
     gf_mulw(c,e,1-2*EDWARDS_D); /* (a-2d)e */

src/GENERATED/c/ed448goldilocks/decaf.c

@@ -57,9 +57,13 @@ const uint8_t decaf_x448_base_point[DECAF_X448_PUBLIC_BYTES] = { 0x05 };
 /* End of template stuff */
 
 /* Sanity */
-#if (COFACTOR == 8) && !IMAGINE_TWIST
+#if (COFACTOR == 8) && !IMAGINE_TWIST && !UNSAFE_CURVE_HAS_POINTS_AT_INFINITY
 /* FUTURE MAGIC: Curve41417 doesn't have these properties. */
-    #error "Currently require IMAGINE_TWIST (and thus p=5 mod 8) for cofactor 8"
+#error "Currently require IMAGINE_TWIST (and thus p=5 mod 8) for cofactor 8"
+        /* OK, but why?
+         * Two reasons: #1: There are bugs when COFACTOR == && IMAGINE_TWIST
+         # #2: 
+         */
 #endif
 
 #if IMAGINE_TWIST && (P_MOD_8 != 5)
@@ -115,15 +119,6 @@ static mask_t gf_lobit(const gf x) {
 /** identity = (0,1) */
 const point_t API_NS(point_identity) = {{{{{0}}},{{{1}}},{{{1}}},{{{0}}}}};
 
-void API_NS(deisogenize) (
-    gf_s *__restrict__ s,
-    gf_s *__restrict__ minus_t_over_s,
-    const point_t p,
-    mask_t toggle_hibit_s,
-    mask_t toggle_hibit_t_over_s,
-    mask_t toggle_rotation
-);
-    
 void API_NS(deisogenize) (
     gf_s *__restrict__ s,
     gf_s *__restrict__ minus_t_over_s,
@@ -163,7 +158,7 @@ void API_NS(deisogenize) (
 #else
     /* More complicated because of rotation */
     /* MAGIC This code is wrong for certain non-Curve25519 curves;
-     * check if it's because of Cofactor==8 or IMAGINE_ROTATION */
+     * check if it's because of Cofactor==8 or IMAGINE_TWIST */
     
     gf c, d;
     gf_s *b = s, *a = minus_t_over_s;
@@ -177,11 +172,11 @@ void API_NS(deisogenize) (
         gf_mul ( c, a, b ); /* "zx" = Z^2 - aX^2 = Z^2 - X^2 */
     #else
         const gf_s *x = p->x, *t = p->t;
-        /* Won't hit the gf_cond_sel below because COFACTOR==8 requires IMAGINE_TWIST for now. */
         gf_sqr ( a, p->z );
         gf_sqr ( b, p->x );
         gf_add ( c, a, b ); /* "zx" = Z^2 - aX^2 = Z^2 + X^2 */
     #endif
+    /* Here: c = "zx" in the SAGE code = Z^2 - aX^2 */
     
     gf_mul ( a, p->z, t ); /* "tz" = T*Z */
     gf_sqr ( b, a );
@@ -201,8 +196,9 @@ void API_NS(deisogenize) (
         /* Curve25519: cond select between zx * 1/tz or sqrt(1-d); y=-x */
         gf_mul ( a, b, c ); 
         gf_cond_sel ( a, a, SQRT_ONE_MINUS_D, rotate );
-        gf_cond_sel ( x, p->y, x, rotate );
+        gf_cond_sel ( e, p->y, x, rotate );
     #else
+        const gf_s *e = x;
         (void)toggle_rotation;
         rotate = 0;
     #endif
@@ -216,7 +212,7 @@ void API_NS(deisogenize) (
     gf_cond_neg ( minus_t_over_s, tg );
     gf_cond_neg ( c, rotate ^ tg );
     gf_add ( d, d, c );
-    gf_mul ( s, d, x ); /* here "x" = y unless rotate */
+    gf_mul ( s, d, e ); /* here "x" = y unless rotate */
     gf_cond_neg ( s, toggle_hibit_s ^ gf_hibit(s) );
 #endif
 }
@@ -236,24 +232,24 @@ decaf_error_t API_NS(point_decode) (
     mask_t succ = gf_deserialize(s, ser, 0);
     mask_t zero = gf_eq(s, ZERO);
     succ &= bool_to_mask(allow_identity) | ~zero;
-    gf_sqr ( a, s );
+    gf_sqr ( a, s ); /* s^2 */
 #if IMAGINE_TWIST
     gf_sub ( f, ONE, a ); /* f = 1-as^2 = 1-s^2*/
 #else
     gf_add ( f, ONE, a ); /* f = 1-as^2 = 1+s^2 */
 #endif
     succ &= ~ gf_eq( f, ZERO );
-    gf_sqr ( b, f ); 
+    gf_sqr ( b, f );  /* (1-as^2)^2 = 1 - 2as^2 + a^2 s^4 */
     gf_mulw ( c, a, 4*IMAGINE_TWIST-4*EDWARDS_D ); 
-    gf_add ( c, c, b ); /* t^2 */
-    gf_mul ( d, f, s ); /* s(1-as^2) for denoms */
-    gf_sqr ( e, d );
-    gf_mul ( b, c, e );
+    gf_add ( c, c, b ); /* t^2 = 1 + (2a-4d) s^2 + s^4 */
+    gf_mul ( d, f, s ); /* s * (1-as^2) for denoms */
+    gf_sqr ( e, d );    /* s^2 * (1-as^2)^2 */
+    gf_mul ( b, c, e ); /* t^2 * s^2 * (1-as^2)^2 */
     
     succ &= gf_isr(e,b) | gf_eq(b,ZERO); /* e = 1/(t s (1-as^2)) */
-    gf_mul ( b, e, d ); /* 1/t */
-    gf_mul ( d, e, c ); /* d = t / (s(1-as^2)) */
-    gf_mul ( e, d, f ); /* t/s */
+    gf_mul ( b, e, d ); /* 1 / t */
+    gf_mul ( d, e, c ); /* t / (s(1-as^2)) */
+    gf_mul ( e, d, f ); /* t / s */
     mask_t negtos = gf_hibit(e);
     gf_cond_neg(b, negtos);
     gf_cond_neg(d, negtos);
@@ -279,8 +275,23 @@ decaf_error_t API_NS(point_decode) (
 #endif
     gf_mul ( p->t, p->x, a ); /* T = 2s (1-as^2)/t */
     
-    p->y->limb[0] -= zero;
+#if UNSAFE_CURVE_HAS_POINTS_AT_INFINITY
+    /* This can't happen for any of the supported configurations.
+     *
+     * If it can happen (because s=1), it's because the curve has points
+     * at infinity, which means that there may be critical security bugs
+     * elsewhere in the library.  In that case, it's better that you hit
+     * the assertion in point_valid, which will happen in the test suite
+     * since it tests s=1.
+     *
+     * This debugging option is to allow testing of IMAGINE_TWIST = 0 on
+     * Ed25519, without hitting that assertion.  Don't use it in
+     * production.
+     */
+    succ &= ~gf_eq(p->z,ZERO);
+#endif
     
+    p->y->limb[0] -= zero;
     assert(API_NS(point_valid)(p) | ~succ);
     
     return decaf_succeed_if(mask_to_bool(succ));
@@ -822,7 +833,7 @@ void API_NS(point_debugging_torque) (
     point_t q,
     const point_t p
 ) {
-#if COFACTOR == 8
+#if COFACTOR == 8 && IMAGINE_TWIST
     gf tmp;
     gf_mul(tmp,p->x,SQRT_MINUS_ONE);
     gf_mul(q->x,p->y,SQRT_MINUS_ONE);

src/GENERATED/c/ed448goldilocks/elligator.c

@@ -61,7 +61,7 @@ void API_NS(point_from_hash_nonuniform) (
     
     /* s@a = +-|N.e| */
     gf_mul(a,N,e);
-    gf_cond_neg(a,gf_hibit(a)^square); /* NB this is - what is listen in the paper */
+    gf_cond_neg(a,gf_hibit(a)^square); /* NB this is - what is listed in the paper */
     
     /* t@b = -+ cN(r-1)((a-2d)e)^2 - 1 */
     gf_mulw(c,e,1-2*EDWARDS_D); /* (a-2d)e */

src/generator/curve_data.py

@@ -122,8 +122,14 @@ for curve,data in curve_data.items():
         data["eddsa_sigma_iso"] = 0
 
     if "imagine_twist" not in data:
+        # This is a HACK.  The real problem is that iso-Ed25519
+        # has points at infinity unless you IMAGINE_TWIST.
+        #
+        # Also there are lots of bugs when cofactor=8 && !IMAGINE_TWIST.
+        # (FIXME, eventually)
         if data["modulus"]%4 == 3: data["imagine_twist"] = 0
         else: data["imagine_twist"] = 1
+        # data["imagine_twist"] = 0
 
     data["q"] = (data["modulus"]+1-data["trace"]) // data["cofactor"]
     data["bits"] = ceil_log2(data["modulus"])

src/per_curve/decaf.tmpl.c

@@ -46,9 +46,13 @@ const uint8_t decaf_x$(gf_shortname)_base_point[DECAF_X$(gf_shortname)_PUBLIC_BY
 /* End of template stuff */
 
 /* Sanity */
-#if (COFACTOR == 8) && !IMAGINE_TWIST
+#if (COFACTOR == 8) && !IMAGINE_TWIST && !UNSAFE_CURVE_HAS_POINTS_AT_INFINITY
 /* FUTURE MAGIC: Curve41417 doesn't have these properties. */
-    #error "Currently require IMAGINE_TWIST (and thus p=5 mod 8) for cofactor 8"
+#error "Currently require IMAGINE_TWIST (and thus p=5 mod 8) for cofactor 8"
+        /* OK, but why?
+         * Two reasons: #1: There are bugs when COFACTOR == && IMAGINE_TWIST
+         # #2: 
+         */
 #endif
 
 #if IMAGINE_TWIST && (P_MOD_8 != 5)
@@ -104,15 +108,6 @@ static mask_t gf_lobit(const gf x) {
 /** identity = (0,1) */
 const point_t API_NS(point_identity) = {{{{{0}}},{{{1}}},{{{1}}},{{{0}}}}};
 
-void API_NS(deisogenize) (
-    gf_s *__restrict__ s,
-    gf_s *__restrict__ minus_t_over_s,
-    const point_t p,
-    mask_t toggle_hibit_s,
-    mask_t toggle_hibit_t_over_s,
-    mask_t toggle_rotation
-);
-    
 void API_NS(deisogenize) (
     gf_s *__restrict__ s,
     gf_s *__restrict__ minus_t_over_s,
@@ -152,7 +147,7 @@ void API_NS(deisogenize) (
 #else
     /* More complicated because of rotation */
     /* MAGIC This code is wrong for certain non-Curve25519 curves;
-     * check if it's because of Cofactor==8 or IMAGINE_ROTATION */
+     * check if it's because of Cofactor==8 or IMAGINE_TWIST */
     
     gf c, d;
     gf_s *b = s, *a = minus_t_over_s;
@@ -166,11 +161,11 @@ void API_NS(deisogenize) (
         gf_mul ( c, a, b ); /* "zx" = Z^2 - aX^2 = Z^2 - X^2 */
     #else
         const gf_s *x = p->x, *t = p->t;
-        /* Won't hit the gf_cond_sel below because COFACTOR==8 requires IMAGINE_TWIST for now. */
         gf_sqr ( a, p->z );
         gf_sqr ( b, p->x );
         gf_add ( c, a, b ); /* "zx" = Z^2 - aX^2 = Z^2 + X^2 */
     #endif
+    /* Here: c = "zx" in the SAGE code = Z^2 - aX^2 */
     
     gf_mul ( a, p->z, t ); /* "tz" = T*Z */
     gf_sqr ( b, a );
@@ -190,8 +185,9 @@ void API_NS(deisogenize) (
         /* Curve25519: cond select between zx * 1/tz or sqrt(1-d); y=-x */
         gf_mul ( a, b, c ); 
         gf_cond_sel ( a, a, SQRT_ONE_MINUS_D, rotate );
-        gf_cond_sel ( x, p->y, x, rotate );
+        gf_cond_sel ( e, p->y, x, rotate );
     #else
+        const gf_s *e = x;
         (void)toggle_rotation;
         rotate = 0;
     #endif
@@ -205,7 +201,7 @@ void API_NS(deisogenize) (
     gf_cond_neg ( minus_t_over_s, tg );
     gf_cond_neg ( c, rotate ^ tg );
     gf_add ( d, d, c );
-    gf_mul ( s, d, x ); /* here "x" = y unless rotate */
+    gf_mul ( s, d, e ); /* here "x" = y unless rotate */
     gf_cond_neg ( s, toggle_hibit_s ^ gf_hibit(s) );
 #endif
 }
@@ -225,24 +221,24 @@ decaf_error_t API_NS(point_decode) (
     mask_t succ = gf_deserialize(s, ser, 0);
     mask_t zero = gf_eq(s, ZERO);
     succ &= bool_to_mask(allow_identity) | ~zero;
-    gf_sqr ( a, s );
+    gf_sqr ( a, s ); /* s^2 */
 #if IMAGINE_TWIST
     gf_sub ( f, ONE, a ); /* f = 1-as^2 = 1-s^2*/
 #else
     gf_add ( f, ONE, a ); /* f = 1-as^2 = 1+s^2 */
 #endif
     succ &= ~ gf_eq( f, ZERO );
-    gf_sqr ( b, f ); 
+    gf_sqr ( b, f );  /* (1-as^2)^2 = 1 - 2as^2 + a^2 s^4 */
     gf_mulw ( c, a, 4*IMAGINE_TWIST-4*EDWARDS_D ); 
-    gf_add ( c, c, b ); /* t^2 */
-    gf_mul ( d, f, s ); /* s(1-as^2) for denoms */
-    gf_sqr ( e, d );
-    gf_mul ( b, c, e );
+    gf_add ( c, c, b ); /* t^2 = 1 + (2a-4d) s^2 + s^4 */
+    gf_mul ( d, f, s ); /* s * (1-as^2) for denoms */
+    gf_sqr ( e, d );    /* s^2 * (1-as^2)^2 */
+    gf_mul ( b, c, e ); /* t^2 * s^2 * (1-as^2)^2 */
     
     succ &= gf_isr(e,b) | gf_eq(b,ZERO); /* e = 1/(t s (1-as^2)) */
-    gf_mul ( b, e, d ); /* 1/t */
-    gf_mul ( d, e, c ); /* d = t / (s(1-as^2)) */
-    gf_mul ( e, d, f ); /* t/s */
+    gf_mul ( b, e, d ); /* 1 / t */
+    gf_mul ( d, e, c ); /* t / (s(1-as^2)) */
+    gf_mul ( e, d, f ); /* t / s */
     mask_t negtos = gf_hibit(e);
     gf_cond_neg(b, negtos);
     gf_cond_neg(d, negtos);
@@ -268,8 +264,23 @@ decaf_error_t API_NS(point_decode) (
 #endif
     gf_mul ( p->t, p->x, a ); /* T = 2s (1-as^2)/t */
     
-    p->y->limb[0] -= zero;
+#if UNSAFE_CURVE_HAS_POINTS_AT_INFINITY
+    /* This can't happen for any of the supported configurations.
+     *
+     * If it can happen (because s=1), it's because the curve has points
+     * at infinity, which means that there may be critical security bugs
+     * elsewhere in the library.  In that case, it's better that you hit
+     * the assertion in point_valid, which will happen in the test suite
+     * since it tests s=1.
+     *
+     * This debugging option is to allow testing of IMAGINE_TWIST = 0 on
+     * Ed25519, without hitting that assertion.  Don't use it in
+     * production.
+     */
+    succ &= ~gf_eq(p->z,ZERO);
+#endif
     
+    p->y->limb[0] -= zero;
     assert(API_NS(point_valid)(p) | ~succ);
     
     return decaf_succeed_if(mask_to_bool(succ));
@@ -811,7 +822,7 @@ void API_NS(point_debugging_torque) (
     point_t q,
     const point_t p
 ) {
-#if COFACTOR == 8
+#if COFACTOR == 8 && IMAGINE_TWIST
     gf tmp;
     gf_mul(tmp,p->x,SQRT_MINUS_ONE);
     gf_mul(q->x,p->y,SQRT_MINUS_ONE);
@@ -1389,7 +1400,13 @@ void decaf_x$(gf_shortname)_derive_public_key (
     point_t p;
     API_NS(precomputed_scalarmul)(p,API_NS(precomputed_base),the_scalar);
     
-    /* Isogenize to Montgomery curve */
+    /* Isogenize to Montgomery curve.
+     *
+     * Why isn't this just a separate function, eg decaf_encode_like_x$(gf_shortname)?
+     * Basically because in general it does the wrong thing if there is a cofactor
+     * component in the input.  In this function though, there isn't a cofactor
+     * component in the input.
+     */
     gf_invert(p->t,p->x); /* 1/x */
     gf_mul(p->z,p->t,p->y); /* y/x */
     gf_sqr(p->y,p->z); /* (y/x)^2 */

src/per_curve/elligator.tmpl.c

@@ -50,7 +50,7 @@ void API_NS(point_from_hash_nonuniform) (
     
     /* s@a = +-|N.e| */
     gf_mul(a,N,e);
-    gf_cond_neg(a,gf_hibit(a)^square); /* NB this is - what is listen in the paper */
+    gf_cond_neg(a,gf_hibit(a)^square); /* NB this is - what is listed in the paper */
     
     /* t@b = -+ cN(r-1)((a-2d)e)^2 - 1 */
     gf_mulw(c,e,1-2*EDWARDS_D); /* (a-2d)e */

test/test_decaf.cxx

@@ -353,7 +353,12 @@ static void test_ec() {
         rng.read(buffer);
         Point r = Point::from_hash(buffer);
         
-        point_check(test,p,q,r,0,0,p,Point(p.serialize()),"round-trip");
+        try {
+            point_check(test,p,q,r,0,0,p,Point(p.serialize()),"round-trip");
+        } catch (CryptoException) {
+            test.fail();
+            printf("    Round-trip raised CryptoException!\n");
+        }
         Point pp = p.debugging_torque().debugging_pscale(rng);
         if (!memeq(pp.serialize(),p.serialize())) {
             test.fail();
@@ -390,8 +395,13 @@ static void test_ec() {
             + Point::from_hash(Buffer(buffer).slice(Point::HASH_BYTES,Point::HASH_BYTES)),
             "unih = hash+add"
         );
-            
-        point_check(test,p,q,r,x,0,Point(x.direct_scalarmul(p.serialize())),x*p,"direct mul");
+        
+        try {
+            point_check(test,p,q,r,x,0,Point(x.direct_scalarmul(p.serialize())),x*p,"direct mul");
+        } catch (CryptoException) {
+            printf("    Direct mul raised CryptoException!\n");
+            test.fail();
+        }
         
         q=p;
         for (int j=1; j<Group::REMOVED_COFACTOR; j<<=1) q = q.times_two();
@@ -531,7 +541,6 @@ static void test_eddsa() {
             printf("    Signature validation failed on sig %d\n", i);
         }    
     }
-    
 }
 
 /* Thanks Johan Pascal */