Introduce limits to prevent malicious keys being able to
authorMark J. Cox <mark@openssl.org>
Thu, 28 Sep 2006 11:29:03 +0000 (11:29 +0000)
committerMark J. Cox <mark@openssl.org>
Thu, 28 Sep 2006 11:29:03 +0000 (11:29 +0000)
cause a denial of service.  (CVE-2006-2940)
[Steve Henson, Bodo Moeller]

Fix ASN.1 parsing of certain invalid structures that can result
in a denial of service.  (CVE-2006-2937)  [Steve Henson]

Fix buffer overflow in SSL_get_shared_ciphers() function.
(CVE-2006-3738) [Tavis Ormandy and Will Drewry, Google Security Team]

Fix SSL client code which could crash if connecting to a
malicious SSLv2 server.  (CVE-2006-4343)
[Tavis Ormandy and Will Drewry, Google Security Team]

18 files changed:
CHANGES
NEWS
crypto/asn1/tasn_dec.c
crypto/dh/dh.h
crypto/dh/dh_err.c
crypto/dh/dh_key.c
crypto/dsa/dsa.h
crypto/dsa/dsa_err.c
crypto/dsa/dsa_ossl.c
crypto/ec/ec.h
crypto/ec/ec_asn1.c
crypto/ec/ec_err.c
crypto/rsa/rsa.h
crypto/rsa/rsa_eay.c
crypto/rsa/rsa_err.c
ssl/s2_clnt.c
ssl/s3_srvr.c
ssl/ssl_lib.c

diff --git a/CHANGES b/CHANGES
index 01642e13e03269b3f45eff44e4fe2ddee126edc4..4e2740be5ebd8fb53daf835080fb250e099bbc28 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,20 @@
 
  Changes between 0.9.8c and 0.9.8d  [xx XXX xxxx]
 
