diff options
author | Billy Brumley <bbrumley@gmail.com> | 2018-11-09 09:25:43 +0200 |
---|---|---|
committer | Nicola Tuveri <nic.tuv@gmail.com> | 2018-11-10 04:14:11 +0200 |
commit | 6f172154f5d389a9e52583c6564b2693cb631e7f (patch) | |
tree | 483b353298f21d1bcf83c3c6b2027aaab318df72 /crypto/bn | |
parent | bdb8897691942931184c38a9ab7b69a0977829b7 (diff) |
[crypto/bn] swap BN_FLG_FIXED_TOP too
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/7599)
(cherry picked from commit dd41956d80686638d74fd203bd67060f90966280)
Diffstat (limited to 'crypto/bn')
-rw-r--r-- | crypto/bn/bn_lib.c | 42 |
1 files changed, 23 insertions, 19 deletions
diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c index 266a3dd304..80f910c807 100644 --- a/crypto/bn/bn_lib.c +++ b/crypto/bn/bn_lib.c @@ -767,26 +767,30 @@ void BN_consttime_swap(BN_ULONG condition, BIGNUM *a, BIGNUM *b, int nwords) b->neg ^= t; /*- - * 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. + * BN_FLG_STATIC_DATA: indicates 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. + * + * BN_FLG_SECURE: must be preserved, because it determines how x->d was + * allocated and hence how to free it. + * + * BN_FLG_CONSTTIME: sufficient to mask and swap + * + * BN_FLG_FIXED_TOP: indicates that we haven't called bn_correct_top() on + * the data, so the d array may be padded with additional 0 values (i.e. + * top could be greater than the minimal value that it could be). We should + * be swapping it */ - t = ((a->flags ^ b->flags) & BN_FLG_CONSTTIME) & condition; + +#define BN_CONSTTIME_SWAP_FLAGS (BN_FLG_CONSTTIME | BN_FLG_FIXED_TOP) + + t = ((a->flags ^ b->flags) & BN_CONSTTIME_SWAP_FLAGS) & condition; a->flags ^= t; b->flags ^= t; |