[openssl-commits] [openssl] OpenSSL_1_1_0-stable update
Andy Polyakov
appro at openssl.org
Fri Jul 27 12:51:20 UTC 2018
The branch OpenSSL_1_1_0-stable has been updated
via 9da6f31c7e61b484dda6c0a59d46c76410981e13 (commit)
via ed04bcf67426888e8f8556b9eb37e9e2cf4eb04b (commit)
via e1c495db1d48c4a8c467d4a5e692e991528d8618 (commit)
via 63ad27165f9abd4f9e55d1a2e8c9bbdb01073a4f (commit)
via 6040bd3f7109dcae508c3194232e7b8ee8654dc0 (commit)
from 2f19065bd35dc84492c4c47ff5b706340300866f (commit)
- Log -----------------------------------------------------------------
commit 9da6f31c7e61b484dda6c0a59d46c76410981e13
Author: Andy Polyakov <appro at openssl.org>
Date: Thu Jul 26 14:38:53 2018 +0200
CHANGES: mention blinding reverting in ECDSA.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6796)
commit ed04bcf67426888e8f8556b9eb37e9e2cf4eb04b
Author: Andy Polyakov <appro at openssl.org>
Date: Wed Jul 25 10:29:51 2018 +0200
bn/bn_mod.c: harmonize BN_mod_add_quick with original implementation.
New implementation failed to correctly reset r->neg flag. Spotted by
OSSFuzz.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6796)
(cherry picked from commit 70a579ae2f37437a1e02331eeaa84e1b68ba021e)
commit e1c495db1d48c4a8c467d4a5e692e991528d8618
Author: Andy Polyakov <appro at openssl.org>
Date: Thu Jul 12 22:27:43 2018 +0200
ec/ecdsa_ossl.c: switch to fixed-length Montgomery multiplication.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6796)
(cherry picked from commit 37132c9702328940a99b1307f742ab094ef754a7)
commit 63ad27165f9abd4f9e55d1a2e8c9bbdb01073a4f
Author: Andy Polyakov <appro at openssl.org>
Date: Fri Jul 6 16:13:29 2018 +0200
ec/ecdsa_ossl.c: formatting and readability fixes.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6796)
(cherry picked from commit fff7a0dcf6e3135c7f93e6cb5fb35e37dd0b384d)
commit 6040bd3f7109dcae508c3194232e7b8ee8654dc0
Author: Andy Polyakov <appro at openssl.org>
Date: Fri Jul 6 15:55:34 2018 +0200
ec/ecdsa_ossl.c: revert blinding in ECDSA signature.
Originally suggested solution for "Return Of the Hidden Number Problem"
is arguably too expensive. While it has marginal impact on slower
curves, none to ~6%, optimized implementations suffer real penalties.
Most notably sign with P-256 went more than 2 times[!] slower. Instead,
just implement constant-time BN_mod_add_quick.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6796)
(cherry picked from commit 3fc7a9b96cbed0c3da6f53c08e34d8d0c982745f)
Resolved conflicts:
crypto/ec/ecdsa_ossl.c
-----------------------------------------------------------------------
Summary of changes:
CHANGES | 4 ++
crypto/bn/bn_mod.c | 67 ++++++++++++++++++--
crypto/ec/ecdsa_ossl.c | 130 +++++++++++----------------------------
crypto/include/internal/bn_int.h | 2 +
4 files changed, 104 insertions(+), 99 deletions(-)
diff --git a/CHANGES b/CHANGES
index 148960a..277654d 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,10 @@
Changes between 1.1.0h and 1.1.0i [xx XXX xxxx]
+ *) Revert blinding in ECDSA sign and instead make problematic addition
+ length-invariant. Switch even to fixed-length Montgomery multiplication.
+ [Andy Polyakov]
+
*) Change generating and checking of primes so that the error rate of not
being prime depends on the intended use based on the size of the input.
For larger primes this will result in more rounds of Miller-Rabin.
diff --git a/crypto/bn/bn_mod.c b/crypto/bn/bn_mod.c
index 13b583f..e33e3f3 100644
--- a/crypto/bn/bn_mod.c
+++ b/crypto/bn/bn_mod.c
@@ -35,18 +35,73 @@ int BN_mod_add(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, const BIGNUM *m,
/*
* BN_mod_add variant that may be used if both a and b are non-negative and
- * less than m
+ * less than m. The original algorithm was
+ *
+ * if (!BN_uadd(r, a, b))
+ * return 0;
+ * if (BN_ucmp(r, m) >= 0)
+ * return BN_usub(r, r, m);
+ *
+ * which is replaced with addition, subtracting modulus, and conditional
+ * move depending on whether or not subtraction borrowed.
*/
-int BN_mod_add_quick(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
- const BIGNUM *m)
+int bn_mod_add_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
+ const BIGNUM *m)
{
- if (!BN_uadd(r, a, b))
+ size_t i, ai, bi, mtop = m->top;
+ BN_ULONG storage[1024 / BN_BITS2];
+ BN_ULONG carry, temp, mask, *rp, *tp = storage;
+ const BN_ULONG *ap, *bp;
+
+ if (bn_wexpand(r, mtop) == NULL)
return 0;
- if (BN_ucmp(r, m) >= 0)
- return BN_usub(r, r, m);
+
+ if (mtop > sizeof(storage) / sizeof(storage[0])
+ && (tp = OPENSSL_malloc(mtop * sizeof(BN_ULONG))) == NULL)
+ return 0;
+
+ ap = a->d != NULL ? a->d : tp;
+ bp = b->d != NULL ? b->d : tp;
+
+ for (i = 0, ai = 0, bi = 0, carry = 0; i < mtop;) {
+ mask = (BN_ULONG)0 - ((i - a->top) >> (8 * sizeof(i) - 1));
+ temp = ((ap[ai] & mask) + carry) & BN_MASK2;
+ carry = (temp < carry);
+
+ mask = (BN_ULONG)0 - ((i - b->top) >> (8 * sizeof(i) - 1));
+ tp[i] = ((bp[bi] & mask) + temp) & BN_MASK2;
+ carry += (tp[i] < temp);
+
+ i++;
+ ai += (i - a->dmax) >> (8 * sizeof(i) - 1);
+ bi += (i - b->dmax) >> (8 * sizeof(i) - 1);
+ }
+ rp = r->d;
+ carry -= bn_sub_words(rp, tp, m->d, mtop);
+ for (i = 0; i < mtop; i++) {
+ rp[i] = (carry & tp[i]) | (~carry & rp[i]);
+ ((volatile BN_ULONG *)tp)[i] = 0;
+ }
+ r->top = mtop;
+ r->neg = 0;
+
+ if (tp != storage)
+ OPENSSL_free(tp);
+
return 1;
}
+int BN_mod_add_quick(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
+ const BIGNUM *m)
+{
+ int ret = bn_mod_add_fixed_top(r, a, b, m);
+
+ if (ret)
+ bn_correct_top(r);
+
+ return ret;
+}
+
int BN_mod_sub(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, const BIGNUM *m,
BN_CTX *ctx)
{
diff --git a/crypto/ec/ecdsa_ossl.c b/crypto/ec/ecdsa_ossl.c
index c103917..bf6f1bc 100644
--- a/crypto/ec/ecdsa_ossl.c
+++ b/crypto/ec/ecdsa_ossl.c
@@ -10,9 +10,8 @@
#include <string.h>
#include <openssl/err.h>
#include <openssl/obj_mac.h>
-#include <openssl/bn.h>
#include <openssl/rand.h>
-#include <openssl/ec.h>
+#include "internal/bn_int.h"
#include "ec_lcl.h"
int ossl_ecdsa_sign(int type, const unsigned char *dgst, int dlen,
@@ -53,13 +52,12 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in,
return 0;
}
- if (ctx_in == NULL) {
+ if ((ctx = ctx_in) == NULL) {
if ((ctx = BN_CTX_new()) == NULL) {
ECerr(EC_F_ECDSA_SIGN_SETUP, ERR_R_MALLOC_FAILURE);
return 0;
}
- } else
- ctx = ctx_in;
+ }
k = BN_new(); /* this value is later returned in *kinvp */
r = BN_new(); /* this value is later returned in *rp */
@@ -73,10 +71,6 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in,
goto err;
}
order = EC_GROUP_get0_order(group);
- if (order == NULL) {
- ECerr(EC_F_ECDSA_SIGN_SETUP, ERR_R_EC_LIB);
- goto err;
- }
/* Preallocate space */
order_bits = BN_num_bits(order);
@@ -87,23 +81,23 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in,
do {
/* get random k */
- do
+ do {
if (dgst != NULL) {
- if (!BN_generate_dsa_nonce
- (k, order, EC_KEY_get0_private_key(eckey), dgst, dlen,
- ctx)) {
+ if (!BN_generate_dsa_nonce(k, order,
+ EC_KEY_get0_private_key(eckey),
+ dgst, dlen, ctx)) {
ECerr(EC_F_ECDSA_SIGN_SETUP,
- EC_R_RANDOM_NUMBER_GENERATION_FAILED);
+ EC_R_RANDOM_NUMBER_GENERATION_FAILED);
goto err;
}
} else {
if (!BN_rand_range(k, order)) {
ECerr(EC_F_ECDSA_SIGN_SETUP,
- EC_R_RANDOM_NUMBER_GENERATION_FAILED);
+ EC_R_RANDOM_NUMBER_GENERATION_FAILED);
goto err;
}
}
- while (BN_is_zero(k));
+ } while (BN_is_zero(k));
/*
* We do not want timing information to leak the length of k, so we
@@ -129,18 +123,16 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in,
}
if (EC_METHOD_get_field_type(EC_GROUP_method_of(group)) ==
NID_X9_62_prime_field) {
- if (!EC_POINT_get_affine_coordinates_GFp
- (group, tmp_point, X, NULL, ctx)) {
+ if (!EC_POINT_get_affine_coordinates_GFp(group, tmp_point, X,
+ NULL, ctx)) {
ECerr(EC_F_ECDSA_SIGN_SETUP, ERR_R_EC_LIB);
goto err;
}
}
#ifndef OPENSSL_NO_EC2M
else { /* NID_X9_62_characteristic_two_field */
-
- if (!EC_POINT_get_affine_coordinates_GF2m(group,
- tmp_point, X, NULL,
- ctx)) {
+ if (!EC_POINT_get_affine_coordinates_GF2m(group, tmp_point, X,
+ NULL, ctx)) {
ECerr(EC_F_ECDSA_SIGN_SETUP, ERR_R_EC_LIB);
goto err;
}
@@ -150,8 +142,7 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in,
ECerr(EC_F_ECDSA_SIGN_SETUP, ERR_R_BN_LIB);
goto err;
}
- }
- while (BN_is_zero(r));
+ } while (BN_is_zero(r));
/* compute the inverse of k */
if (EC_GROUP_get_mont_data(group) != NULL) {
@@ -210,8 +201,7 @@ ECDSA_SIG *ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len,
EC_KEY *eckey)
{
int ok = 0, i;
- BIGNUM *kinv = NULL, *s, *m = NULL, *tmp = NULL, *blind = NULL;
- BIGNUM *blindm = NULL;
+ BIGNUM *kinv = NULL, *s, *m = NULL;
const BIGNUM *order, *ckinv;
BN_CTX *ctx = NULL;
const EC_GROUP *group;
@@ -244,27 +234,13 @@ ECDSA_SIG *ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len,
}
s = ret->s;
- ctx = BN_CTX_secure_new();
- if (ctx == NULL) {
- ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_MALLOC_FAILURE);
- goto err;
- }
-
- BN_CTX_start(ctx);
- tmp = BN_CTX_get(ctx);
- m = BN_CTX_get(ctx);
- blind = BN_CTX_get(ctx);
- blindm = BN_CTX_get(ctx);
- if (blindm == NULL) {
+ if ((ctx = BN_CTX_new()) == NULL
+ || (m = BN_new()) == NULL) {
ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_MALLOC_FAILURE);
goto err;
}
order = EC_GROUP_get0_order(group);
- if (order == NULL) {
- ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_EC_LIB);
- goto err;
- }
i = BN_num_bits(order);
/*
* Need to truncate digest if it is too long: first truncate whole bytes.
@@ -275,7 +251,7 @@ ECDSA_SIG *ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len,
ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_BN_LIB);
goto err;
}
- /* If still too long truncate remaining bits with a shift */
+ /* If still too long, truncate remaining bits with a shift */
if ((8 * dgst_len > i) && !BN_rshift(m, m, 8 - (i & 0x7))) {
ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_BN_LIB);
goto err;
@@ -296,59 +272,27 @@ ECDSA_SIG *ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len,
}
/*
- * The normal signature calculation is:
- *
- * s := k^-1 * (m + r * priv_key) mod order
- *
- * We will blind this to protect against side channel attacks
- *
- * s := blind^-1 * k^-1 * (blind * m + blind * r * priv_key) mod order
+ * With only one multiplicant being in Montgomery domain
+ * multiplication yields real result without post-conversion.
+ * Also note that all operations but last are performed with
+ * zero-padded vectors. Last operation, BN_mod_mul_montgomery
+ * below, returns user-visible value with removed zero padding.
*/
-
- /* Generate a blinding value */
- do {
- if (!BN_rand(blind, BN_num_bits(order) - 1, BN_RAND_TOP_ANY,
- BN_RAND_BOTTOM_ANY))
- goto err;
- } while (BN_is_zero(blind));
- BN_set_flags(blind, BN_FLG_CONSTTIME);
- BN_set_flags(blindm, BN_FLG_CONSTTIME);
- BN_set_flags(tmp, BN_FLG_CONSTTIME);
-
- /* tmp := blind * priv_key * r mod order */
- if (!BN_mod_mul(tmp, blind, priv_key, order, ctx)) {
- ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_BN_LIB);
- goto err;
- }
- if (!BN_mod_mul(tmp, tmp, ret->r, order, ctx)) {
- ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_BN_LIB);
- goto err;
- }
-
- /* blindm := blind * m mod order */
- if (!BN_mod_mul(blindm, blind, m, order, ctx)) {
- ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_BN_LIB);
- goto err;
- }
-
- /* s : = (blind * priv_key * r) + (blind * m) mod order */
- if (!BN_mod_add_quick(s, tmp, blindm, order)) {
- ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_BN_LIB);
- goto err;
- }
-
- /* s := s * k^-1 mod order */
- if (!BN_mod_mul(s, s, ckinv, order, ctx)) {
+ if (!bn_to_mont_fixed_top(s, ret->r, group->mont_data, ctx)
+ || !bn_mul_mont_fixed_top(s, s, priv_key, group->mont_data, ctx)) {
ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_BN_LIB);
goto err;
}
-
- /* s:= s * blind^-1 mod order */
- if (BN_mod_inverse(blind, blind, order, ctx) == NULL) {
+ if (!bn_mod_add_fixed_top(s, s, m, order)) {
ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_BN_LIB);
goto err;
}
- if (!BN_mod_mul(s, s, blind, order, ctx)) {
+ /*
+ * |s| can still be larger than modulus, because |m| can be. In
+ * such case we count on Montgomery reduction to tie it up.
+ */
+ if (!bn_to_mont_fixed_top(s, s, group->mont_data, ctx)
+ || !BN_mod_mul_montgomery(s, s, ckinv, group->mont_data, ctx)) {
ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, ERR_R_BN_LIB);
goto err;
}
@@ -362,11 +306,11 @@ ECDSA_SIG *ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len,
ECerr(EC_F_OSSL_ECDSA_SIGN_SIG, EC_R_NEED_NEW_SETUP_VALUES);
goto err;
}
- } else
+ } else {
/* s != 0 => we have a valid signature */
break;
- }
- while (1);
+ }
+ } while (1);
ok = 1;
err:
@@ -374,8 +318,8 @@ ECDSA_SIG *ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len,
ECDSA_SIG_free(ret);
ret = NULL;
}
- BN_CTX_end(ctx);
BN_CTX_free(ctx);
+ BN_clear_free(m);
BN_clear_free(kinv);
return ret;
}
diff --git a/crypto/include/internal/bn_int.h b/crypto/include/internal/bn_int.h
index 3501ffb..32eb581 100644
--- a/crypto/include/internal/bn_int.h
+++ b/crypto/include/internal/bn_int.h
@@ -85,6 +85,8 @@ int bn_mul_mont_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
BN_MONT_CTX *mont, BN_CTX *ctx);
int bn_to_mont_fixed_top(BIGNUM *r, const BIGNUM *a, BN_MONT_CTX *mont,
BN_CTX *ctx);
+int bn_mod_add_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
+ const BIGNUM *m);
#ifdef __cplusplus
}
More information about the openssl-commits
mailing list