+  *) Introduce limits to prevent malicious keys being able to
+     cause a denial of service.  (CVE-2006-2940)
+     [Steve Henson, Bodo Moeller]
+
+  *) Fix ASN.1 parsing of certain invalid structures that can result
+     in a denial of service.  (CVE-2006-2937)  [Steve Henson]
+
+  *) Fix buffer overflow in SSL_get_shared_ciphers() function. 
+     (CVE-2006-3738) [Tavis Ormandy and Will Drewry, Google Security Team]
+
+  *) Fix SSL client code which could crash if connecting to a
+     malicious SSLv2 server.  (CVE-2006-4343)
+     [Tavis Ormandy and Will Drewry, Google Security Team]
+
   *) Since 0.9.8b, ciphersuite strings naming explicit ciphersuites
      match only those.  Before that, "AES256-SHA" would be interpreted
      as a pattern and match "AES128-SHA" too (since AES128-SHA got
diff --git a/NEWS b/NEWS
index 6bda70f39b3e1ebba96aae04d34e886c092fea0f..ad8033a81b617dd4f6d659e3118ca56359822f2d 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,12 @@
   This file gives a brief overview of the major changes between each OpenSSL
   release. For more details please read the CHANGES file.
 
+  Major changes between OpenSSL 0.9.7c and OpenSSL 0.9.8d:
+
+      o Introduce limits to prevent malicious key DoS  (CVE-2006-2940)
+      o Fix security issues (CVE-2006-2937, CVE-2006-3737, CVE-2006-4343)
+      o Changes to ciphersuite selection algorithm
+
   Major changes between OpenSSL 0.9.8b and OpenSSL 0.9.8c:
 
       o Fix Daniel Bleichenbacher forged signature attack, CVE-2006-4339
index 0294d8e766a4b9b1c207bc1ecb9b4b6904907335..ff2f77b9d8c16b5805bc6a2b02fd45387ea9830b 100644 (file)
@@ -832,6 +832,7 @@ static int asn1_d2i_ex_primitive(ASN1_VALUE **pval,
                }
        else if (ret == -1)
                return -1;
+        ret = 0;
        /* SEQUENCE, SET and "OTHER" are left in encoded form */
        if ((utype == V_ASN1_SEQUENCE)
                || (utype == V_ASN1_SET) || (utype == V_ASN1_OTHER))
index 4d0c5653166fe9182c27c563c830b1e35519bc39..ccdf35ae1c27d20ab0c228ef272105504fc811d8 100644 (file)
 #include <openssl/bn.h>
 #endif
        
+#ifndef OPENSSL_DH_MAX_MODULUS_BITS
+# define OPENSSL_DH_MAX_MODULUS_BITS   10000
+#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
@@ -221,6 +225,7 @@ void ERR_load_DH_strings(void);
 /* Reason codes. */
 #define DH_R_BAD_GENERATOR                              101
 #define DH_R_INVALID_PUBKEY                             102
+#define DH_R_MODULUS_TOO_LARGE                          103
 #define DH_R_NO_PRIVATE_VALUE                           100
 
 #ifdef  __cplusplus
index b14a94f36a43f2eb700805db1a29a9d33081f560..783bb4754c52391fc6ef2967eb3376d9b423ab32 100644 (file)
@@ -84,6 +84,7 @@ static ERR_STRING_DATA DH_str_reasons[]=
        {
 {ERR_REASON(DH_R_BAD_GENERATOR)          ,"bad generator"},
 {ERR_REASON(DH_R_INVALID_PUBKEY)         ,"invalid public key"},
+{ERR_REASON(DH_R_MODULUS_TOO_LARGE)      ,"modulus too large"},
 {ERR_REASON(DH_R_NO_PRIVATE_VALUE)       ,"no private value"},
 {0,NULL}
        };
index 79984e13bc927111d08b81ddb2b98a389f78356b..cb5abdcf47c4a18fd1d15d7d08d7c0f3801617f6 100644 (file)
@@ -179,6 +179,12 @@ static int compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
        int ret= -1;
         int check_result;
 
+       if (BN_num_bits(dh->p) > OPENSSL_DH_MAX_MODULUS_BITS)
+               {
+               DHerr(DH_F_COMPUTE_KEY,DH_R_MODULUS_TOO_LARGE);
+               goto err;
+               }
+
        ctx = BN_CTX_new();
        if (ctx == NULL) goto err;
        BN_CTX_start(ctx);
index b12db98b1303a60fc62284433747c9d4ae3f72ca..3a8fe5b56bbd76a7f5fd7fa1f155cfc032ca7979 100644 (file)
 #endif
 #endif
 
+#ifndef OPENSSL_DSA_MAX_MODULUS_BITS
+# define OPENSSL_DSA_MAX_MODULUS_BITS  10000
+#endif
+
 #define DSA_FLAG_CACHE_MONT_P  0x01
 #define DSA_FLAG_NO_EXP_CONSTTIME       0x02 /* new with 0.9.7h; the built-in DSA
                                               * implementation now uses constant time
@@ -270,8 +274,10 @@ void ERR_load_DSA_strings(void);
 #define DSA_F_SIG_CB                                    114
 
 /* Reason codes. */
+#define DSA_R_BAD_Q_VALUE                               102
 #define DSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE               100
 #define DSA_R_MISSING_PARAMETERS                        101
+#define DSA_R_MODULUS_TOO_LARGE                                 103
 
 #ifdef  __cplusplus
 }
index fd42053572bcc229e280de41b031b5757b66e9bb..d7fac691546d1be08e6c8fb0895e9391aa9af988 100644 (file)
@@ -89,8 +89,10 @@ static ERR_STRING_DATA DSA_str_functs[]=
 
 static ERR_STRING_DATA DSA_str_reasons[]=
        {
+{ERR_REASON(DSA_R_BAD_Q_VALUE)           ,"bad q value"},
 {ERR_REASON(DSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE),"data too large for key size"},
 {ERR_REASON(DSA_R_MISSING_PARAMETERS)    ,"missing parameters"},
+{ERR_REASON(DSA_R_MODULUS_TOO_LARGE)     ,"modulus too large"},
 {0,NULL}
        };
 
