Fix side channel in the ecp_nistz256.c reference implementation
This is only used if configured with
./config -DECP_NISTZ256_REFERENCE_IMPLEMENTATION
@@ -358,16 +358,47 @@ static void ecp_nistz256_point_add(P256_POINT *r,
ecp_nistz256_sub(H, U2, U1); /* H = U2 - U1 */
/*
- * This should not happen during sign/ecdh, so no constant time violation
+ * The formulae are incorrect if the points are equal so we check for
+ * this and do doubling if this happens.
+ *
+ * Points here are in Jacobian projective coordinates (Xi, Yi, Zi)
+ * that are bound to the affine coordinates (xi, yi) by the following
+ * equations:
+ * - xi = Xi / (Zi)^2
+ * - y1 = Yi / (Zi)^3
+ *
+ * For the sake of optimization, the algorithm operates over
+ * intermediate variables U1, U2 and S1, S2 that are derived from
+ * the projective coordinates:
+ * - U1 = X1 * (Z2)^2 ; U2 = X2 * (Z1)^2
+ * - S1 = Y1 * (Z2)^3 ; S2 = Y2 * (Z1)^3
+ *
+ * It is easy to prove that is_equal(U1, U2) implies that the affine
+ * x-coordinates are equal, or either point is at infinity.
+ * Likewise is_equal(S1, S2) implies that the affine y-coordinates are
+ * equal, or either point is at infinity.
+ *
+ * The special case of either point being the point at infinity (Z1 or Z2
+ * is zero), is handled separately later on in this function, so we avoid
+ * jumping to point_double here in those special cases.
+ *
+ * When both points are inverse of each other, we know that the affine
+ * x-coordinates are equal, and the y-coordinates have different sign.
+ * Therefore since U1 = U2, we know H = 0, and therefore Z3 = H*Z1*Z2
+ * will equal 0, thus the result is infinity, if we simply let this
+ * function continue normally.
+ *
+ * We use bitwise operations to avoid potential side-channels introduced by
+ * the short-circuiting behaviour of boolean operators.
+ */
+ if (is_equal(U1, U2) & ~in1infty & ~in2infty & is_equal(S1, S2)) {
+ /*
+ * This is obviously not constant-time but it should never happen during
+ * single point multiplication, so there is no timing leak for ECDH or
+ * ECDSA signing.
*/
- if (is_equal(U1, U2) && !in1infty && !in2infty) {
- if (is_equal(S1, S2)) {
ecp_nistz256_point_double(r, a);
return;
- } else {
- memset(r, 0, sizeof(*r));
- return;
- }
}
ecp_nistz256_sqr_mont(Rsqr, R); /* R^2 */
