Skip to content

Commit

Permalink
Fix configuration of TLSv1.3 ciphersuites
Browse files Browse the repository at this point in the history 
Configuration of TLSv1.3 ciphersuites wasn't working in some cases.

Fixes openssl#5740

Reviewed-by: Rich Salz <[email protected]>
(Merged from openssl#5855)
  • Loading branch information
@mattcaswell
mattcaswell committed Apr 4, 2018
1 parent 034cb87 commit a53b5be
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 105 deletions.
136 changes: 128 additions & 8 deletions ssl/ssl_ciph.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <openssl/comp.h>
#include <openssl/engine.h>
#include <openssl/crypto.h>
#include <openssl/conf.h>
#include "internal/nelem.h"
#include "ssl_locl.h"
#include "internal/thread_once.h"
Expand Down Expand Up @@ -1274,6 +1275,131 @@ static int check_suiteb_cipher_list(const SSL_METHOD *meth, CERT *c,
}
#endif

static int ciphersuite_cb(const char *elem, int len, void *arg)
{
STACK_OF(SSL_CIPHER) *ciphersuites = (STACK_OF(SSL_CIPHER) *)arg;
const SSL_CIPHER *cipher;
/* Arbitrary sized temp buffer for the cipher name. Should be big enough */
char name[80];

if (len > (int)(sizeof(name) - 1)) {
SSLerr(SSL_F_CIPHERSUITE_CB, SSL_R_NO_CIPHER_MATCH);
return 0;
}

memcpy(name, elem, len);
name[len] = '\0';

cipher = ssl3_get_cipher_by_std_name(name);
if (cipher == NULL) {
SSLerr(SSL_F_CIPHERSUITE_CB, SSL_R_NO_CIPHER_MATCH);
return 0;
}

if (!sk_SSL_CIPHER_push(ciphersuites, cipher)) {
SSLerr(SSL_F_CIPHERSUITE_CB, ERR_R_INTERNAL_ERROR);
return 0;
}

return 1;
}

int set_ciphersuites(STACK_OF(SSL_CIPHER) **currciphers, const char *str)
{
STACK_OF(SSL_CIPHER) *newciphers = sk_SSL_CIPHER_new_null();

if (newciphers == NULL)
return 0;

/* Parse the list. We explicitly allow an empty list */
if (*str != '\0'
&& !CONF_parse_list(str, ':', 1, ciphersuite_cb, newciphers)) {
sk_SSL_CIPHER_free(newciphers);
return 0;
}
sk_SSL_CIPHER_free(*currciphers);
*currciphers = newciphers;

return 1;
}

static int update_cipher_list_by_id(STACK_OF(SSL_CIPHER) **cipher_list_by_id,
STACK_OF(SSL_CIPHER) *cipherstack)
{
STACK_OF(SSL_CIPHER) *tmp_cipher_list = sk_SSL_CIPHER_dup(cipherstack);

if (tmp_cipher_list == NULL) {
return 0;
}

sk_SSL_CIPHER_free(*cipher_list_by_id);
*cipher_list_by_id = tmp_cipher_list;

(void)sk_SSL_CIPHER_set_cmp_func(*cipher_list_by_id, ssl_cipher_ptr_id_cmp);
sk_SSL_CIPHER_sort(*cipher_list_by_id);

return 1;
}

static int update_cipher_list(STACK_OF(SSL_CIPHER) **cipher_list,
STACK_OF(SSL_CIPHER) **cipher_list_by_id,
STACK_OF(SSL_CIPHER) *tls13_ciphersuites)
{
int i;
STACK_OF(SSL_CIPHER) *tmp_cipher_list = sk_SSL_CIPHER_dup(*cipher_list);

if (tmp_cipher_list == NULL)
return 0;

/*
* Delete any existing TLSv1.3 ciphersuites. These are always first in the
* list.
*/
while (sk_SSL_CIPHER_num(tmp_cipher_list) > 0
&& sk_SSL_CIPHER_value(tmp_cipher_list, 0)->min_tls
== TLS1_3_VERSION)
sk_SSL_CIPHER_delete(tmp_cipher_list, 0);

/* Insert the new TLSv1.3 ciphersuites */
for (i = 0; i < sk_SSL_CIPHER_num(tls13_ciphersuites); i++)
sk_SSL_CIPHER_insert(tmp_cipher_list,
sk_SSL_CIPHER_value(tls13_ciphersuites, i), i);

if (!update_cipher_list_by_id(cipher_list_by_id, tmp_cipher_list))
return 0;

sk_SSL_CIPHER_free(*cipher_list);
*cipher_list = tmp_cipher_list;

return 1;
}

int SSL_CTX_set_ciphersuites(SSL_CTX *ctx, const char *str)
{
int ret = set_ciphersuites(&(ctx->tls13_ciphersuites), str);

if (ret && ctx->cipher_list != NULL) {
/* We already have a cipher_list, so we need to update it */
return update_cipher_list(&ctx->cipher_list, &ctx->cipher_list_by_id,
ctx->tls13_ciphersuites);
}

return ret;
}

int SSL_set_ciphersuites(SSL *s, const char *str)
{
int ret = set_ciphersuites(&(s->tls13_ciphersuites), str);

if (ret && s->cipher_list != NULL) {
/* We already have a cipher_list, so we need to update it */
return update_cipher_list(&s->cipher_list, &s->cipher_list_by_id,
s->tls13_ciphersuites);
}

return ret;
}

STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(const SSL_METHOD *ssl_method,
STACK_OF(SSL_CIPHER) *tls13_ciphersuites,
STACK_OF(SSL_CIPHER) **cipher_list,
Expand All @@ -1283,7 +1409,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, i;
uint32_t disabled_mkey, disabled_auth, disabled_enc, disabled_mac;
STACK_OF(SSL_CIPHER) *cipherstack, *tmp_cipher_list;
STACK_OF(SSL_CIPHER) *cipherstack;
const char *rule_p;
CIPHER_ORDER *co_list = NULL, *head = NULL, *tail = NULL, *curr;
const SSL_CIPHER **ca_list = NULL;
Expand Down Expand Up @@ -1498,19 +1624,13 @@ STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(const SSL_METHOD *ssl_method,
}
OPENSSL_free(co_list); /* Not needed any longer */

tmp_cipher_list = sk_SSL_CIPHER_dup(cipherstack);
if (tmp_cipher_list == NULL) {
if (!update_cipher_list_by_id(cipher_list_by_id, cipherstack)) {
sk_SSL_CIPHER_free(cipherstack);
return 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;
(void)sk_SSL_CIPHER_set_cmp_func(*cipher_list_by_id, ssl_cipher_ptr_id_cmp);

sk_SSL_CIPHER_sort(*cipher_list_by_id);
return cipherstack;
}

Expand Down
93 changes: 0 additions & 93 deletions ssl/ssl_lib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2549,99 +2549,6 @@ int SSL_set_cipher_list(SSL *s, const char *str)
return 1;
}

static int ciphersuite_cb(const char *elem, int len, void *arg)
{
STACK_OF(SSL_CIPHER) *ciphersuites = (STACK_OF(SSL_CIPHER) *)arg;
const SSL_CIPHER *cipher;
/* Arbitrary sized temp buffer for the cipher name. Should be big enough */
char name[80];

if (len > (int)(sizeof(name) - 1)) {
SSLerr(SSL_F_CIPHERSUITE_CB, SSL_R_NO_CIPHER_MATCH);
return 0;
}

memcpy(name, elem, len);
name[len] = '\0';

cipher = ssl3_get_cipher_by_std_name(name);
if (cipher == NULL) {
SSLerr(SSL_F_CIPHERSUITE_CB, SSL_R_NO_CIPHER_MATCH);
return 0;
}

if (!sk_SSL_CIPHER_push(ciphersuites, cipher)) {
SSLerr(SSL_F_CIPHERSUITE_CB, ERR_R_INTERNAL_ERROR);
return 0;
}

return 1;
}

static int set_ciphersuites(STACK_OF(SSL_CIPHER) **currciphers, const char *str)
{
STACK_OF(SSL_CIPHER) *newciphers = sk_SSL_CIPHER_new_null();

if (newciphers == NULL)
return 0;

/* Parse the list. We explicitly allow an empty list */
if (*str != '\0'
&& !CONF_parse_list(str, ':', 1, ciphersuite_cb, newciphers)) {
sk_SSL_CIPHER_free(newciphers);
return 0;
}
sk_SSL_CIPHER_free(*currciphers);
*currciphers = newciphers;

return 1;
}

static int update_cipher_list(STACK_OF(SSL_CIPHER) *cipher_list,
STACK_OF(SSL_CIPHER) *tls13_ciphersuites)
{
int i;

/*
* Delete any existing TLSv1.3 ciphersuites. These are always first in the
* list.
*/
while (sk_SSL_CIPHER_num(cipher_list) > 0
&& sk_SSL_CIPHER_value(cipher_list, 0)->min_tls == TLS1_3_VERSION)
sk_SSL_CIPHER_delete(cipher_list, 0);

/* Insert the new TLSv1.3 ciphersuites */
for (i = 0; i < sk_SSL_CIPHER_num(tls13_ciphersuites); i++)
sk_SSL_CIPHER_insert(cipher_list,
sk_SSL_CIPHER_value(tls13_ciphersuites, i), i);

return 1;
}

int SSL_CTX_set_ciphersuites(SSL_CTX *ctx, const char *str)
{
int ret = set_ciphersuites(&(ctx->tls13_ciphersuites), str);

if (ret && ctx->cipher_list != NULL) {
/* We already have a cipher_list, so we need to update it */
return update_cipher_list(ctx->cipher_list, ctx->tls13_ciphersuites);
}

return ret;
}

int SSL_set_ciphersuites(SSL *s, const char *str)
{
int ret = set_ciphersuites(&(s->tls13_ciphersuites), str);

if (ret && s->cipher_list != NULL) {
/* We already have a cipher_list, so we need to update it */
return update_cipher_list(s->cipher_list, s->tls13_ciphersuites);
}

return ret;
}

char *SSL_get_shared_ciphers(const SSL *s, char *buf, int len)
{
char *p;
Expand Down
1 change: 1 addition & 0 deletions ssl/ssl_locl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2210,6 +2210,7 @@ __owur int ssl_cipher_id_cmp(const SSL_CIPHER *a, const SSL_CIPHER *b);
DECLARE_OBJ_BSEARCH_GLOBAL_CMP_FN(SSL_CIPHER, SSL_CIPHER, ssl_cipher_id);
__owur int ssl_cipher_ptr_id_cmp(const SSL_CIPHER *const *ap,
const SSL_CIPHER *const *bp);
__owur int set_ciphersuites(STACK_OF(SSL_CIPHER) **currciphers, const char *str);
__owur STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(const SSL_METHOD *ssl_method,
STACK_OF(SSL_CIPHER) *tls13_ciphersuites,
STACK_OF(SSL_CIPHER) **cipher_list,
Expand Down
6 changes: 2 additions & 4 deletions test/sslcorrupttest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,9 @@ static int test_ssl_corrupt(int testidx)
&sctx, &cctx, cert, privkey)))
return 0;

if (!TEST_true(SSL_CTX_set_cipher_list(cctx, cipher_list[testidx])))
goto end;

if (!TEST_ptr(ciphers = SSL_CTX_get_ciphers(cctx))
if (!TEST_true(SSL_CTX_set_cipher_list(cctx, cipher_list[testidx]))
|| !TEST_true(SSL_CTX_set_ciphersuites(cctx, ""))
|| !TEST_ptr(ciphers = SSL_CTX_get_ciphers(cctx))
|| !TEST_int_eq(sk_SSL_CIPHER_num(ciphers), 1)
|| !TEST_ptr(currcipher = sk_SSL_CIPHER_value(ciphers, 0)))
goto end;
Expand Down

0 comments on commit a53b5be

Please sign in to comment.