Re: Disable DSA_FLAG_NO_EXP_CONSTTIME, always enable constant-time behavior
No problem, I undid that bit. Thanks all. On Mon, Jun 20, 2016 at 11:32 AM, Ted Unangst wrote: > Brent Cook wrote: > > 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 > > -#endif > > +#endif > > \ No newline at end of file > > can we please keep the newline at the end of the file? >
Re: Disable DSA_FLAG_NO_EXP_CONSTTIME, always enable constant-time behavior
Brent Cook wrote: > 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 > -#endif > +#endif > \ No newline at end of file can we please keep the newline at the end of the file?
Re: Disable DSA_FLAG_NO_EXP_CONSTTIME, always enable constant-time behavior
On 2016/06/20 16:55, Marc Espie wrote: > The only thing I'm wondering about is if there's somebody out there who > just uses the "big integer arithmetic" part of openssl, and doesn't want > to go libgmp for licensing reasons. > > Like, if you're in it for (say) trying to break codes, having code that > goes as fast as it can might be useful. > > Is this a valid concern ? > If someone is doing this and needs the speed, they're going to need to find a different solution for OpenSSL as well, they have already enforced constant-time DSA in their tree.
Re: Disable DSA_FLAG_NO_EXP_CONSTTIME, always enable constant-time behavior
The only thing I'm wondering about is if there's somebody out there who just uses the "big integer arithmetic" part of openssl, and doesn't want to go libgmp for licensing reasons. Like, if you're in it for (say) trying to break codes, having code that goes as fast as it can might be useful. Is this a valid concern ?
Re: Disable DSA_FLAG_NO_EXP_CONSTTIME, always enable constant-time behavior
Reads good to me, and passes the regress here, so OK from me.
On Mon, Jun 20, 2016 at 04:40:25AM -0500, Brent Cook wrote:
> 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_P0x01
> -#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_
Disable DSA_FLAG_NO_EXP_CONSTTIME, always enable constant-time behavior
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 (B