index 3fd8a35613d32dce4d252a6c65986df615210fe7..e6aad85825de844d03f71d42c6b3f5dc4a6a19b9 100644 (file)
@@ -304,6 +304,18 @@ static int dsa_do_verify(const unsigned char *dgst, int dgst_len, DSA_SIG *sig,
                return -1;
                }
 
+       if (BN_num_bits(dsa->q) != 160)
+               {
+               DSAerr(DSA_F_DSA_DO_VERIFY,DSA_R_BAD_Q_VALUE);
+               return -1;
+               }
+
+       if (BN_num_bits(dsa->p) > OPENSSL_DSA_MAX_MODULUS_BITS)
+               {
+               DSAerr(DSA_F_DSA_DO_VERIFY,DSA_R_MODULUS_TOO_LARGE);
+               return -1;
+               }
+
        BN_init(&u1);
        BN_init(&u2);
        BN_init(&t1);
index 919c736388dc60d49d7c56aa4bdb646c9841cd1a..3c96fbd0d86d6ec8f31d42da1c3f73e43445e785 100644 (file)
@@ -93,6 +93,10 @@ extern "C" {
 #endif
 
 
+#ifndef OPENSSL_ECC_MAX_FIELD_BITS
+# define OPENSSL_ECC_MAX_FIELD_BITS 661
+#endif
+
 typedef enum {
        /* values as defined in X9.62 (ECDSA) and elsewhere */
        POINT_CONVERSION_COMPRESSED = 2,
@@ -482,6 +486,7 @@ void ERR_load_EC_strings(void);
 #define EC_R_D2I_ECPKPARAMETERS_FAILURE                         117
 #define EC_R_DISCRIMINANT_IS_ZERO                       118
 #define EC_R_EC_GROUP_NEW_BY_NAME_FAILURE               119
+#define EC_R_FIELD_TOO_LARGE                            138
 #define EC_R_GROUP2PKPARAMETERS_FAILURE                         120
 #define EC_R_I2D_ECPKPARAMETERS_FAILURE                         121
 #define EC_R_INCOMPATIBLE_OBJECTS                       101
@@ -492,7 +497,9 @@ void ERR_load_EC_strings(void);
 #define EC_R_INVALID_FIELD                              103
 #define EC_R_INVALID_FORM                               104
 #define EC_R_INVALID_GROUP_ORDER                        122
+#define EC_R_INVALID_PENTANOMIAL_BASIS                  132
 #define EC_R_INVALID_PRIVATE_KEY                        123
+#define EC_R_INVALID_TRINOMIAL_BASIS                    137
 #define EC_R_MISSING_PARAMETERS                                 124
 #define EC_R_MISSING_PRIVATE_KEY                        125
 #define EC_R_NOT_A_NIST_PRIME                           135
index dec913b8addcc5c4ea589de22feaea9c0937f444..66ef129293c27557863ca635ae847d3ba23db3d8 100644 (file)
@@ -741,6 +741,7 @@ static EC_GROUP *ec_asn1_parameters2group(const ECPARAMETERS *params)
        EC_GROUP                *ret = NULL;
        BIGNUM                  *p = NULL, *a = NULL, *b = NULL;
        EC_POINT                *point=NULL;
+       long                    field_bits;
 
        if (!params->fieldID || !params->fieldID->fieldType || 
            !params->fieldID->p.ptr)
@@ -779,6 +780,13 @@ static EC_GROUP *ec_asn1_parameters2group(const ECPARAMETERS *params)
 
                char_two = params->fieldID->p.char_two;
 
+               field_bits = char_two->m;
+               if (field_bits > OPENSSL_ECC_MAX_FIELD_BITS)
+                       {
+                       ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, EC_R_FIELD_TOO_LARGE);
+                       goto err;
+                       }
+
                if ((p = BN_new()) == NULL)
                        {
                        ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, ERR_R_MALLOC_FAILURE);
@@ -799,6 +807,13 @@ static EC_GROUP *ec_asn1_parameters2group(const ECPARAMETERS *params)
                                }
 
                        tmp_long = ASN1_INTEGER_get(char_two->p.tpBasis);
