Hi, This is a patch from Cesar Pereida, removing support for DSA_FLAG_NO_EXP_CONSTTIME by making DSA always operate in constant time.
See https://github.com/libressl-portable/openbsd/pull/61 for more details. ok? diff --git a/src/lib/libssl/src/crypto/dsa/dsa.h b/src/lib/libssl/src/crypto/dsa/dsa.h index 909096d..d2d1d5f 100644 --- a/src/lib/libssl/src/crypto/dsa/dsa.h +++ b/src/lib/libssl/src/crypto/dsa/dsa.h @@ -89,12 +89,8 @@ #endif #define DSA_FLAG_CACHE_MONT_P 0x01 -#define DSA_FLAG_NO_EXP_CONSTTIME 0x02 /* new with 0.9.7h; the built-in DSA - * implementation now uses constant time - * modular exponentiation for secret exponents - * by default. This flag causes the - * faster variable sliding window method to - * be used for all exponents. +#define DSA_FLAG_NO_EXP_CONSTTIME 0x00 /* Does nothing. Previously this switched off + * constant time behaviour. */ /* If this flag is set the DSA method is FIPS compliant and can be used diff --git a/src/lib/libssl/src/crypto/dsa/dsa_key.c b/src/lib/libssl/src/crypto/dsa/dsa_key.c index 2968fa2..e01bacb 100644 --- a/src/lib/libssl/src/crypto/dsa/dsa_key.c +++ b/src/lib/libssl/src/crypto/dsa/dsa_key.c @@ -104,18 +104,18 @@ dsa_builtin_keygen(DSA *dsa) pub_key=dsa->pub_key; { - BIGNUM local_prk; - BIGNUM *prk; + BIGNUM *prk = BN_new(); - if ((dsa->flags & DSA_FLAG_NO_EXP_CONSTTIME) == 0) { - BN_init(&local_prk); - prk = &local_prk; - BN_with_flags(prk, priv_key, BN_FLG_CONSTTIME); - } else - prk = priv_key; + if (prk == NULL) + goto err; + + BN_with_flags(prk, priv_key, BN_FLG_CONSTTIME); - if (!BN_mod_exp(pub_key, dsa->g, prk, dsa->p, ctx)) + if (!BN_mod_exp(pub_key, dsa->g, prk, dsa->p, ctx)) { + BN_free(prk); goto err; + } + BN_free(prk); } dsa->priv_key = priv_key; @@ -130,4 +130,4 @@ err: BN_CTX_free(ctx); return ok; } -#endif +#endif \ No newline at end of file diff --git a/src/lib/libssl/src/crypto/dsa/dsa_ossl.c b/src/lib/libssl/src/crypto/dsa/dsa_ossl.c index 726e6c7..4a3b417 100644 --- a/src/lib/libssl/src/crypto/dsa/dsa_ossl.c +++ b/src/lib/libssl/src/crypto/dsa/dsa_ossl.c @@ -83,46 +83,6 @@ static DSA_METHOD openssl_dsa_meth = { .finish = dsa_finish }; -/* - * These macro wrappers replace attempts to use the dsa_mod_exp() and - * bn_mod_exp() handlers in the DSA_METHOD structure. We avoid the problem of - * having a the macro work as an expression by bundling an "err_instr". So; - * - * if (!dsa->meth->bn_mod_exp(dsa, r,dsa->g,&k,dsa->p,ctx, - * dsa->method_mont_p)) goto err; - * - * can be replaced by; - * - * DSA_BN_MOD_EXP(goto err, dsa, r, dsa->g, &k, dsa->p, ctx, - * dsa->method_mont_p); - */ - -#define DSA_MOD_EXP(err_instr,dsa,rr,a1,p1,a2,p2,m,ctx,in_mont) \ -do { \ - int _tmp_res53; \ - if ((dsa)->meth->dsa_mod_exp) \ - _tmp_res53 = (dsa)->meth->dsa_mod_exp((dsa), (rr), \ - (a1), (p1), (a2), (p2), (m), (ctx), (in_mont)); \ - else \ - _tmp_res53 = BN_mod_exp2_mont((rr), (a1), \ - (p1), (a2), (p2), (m), (ctx), (in_mont)); \ - if (!_tmp_res53) \ - err_instr; \ -} while(0) - -#define DSA_BN_MOD_EXP(err_instr,dsa,r,a,p,m,ctx,m_ctx) \ -do { \ - int _tmp_res53; \ - if ((dsa)->meth->bn_mod_exp) \ - _tmp_res53 = (dsa)->meth->bn_mod_exp((dsa), (r), \ - (a), (p), (m), (ctx), (m_ctx)); \ - else \ - _tmp_res53 = BN_mod_exp_mont((r), (a), (p), (m), \ - (ctx), (m_ctx)); \ - if (!_tmp_res53) \ - err_instr; \ -} while(0) - const DSA_METHOD * DSA_OpenSSL(void) { @@ -222,7 +182,7 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp) { BN_CTX *ctx; - BIGNUM k, kq, *K, *kinv = NULL, *r = NULL; + BIGNUM k, *kinv = NULL, *r = NULL; int ret = 0; if (!dsa->p || !dsa->q || !dsa->g) { @@ -231,7 +191,6 @@ dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp) } BN_init(&k); - BN_init(&kq); if (ctx_in == NULL) { if ((ctx = BN_CTX_new()) == NULL) @@ -248,6 +207,8 @@ dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp) goto err; } while (BN_is_zero(&k)); + BN_set_flags(&k, BN_FLG_CONSTTIME); + if (dsa->flags & DSA_FLAG_CACHE_MONT_P) { if (!BN_MONT_CTX_set_locked(&dsa->method_mont_p, CRYPTO_LOCK_DSA, dsa->p, ctx)) @@ -256,37 +217,31 @@ dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp) /* Compute r = (g^k mod p) mod q */ - if ((dsa->flags & DSA_FLAG_NO_EXP_CONSTTIME) == 0) { - if (!BN_copy(&kq, &k)) - goto err; - - /* - * 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.) - */ + /* + * 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.) + */ - if (!BN_add(&kq, &kq, dsa->q)) + if (!BN_add(&k, &k, dsa->q)) + goto err; + if (BN_num_bits(&k) <= BN_num_bits(dsa->q)) { + if (!BN_add(&k, &k, dsa->q)) goto err; - if (BN_num_bits(&kq) <= BN_num_bits(dsa->q)) { - if (!BN_add(&kq, &kq, dsa->q)) - goto err; - } + } - K = &kq; + if (dsa->meth->bn_mod_exp != NULL) { + if (!dsa->meth->bn_mod_exp(dsa, r, dsa->g, &k, dsa->p, ctx, + dsa->method_mont_p)) + goto err; } else { - K = &k; + if (!BN_mod_exp_mont(r, dsa->g, &k, dsa->p, ctx, dsa->method_mont_p)) + goto err; } - if ((dsa->flags & DSA_FLAG_NO_EXP_CONSTTIME) == 0) { - BN_set_flags(K, BN_FLG_CONSTTIME); - } - - DSA_BN_MOD_EXP(goto err, dsa, r, dsa->g, K, dsa->p, ctx, - dsa->method_mont_p); if (!BN_mod(r,r,dsa->q,ctx)) goto err; @@ -308,7 +263,6 @@ err: if (ctx_in == NULL) BN_CTX_free(ctx); BN_clear_free(&k); - BN_clear_free(&kq); return ret; } @@ -386,8 +340,16 @@ dsa_do_verify(const unsigned char *dgst, int dgst_len, DSA_SIG *sig, DSA *dsa) goto err; } - DSA_MOD_EXP(goto err, dsa, &t1, dsa->g, &u1, dsa->pub_key, &u2, dsa->p, - ctx, mont); + if (dsa->meth->dsa_mod_exp != NULL) { + if (!dsa->meth->dsa_mod_exp(dsa, &t1, dsa->g, &u1, dsa->pub_key, &u2, + dsa->p, ctx, mont)) + goto err; + } else { + if (!BN_mod_exp2_mont(&t1, dsa->g, &u1, dsa->pub_key, &u2, dsa->p, ctx, + mont)) + goto err; + } + /* BN_copy(&u1,&t1); */ /* let u1 = u1 mod q */ if (!BN_mod(&u1, &t1, dsa->q, ctx))
