ecc: return infinity for P + (-P) with equal z in the projective add

tests/api/test_ossl_ec.c

@@ -532,6 +532,7 @@ int test_wolfSSL_EC_POINT(void)
     !defined(WOLF_CRYPTO_CB_ONLY_ECC) && !defined(WOLFSSL_NO_ECC_SW)
     {
         EC_POINT* orig_point = NULL;
+        EC_POINT* neg_g = NULL;
         ExpectNotNull(orig_point = EC_POINT_new(group));
         ExpectIntEQ(EC_POINT_add(group, orig_point, set_point, set_point, NULL),
                     1);
@@ -546,6 +547,22 @@ int test_wolfSSL_EC_POINT(void)
         ExpectIntEQ(EC_POINT_add(group, orig_point, orig_point, new_point,
                                  NULL), 1);
         ExpectIntEQ(EC_POINT_cmp(group, orig_point, set_point, NULL), 0);
+#if !defined(HAVE_FIPS) || FIPS_VERSION_GE(7,0)
+        /* new_point is set_point inverted, so the sum is infinity */
+        ExpectIntEQ(EC_POINT_add(group, orig_point, set_point, new_point,
+                                 NULL), 1);
+        ExpectIntEQ(EC_POINT_is_at_infinity(group, orig_point), 1);
+        ExpectIntEQ(EC_POINT_add(group, orig_point, new_point, set_point,
+                                 NULL), 1);
+        ExpectIntEQ(EC_POINT_is_at_infinity(group, orig_point), 1);
+        /* 1*G + 1*(-G) is infinity */
+        ExpectNotNull(neg_g = EC_POINT_dup(Gxy, group));
+        ExpectIntEQ(EC_POINT_invert(group, neg_g, NULL), 1);
+        ExpectIntEQ(EC_POINT_mul(group, orig_point, BN_value_one(), neg_g,
+                                 BN_value_one(), NULL), 1);
+        ExpectIntEQ(EC_POINT_is_at_infinity(group, orig_point), 1);
+#endif
+        EC_POINT_free(neg_g);
         EC_POINT_free(orig_point);
     }
 #endif

tests/unit-mcdc/test_ecc_fault_whitebox.c

@@ -796,7 +796,7 @@ static void wb_degenerate_points(void)
     }
     (void)_ecc_projective_add_point(P, Q, R, &a, &prime, mp);
 
-    /* Q = -P: x and z equal, y differs but equals modulus - P->y */
+    /* Q = -P: x and z equal, y differs -> the general add gives Z == 0 */
     (void)mp_sub(&prime, P->y, Q->y);
     (void)_ecc_projective_add_point(P, Q, R, &a, &prime, mp);
 
@@ -814,7 +814,7 @@ static void wb_degenerate_points(void)
     (void)mp_set(Q->x, 3);
     (void)_ecc_projective_add_point(P, Q, R, &a, &prime, mp);
 
-    /* y differs and is NOT the negation either: the last operand FALSE */
+    /* y differs and Q is not -P either: the last operand FALSE */
     (void)wc_ecc_copy_point(P, Q);
     (void)mp_set(Q->y, 5);
     (void)_ecc_projective_add_point(P, Q, R, &a, &prime, mp);
@@ -831,9 +831,9 @@ static void wb_degenerate_points(void)
 
     /* --- the _safe wrappers --- */
 
-    /* A = -B: the wrapper's own set-to-infinity chain, with and without an
-     * infinity out-pointer, and with the chain faulted. The result must be
-     * the (0, 0, 1) infinity representation with the flag raised. */
+    /* A = -B with the same Z: the add returns Z == 0, so the result must be
+     * (0, 0, 1) with the flag raised. Run with and without an infinity
+     * out-pointer, and with the first mp_* calls of the add faulted. */
     (void)wc_ecc_copy_point(P, Q);
     (void)mp_sub(&prime, P->y, Q->y);
     inf = 0;
@@ -861,15 +861,9 @@ static void wb_degenerate_points(void)
     (void)ecc_projective_add_point_safe(P, Q, R, &a, &prime, mp, &inf);
     (void)ecc_projective_add_point_safe(P, Q, R, &a, &prime, mp, NULL);
     (void)ecc_projective_add_point_safe(Q, P, R, &a, &prime, mp, &inf);