+
+                       if (!(char_two->m > tmp_long && tmp_long > 0))
+                               {
+                               ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, EC_R_INVALID_TRINOMIAL_BASIS);
+                               goto err;
+                               }
+                       
                        /* create the polynomial */
                        if (!BN_set_bit(p, (int)char_two->m))
                                goto err;
@@ -817,6 +832,13 @@ static EC_GROUP *ec_asn1_parameters2group(const ECPARAMETERS *params)
                                ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, EC_R_ASN1_ERROR);
                                goto err;
                                }
+
+                       if (!(char_two->m > penta->k3 && penta->k3 > penta->k2 && penta->k2 > penta->k1 && penta->k1 > 0))
+                               {
+                               ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, EC_R_INVALID_PENTANOMIAL_BASIS);
+                               goto err;
+                               }
+                       
                        /* create the polynomial */
                        if (!BN_set_bit(p, (int)char_two->m)) goto err;
                        if (!BN_set_bit(p, (int)penta->k1)) goto err;
@@ -853,6 +875,20 @@ static EC_GROUP *ec_asn1_parameters2group(const ECPARAMETERS *params)
                        ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, ERR_R_ASN1_LIB);
                        goto err;
                        }
+
+               if (BN_is_negative(p) || BN_is_zero(p))
+                       {
+                       ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, EC_R_INVALID_FIELD);
+                       goto err;
+                       }
+
+               field_bits = BN_num_bits(p);
+               if (field_bits > OPENSSL_ECC_MAX_FIELD_BITS)
+                       {
+                       ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, EC_R_FIELD_TOO_LARGE);
+                       goto err;
+                       }
+
                /* create the EC_GROUP structure */
                ret = EC_GROUP_new_curve_GFp(p, a, b, NULL);
                }
@@ -910,6 +946,16 @@ static EC_GROUP *ec_asn1_parameters2group(const ECPARAMETERS *params)
                ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, ERR_R_ASN1_LIB);
                goto err;
                }
+       if (BN_is_negative(a) || BN_is_zero(a))
+               {
+               ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, EC_R_INVALID_GROUP_ORDER);
+               goto err;
+               }
+       if (BN_num_bits(a) > (int)field_bits + 1) /* Hasse bound */
+               {
+               ECerr(EC_F_EC_ASN1_PARAMETERS2GROUP, EC_R_INVALID_GROUP_ORDER);
+               goto err;
+               }
        
        /* extract the cofactor (optional) */
        if (params->cofactor == NULL)
index 38302b9b549d3616b4bde11124ff30abcbe073c9..031c54d0b57fec4fb311263a1769eb13e7489993 100644 (file)
@@ -188,6 +188,7 @@ static ERR_STRING_DATA EC_str_reasons[]=
 {ERR_REASON(EC_R_D2I_ECPKPARAMETERS_FAILURE),"d2i ecpkparameters failure"},
 {ERR_REASON(EC_R_DISCRIMINANT_IS_ZERO)   ,"discriminant is zero"},
 {ERR_REASON(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE),"ec group new by name failure"},
+{ERR_REASON(EC_R_FIELD_TOO_LARGE)        ,"field too large"},
 {ERR_REASON(EC_R_GROUP2PKPARAMETERS_FAILURE),"group2pkparameters failure"},
 {ERR_REASON(EC_R_I2D_ECPKPARAMETERS_FAILURE),"i2d ecpkparameters failure"},
 {ERR_REASON(EC_R_INCOMPATIBLE_OBJECTS)   ,"incompatible objects"},
@@ -198,7 +199,9 @@ static ERR_STRING_DATA EC_str_reasons[]=
 {ERR_REASON(EC_R_INVALID_FIELD)          ,"invalid field"},
 {ERR_REASON(EC_R_INVALID_FORM)           ,"invalid form"},
 {ERR_REASON(EC_R_INVALID_GROUP_ORDER)    ,"invalid group order"},
