Commit f0747cd9 authored by Nils Larsch's avatar Nils Larsch
Browse files

- 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
parent 21ac2b96

ssl/ssl.h

+2 −0
Original line number Diff line number Diff line
@@ -1655,6 +1655,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
@@ -1685,6 +1686,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

ssl/ssl_ciph.c

+23 −50
Original line number Diff line number Diff line
@@ -740,11 +740,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;
@@ -798,7 +805,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;
@@ -806,7 +813,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)

		{
@@ -912,54 +920,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)



	tmp_cipher_list = sk_SSL_CIPHER_dup(cipherstack);

	if (tmp_cipher_list == NULL)

		{

		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))

		{

		sk_SSL_CIPHER_free(cipherstack);

		return(NULL);

		}



	*cipher_list_by_id = tmp_cipher_list;

	sk_SSL_CIPHER_set_cmp_func(*cipher_list_by_id,ssl_cipher_ptr_id_cmp);



	return(cipherstack);

ssl/ssl_err.c

+2 −0
Original line number Diff line number Diff line
@@ -183,6 +183,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"},
@@ -213,6 +214,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"},

ssl/ssl_lib.c

+26 −6
Original line number Diff line number Diff line
@@ -1153,8 +1153,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 */
@@ -1164,8 +1177,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 */