OpenSSL allows spaces, commas and semi-colons to be used as separators in cipher strings, in addition to the usual colons. This change documents that spaces cannot be used in equal-preference groups and forbids these alternative separators in strict mode. Change-Id: I3879e25aed54539c281511627e6a282e9463bdc3 Reviewed-on: https://boringssl-review.googlesource.com/18424 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>kris/onging/CECPQ3_patch15
@@ -1350,8 +1350,9 @@ OPENSSL_EXPORT int SSL_CIPHER_get_bits(const SSL_CIPHER *cipher, | |||||
* be used. | * be used. | ||||
* | * | ||||
* Unknown rules are silently ignored by legacy APIs, and rejected by APIs with | * Unknown rules are silently ignored by legacy APIs, and rejected by APIs with | ||||
* "strict" in the name, which should be preferred. Cipher lists can be long and | |||||
* it's easy to commit typos. | |||||
* "strict" in the name, which should be preferred. Cipher lists can be long | |||||
* and it's easy to commit typos. Strict functions will also reject the use of | |||||
* spaces, semi-colons and commas as alternative separators. | |||||
* | * | ||||
* The special |@STRENGTH| directive will sort all enabled ciphers by strength. | * The special |@STRENGTH| directive will sort all enabled ciphers by strength. | ||||
* | * | ||||
@@ -1369,7 +1370,7 @@ OPENSSL_EXPORT int SSL_CIPHER_get_bits(const SSL_CIPHER *cipher, | |||||
* [ECDHE-ECDSA-CHACHA20-POLY1305|ECDHE-ECDSA-AES128-GCM-SHA256] | * [ECDHE-ECDSA-CHACHA20-POLY1305|ECDHE-ECDSA-AES128-GCM-SHA256] | ||||
* | * | ||||
* Once an equal-preference group is used, future directives must be | * Once an equal-preference group is used, future directives must be | ||||
* opcode-less. | |||||
* opcode-less. Inside an equal-preference group, spaces are not allowed. | |||||
* | * | ||||
* TLS 1.3 ciphers do not participate in this mechanism and instead have a | * TLS 1.3 ciphers do not participate in this mechanism and instead have a | ||||
* built-in preference order. Functions to set cipher lists do not affect TLS | * built-in preference order. Functions to set cipher lists do not affect TLS | ||||
@@ -756,8 +756,12 @@ const EVP_MD *ssl_get_handshake_digest(uint32_t algorithm_prf, | |||||
} | } | ||||
} | } | ||||
#define ITEM_SEP(a) \ | |||||
(((a) == ':') || ((a) == ' ') || ((a) == ';') || ((a) == ',')) | |||||
static bool is_cipher_list_separator(char c, int is_strict) { | |||||
if (c == ':') { | |||||
return true; | |||||
} | |||||
return !is_strict && (c == ' ' || c == ';' || c == ','); | |||||
} | |||||
/* rule_equals returns one iff the NUL-terminated string |rule| is equal to the | /* rule_equals returns one iff the NUL-terminated string |rule| is equal to the | ||||
* |buf_len| bytes at |buf|. */ | * |buf_len| bytes at |buf|. */ | ||||
@@ -1092,7 +1096,7 @@ static int ssl_cipher_process_rulestr(const SSL_PROTOCOL_METHOD *ssl_method, | |||||
return 0; | return 0; | ||||
} | } | ||||
if (ITEM_SEP(ch)) { | |||||
if (is_cipher_list_separator(ch, strict)) { | |||||
l++; | l++; | ||||
continue; | continue; | ||||
} | } | ||||
@@ -1186,7 +1190,7 @@ static int ssl_cipher_process_rulestr(const SSL_PROTOCOL_METHOD *ssl_method, | |||||
/* We do not support any "multi" options together with "@", so throw away | /* We do not support any "multi" options together with "@", so throw away | ||||
* the rest of the command, if any left, until end or ':' is found. */ | * the rest of the command, if any left, until end or ':' is found. */ | ||||
while (*l != '\0' && !ITEM_SEP(*l)) { | |||||
while (*l != '\0' && !is_cipher_list_separator(*l, strict)) { | |||||
l++; | l++; | ||||
} | } | ||||
} else if (!skip_rule) { | } else if (!skip_rule) { | ||||
@@ -279,6 +279,19 @@ static const CipherTest kCipherTests[] = { | |||||
}, | }, | ||||
false, | false, | ||||
}, | }, | ||||
// Spaces, semi-colons and commas are separators. | |||||
{ | |||||
"AES128-SHA: AES128-SHA256 AES256-SHA ,AES256-SHA256 ; AES128-GCM-SHA256", | |||||
{ | |||||
{TLS1_CK_RSA_WITH_AES_128_SHA, 0}, | |||||
{TLS1_CK_RSA_WITH_AES_128_SHA256, 0}, | |||||
{TLS1_CK_RSA_WITH_AES_256_SHA, 0}, | |||||
{TLS1_CK_RSA_WITH_AES_256_SHA256, 0}, | |||||
{TLS1_CK_RSA_WITH_AES_128_GCM_SHA256, 0}, | |||||
}, | |||||
// …but not in strict mode. | |||||
true, | |||||
}, | |||||
}; | }; | ||||
static const char *kBadRules[] = { | static const char *kBadRules[] = { | ||||
@@ -304,6 +317,8 @@ static const char *kBadRules[] = { | |||||
"[ECDHE-RSA-CHACHA20-POLY1305|ECDHE-RSA-AES128-GCM-SHA256]:@STRENGTH", | "[ECDHE-RSA-CHACHA20-POLY1305|ECDHE-RSA-AES128-GCM-SHA256]:@STRENGTH", | ||||
// Opcode supplied, but missing selector. | // Opcode supplied, but missing selector. | ||||
"+", | "+", | ||||
// Spaces are forbidden in equal-preference groups. | |||||
"[AES128-SHA | AES128-SHA256]", | |||||
}; | }; | ||||
static const char *kMustNotIncludeNull[] = { | static const char *kMustNotIncludeNull[] = { | ||||