ECDSA: remove nonce padding (delegated to EC_POINT_mul)

* EC_POINT_mul is now responsible for constant time point multiplication
  (for single fixed or variable point multiplication, when the scalar is
  in the range [0,group_order), so we need to strip the nonce padding
  from ECDSA.
* Entry added to CHANGES
* Updated EC_POINT_mul documentation
  - Integrate existing EC_POINT_mul and EC_POINTs_mul entries in the
    manpage to reflect the shift in constant-time expectations when
    performing a single fixed or variable point multiplication;
  - Add documentation to ec_method_st to reflect the updated "contract"
    between callers and implementations of ec_method_st.mul.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6070)

CHANGES

@@ -9,6 +9,10 @@
 
  Changes between 1.1.0h and 1.1.1 [xx XXX xxxx]
 
+  *) Remove ECDSA nonce padding: EC_POINT_mul is now responsible for
+     constant time fixed point multiplication.
+     [Billy Bob Brumley]
+
   *) Updated CONTRIBUTING
      [Rich Salz]
 

crypto/ec/ec_lcl.h

@@ -120,6 +120,23 @@ struct ec_method_st {
      * EC_POINT_have_precompute_mult (default implementations are used if the
      * 'mul' pointer is 0):
      */
+    /*-
+     * mul() calculates the value
+     *
+     *   r := generator * scalar
+     *        + points[0] * scalars[0]
+     *        + ...
+     *        + points[num-1] * scalars[num-1].
+     *
+     * For a fixed point multiplication (scalar != NULL, num == 0)
+     * or a variable point multiplication (scalar == NULL, num == 1),
+     * mul() must use a constant time algorithm: in both cases callers
+     * should provide an input scalar (either scalar or scalars[0])
+     * in the range [0, ec_group_order); for robustness, implementers
+     * should handle the case when the scalar has not been reduced, but
+     * may treat it as an unusual input, without any constant-timeness
+     * guarantee.
+     */
     int (*mul) (const EC_GROUP *group, EC_POINT *r, const BIGNUM *scalar,
                 size_t num, const EC_POINT *points[], const BIGNUM *scalars[],
                 BN_CTX *);

crypto/ec/ec_mult.c

@@ -113,9 +113,9 @@ void EC_ec_pre_comp_free(EC_PRE_COMP *pre)
  *
  * At a high level, it is Montgomery ladder with conditional swaps.
  *
- * It performs either a fixed scalar point multiplication
+ * It performs either a fixed point multiplication
  *          (scalar * generator)
- * when point is NULL, or a generic scalar point multiplication
+ * when point is NULL, or a variable point multiplication
  *          (scalar * point)
  * when point is not NULL.
  *

crypto/ec/ecdsa_ossl.c

@@ -105,23 +105,6 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in,
             }
         while (BN_is_zero(k));
 
-        /*
-         * We do not want timing information to leak the length of k, so we
-         * compute G*k using an equivalent scalar of fixed bit-length.
-         *
-         * We unconditionally perform both of these additions to prevent a
-         * small timing information leakage.  We then choose the sum that is
-         * one bit longer than the order.  This guarantees the code
-         * path used in the constant time implementations elsewhere.
-         *
-         * TODO: revisit the BN_copy aiming for a memory access agnostic
-         * conditional copy.
-         */
-        if (!BN_add(r, k, order)
-            || !BN_add(X, r, order)
-            || !BN_copy(k, BN_num_bits(r) > order_bits ? r : X))
-            goto err;
-
         /* compute r the x-coordinate of generator * k */
         if (!EC_POINT_mul(group, tmp_point, k, NULL, NULL, ctx)) {
             ECerr(EC_F_ECDSA_SIGN_SETUP, ERR_R_EC_LIB);

doc/man3/EC_POINT_add.pod

@@ -43,10 +43,12 @@ The functions EC_POINT_make_affine and EC_POINTs_make_affine force the internal
 co-ordinate system. In the case of EC_POINTs_make_affine the value B<num> provides the number of points in the array B<points> to be
 forced.
 
-EC_POINT_mul calculates the value generator * B<n> + B<q> * B<m> and stores the result in B<r>. The value B<n> may be NULL in which case the result is just B<q> * B<m>.
+EC_POINT_mul is a convenient interface to EC_POINTs_mul: it calculates the value generator * B<n> + B<q> * B<m> and stores the result in B<r>.
+The value B<n> may be NULL in which case the result is just B<q> * B<m> (variable point multiplication). Alternatively, both B<q> and B<m> may be NULL, and B<n> non-NULL, in which case the result is just generator * B<n> (fixed point multiplication).
+When performing a single fixed or variable point multiplication, the underlying implementation uses a constant time algorithm, when the input scalar (either B<n> or B<m>) is in the range [0, ec_group_order).
 
-EC_POINTs_mul calculates the value generator * B<n> + B<q[0]> * B<m[0]> + ... + B<q[num-1]> * B<m[num-1]>. As for EC_POINT_mul the value
+EC_POINTs_mul calculates the value generator * B<n> + B<q[0]> * B<m[0]> + ... + B<q[num-1]> * B<m[num-1]>. As for EC_POINT_mul the value B<n> may be NULL or B<num> may be zero.
-B<n> may be NULL.
+When performing a fixed point multiplication (B<n> is non-NULL and B<num> is 0) or a variable point multiplication (B<n> is NULL and B<num> is 1), the underlying implementation uses a constant time algorithm, when the input scalar (either B<n> or B<m[0]>) is in the range [0, ec_group_order).
 
 The function EC_GROUP_precompute_mult stores multiples of the generator for faster point multiplication, whilst
 EC_GROUP_have_precompute_mult tests whether precomputation has already been done. See L<EC_GROUP_copy(3)> for information