diff options
| author | tb <> | 2018-04-28 14:22:21 +0000 |
|---|---|---|
| committer | tb <> | 2018-04-28 14:22:21 +0000 |
| commit | 3cea2322ceb21eb9865410e66501a89edc3bbf08 (patch) | |
| tree | 2b9bf7013db68f28acde937c2f3ea0071b53af1a /src/lib/libcrypto/dsa/dsa_ossl.c | |
| parent | 7018cf0029cc86e859990723d6340037f6f9402c (diff) | |
| download | openbsd-3cea2322ceb21eb9865410e66501a89edc3bbf08.tar.gz openbsd-3cea2322ceb21eb9865410e66501a89edc3bbf08.tar.bz2 openbsd-3cea2322ceb21eb9865410e66501a89edc3bbf08.zip | |
Fix a small timing side channel in dsa_sign_setup(). Simple adaptation
of OpenSSL commit c0caa945f6ef30363e0d01d75155f20248403df4 to our
version of this function.
ok beck, jsing
Original commit message:
commit c0caa945f6ef30363e0d01d75155f20248403df4
Author: Pauli <paul.dale@oracle.com>
Date: Wed Nov 1 06:58:13 2017 +1000
Address a timing side channel whereby it is possible to determine some
information about the length of the scalar used in DSA 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: Andy Polyakov <appro@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4576)]
Diffstat (limited to 'src/lib/libcrypto/dsa/dsa_ossl.c')
| -rw-r--r-- | src/lib/libcrypto/dsa/dsa_ossl.c | 37 |
1 files changed, 25 insertions, 12 deletions
diff --git a/src/lib/libcrypto/dsa/dsa_ossl.c b/src/lib/libcrypto/dsa/dsa_ossl.c index f1013fe547..301cdd5095 100644 --- a/src/lib/libcrypto/dsa/dsa_ossl.c +++ b/src/lib/libcrypto/dsa/dsa_ossl.c | |||
| @@ -1,4 +1,4 @@ | |||
| 1 | /* $OpenBSD: dsa_ossl.c,v 1.30 2017/01/29 17:49:22 beck Exp $ */ | 1 | /* $OpenBSD: dsa_ossl.c,v 1.31 2018/04/28 14:22:21 tb Exp $ */ |
| 2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) | 2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) |
| 3 | * All rights reserved. | 3 | * All rights reserved. |
| 4 | * | 4 | * |
| @@ -184,8 +184,8 @@ static int | |||
| 184 | dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp) | 184 | dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp) |
| 185 | { | 185 | { |
| 186 | BN_CTX *ctx; | 186 | BN_CTX *ctx; |
| 187 | BIGNUM k, *kinv = NULL, *r = NULL; | 187 | BIGNUM k, l, m, *kinv = NULL, *r = NULL; |
| 188 | int ret = 0; | 188 | int q_bits, ret = 0; |
| 189 | 189 | ||
| 190 | if (!dsa->p || !dsa->q || !dsa->g) { | 190 | if (!dsa->p || !dsa->q || !dsa->g) { |
| 191 | DSAerror(DSA_R_MISSING_PARAMETERS); | 191 | DSAerror(DSA_R_MISSING_PARAMETERS); |
| @@ -193,6 +193,8 @@ dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp) | |||
| 193 | } | 193 | } |
| 194 | 194 | ||
| 195 | BN_init(&k); | 195 | BN_init(&k); |
| 196 | BN_init(&l); | ||
| 197 | BN_init(&m); | ||
| 196 | 198 | ||
| 197 | if (ctx_in == NULL) { | 199 | if (ctx_in == NULL) { |
| 198 | if ((ctx = BN_CTX_new()) == NULL) | 200 | if ((ctx = BN_CTX_new()) == NULL) |
| @@ -203,6 +205,13 @@ dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp) | |||
| 203 | if ((r = BN_new()) == NULL) | 205 | if ((r = BN_new()) == NULL) |
| 204 | goto err; | 206 | goto err; |
| 205 | 207 | ||
| 208 | /* Preallocate space */ | ||
| 209 | q_bits = BN_num_bits(dsa->q); | ||
| 210 | if (!BN_set_bit(&k, q_bits) || | ||
| 211 | !BN_set_bit(&l, q_bits) || | ||
| 212 | !BN_set_bit(&m, q_bits)) | ||
| 213 | goto err; | ||
| 214 | |||
| 206 | /* Get random k */ | 215 | /* Get random k */ |
| 207 | do { | 216 | do { |
| 208 | if (!BN_rand_range(&k, dsa->q)) | 217 | if (!BN_rand_range(&k, dsa->q)) |
| @@ -221,19 +230,21 @@ dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp) | |||
| 221 | 230 | ||
| 222 | /* | 231 | /* |
| 223 | * We do not want timing information to leak the length of k, | 232 | * We do not want timing information to leak the length of k, |
| 224 | * so we compute g^k using an equivalent exponent of fixed | 233 | * so we compute G^k using an equivalent exponent of fixed |
| 225 | * length. | 234 | * bit-length. |
| 235 | * | ||
| 236 | * We unconditionally perform both of these additions to prevent a | ||
| 237 | * small timing information leakage. We then choose the sum that is | ||
| 238 | * one bit longer than the modulus. | ||
| 226 | * | 239 | * |
| 227 | * (This is a kludge that we need because the BN_mod_exp_mont() | 240 | * TODO: revisit the BN_copy aiming for a memory access agnostic |
| 228 | * does not let us specify the desired timing behaviour.) | 241 | * conditional copy. |
| 229 | */ | 242 | */ |
| 230 | 243 | ||
| 231 | if (!BN_add(&k, &k, dsa->q)) | 244 | if (!BN_add(&l, &k, dsa->q) || |
| 245 | !BN_add(&m, &l, dsa->q) || | ||
| 246 | !BN_copy(&k, BN_num_bits(&l) > q_bits ? &l : &m)) | ||
| 232 | goto err; | 247 | goto err; |
| 233 | if (BN_num_bits(&k) <= BN_num_bits(dsa->q)) { | ||
| 234 | if (!BN_add(&k, &k, dsa->q)) | ||
| 235 | goto err; | ||
| 236 | } | ||
| 237 | 248 | ||
| 238 | if (dsa->meth->bn_mod_exp != NULL) { | 249 | if (dsa->meth->bn_mod_exp != NULL) { |
| 239 | if (!dsa->meth->bn_mod_exp(dsa, r, dsa->g, &k, dsa->p, ctx, | 250 | if (!dsa->meth->bn_mod_exp(dsa, r, dsa->g, &k, dsa->p, ctx, |
| @@ -265,6 +276,8 @@ err: | |||
| 265 | if (ctx_in == NULL) | 276 | if (ctx_in == NULL) |
| 266 | BN_CTX_free(ctx); | 277 | BN_CTX_free(ctx); |
| 267 | BN_clear_free(&k); | 278 | BN_clear_free(&k); |
| 279 | BN_clear_free(&l); | ||
| 280 | BN_clear_free(&m); | ||
| 268 | return ret; | 281 | return ret; |
| 269 | } | 282 | } |
| 270 | 283 | ||