+{ERR_REASON(EC_R_INVALID_PENTANOMIAL_BASIS),"invalid pentanomial basis"},
 {ERR_REASON(EC_R_INVALID_PRIVATE_KEY)    ,"invalid private key"},
+{ERR_REASON(EC_R_INVALID_TRINOMIAL_BASIS),"invalid trinomial basis"},
 {ERR_REASON(EC_R_MISSING_PARAMETERS)     ,"missing parameters"},
 {ERR_REASON(EC_R_MISSING_PRIVATE_KEY)    ,"missing private key"},
 {ERR_REASON(EC_R_NOT_A_NIST_PRIME)       ,"not a NIST prime"},
index d302254bb1b1c1e1422dc642dddc45a885f43bba..b19c556930f46cad75850e62db4c5838e9faa714 100644 (file)
@@ -159,6 +159,17 @@ struct rsa_st
        BN_BLINDING *mt_blinding;
        };
 
+#ifndef OPENSSL_RSA_MAX_MODULUS_BITS
+# define OPENSSL_RSA_MAX_MODULUS_BITS  16384
+#endif
+
+#ifndef OPENSSL_RSA_SMALL_MODULUS_BITS
+# define OPENSSL_RSA_SMALL_MODULUS_BITS        3072
+#endif
+#ifndef OPENSSL_RSA_MAX_PUBEXP_BITS
+# define OPENSSL_RSA_MAX_PUBEXP_BITS   64 /* exponent limit enforced for "large" modulus only */
+#endif
+
 #define RSA_3  0x3L
 #define RSA_F4 0x10001L
 
@@ -407,6 +418,7 @@ void ERR_load_RSA_strings(void);
 #define RSA_R_IQMP_NOT_INVERSE_OF_Q                     126
 #define RSA_R_KEY_SIZE_TOO_SMALL                        120
 #define RSA_R_LAST_OCTET_INVALID                        134
+#define RSA_R_MODULUS_TOO_LARGE                                 105
 #define RSA_R_NO_PUBLIC_EXPONENT                        140
 #define RSA_R_NULL_BEFORE_BLOCK_MISSING                         113
 #define RSA_R_N_DOES_NOT_EQUAL_P_Q                      127
