kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Add some addition tests for the cipher parsing code and tidy.

Browse Source 
The in_group check is redundant and test an extremely absurd corner of
the syntax.

Change-Id: Ia54bcd7cda7ba05415d3a250ee93e1acedcc43d6
Reviewed-on: https://boringssl-review.googlesource.com/17542
Reviewed-by: David Benjamin <davidben@google.com>
This commit is contained in:
David Benjamin 2017-07-01 11:13:53 -04:00
parent 634f475255
commit bf5f192310
2 changed files with 32 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
ssl/ssl_cipher.c
View File

@ -1035,7 +1035,7 @@ static int ssl_cipher_process_rulestr(const SSL_PROTOCOL_METHOD *ssl_method,
uint32_t alg_mkey, alg_auth, alg_enc, alg_mac; uint32_t alg_mkey, alg_auth, alg_enc, alg_mac;
uint16_t min_version; uint16_t min_version;
const char *l, *buf; const char *l, *buf;
int multi, skip_rule, rule, ok, in_group = 0, has_group = 0; int multi, skip_rule, rule, in_group = 0, has_group = 0;
size_t j, buf_len; size_t j, buf_len;
uint32_t cipher_id; uint32_t cipher_id;
char ch; char ch;
@ -1082,10 +1082,7 @@ static int ssl_cipher_process_rulestr(const SSL_PROTOCOL_METHOD *ssl_method,
rule = CIPHER_SPECIAL; rule = CIPHER_SPECIAL;
l++; l++;
} else if (ch == '[') { } else if (ch == '[') {
if (in_group) { assert(!in_group);
OPENSSL_PUT_ERROR(SSL, SSL_R_NESTED_GROUP);
return 0;
}
in_group = 1; in_group = 1;
has_group = 1; has_group = 1;
l++; l++;
@ -1185,15 +1182,11 @@ static int ssl_cipher_process_rulestr(const SSL_PROTOCOL_METHOD *ssl_method,
/* Ok, we have the rule, now apply it. */ /* Ok, we have the rule, now apply it. */
if (rule == CIPHER_SPECIAL) { if (rule == CIPHER_SPECIAL) {
/* special command */ if (buf_len != 8 || strncmp(buf, "STRENGTH", 8) != 0) {
ok = 0;
if (buf_len == 8 && !strncmp(buf, "STRENGTH", 8)) {
ok = ssl_cipher_strength_sort(head_p, tail_p);
} else {
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_COMMAND); OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_COMMAND);
return 0;
} }
if (!ssl_cipher_strength_sort(head_p, tail_p)) {
if (ok == 0) {
return 0; return 0;
} }

28
ssl/ssl_test.cc
View File

@ -180,7 +180,7 @@ static const CipherTest kCipherTests[] = {
// Standard names may be used instead of OpenSSL names. // Standard names may be used instead of OpenSSL names.
{ {
"[TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256|" "[TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256|"
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256]:" "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256]:"
"[TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256]:" "[TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256]:"
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
{ {
@ -214,6 +214,32 @@ static const CipherTest kCipherTests[] = {
}, },
false, false,
}, },
// Additional masks after @STRENGTH get silently discarded.
//
// TODO(davidben): Make this an error. If not silently discarded, they get
// interpreted as + opcodes which are very different.
{
"ECDHE-RSA-AES128-GCM-SHA256:"
"ECDHE-RSA-AES256-GCM-SHA384:"
"@STRENGTH+AES256",
{
{TLS1_CK_ECDHE_RSA_WITH_AES_256_GCM_SHA384, 0},
{TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 0},
},
false,
},
{
"ECDHE-RSA-AES128-GCM-SHA256:"
"ECDHE-RSA-AES256-GCM-SHA384:"
"@STRENGTH+AES256:"
"ECDHE-RSA-CHACHA20-POLY1305",
{
{TLS1_CK_ECDHE_RSA_WITH_AES_256_GCM_SHA384, 0},
{TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 0},
{TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, 0},
},
false,
},
// Exact ciphers may not be used in multi-part rules; they are treated // Exact ciphers may not be used in multi-part rules; they are treated
// as unknown aliases. // as unknown aliases.
{ {