diff options
author | Pauli <paul.dale@oracle.com> | 2017-11-01 06:58:13 +1000 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2017-11-01 15:55:11 +0000 |
commit | 71844800d543162f709c6a223d993a50506028c2 (patch) | |
tree | b5c91928a759905c57707d01eff8b2bf389cb61b | |
parent | bb0c588ab2c6dae55bb8adbcf72cab3bca390e0f (diff) |
Address a timing side channel whereby it is possible to determine some
information about the length of a value used in DSA operations from
a large number 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: Andy Polyakov <appro@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4576)
(cherry picked from commit c0caa945f6ef30363e0d01d75155f20248403df4)
-rw-r--r-- | crypto/dsa/dsa_ossl.c | 35 |
1 files changed, 25 insertions, 10 deletions
diff --git a/crypto/dsa/dsa_ossl.c b/crypto/dsa/dsa_ossl.c index 479337763b..7f48cf2e33 100644 --- a/crypto/dsa/dsa_ossl.c +++ b/crypto/dsa/dsa_ossl.c @@ -148,7 +148,9 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, { BN_CTX *ctx = NULL; BIGNUM *k, *kinv = NULL, *r = *rp; + BIGNUM *l, *m; int ret = 0; + int q_bits; if (!dsa->p || !dsa->q || !dsa->g) { DSAerr(DSA_F_DSA_SIGN_SETUP, DSA_R_MISSING_PARAMETERS); @@ -156,7 +158,9 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, } k = BN_new(); - if (k == NULL) + l = BN_new(); + m = BN_new(); + if (k == NULL || l == NULL || m == NULL) goto err; if (ctx_in == NULL) { @@ -165,6 +169,13 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, } else ctx = ctx_in; + /* Preallocate space */ + q_bits = BN_num_bits(dsa->q); + if (!BN_set_bit(k, q_bits) + || !BN_set_bit(l, q_bits) + || !BN_set_bit(m, q_bits)) + goto err; + /* Get random k */ do { if (dgst != NULL) { @@ -191,17 +202,19 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, /* * We do not want timing information to leak the length of k, so we - * compute g^k using an equivalent exponent of fixed length. (This - * is a kludge that we need because the BN_mod_exp_mont() does not - * let us specify the desired timing behaviour.) + * 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 modulus. + * + * TODO: revisit the BN_copy aiming for a memory access agnostic + * conditional copy. */ - - if (!BN_add(k, k, dsa->q)) + if (!BN_add(l, k, dsa->q) + || !BN_add(m, l, dsa->q) + || !BN_copy(k, BN_num_bits(l) > q_bits ? l : m)) goto err; - if (BN_num_bits(k) <= BN_num_bits(dsa->q)) { - if (!BN_add(k, k, dsa->q)) - goto err; - } if ((dsa)->meth->bn_mod_exp != NULL) { if (!dsa->meth->bn_mod_exp(dsa, r, dsa->g, k, dsa->p, ctx, @@ -229,6 +242,8 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, if (ctx != ctx_in) BN_CTX_free(ctx); BN_clear_free(k); + BN_clear_free(l); + BN_clear_free(m); return ret; } |