-    /* The set-to-infinity chain sits behind the whole add, so its
-     * "err == MP_OKAY" operand only goes FALSE when the armed index lands
-     * exactly on one of those mp_set calls -- hence a sweep long enough to
-     * walk past every mp_* the add itself performs. */
-    /* Negatives of each other on DIFFERENT projective representatives:
-     * B = (x*L^2, -y*L^3, z*L). The wrapper's x/z equality test misses it, so
-     * the raw add runs and returns Z == 0 with X and Y non-zero -- the
-     * "only Z zero -> result is infinity" arm, which the all-zero shape above
-     * never reaches. */
+    /* B = (x*L^2, -y*L^3, z*L) is -P on a different representative: the add
+     * returns Z == 0 with X and Y non-zero. The sweep runs past every mp_*
+     * of the add so a fault lands on the mp_set calls that write infinity. */
     (void)wc_ecc_copy_point(P, Q);
     (void)mp_set(&zero, 4);
     (void)mp_mulmod(P->x, &zero, &prime, Q->x);

wolfcrypt/src/ecc.c

@@ -2071,7 +2071,7 @@ static int _ecc_projective_dbl_point(ecc_point *P, ecc_point *R, mp_int* a,
    Add two ECC points
    P        The point to add
    Q        The point to add
-   R        [out] The destination of the double
+   R        [out] The sum; the point at infinity has a z of zero
    a        ECC curve parameter a
    modulus  The modulus of the field the ECC curve is in
    mp       The "b" value from montgomery_setup()
@@ -2166,17 +2166,10 @@ static int _ecc_projective_add_point(ecc_point* P, ecc_point* Q, ecc_point* R,
    }
 
    /* should we dbl instead? */
-   if (err == MP_OKAY) {
-#ifdef ECC_TIMING_RESISTANT
-      err = mp_submod_ct(modulus, Q->y, modulus, t1);
-#else
-      err = mp_sub(modulus, Q->y, t1);
-#endif
-   }
    if (err == MP_OKAY) {
       if ( (mp_cmp(P->x, Q->x) == MP_EQ) &&
            (mp_get_digit_count(Q->z) && mp_cmp(P->z, Q->z) == MP_EQ) &&
-           (mp_cmp(P->y, Q->y) == MP_EQ || mp_cmp(P->y, t1) == MP_EQ)) {
+           (mp_cmp(P->y, Q->y) == MP_EQ)) {
           mp_clear(t1);
           mp_clear(t2);
    #ifdef WOLFSSL_SMALL_STACK
@@ -8737,22 +8730,20 @@ static int ecc_cond_copy_point(ecc_point* S, int copy, ecc_point* D)
  *
  *   A is infinity     -> B copied over the result.
  *   B is infinity     -> A copied over the result.
- *   A = -B, same z    -> The add doubles instead; result set to infinity.
  *   A = B             -> Doubled by the add itself when the z values match;
  *                        all-zero result when they differ - doubled here.
  *   A + B = infinity  -> Add result has z == 0, x/y not 0; set to infinity.
  *
- * The infinity out-flag is set only for the last two cases - a finite pair
- * whose sum is infinity - matching the original branching version, which
- * never touched the flag when copying an operand out.
+ * The infinity out-flag is set only for the last case - a finite pair whose
+ * sum is infinity - matching the original branching version, which never
+ * touched the flag when copying an operand out.
  */
 int ecc_projective_add_point_safe(ecc_point* A, ecc_point* B, ecc_point* R,
     mp_int* a, mp_int* modulus, mp_digit mp, int* infinity)
 {
     int err;
     int aInf;
     int bInf;
-    int nInf;
     int rInf = 0;
 #ifdef WOLFSSL_SMALL_STACK
     ecc_point* T = NULL;
@@ -8771,15 +8762,6 @@ int ecc_projective_add_point_safe(ecc_point* A, ecc_point* B, ecc_point* R,
     bInf  = (mp_iszero(B->x) == MP_YES);
     bInf &= (mp_iszero(B->y) == MP_YES);
 
-    /* A = -B with matching z gives infinity, which the add turns into a
-     * double instead: note it now and write infinity over the result after
-     * the add. A = B needs no note - the add doubles it correctly. */
-    nInf  = (mp_cmp(A->x, B->x) == MP_EQ);
-    nInf &= (mp_cmp(A->z, B->z) == MP_EQ);
-    nInf &= (mp_cmp(A->y, B->y) != MP_EQ);
-    nInf &= (aInf == 0);
-    nInf &= (bInf == 0);
-
     /* Off the stack unless the build asked for small stacks. Inherit the
      * key's heap and small-stack cache from R so the temporary behaves like
      * the caller's own points - this is the scalar multiplication inner
@@ -8820,12 +8802,6 @@ int ecc_projective_add_point_safe(ecc_point* A, ecc_point* B, ecc_point* R,
         }
     }
 
-    /* A = -B with matching z: replace the double the add produced. */
-    if ((err == MP_OKAY) && nInf) {
-        err = ecc_set_point_infinity(T);
-        rInf = 1;
-    }
-
     /* Select without branching on which operand was infinity: A infinity
      * gives B, B infinity gives A. Both infinity leaves A, itself infinity,
      * so needs no separate case. */