diff options
author | Bodo Möller <bodo@openssl.org> | 2008-09-14 13:42:34 +0000 |
---|---|---|
committer | Bodo Möller <bodo@openssl.org> | 2008-09-14 13:42:34 +0000 |
commit | f8d6be3f8170c4aa3bea1618994f912629f3d0c3 (patch) | |
tree | 18c4c5d270339284cc408289fff7cbf6994b6163 | |
parent | d493899579429374026bc11c60859d5af9c2a0dc (diff) |
Some precautions to avoid potential security-relevant problems.
-rw-r--r-- | CHANGES | 16 | ||||
-rw-r--r-- | crypto/bn/bn_div.c | 15 | ||||
-rw-r--r-- | crypto/bn/bn_nist.c | 183 | ||||
-rw-r--r-- | crypto/md32_common.h | 2 | ||||
-rw-r--r-- | ssl/ssl_asn1.c | 2 |
5 files changed, 165 insertions, 53 deletions
@@ -705,6 +705,22 @@ Changes between 0.9.8h and 0.9.8i [xx XXX xxxx] + *) Various precautionary measures: + + - Avoid size_t integer overflow in HASH_UPDATE (md32_common.h). + + - Avoid a buffer overflow in d2i_SSL_SESSION() (ssl_asn1.c). + (NB: This would require knowledge of the secret session ticket key + to exploit, in which case you'd be SOL either way.) + + - Change bn_nist.c so that it will properly handle input BIGNUMs + outside the expected range. + + - Enforce the 'num' check in BN_div() (bn_div.c) for non-BN_DEBUG + builds. + + [Neel Mehta, Bodo Moeller] + *) Add support for Local Machine Keyset attribute in PKCS#12 files. [Steve Henson] diff --git a/crypto/bn/bn_div.c b/crypto/bn/bn_div.c index 8adf21430e..6db472f8c3 100644 --- a/crypto/bn/bn_div.c +++ b/crypto/bn/bn_div.c @@ -187,6 +187,17 @@ int BN_div(BIGNUM *dv, BIGNUM *rm, const BIGNUM *num, const BIGNUM *divisor, BN_ULONG d0,d1; int num_n,div_n; + /* Invalid zero-padding would have particularly bad consequences + * in the case of 'num', so don't just rely on bn_check_top() for this one + * (bn_check_top() works only for BN_DEBUG builds) */ + if (num->top > 0 && num->d[num->top - 1] == 0) + { + BNerr(BN_F_BN_DIV,BN_R_NOT_INITIALIZED); + return 0; + } + + bn_check_top(num); + if ((BN_get_flags(num, BN_FLG_CONSTTIME) != 0) || (BN_get_flags(divisor, BN_FLG_CONSTTIME) != 0)) { return BN_div_no_branch(dv, rm, num, divisor, ctx); @@ -194,7 +205,7 @@ int BN_div(BIGNUM *dv, BIGNUM *rm, const BIGNUM *num, const BIGNUM *divisor, bn_check_top(dv); bn_check_top(rm); - bn_check_top(num); + /* bn_check_top(num); */ /* 'num' has been checked already */ bn_check_top(divisor); if (BN_is_zero(divisor)) @@ -422,7 +433,7 @@ static int BN_div_no_branch(BIGNUM *dv, BIGNUM *rm, const BIGNUM *num, bn_check_top(dv); bn_check_top(rm); - bn_check_top(num); + /* bn_check_top(num); */ /* 'num' has been checked in BN_div() */ bn_check_top(divisor); if (BN_is_zero(divisor)) diff --git a/crypto/bn/bn_nist.c b/crypto/bn/bn_nist.c index 9419f3b58d..bd04e7e29d 100644 --- a/crypto/bn/bn_nist.c +++ b/crypto/bn/bn_nist.c @@ -59,6 +59,7 @@ #include "bn_lcl.h" #include "cryptlib.h" + #define BN_NIST_192_TOP (192+BN_BITS2-1)/BN_BITS2 #define BN_NIST_224_TOP (224+BN_BITS2-1)/BN_BITS2 #define BN_NIST_256_TOP (256+BN_BITS2-1)/BN_BITS2 @@ -152,60 +153,98 @@ static const BN_ULONG _nist_p_521[] = {0xFFFFFFFF,0xFFFFFFFF,0xFFFFFFFF, #error "unsupported BN_BITS2" #endif + +static const BIGNUM _bignum_nist_p_192 = + { + (BN_ULONG *)_nist_p_192[0], + BN_NIST_192_TOP, + BN_NIST_192_TOP, + 0, + BN_FLG_STATIC_DATA + }; + +static const BIGNUM _bignum_nist_p_224 = + { + (BN_ULONG *)_nist_p_224[0], + BN_NIST_224_TOP, + BN_NIST_224_TOP, + 0, + BN_FLG_STATIC_DATA + }; + +static const BIGNUM _bignum_nist_p_256 = + { + (BN_ULONG *)_nist_p_256[0], + BN_NIST_256_TOP, + BN_NIST_256_TOP, + 0, + BN_FLG_STATIC_DATA + }; + +static const BIGNUM _bignum_nist_p_384 = + { + (BN_ULONG *)_nist_p_384[0], + BN_NIST_384_TOP, + BN_NIST_384_TOP, + 0, + BN_FLG_STATIC_DATA + }; + +static const BIGNUM _bignum_nist_p_521 = + { + (BN_ULONG *)_nist_p_521, + BN_NIST_521_TOP, + BN_NIST_521_TOP, + 0, + BN_FLG_STATIC_DATA + }; + + const BIGNUM *BN_get0_nist_prime_192(void) { - static BIGNUM const_nist_192 = { (BN_ULONG *)_nist_p_192[0], - BN_NIST_192_TOP, BN_NIST_192_TOP, 0, BN_FLG_STATIC_DATA }; - return &const_nist_192; + return &_bignum_nist_p_192; } const BIGNUM *BN_get0_nist_prime_224(void) { - static BIGNUM const_nist_224 = { (BN_ULONG *)_nist_p_224[0], - BN_NIST_224_TOP, BN_NIST_224_TOP, 0, BN_FLG_STATIC_DATA }; - return &const_nist_224; + return &_bignum_nist_p_224; } const BIGNUM *BN_get0_nist_prime_256(void) { - static BIGNUM const_nist_256 = { (BN_ULONG *)_nist_p_256[0], - BN_NIST_256_TOP, BN_NIST_256_TOP, 0, BN_FLG_STATIC_DATA }; - return &const_nist_256; + return &_bignum_nist_p_256; } const BIGNUM *BN_get0_nist_prime_384(void) { - static BIGNUM const_nist_384 = { (BN_ULONG *)_nist_p_384[0], - BN_NIST_384_TOP, BN_NIST_384_TOP, 0, BN_FLG_STATIC_DATA }; - return &const_nist_384; + return &_bignum_nist_p_384; } const BIGNUM *BN_get0_nist_prime_521(void) { - static BIGNUM const_nist_521 = { (BN_ULONG *)_nist_p_521, - BN_NIST_521_TOP, BN_NIST_521_TOP, 0, BN_FLG_STATIC_DATA }; - return &const_nist_521; + return &_bignum_nist_p_521; } -#define BN_NIST_ADD_ONE(a) while (!(*(a)=(*(a)+1)&BN_MASK2)) ++(a); static void nist_cp_bn_0(BN_ULONG *buf, BN_ULONG *a, int top, int max) - { + { int i; - BN_ULONG *_tmp1 = (buf), *_tmp2 = (a); - for (i = (top); i != 0; i--) - *_tmp1++ = *_tmp2++; - for (i = (max) - (top); i != 0; i--) - *_tmp1++ = (BN_ULONG) 0; - } + BN_ULONG *_tmp1 = (buf), *_tmp2 = (a); + + OPENSSL_assert(top <= max); + for (i = (top); i != 0; i--) + *_tmp1++ = *_tmp2++; + for (i = (max) - (top); i != 0; i--) + *_tmp1++ = (BN_ULONG) 0; + } static void nist_cp_bn(BN_ULONG *buf, BN_ULONG *a, int top) - { + { int i; - BN_ULONG *_tmp1 = (buf), *_tmp2 = (a); - for (i = (top); i != 0; i--) - *_tmp1++ = *_tmp2++; - } + BN_ULONG *_tmp1 = (buf), *_tmp2 = (a); + for (i = (top); i != 0; i--) + *_tmp1++ = *_tmp2++; + } #if BN_BITS2 == 64 #define bn_cp_64(to, n, from, m) (to)[n] = (m>=0)?((from)[m]):0; @@ -255,6 +294,11 @@ int BN_nist_mod_192(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, *res; size_t mask; + field = &_bignum_nist_p_192; /* just to make sure */ + + if (BN_is_negative(a) || a->top > 2*BN_NIST_192_TOP) + return BN_nnmod(r, field, a, ctx); + i = BN_ucmp(field, a); if (i == 0) { @@ -264,9 +308,6 @@ int BN_nist_mod_192(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, else if (i > 0) return (r == a) ? 1 : (BN_copy(r ,a) != NULL); - if (top == BN_NIST_192_TOP) - return BN_usub(r, a, field); - if (r != a) { if (!bn_wexpand(r, BN_NIST_192_TOP)) @@ -304,6 +345,11 @@ int BN_nist_mod_192(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, r->top = BN_NIST_192_TOP; bn_correct_top(r); + if (BN_ucmp(field, r) <= 0) + { + if (!BN_usub(r, r, field)) return 0; + } + return 1; } @@ -333,6 +379,11 @@ int BN_nist_mod_224(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, size_t mask; union { bn_addsub_f f; size_t p; } u; + field = &_bignum_nist_p_224; /* just to make sure */ + + if (BN_is_negative(a) || a->top > 2*BN_NIST_224_TOP) + return BN_nnmod(r, field, a, ctx); + i = BN_ucmp(field, a); if (i == 0) { @@ -342,9 +393,6 @@ int BN_nist_mod_224(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, else if (i > 0) return (r == a)? 1 : (BN_copy(r ,a) != NULL); - if (top == BN_NIST_224_TOP) - return BN_usub(r, a, field); - if (r != a) { if (!bn_wexpand(r, BN_NIST_224_TOP)) @@ -408,6 +456,11 @@ int BN_nist_mod_224(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, r->top = BN_NIST_224_TOP; bn_correct_top(r); + if (BN_ucmp(field, r) <= 0) + { + if (!BN_usub(r, r, field)) return 0; + } + return 1; } @@ -436,6 +489,11 @@ int BN_nist_mod_256(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, size_t mask; union { bn_addsub_f f; size_t p; } u; + field = &_bignum_nist_p_256; /* just to make sure */ + + if (BN_is_negative(a) || a->top > 2*BN_NIST_256_TOP) + return BN_nnmod(r, field, a, ctx); + i = BN_ucmp(field, a); if (i == 0) { @@ -445,9 +503,6 @@ int BN_nist_mod_256(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, else if (i > 0) return (r == a)? 1 : (BN_copy(r ,a) != NULL); - if (top == BN_NIST_256_TOP) - return BN_usub(r, a, field); - if (r != a) { if (!bn_wexpand(r, BN_NIST_256_TOP)) @@ -519,6 +574,11 @@ int BN_nist_mod_256(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, r->top = BN_NIST_256_TOP; bn_correct_top(r); + if (BN_ucmp(field, r) <= 0) + { + if (!BN_usub(r, r, field)) return 0; + } + return 1; } @@ -551,6 +611,11 @@ int BN_nist_mod_384(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, size_t mask; union { bn_addsub_f f; size_t p; } u; + field = &_bignum_nist_p_384; /* just to make sure */ + + if (BN_is_negative(a) || a->top > 2*BN_NIST_384_TOP) + return BN_nnmod(r, field, a, ctx); + i = BN_ucmp(field, a); if (i == 0) { @@ -560,9 +625,6 @@ int BN_nist_mod_384(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, else if (i > 0) return (r == a)? 1 : (BN_copy(r ,a) != NULL); - if (top == BN_NIST_384_TOP) - return BN_usub(r, a, field); - if (r != a) { if (!bn_wexpand(r, BN_NIST_384_TOP)) @@ -636,6 +698,11 @@ int BN_nist_mod_384(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, r->top = BN_NIST_384_TOP; bn_correct_top(r); + if (BN_ucmp(field, r) <= 0) + { + if (!BN_usub(r, r, field)) return 0; + } + return 1; } @@ -651,11 +718,33 @@ int BN_nist_mod_521(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, BN_ULONG *r_d; BIGNUM *tmp; + field = &_bignum_nist_p_521; /* just to make sure */ + + if (BN_is_negative(a)) + return BN_nnmod(r, field, a, ctx); + /* check whether a reduction is necessary */ top = a->top; if (top < BN_NIST_521_TOP || ( top == BN_NIST_521_TOP && - (!(a->d[BN_NIST_521_TOP-1] & ~(BN_NIST_521_TOP_MASK))))) - return (r == a)? 1 : (BN_copy(r ,a) != NULL); + (!(a->d[BN_NIST_521_TOP-1] & ~(BN_NIST_521_TOP_MASK))))) + { + int i = BN_ucmp(field, a); + if (i == 0) + { + BN_zero(r); + return 1; + } + else + { +#ifdef BN_DEBUG + OPENSSL_assert(i > 0); /* because 'field' is 1111...1111 */ +#endif + return (r == a)? 1 : (BN_copy(r ,a) != NULL); + } + } + + if (BN_num_bits(a) > 2*521) + return BN_nnmod(r, field, a, ctx); BN_CTX_start(ctx); tmp = BN_CTX_get(ctx); @@ -675,15 +764,11 @@ int BN_nist_mod_521(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, if (!BN_uadd(r, tmp, r)) goto err; - top = r->top; - r_d = r->d; - if (top == BN_NIST_521_TOP && - (r_d[BN_NIST_521_TOP-1] & ~(BN_NIST_521_TOP_MASK))) + + if (BN_ucmp(field, r) <= 0) { - BN_NIST_ADD_ONE(r_d) - r->d[BN_NIST_521_TOP-1] &= BN_NIST_521_TOP_MASK; + if (!BN_usub(r, r, field)) goto err; } - bn_correct_top(r); ret = 1; err: diff --git a/crypto/md32_common.h b/crypto/md32_common.h index 089c450290..61bcd9786f 100644 --- a/crypto/md32_common.h +++ b/crypto/md32_common.h @@ -301,7 +301,7 @@ int HASH_UPDATE (HASH_CTX *c, const void *data_, size_t len) { p=(unsigned char *)c->data; - if ((n+len) >= HASH_CBLOCK) + if (len >= HASH_CBLOCK || len+n >= HASH_CBLOCK) { memcpy (p+n,data,HASH_CBLOCK-n); HASH_BLOCK_DATA_ORDER (c,p,1); diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c index 8630d3ecee..8c4840219b 100644 --- a/ssl/ssl_asn1.c +++ b/ssl/ssl_asn1.c @@ -413,7 +413,7 @@ SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const unsigned char **pp, memcpy(ret->session_id,os.data,os.length); M_ASN1_D2I_get_x(ASN1_OCTET_STRING,osp,d2i_ASN1_OCTET_STRING); - if (ret->master_key_length > SSL_MAX_MASTER_KEY_LENGTH) + if (os.length > SSL_MAX_MASTER_KEY_LENGTH) ret->master_key_length=SSL_MAX_MASTER_KEY_LENGTH; else ret->master_key_length=os.length; |