Commit ab919525 authored by Pauli's avatar Pauli Committed by Matt Caswell
Browse files

Address a timing side channel whereby it is possible to determine some 



information about the length of the scalar used in ECDSA operations
from a large number (2^32) of signatures.

This doesn't rate as a CVE because:

* For the non-constant time code, there are easier ways to extract
  more information.

* For the constant time code, it requires a significant number of signatures
  to leak a small amount of information.

Thanks to Neals Fournaise, Eliane Jaulmes and Jean-Rene Reinhard for
reporting this issue.

Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4576)

(cherry picked from commit 4a089bbd)
parent 71844800

crypto/ec/ecdsa_ossl.c

+20 −6
Original line number Diff line number Diff line 
/*

 * Copyright 2002-2016 The OpenSSL Project Authors. All Rights Reserved.

 * Copyright 2002-2017 The OpenSSL Project Authors. All Rights Reserved.

 *

 * Licensed under the OpenSSL license (the "License").  You may not use

 * this file except in compliance with the License.  You can obtain a copy
@@ -41,6 +41,7 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, 
    EC_POINT *tmp_point = NULL;

    const EC_GROUP *group;

    int ret = 0;

    int order_bits;



    if (eckey == NULL || (group = EC_KEY_get0_group(eckey)) == NULL) {

        ECerr(EC_F_ECDSA_SIGN_SETUP, ERR_R_PASSED_NULL_PARAMETER);
@@ -77,6 +78,13 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, 
        goto err;

    }



    /* Preallocate space */

    order_bits = BN_num_bits(order);

    if (!BN_set_bit(k, order_bits)

        || !BN_set_bit(r, order_bits)

        || !BN_set_bit(X, order_bits))

        goto err;



    do {

        /* get random k */

        do
@@ -100,12 +108,18 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, 
        /*

         * 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(k, k, order))

            goto err;

        if (BN_num_bits(k) <= BN_num_bits(order))

            if (!BN_add(k, k, order))

        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 */