- let SSL_CTX_set_cipher_list and SSL_set_cipher_list return an

  error if the cipher list is empty
- fix last commit in ssl_create_cipher_list
- clean up ssl_create_cipher_list

diff --git a/ssl/ssl.h b/ssl/ssl.h
index 3161f532cf6c1c249b0fd7a168e6278c78d311f1..13b8abe3c24d9db71cb3e8ff88be4173fcd23f7c 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -1567,6 +1567,7 @@ void ERR_load_SSL_strings(void);
 #define SSL_F_SSL_CTRL                                  232
 #define SSL_F_SSL_CTX_CHECK_PRIVATE_KEY                         168
 #define SSL_F_SSL_CTX_NEW                               169
+#define SSL_F_SSL_CTX_SET_CIPHER_LIST                   1026
 #define SSL_F_SSL_CTX_SET_PURPOSE                       226
 #define SSL_F_SSL_CTX_SET_SESSION_ID_CONTEXT            219
 #define SSL_F_SSL_CTX_SET_SSL_VERSION                   170
@@ -1596,6 +1597,7 @@ void ERR_load_SSL_strings(void);
 #define SSL_F_SSL_SESSION_PRINT_FP                      190
 #define SSL_F_SSL_SESS_CERT_NEW                                 225
 #define SSL_F_SSL_SET_CERT                              191
+#define SSL_F_SSL_SET_CIPHER_LIST                       1027
 #define SSL_F_SSL_SET_FD                                192
 #define SSL_F_SSL_SET_PKEY                              193
 #define SSL_F_SSL_SET_PURPOSE                           227

diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c
index 2b8a37397c1b9743dbbb00b1e63bcfdeeb97fa89..3df5e2fa805576ce1bc6f70cc43f38605a7d923d 100644 (file)
--- a/ssl/ssl_ciph.c
+++ b/ssl/ssl_ciph.c
@@ -700,11 +700,18 @@ static int ssl_cipher_process_rulestr(const char *rule_str,
                        if (!found)
                                break;  /* ignore this entry */
 
-                       algorithms = (ca_list[j]->algorithms & ~mask) |
-                                    (ca_list[j]->algorithms &  algorithms & mask);
+                       /* New algorithms:
+                        *  1 - any old restrictions apply outside new mask
+                        *  2 - any new restrictions apply outside old mask
+                        *  3 - enforce old & new where masks intersect
+                        */
+                       algorithms = (algorithms & ~ca_list[j]->mask) |         /* 1 */
+                                    (ca_list[j]->algorithms & ~mask) |         /* 2 */
+                                    (algorithms & ca_list[j]->algorithms);     /* 3 */
                        mask |= ca_list[j]->mask;
-                       algo_strength = (ca_list[j]->algo_strength & ~mask_strength) |
-                                       (ca_list[j]->algo_strength & algo_strength & mask_strength);
+                       algo_strength = (algo_strength & ~ca_list[j]->mask_strength) |
+                                       (ca_list[j]->algo_strength & ~mask_strength) |
+                                       (algo_strength & ca_list[j]->algo_strength);
                        mask_strength |= ca_list[j]->mask_strength;
 
                        if (!multi) break;
@@ -758,7 +765,7 @@ STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(const SSL_METHOD *ssl_method,
        {
        int ok, num_of_ciphers, num_of_alias_max, num_of_group_aliases;
        unsigned long disabled_mask;
-       STACK_OF(SSL_CIPHER) *cipherstack;
+       STACK_OF(SSL_CIPHER) *cipherstack, *tmp_cipher_list;
        const char *rule_p;
        CIPHER_ORDER *co_list = NULL, *head = NULL, *tail = NULL, *curr;
        SSL_CIPHER **ca_list = NULL;
@@ -766,7 +773,8 @@ STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(const SSL_METHOD *ssl_method,
        /*
         * Return with error if nothing to do.
         */
-       if (rule_str == NULL) return(NULL);
+       if (rule_str == NULL || cipher_list == NULL || cipher_list_by_id == NULL)
+               return NULL;
 
        if (init_ciphers)
                {
@@ -876,54 +884,19 @@ STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(const SSL_METHOD *ssl_method,
                        }
                }
        OPENSSL_free(co_list);  /* Not needed any longer */
-       /* if no ciphers where selected let's return NULL */
-       if (sk_SSL_CIPHER_num(cipherstack) == 0)
-               {
-               SSLerr(SSL_F_SSL_CREATE_CIPHER_LIST, SSL_R_NO_CIPHER_MATCH);
-               sk_SSL_CIPHER_free(cipherstack);
-               return NULL;
-               }
 
-       /*
-        * The following passage is a little bit odd. If pointer variables
-        * were supplied to hold STACK_OF(SSL_CIPHER) return information,
-        * the old memory pointed to is free()ed. Then, however, the
-        * cipher_list entry will be assigned just a copy of the returned
-        * cipher stack. For cipher_list_by_id a copy of the cipher stack
-        * will be created. See next comment...
-        */
-       if (cipher_list != NULL)
-               {
-               if (*cipher_list != NULL)
-                       sk_SSL_CIPHER_free(*cipher_list);
-               *cipher_list = cipherstack;
-               }
-
-       if (cipher_list_by_id != NULL)
-               {
-               if (*cipher_list_by_id != NULL)
-                       sk_SSL_CIPHER_free(*cipher_list_by_id);
-               *cipher_list_by_id = sk_SSL_CIPHER_dup(cipherstack);
-               }
-
-       /*
-        * Now it is getting really strange. If something failed during
-        * the previous pointer assignment or if one of the pointers was
-        * not requested, the error condition is met. That might be
-        * discussable. The strange thing is however that in this case
-        * the memory "ret" pointed to is "free()ed" and hence the pointer
-        * cipher_list becomes wild. The memory reserved for
-        * cipher_list_by_id however is not "free()ed" and stays intact.
-        */
-       if (    (cipher_list_by_id == NULL) ||
-               (*cipher_list_by_id == NULL) ||
-               (cipher_list == NULL) ||
-               (*cipher_list == NULL))
+       tmp_cipher_list = sk_SSL_CIPHER_dup(cipherstack);
+       if (tmp_cipher_list == NULL)
                {
                sk_SSL_CIPHER_free(cipherstack);
-               return(NULL);
+               return NULL;
                }
-
+       if (*cipher_list != NULL)
+               sk_SSL_CIPHER_free(*cipher_list);
+       *cipher_list = cipherstack;
+       if (*cipher_list_by_id != NULL)
+               sk_SSL_CIPHER_free(*cipher_list_by_id);
+       *cipher_list_by_id = tmp_cipher_list;
        sk_SSL_CIPHER_set_cmp_func(*cipher_list_by_id,ssl_cipher_ptr_id_cmp);
 
        return(cipherstack);

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 9470822dfdfb7151714983b24f80247ceb391482..825ff252e62da3732906f19556e17e8feafc395e 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -159,6 +159,7 @@ static ERR_STRING_DATA SSL_str_functs[]=
 {ERR_FUNC(SSL_F_SSL_CTRL),     "SSL_ctrl"},
 {ERR_FUNC(SSL_F_SSL_CTX_CHECK_PRIVATE_KEY),    "SSL_CTX_check_private_key"},
 {ERR_FUNC(SSL_F_SSL_CTX_NEW),  "SSL_CTX_new"},
+{ERR_FUNC(SSL_F_SSL_CTX_SET_CIPHER_LIST),      "SSL_CTX_set_cipher_list"},
 {ERR_FUNC(SSL_F_SSL_CTX_SET_PURPOSE),  "SSL_CTX_set_purpose"},
 {ERR_FUNC(SSL_F_SSL_CTX_SET_SESSION_ID_CONTEXT),       "SSL_CTX_set_session_id_context"},
 {ERR_FUNC(SSL_F_SSL_CTX_SET_SSL_VERSION),      "SSL_CTX_set_ssl_version"},
@@ -188,6 +189,7 @@ static ERR_STRING_DATA SSL_str_functs[]=
 {ERR_FUNC(SSL_F_SSL_SESSION_PRINT_FP), "SSL_SESSION_print_fp"},
 {ERR_FUNC(SSL_F_SSL_SESS_CERT_NEW),    "SSL_SESS_CERT_NEW"},
 {ERR_FUNC(SSL_F_SSL_SET_CERT), "SSL_SET_CERT"},
+{ERR_FUNC(SSL_F_SSL_SET_CIPHER_LIST),  "SSL_set_cipher_list"},
 {ERR_FUNC(SSL_F_SSL_SET_FD),   "SSL_set_fd"},
 {ERR_FUNC(SSL_F_SSL_SET_PKEY), "SSL_SET_PKEY"},
 {ERR_FUNC(SSL_F_SSL_SET_PURPOSE),      "SSL_set_purpose"},

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index f1ded07c46d4e73bd079550348f640d7c83c292b..6cdc7f6156c5dfdbba738cb3cc6faa92776569fa 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1130,8 +1130,21 @@ int SSL_CTX_set_cipher_list(SSL_CTX *ctx, const char *str)
        
        sk=ssl_create_cipher_list(ctx->method,&ctx->cipher_list,
                &ctx->cipher_list_by_id,str);
-/* XXXX */
-       return((sk == NULL)?0:1);
+       /* ssl_create_cipher_list may return an empty stack if it
+        * was unable to find a cipher matching the given rule string
+        * (for example if the rule string specifies a cipher which
+        * has been disabled). This is not an error as far as 
+        * ssl_create_cipher_list is concerned, and hence 
+        * ctx->cipher_list and ctx->cipher_list_by_id has been
+        * updated. */
+       if (sk == NULL)
+               return 0;
+       else if (sk_SSL_CIPHER_num(sk) == 0)
+               {
+               SSLerr(SSL_F_SSL_CTX_SET_CIPHER_LIST, SSL_R_NO_CIPHER_MATCH);
+               return 0;
+               }
+       return 1;
        }
 
 /** specify the ciphers to be used by the SSL */
@@ -1141,8 +1154,15 @@ int SSL_set_cipher_list(SSL *s,const char *str)
        
        sk=ssl_create_cipher_list(s->ctx->method,&s->cipher_list,
                &s->cipher_list_by_id,str);
-/* XXXX */
-       return((sk == NULL)?0:1);
+       /* see comment in SSL_CTX_set_cipher_list */
+       if (sk == NULL)
+               return 0;
+       else if (sk_SSL_CIPHER_num(sk) == 0)
+               {
+               SSLerr(SSL_F_SSL_SET_CIPHER_LIST, SSL_R_NO_CIPHER_MATCH);
+               return 0;
+               }
+       return 1;
        }
 
 /* works well for SSLv2, not so good for SSLv3 */