index 69cabd2716445bb1a88b2ef0273b6d498b8ecc41..e7b7a9c4fc381ca01f5b3fc826bf3864057e88cb 100644 (file)
@@ -168,6 +168,28 @@ static int RSA_eay_public_encrypt(int flen, const unsigned char *from,
        unsigned char *buf=NULL;
        BN_CTX *ctx=NULL;
 
+       if (BN_num_bits(rsa->n) > OPENSSL_RSA_MAX_MODULUS_BITS)
+               {
+               RSAerr(RSA_F_RSA_EAY_PUBLIC_ENCRYPT, RSA_R_MODULUS_TOO_LARGE);
+               return -1;
+               }
+
+       if (BN_ucmp(rsa->n, rsa->e) <= 0)
+               {
+               RSAerr(RSA_F_RSA_EAY_PUBLIC_ENCRYPT, RSA_R_BAD_E_VALUE);
+               return -1;
+               }
+
+       /* for large moduli, enforce exponent limit */
+       if (BN_num_bits(rsa->n) > OPENSSL_RSA_SMALL_MODULUS_BITS)
+               {
+               if (BN_num_bits(rsa->e) > OPENSSL_RSA_MAX_PUBEXP_BITS)
+                       {
+                       RSAerr(RSA_F_RSA_EAY_PUBLIC_ENCRYPT, RSA_R_BAD_E_VALUE);
+                       return -1;
+                       }
+               }
+       
        if ((ctx=BN_CTX_new()) == NULL) goto err;
        BN_CTX_start(ctx);
        f = BN_CTX_get(ctx);
@@ -597,6 +619,28 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from,
        unsigned char *buf=NULL;
        BN_CTX *ctx=NULL;
 
+       if (BN_num_bits(rsa->n) > OPENSSL_RSA_MAX_MODULUS_BITS)
+               {
+               RSAerr(RSA_F_RSA_EAY_PUBLIC_DECRYPT, RSA_R_MODULUS_TOO_LARGE);
+               return -1;
+               }
+
+       if (BN_ucmp(rsa->n, rsa->e) <= 0)
+               {
+               RSAerr(RSA_F_RSA_EAY_PUBLIC_DECRYPT, RSA_R_BAD_E_VALUE);
+               return -1;
+               }
+
+       /* for large moduli, enforce exponent limit */
+       if (BN_num_bits(rsa->n) > OPENSSL_RSA_SMALL_MODULUS_BITS)
+               {
+               if (BN_num_bits(rsa->e) > OPENSSL_RSA_MAX_PUBEXP_BITS)
+                       {
+                       RSAerr(RSA_F_RSA_EAY_PUBLIC_DECRYPT, RSA_R_BAD_E_VALUE);
+                       return -1;
+                       }
+               }
+       
        if((ctx = BN_CTX_new()) == NULL) goto err;
        BN_CTX_start(ctx);
        f = BN_CTX_get(ctx);
index f82b2d6ad994692aa007f2e57d93013be97b1e1f..da7a4fb4c2613d78cc42c1bcbd909d8ff38d9aaf 100644 (file)
@@ -137,6 +137,7 @@ static ERR_STRING_DATA RSA_str_reasons[]=
 {ERR_REASON(RSA_R_IQMP_NOT_INVERSE_OF_Q) ,"iqmp not inverse of q"},
 {ERR_REASON(RSA_R_KEY_SIZE_TOO_SMALL)    ,"key size too small"},
 {ERR_REASON(RSA_R_LAST_OCTET_INVALID)    ,"last octet invalid"},
+{ERR_REASON(RSA_R_MODULUS_TOO_LARGE)     ,"modulus too large"},
 {ERR_REASON(RSA_R_NO_PUBLIC_EXPONENT)    ,"no public exponent"},
 {ERR_REASON(RSA_R_NULL_BEFORE_BLOCK_MISSING),"null before block missing"},
 {ERR_REASON(RSA_R_N_DOES_NOT_EQUAL_P_Q)  ,"n does not equal p q"},
index efb52485a71439e5807904eebca704d6b5dc331e..ce60de630a7a8425224abbc1094fba0ba304df7b 100644 (file)
@@ -520,7 +520,8 @@ static int get_server_hello(SSL *s)
                CRYPTO_add(&s->session->peer->references, 1, CRYPTO_LOCK_X509);
                }
 
-       if (s->session->peer != s->session->sess_cert->peer_key->x509)
+       if (s->session->sess_cert == NULL 
+      || s->session->peer != s->session->sess_cert->peer_key->x509)
                /* can't happen */
                {
                ssl2_return_error(s, SSL2_PE_UNDEFINED_ERROR);
index a8c5df822c776a30da42b591426f52532c643f23..098eea13ce7750cc11b6709c4f5917d4f5eae489 100644 (file)
@@ -2003,7 +2003,7 @@ int ssl3_get_client_key_exchange(SSL *s)
 
                 if (kssl_ctx->client_princ)
                         {
-                        int len = strlen(kssl_ctx->client_princ);
+                        size_t len = strlen(kssl_ctx->client_princ);
                         if ( len < SSL_MAX_KRB5_PRINCIPAL_LENGTH ) 
                                 {
                                 s->session->krb5_client_princ_len = len;
index 28c90fc68e204a971930f6122af7423992834c5e..4971b34375fb802c6328d7555911bdb81fa8b8b0 100644 (file)
@@ -1219,7 +1219,7 @@ char *SSL_get_shared_ciphers(const SSL *s,char *buf,int len)
                c=sk_SSL_CIPHER_value(sk,i);
                for (cp=c->name; *cp; )
                        {
-                       if (len-- == 0)
+                       if (len-- <= 0)
                                {
                                *p='\0';
                                return(buf);