From 39df51522ba2e3773ae2f1d4df5a6031ef41c1ba Mon Sep 17 00:00:00 2001 From: Billy Brumley Date: Mon, 23 Apr 2018 14:34:11 +0300 Subject: [PATCH] Remove superfluous NULL checks. Add Andy's BN_FLG comment. Reviewed-by: Andy Polyakov Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/6009) --- crypto/bn/bn_lib.c | 25 ++++++++++++++++++++----- crypto/ec/ec_mult.c | 5 ----- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c index a446880ec7..91553d4391 100644 --- a/crypto/bn/bn_lib.c +++ b/crypto/bn/bn_lib.c @@ -743,12 +743,27 @@ void BN_consttime_swap(BN_ULONG condition, BIGNUM *a, BIGNUM *b, int nwords) a->neg ^= t; b->neg ^= t; - /* - * cannot just arbitrarily swap flags. - * The way a->d is allocated etc. - * BN_FLG_MALLOCED, BN_FLG_STATIC_DATA, ... + /*- + * Idea behind BN_FLG_STATIC_DATA is actually to + * indicate that data may not be written to. + * Intention is actually to treat it as it's + * read-only data, and some (if not most) of it does + * reside in read-only segment. In other words + * observation of BN_FLG_STATIC_DATA in + * BN_consttime_swap should be treated as fatal + * condition. It would either cause SEGV or + * effectively cause data corruption. + * BN_FLG_MALLOCED refers to BN structure itself, + * and hence must be preserved. Remaining flags are + * BN_FLG_CONSTIME and BN_FLG_SECURE. Latter must be + * preserved, because it determines how x->d was + * allocated and hence how to free it. This leaves + * BN_FLG_CONSTTIME that one can do something about. + * To summarize it's sufficient to mask and swap + * BN_FLG_CONSTTIME alone. BN_FLG_STATIC_DATA should + * be treated as fatal. */ - t = (a->flags ^ b->flags) & condition & BN_FLG_CONSTTIME; + t = ((a->flags ^ b->flags) & BN_FLG_CONSTTIME) & condition; a->flags ^= t; b->flags ^= t; diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c index 1ed7449228..0779e4f7bb 100644 --- a/crypto/ec/ec_mult.c +++ b/crypto/ec/ec_mult.c @@ -142,9 +142,6 @@ static int ec_mul_consttime(const EC_GROUP *group, EC_POINT *r, if (ctx == NULL && (ctx = new_ctx = BN_CTX_secure_new()) == NULL) goto err; - if ((group->order == NULL) || (group->field == NULL)) - goto err; - order_bits = BN_num_bits(group->order); s = EC_POINT_new(group); @@ -152,8 +149,6 @@ static int ec_mul_consttime(const EC_GROUP *group, EC_POINT *r, goto err; if (point == NULL) { - if (group->generator == NULL) - goto err; if (!EC_POINT_copy(s, group->generator)) goto err; } else { -- 2.25.1