I searched around a bit and could find only one open-source instance
of RSA_FLAG_NO_CONSTTIME in the wild - something called 'anon-proxy' that
lives as a zombie in the Debian repos, but hasn't had a real release since
2008 and seems to be orphaned from its original source site. I didn't even
see any scripting language bindings for these flags, which is especially
unusual when a lot of OpenSSL bindings export every trivial feature
otherwise.
So yeah, not a lot should change in the world in removing the defines. Mind
if we wrap in #ifndef LIBRESSL_INTERNAL guards first though?
On Wed, Jun 22, 2016 at 12:52 PM, Bob Beck <b...@openbsd.org> wrote:
> I'm wondering outloud it we should remove the #define, instead of
> leaving it in there. I.E. should we be deliberately
> breaking anything making use of that?
>
> At the very least this (along with the DH one) can probably #ifndef
> LIBRESSL_INTERNAL - and failing that should
> we nuke them and bump majors?
>
>
> On Wed, Jun 22, 2016 at 7:44 AM, Brent Cook <bust...@gmail.com> wrote:
> > This is another patch from César Pereida that disables the DH and RSA
> > non-constant-time flags as well.
> >
> > ok?
> >
> > Index: src/crypto/dh/dh.h
> > ===================================================================
> > RCS file: /cvs/src/lib/libssl/src/crypto/dh/dh.h,v
> > retrieving revision 1.16
> > diff -u -p -u -p -r1.16 dh.h
> > --- src/crypto/dh/dh.h 12 Jun 2014 15:49:28 -0000 1.16
> > +++ src/crypto/dh/dh.h 22 Jun 2016 13:37:33 -0000
> > @@ -78,12 +78,8 @@
> > #endif
> >
> > #define DH_FLAG_CACHE_MONT_P 0x01
> > -#define DH_FLAG_NO_EXP_CONSTTIME 0x02 /* new with 0.9.7h; the built-in
> DH
> > - * 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 DH_FLAG_NO_EXP_CONSTTIME 0x00 /* Does nothing. Previously this
> switched off
> > + * constant time behaviour.
> > */
> >
> > /* If this flag is set the DH method is FIPS compliant and can be used
> > Index: src/crypto/dh/dh_key.c
> > ===================================================================
> > RCS file: /cvs/src/lib/libssl/src/crypto/dh/dh_key.c,v
> > retrieving revision 1.23
> > diff -u -p -u -p -r1.23 dh_key.c
> > --- src/crypto/dh/dh_key.c 9 Feb 2015 15:49:22 -0000 1.23
> > +++ src/crypto/dh/dh_key.c 22 Jun 2016 13:37:33 -0000
> > @@ -147,21 +147,21 @@ generate_key(DH *dh)
> > }
> >
> > {
> > - BIGNUM local_prk;
> > - BIGNUM *prk;
> > + BIGNUM *prk = BN_new();
> >
> > - if ((dh->flags & DH_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 (!dh->meth->bn_mod_exp(dh, pub_key, dh->g, prk,
> dh->p, ctx,
> > - mont))
> > + mont)) {
> > + BN_free(prk);
> > goto err;
> > + }
> > + BN_free(prk);
> > }
> > -
> > +
> > dh->pub_key = pub_key;
> > dh->priv_key = priv_key;
> > ok = 1;
> > @@ -206,10 +206,9 @@ compute_key(unsigned char *key, const BI
> > if (dh->flags & DH_FLAG_CACHE_MONT_P) {
> > mont = BN_MONT_CTX_set_locked(&dh->method_mont_p,
> > CRYPTO_LOCK_DH, dh->p, ctx);
> > - if ((dh->flags & DH_FLAG_NO_EXP_CONSTTIME) == 0) {
> > - /* XXX */
> > - BN_set_flags(dh->priv_key, BN_FLG_CONSTTIME);
> > - }
> > +
> > + BN_set_flags(dh->priv_key, BN_FLG_CONSTTIME);
> > +
> > if (!mont)
> > goto err;
> > }
> > @@ -238,16 +237,7 @@ static int
> > dh_bn_mod_exp(const DH *dh, BIGNUM *r, const BIGNUM *a, const BIGNUM *p,
> > const BIGNUM *m, BN_CTX *ctx, BN_MONT_CTX *m_ctx)
> > {
> > - /*
> > - * If a is only one word long and constant time is false, use
> the faster
> > - * exponenentiation function.
> > - */
> > - if (a->top == 1 && (dh->flags & DH_FLAG_NO_EXP_CONSTTIME) != 0) {
> > - BN_ULONG A = a->d[0];
> > -
> > - return BN_mod_exp_mont_word(r, A, p, m, ctx, m_ctx);
> > - } else
> > - return BN_mod_exp_mont(r, a, p, m, ctx, m_ctx);
> > + return BN_mod_exp_mont(r, a, p, m, ctx, m_ctx);
> > }
> >
> > static int
> > Index: src/crypto/rsa/rsa.h
> > ===================================================================
> > RCS file: /cvs/src/lib/libssl/src/crypto/rsa/rsa.h,v
> > retrieving revision 1.27
> > diff -u -p -u -p -r1.27 rsa.h
> > --- src/crypto/rsa/rsa.h 14 Feb 2015 15:10:39 -0000 1.27
> > +++ src/crypto/rsa/rsa.h 22 Jun 2016 13:37:33 -0000
> > @@ -195,13 +195,9 @@ struct rsa_st {
> > #define RSA_FLAG_NO_BLINDING 0x0080
> >
> > /*
> > - * The built-in RSA implementation uses constant time operations by
> default
> > - * in private key operations, e.g., constant time modular
> exponentiation,
> > - * modular inverse without leaking branches, division without leaking
> branches.
> > - * This flag disables these constant time operations and results in
> faster RSA
> > - * private key operations.
> > + * Does nothing. Previously this switched off constant time behaviour.
> > */
> > -#define RSA_FLAG_NO_CONSTTIME 0x0100
> > +#define RSA_FLAG_NO_CONSTTIME 0x0000
> >
> >
> > #define EVP_PKEY_CTX_set_rsa_padding(ctx, pad) \
> > Index: src/crypto/rsa/rsa_crpt.c
> > ===================================================================
> > RCS file: /cvs/src/lib/libssl/src/crypto/rsa/rsa_crpt.c,v
> > retrieving revision 1.14
> > diff -u -p -u -p -r1.14 rsa_crpt.c
> > --- src/crypto/rsa/rsa_crpt.c 11 Feb 2015 03:19:37 -0000 1.14
> > +++ src/crypto/rsa/rsa_crpt.c 22 Jun 2016 13:37:33 -0000
> > @@ -169,8 +169,8 @@ err:
> > BN_BLINDING *
> > RSA_setup_blinding(RSA *rsa, BN_CTX *in_ctx)
> > {
> > - BIGNUM local_n;
> > - BIGNUM *e, *n;
> > + BIGNUM *e;
> > + BIGNUM *n = BN_new();
> > BN_CTX *ctx;
> > BN_BLINDING *ret = NULL;
> >
> > @@ -192,15 +192,16 @@ RSA_setup_blinding(RSA *rsa, BN_CTX *in_
> > } else
> > e = rsa->e;
> >
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - /* Set BN_FLG_CONSTTIME flag */
> > - n = &local_n;
> > - BN_with_flags(n, rsa->n, BN_FLG_CONSTTIME);
> > - } else
> > - n = rsa->n;
> > + if (n == NULL)
> > + goto err;
> > +
> > + BN_with_flags(n, rsa->n, BN_FLG_CONSTTIME);
> >
> > ret = BN_BLINDING_create_param(NULL, e, n, ctx,
> rsa->meth->bn_mod_exp,
> > rsa->_method_mod_n);
> > +
> > + BN_free(n);
> > +
> > if (ret == NULL) {
> > RSAerr(RSA_F_RSA_SETUP_BLINDING, ERR_R_BN_LIB);
> > goto err;
> > Index: src/crypto/rsa/rsa_eay.c
> > ===================================================================
> > RCS file: /cvs/src/lib/libssl/src/crypto/rsa/rsa_eay.c,v
> > retrieving revision 1.40
> > diff -u -p -u -p -r1.40 rsa_eay.c
> > --- src/crypto/rsa/rsa_eay.c 10 Sep 2015 15:56:25 -0000 1.40
> > +++ src/crypto/rsa/rsa_eay.c 22 Jun 2016 13:37:33 -0000
> > @@ -426,24 +426,26 @@ RSA_eay_private_encrypt(int flen, const
> > if (!rsa->meth->rsa_mod_exp(ret, f, rsa, ctx))
> > goto err;
> > } else {
> > - BIGNUM local_d;
> > - BIGNUM *d = NULL;
> > + BIGNUM *d = BN_new();
> >
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - BN_init(&local_d);
> > - d = &local_d;
> > - BN_with_flags(d, rsa->d, BN_FLG_CONSTTIME);
> > - } else
> > - d = rsa->d;
> > + if (d == NULL)
> > + goto err;
> > +
> > + BN_with_flags(d, rsa->d, BN_FLG_CONSTTIME);
> >
> > if (rsa->flags & RSA_FLAG_CACHE_PUBLIC)
> > if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_n,
> > - CRYPTO_LOCK_RSA, rsa->n, ctx))
> > + CRYPTO_LOCK_RSA, rsa->n, ctx)) {
> > + BN_free(d);
> > goto err;
> > + }
> >
> > if (!rsa->meth->bn_mod_exp(ret, f, d, rsa->n, ctx,
> > - rsa->_method_mod_n))
> > + rsa->_method_mod_n)) {
> > + BN_free(d);
> > goto err;
> > + }
> > + BN_free(d);
> > }
> >
> > if (blinding)
> > @@ -553,22 +555,26 @@ RSA_eay_private_decrypt(int flen, const
> > if (!rsa->meth->rsa_mod_exp(ret, f, rsa, ctx))
> > goto err;
> > } else {
> > - BIGNUM local_d;
> > - BIGNUM *d = NULL;
> > + BIGNUM *d = BN_new();
> >
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - d = &local_d;
> > - BN_with_flags(d, rsa->d, BN_FLG_CONSTTIME);
> > - } else
> > - d = rsa->d;
> > + if (d == NULL)
> > + goto err;
> > +
> > + BN_with_flags(d, rsa->d, BN_FLG_CONSTTIME);
> >
> > if (rsa->flags & RSA_FLAG_CACHE_PUBLIC)
> > if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_n,
> > - CRYPTO_LOCK_RSA, rsa->n, ctx))
> > + CRYPTO_LOCK_RSA, rsa->n, ctx)) {
> > + BN_free(d);
> > goto err;
> > + }
> > +
> > if (!rsa->meth->bn_mod_exp(ret, f, d, rsa->n, ctx,
> > - rsa->_method_mod_n))
> > + rsa->_method_mod_n)) {
> > + BN_free(d);
> > goto err;
> > + }
> > + BN_free(d);
> > }
> >
> > if (blinding)
> > @@ -723,8 +729,10 @@ static int
> > RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx)
> > {
> > BIGNUM *r1, *m1, *vrfy;
> > - BIGNUM local_dmp1, local_dmq1, local_c, local_r1;
> > - BIGNUM *dmp1, *dmq1, *c, *pr1;
> > + BIGNUM *dmp1 = BN_new();
> > + BIGNUM *dmq1 = BN_new();
> > + BIGNUM *pr1 = BN_new();
> > + BIGNUM *c = BN_new();
> > int ret = 0;
> >
> > BN_CTX_start(ctx);
> > @@ -737,34 +745,33 @@ RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM
> > }
> >
> > {
> > - BIGNUM local_p, local_q;
> > - BIGNUM *p = NULL, *q = NULL;
> > + BIGNUM *p = BN_new();
> > + BIGNUM *q = BN_new();
> >
> > /*
> > * Make sure BN_mod_inverse in Montgomery intialization
> uses the
> > - * BN_FLG_CONSTTIME flag (unless RSA_FLAG_NO_CONSTTIME
> is set)
> > + * BN_FLG_CONSTTIME flag
> > */
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - BN_init(&local_p);
> > - p = &local_p;
> > - BN_with_flags(p, rsa->p, BN_FLG_CONSTTIME);
> > -
> > - BN_init(&local_q);
> > - q = &local_q;
> > - BN_with_flags(q, rsa->q, BN_FLG_CONSTTIME);
> > - } else {
> > - p = rsa->p;
> > - q = rsa->q;
> > + if (p == NULL || q == NULL) {
> > + BN_free(p);
> > + BN_free(q);
> > + goto err;
> > }
> > + BN_with_flags(p, rsa->p, BN_FLG_CONSTTIME);
> > + BN_with_flags(q, rsa->q, BN_FLG_CONSTTIME);
> >
> > if (rsa->flags & RSA_FLAG_CACHE_PRIVATE) {
> > - if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_p,
> > - CRYPTO_LOCK_RSA, p, ctx))
> > - goto err;
> > - if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_q,
> > - CRYPTO_LOCK_RSA, q, ctx))
> > + if (!BN_MONT_CTX_set_locked
> > + (&rsa->_method_mod_p,CRYPTO_LOCK_RSA, p,
> ctx)
> > + ||
> !BN_MONT_CTX_set_locked(&rsa->_method_mod_q,
> > + CRYPTO_LOCK_RSA, q, ctx)) {
> > + BN_free(p);
> > + BN_free(q);
> > goto err;
> > + }
> > }
> > + BN_free(p);
> > + BN_free(q);
> > }
> >
> > if (rsa->flags & RSA_FLAG_CACHE_PUBLIC)
> > @@ -773,46 +780,55 @@ RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM
> > goto err;
> >
> > /* compute I mod q */
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - c = &local_c;
> > - BN_with_flags(c, I, BN_FLG_CONSTTIME);
> > - if (!BN_mod(r1, c, rsa->q, ctx))
> > - goto err;
> > - } else {
> > - if (!BN_mod(r1, I, rsa->q, ctx))
> > - goto err;
> > + if (c == NULL)
> > + goto err;
> > +
> > + BN_with_flags(c, I, BN_FLG_CONSTTIME);
> > +
> > + if (!BN_mod(r1, c, rsa->q, ctx)) {
> > + BN_free(c);
> > + goto err;
> > }
> >
> > /* compute r1^dmq1 mod q */
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - dmq1 = &local_dmq1;
> > - BN_with_flags(dmq1, rsa->dmq1, BN_FLG_CONSTTIME);
> > - } else
> > - dmq1 = rsa->dmq1;
> > + if (dmq1 == NULL) {
> > + BN_free(c);
> > + goto err;
> > + }
> > + BN_with_flags(dmq1, rsa->dmq1, BN_FLG_CONSTTIME);
> > +
> > if (!rsa->meth->bn_mod_exp(m1, r1, dmq1, rsa->q, ctx,
> > - rsa->_method_mod_q))
> > + rsa->_method_mod_q)) {
> > + BN_free(c);
> > + BN_free(dmq1);
> > goto err;
> > + }
> > + BN_free(dmq1);
> >
> > /* compute I mod p */
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - c = &local_c;
> > - BN_with_flags(c, I, BN_FLG_CONSTTIME);
> > - if (!BN_mod(r1, c, rsa->p, ctx))
> > - goto err;
> > - } else {
> > - if (!BN_mod(r1, I, rsa->p, ctx))
> > - goto err;
> > + if (c == NULL)
> > + goto err;
> > +
> > + BN_with_flags(c, I, BN_FLG_CONSTTIME);
> > +
> > + if (!BN_mod(r1, c, rsa->p, ctx)) {
> > + BN_free(c);
> > + goto err;
> > }
> > + BN_free(c);
> >
> > /* compute r1^dmp1 mod p */
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - dmp1 = &local_dmp1;
> > - BN_with_flags(dmp1, rsa->dmp1, BN_FLG_CONSTTIME);
> > - } else
> > - dmp1 = rsa->dmp1;
> > + if (dmp1 == NULL)
> > + goto err;
> > +
> > + BN_with_flags(dmp1, rsa->dmp1, BN_FLG_CONSTTIME);
> > +
> > if (!rsa->meth->bn_mod_exp(r0, r1, dmp1, rsa->p, ctx,
> > - rsa->_method_mod_p))
> > + rsa->_method_mod_p)) {
> > + BN_free(dmp1);
> > goto err;
> > + }
> > + BN_free(dmp1);
> >
> > if (!BN_sub(r0, r0, m1))
> > goto err;
> > @@ -828,14 +844,17 @@ RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM
> > goto err;
> >
> > /* Turn BN_FLG_CONSTTIME flag on before division operation */
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - pr1 = &local_r1;
> > - BN_with_flags(pr1, r1, BN_FLG_CONSTTIME);
> > - } else
> > - pr1 = r1;
> > - if (!BN_mod(r0, pr1, rsa->p, ctx))
> > + if (pr1 == NULL)
> > goto err;
> >
> > + BN_with_flags(pr1, r1, BN_FLG_CONSTTIME);
> > +
> > + if (!BN_mod(r0, pr1, rsa->p, ctx)) {
> > + BN_free(pr1);
> > + goto err;
> > + }
> > + BN_free(pr1);
> > +
> > /*
> > * If p < q it is occasionally possible for the correction of
> > * adding 'p' if r0 is negative above to leave the result still
> > @@ -876,17 +895,19 @@ RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM
> > * mod_exp and return that instead.
> > */
> >
> > - BIGNUM local_d;
> > - BIGNUM *d = NULL;
> > + BIGNUM *d = BN_new();
> > +
> > + if (d == NULL)
> > + goto err;
> > +
> > + BN_with_flags(d, rsa->d, BN_FLG_CONSTTIME);
> >
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - d = &local_d;
> > - BN_with_flags(d, rsa->d,
> BN_FLG_CONSTTIME);
> > - } else
> > - d = rsa->d;
> > if (!rsa->meth->bn_mod_exp(r0, I, d, rsa->n, ctx,
> > - rsa->_method_mod_n))
> > + rsa->_method_mod_n)) {
> > + BN_free(d);
> > goto err;
> > + }
> > + BN_free(d);
> > }
> > }
> > ret = 1;
> > Index: src/crypto/rsa/rsa_gen.c
> > ===================================================================
> > RCS file: /cvs/src/lib/libssl/src/crypto/rsa/rsa_gen.c,v
> > retrieving revision 1.17
> > diff -u -p -u -p -r1.17 rsa_gen.c
> > --- src/crypto/rsa/rsa_gen.c 9 Feb 2015 15:49:22 -0000 1.17
> > +++ src/crypto/rsa/rsa_gen.c 22 Jun 2016 13:37:33 -0000
> > @@ -90,8 +90,9 @@ static int
> > rsa_builtin_keygen(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb)
> > {
> > BIGNUM *r0 = NULL, *r1 = NULL, *r2 = NULL, *r3 = NULL, *tmp;
> > - BIGNUM local_r0, local_d, local_p;
> > - BIGNUM *pr0, *d, *p;
> > + BIGNUM *pr0 = BN_new();
> > + BIGNUM *d = BN_new();
> > + BIGNUM *p = BN_new();
> > int bitsp, bitsq, ok = -1, n = 0;
> > BN_CTX *ctx = NULL;
> >
> > @@ -193,37 +194,39 @@ rsa_builtin_keygen(RSA *rsa, int bits, B
> > goto err;
> > if (!BN_mul(r0, r1, r2, ctx)) /* (p-1)(q-1) */
> > goto err;
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - pr0 = &local_r0;
> > - BN_with_flags(pr0, r0, BN_FLG_CONSTTIME);
> > - } else
> > - pr0 = r0;
> > - if (!BN_mod_inverse(rsa->d, rsa->e, pr0, ctx)) /* d */
> > + if (pr0 == NULL)
> > goto err;
> > + BN_with_flags(pr0, r0, BN_FLG_CONSTTIME);
> > + if (!BN_mod_inverse(rsa->d, rsa->e, pr0, ctx)) { /* d */
> > + BN_free(pr0);
> > + goto err;
> > + }
> >
> > /* set up d for correct BN_FLG_CONSTTIME flag */
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - d = &local_d;
> > - BN_with_flags(d, rsa->d, BN_FLG_CONSTTIME);
> > - } else
> > - d = rsa->d;
> > + if (d == NULL)
> > + goto err;
> > + BN_with_flags(d, rsa->d, BN_FLG_CONSTTIME);
> >
> > /* calculate d mod (p-1) */
> > - if (!BN_mod(rsa->dmp1, d, r1, ctx))
> > + if (!BN_mod(rsa->dmp1, d, r1, ctx)) {
> > + BN_free(d);
> > goto err;
> > + }
> >
> > /* calculate d mod (q-1) */
> > - if (!BN_mod(rsa->dmq1, d, r2, ctx))
> > + if (!BN_mod(rsa->dmq1, d, r2, ctx)) {
> > + BN_free(d);
> > goto err;
> > + }
> >
> > /* calculate inverse of q mod p */
> > - if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME)) {
> > - p = &local_p;
> > - BN_with_flags(p, rsa->p, BN_FLG_CONSTTIME);
> > - } else
> > - p = rsa->p;
> > - if (!BN_mod_inverse(rsa->iqmp, rsa->q, p, ctx))
> > + if (p == NULL)
> > + goto err;
> > + BN_with_flags(p, rsa->p, BN_FLG_CONSTTIME);
> > + if (!BN_mod_inverse(rsa->iqmp, rsa->q, p, ctx)) {
> > + BN_free(p);
> > goto err;
> > + }
> >
> > ok = 1;
> > err:
> >
>
