Detach TLS 1.3 cipher configuration from the cipher language.
TLS 1.3 ciphers are now always enabled and come with a hard-coded
preference order.
BUG=110
Change-Id: Idd9cb0d75fb6bf2676ecdee27d88893ff974c4a3
Reviewed-on: https://boringssl-review.googlesource.com/12025
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 417b194..449bd8c 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1306,7 +1306,12 @@
* [ECDHE-ECDSA-CHACHA20-POLY1305|ECDHE-ECDSA-AES128-GCM-SHA256]
*
* Once an equal-preference group is used, future directives must be
- * opcode-less. */
+ * opcode-less.
+ *
+ * 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
+ * 1.3, and functions to query the cipher list do not include TLS 1.3
+ * ciphers. */
/* SSL_DEFAULT_CIPHER_LIST is the default cipher suite configuration. It is
* substituted when a cipher string starts with 'DEFAULT'. */
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index b26b012..67a4c09 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -152,6 +152,7 @@
#include <assert.h>
#include <string.h>
+#include <openssl/aead.h>
#include <openssl/bn.h>
#include <openssl/buf.h>
#include <openssl/bytestring.h>
@@ -605,30 +606,48 @@
return 0;
}
- STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(ssl);
-
- int any_enabled = 0;
- for (size_t i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) {
- const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(ciphers, i);
- /* Skip disabled ciphers */
- if ((cipher->algorithm_mkey & ssl->cert->mask_k) ||
- (cipher->algorithm_auth & ssl->cert->mask_a)) {
- continue;
+ /* Add TLS 1.3 ciphers. Order ChaCha20-Poly1305 relative to AES-GCM based on
+ * hardware support. */
+ if (max_version >= TLS1_3_VERSION) {
+ if (!EVP_has_aes_hardware() &&
+ !CBB_add_u16(&child, TLS1_CK_CHACHA20_POLY1305_SHA256 & 0xffff)) {
+ return 0;
}
- if (SSL_CIPHER_get_min_version(cipher) > max_version ||
- SSL_CIPHER_get_max_version(cipher) < min_version) {
- continue;
+ if (!CBB_add_u16(&child, TLS1_CK_AES_128_GCM_SHA256 & 0xffff) ||
+ !CBB_add_u16(&child, TLS1_CK_AES_256_GCM_SHA384 & 0xffff)) {
+ return 0;
}
- any_enabled = 1;
- if (!CBB_add_u16(&child, ssl_cipher_get_value(cipher))) {
+ if (EVP_has_aes_hardware() &&
+ !CBB_add_u16(&child, TLS1_CK_CHACHA20_POLY1305_SHA256 & 0xffff)) {
return 0;
}
}
- /* If all ciphers were disabled, return the error to the caller. */
- if (!any_enabled) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHERS_AVAILABLE);
- return 0;
+ if (min_version < TLS1_3_VERSION) {
+ STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(ssl);
+ int any_enabled = 0;
+ for (size_t i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) {
+ const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(ciphers, i);
+ /* Skip disabled ciphers */
+ if ((cipher->algorithm_mkey & ssl->cert->mask_k) ||
+ (cipher->algorithm_auth & ssl->cert->mask_a)) {
+ continue;
+ }
+ if (SSL_CIPHER_get_min_version(cipher) > max_version ||
+ SSL_CIPHER_get_max_version(cipher) < min_version) {
+ continue;
+ }
+ any_enabled = 1;
+ if (!CBB_add_u16(&child, ssl_cipher_get_value(cipher))) {
+ return 0;
+ }
+ }
+
+ /* If all ciphers were disabled, return the error to the caller. */
+ if (!any_enabled && max_version < TLS1_3_VERSION) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHERS_AVAILABLE);
+ return 0;
+ }
}
/* For SSLv3, the SCSV is added. Otherwise the renegotiation extension is
diff --git a/ssl/internal.h b/ssl/internal.h
index 461de2c..b217017 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1658,6 +1658,9 @@
void ssl_cipher_preference_list_free(
struct ssl_cipher_preference_list_st *cipher_list);
+
+/* ssl_get_cipher_preferences returns the cipher preference list for TLS 1.2 and
+ * below. */
const struct ssl_cipher_preference_list_st *ssl_get_cipher_preferences(
const SSL *ssl);
@@ -1712,7 +1715,7 @@
/* ssl_is_valid_cipher checks that |cipher| is valid according to the current
* server configuration in |ssl|. It returns 1 if valid, and 0 otherwise. */
-int ssl_is_valid_cipher(SSL *ssl, const SSL_CIPHER *cipher);
+int ssl_is_valid_cipher(const SSL *ssl, const SSL_CIPHER *cipher);
const SSL_CIPHER *ssl3_choose_cipher(
SSL *ssl, const struct ssl_early_callback_ctx *client_hello,
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 2a7c91a..f119523 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -241,15 +241,21 @@
return NULL;
}
-int ssl_is_valid_cipher(SSL *ssl, const SSL_CIPHER *cipher) {
+int ssl_is_valid_cipher(const SSL *ssl, const SSL_CIPHER *cipher) {
/* Check the TLS version. */
- if (SSL_CIPHER_get_min_version(cipher) > ssl3_protocol_version(ssl) ||
- SSL_CIPHER_get_max_version(cipher) < ssl3_protocol_version(ssl)) {
+ uint16_t version = ssl3_protocol_version(ssl);
+ if (SSL_CIPHER_get_min_version(cipher) > version ||
+ SSL_CIPHER_get_max_version(cipher) < version) {
return 0;
}
- return sk_SSL_CIPHER_find(ssl_get_cipher_preferences(ssl)->ciphers,
- NULL, cipher);
+ /* TLS 1.3 ciphers are not configurable. */
+ if (version >= TLS1_3_VERSION) {
+ return 1;
+ }
+
+ return sk_SSL_CIPHER_find(ssl_get_cipher_preferences(ssl)->ciphers, NULL,
+ cipher);
}
const SSL_CIPHER *ssl3_choose_cipher(
diff --git a/ssl/ssl_cipher.c b/ssl/ssl_cipher.c
index deb463e..9ca7f24 100644
--- a/ssl/ssl_cipher.c
+++ b/ssl/ssl_cipher.c
@@ -738,9 +738,6 @@
{"TLSv1", ~SSL_kCECPQ1, ~0u, ~SSL_eNULL, ~0u, SSL3_VERSION},
{"TLSv1.2", ~SSL_kCECPQ1, ~0u, ~SSL_eNULL, ~0u, TLS1_2_VERSION},
- /* AEAD-only ciphers for TLS 1.3. */
- {"GENERIC", SSL_kGENERIC, SSL_aGENERIC, ~0u, ~0u, 0},
-
/* Legacy strength classes. */
{"HIGH", ~SSL_kCECPQ1, ~0u, ~SSL_eNULL, ~0u, 0},
{"FIPS", ~SSL_kCECPQ1, ~0u, ~SSL_eNULL, ~0u, 0},
@@ -943,7 +940,9 @@
size_t co_list_num = 0;
for (size_t i = 0; i < kCiphersLen; i++) {
const SSL_CIPHER *cipher = &kCiphers[i];
- if (ssl_method->supports_cipher(cipher)) {
+ if (ssl_method->supports_cipher(cipher) &&
+ /* TLS 1.3 ciphers do not participate in this mechanism. */
+ cipher->algorithm_mkey != SSL_kGENERIC) {
co_list[co_list_num].cipher = cipher;
co_list[co_list_num].next = NULL;
co_list[co_list_num].prev = NULL;
@@ -1386,11 +1385,8 @@
/* Now arrange all ciphers by preference:
* TODO(davidben): Compute this order once and copy it. */
- /* Everything else being equal, prefer TLS 1.3 ciphers then ECDHE_ECDSA then
- * ECDHE_RSA over other key exchange mechanisms */
-
- ssl_cipher_apply_rule(0, SSL_kGENERIC, SSL_aGENERIC, ~0u, ~0u, 0, CIPHER_ADD,
- -1, 0, &head, &tail);
+ /* Everything else being equal, prefer ECDHE_ECDSA and ECDHE_RSA over other
+ * key exchange mechanisms */
ssl_cipher_apply_rule(0, SSL_kECDHE, SSL_aECDSA, ~0u, ~0u, 0, CIPHER_ADD, -1,
0, &head, &tail);
ssl_cipher_apply_rule(0, SSL_kECDHE, ~0u, ~0u, ~0u, 0, CIPHER_ADD, -1, 0,
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 4c658d3..ed32c21 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -269,7 +269,6 @@
"SSLv3",
"TLSv1",
"TLSv1.2",
- "GENERIC",
};
static const char *kMustNotIncludeCECPQ1[] = {
@@ -294,7 +293,6 @@
"AES256",
"AESGCM",
"CHACHA20",
- "GENERIC",
};
static const CurveTest kCurveTests[] = {
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 2e2f580..ed17aa8 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -40,6 +40,7 @@
#include <inttypes.h>
#include <string.h>
+#include <openssl/aead.h>
#include <openssl/bio.h>
#include <openssl/buf.h>
#include <openssl/bytestring.h>
@@ -1320,6 +1321,25 @@
}
}
+ uint16_t cipher_id =
+ static_cast<uint16_t>(SSL_CIPHER_get_id(SSL_get_current_cipher(ssl)));
+ if (config->expect_cipher_aes != 0 &&
+ EVP_has_aes_hardware() &&
+ static_cast<uint16_t>(config->expect_cipher_aes) != cipher_id) {
+ fprintf(stderr, "Cipher ID was %04x, wanted %04x (has AES hardware)\n",
+ cipher_id, static_cast<uint16_t>(config->expect_cipher_aes));
+ return false;
+ }
+
+ if (config->expect_cipher_no_aes != 0 &&
+ !EVP_has_aes_hardware() &&
+ static_cast<uint16_t>(config->expect_cipher_no_aes) != cipher_id) {
+ fprintf(stderr, "Cipher ID was %04x, wanted %04x (no AES hardware)\n",
+ cipher_id, static_cast<uint16_t>(config->expect_cipher_no_aes));
+ return false;
+ }
+
+
if (!config->psk.empty()) {
if (SSL_get_peer_cert_chain(ssl) != nullptr) {
fprintf(stderr, "Received peer certificate on a PSK cipher.\n");
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ae1d0b6..bc4e570 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5508,26 +5508,6 @@
expectResumeRejected: true,
})
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Resume-Server-DeclineBadCipher-2-TLS13",
- resumeSession: true,
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- FilterTicket: func(in []byte) ([]byte, error) {
- return SetShimTicketCipherSuite(in, TLS_AES_256_GCM_SHA384)
- },
- },
- },
- flags: []string{
- "-cipher", "AES128",
- "-ticket-key",
- base64.StdEncoding.EncodeToString(TestShimTicketKey),
- },
- expectResumeRejected: true,
- })
-
// Sessions may not be resumed at a different cipher.
testCases = append(testCases, testCase{
name: "Resume-Client-CipherMismatch",
@@ -8973,6 +8953,58 @@
})
}
+func addTLS13CipherPreferenceTests() {
+ // Test that client preference is honored if the shim has AES hardware
+ // and ChaCha20-Poly1305 is preferred otherwise.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "TLS13-CipherPreference-Server-ChaCha20-AES",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ CipherSuites: []uint16{
+ TLS_CHACHA20_POLY1305_SHA256,
+ TLS_AES_128_GCM_SHA256,
+ },
+ },
+ flags: []string{
+ "-expect-cipher-aes", strconv.Itoa(int(TLS_CHACHA20_POLY1305_SHA256)),
+ "-expect-cipher-no-aes", strconv.Itoa(int(TLS_CHACHA20_POLY1305_SHA256)),
+ },
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "TLS13-CipherPreference-Server-AES-ChaCha20",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ CipherSuites: []uint16{
+ TLS_AES_128_GCM_SHA256,
+ TLS_CHACHA20_POLY1305_SHA256,
+ },
+ },
+ flags: []string{
+ "-expect-cipher-aes", strconv.Itoa(int(TLS_AES_128_GCM_SHA256)),
+ "-expect-cipher-no-aes", strconv.Itoa(int(TLS_CHACHA20_POLY1305_SHA256)),
+ },
+ })
+
+ // Test that the client orders ChaCha20-Poly1305 and AES-GCM based on
+ // whether it has AES hardware.
+ testCases = append(testCases, testCase{
+ name: "TLS13-CipherPreference-Client",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ // Use the client cipher order. (This is the default but
+ // is listed to be explicit.)
+ PreferServerCipherSuites: false,
+ },
+ flags: []string{
+ "-expect-cipher-aes", strconv.Itoa(int(TLS_AES_128_GCM_SHA256)),
+ "-expect-cipher-no-aes", strconv.Itoa(int(TLS_CHACHA20_POLY1305_SHA256)),
+ },
+ })
+}
+
func addPeekTests() {
// Test SSL_peek works, including on empty records.
testCases = append(testCases, testCase{
@@ -9160,6 +9192,7 @@
addWrongMessageTypeTests()
addTrailingMessageDataTests()
addTLS13HandshakeTests()
+ addTLS13CipherPreferenceTests()
addPeekTests()
var wg sync.WaitGroup
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 112d642..3d3ecd8 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -158,6 +158,8 @@
{ "-expect-dhe-group-size", &TestConfig::expect_dhe_group_size },
{ "-initial-timeout-duration-ms", &TestConfig::initial_timeout_duration_ms },
{ "-max-cert-list", &TestConfig::max_cert_list },
+ { "-expect-cipher-aes", &TestConfig::expect_cipher_aes },
+ { "-expect-cipher-no-aes", &TestConfig::expect_cipher_no_aes },
};
const Flag<std::vector<int>> kIntVectorFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index d91a74b..34c02fb 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -117,6 +117,8 @@
int max_cert_list = 0;
std::string ticket_key;
bool use_exporter_between_reads = false;
+ int expect_cipher_aes = 0;
+ int expect_cipher_no_aes = 0;
};
bool ParseConfig(int argc, char **argv, TestConfig *out_config);
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index 3b85442..409dc24 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -218,12 +218,9 @@
return ssl_hs_error;
}
- /* Check if the cipher is disabled. */
- if ((cipher->algorithm_mkey & ssl->cert->mask_k) ||
- (cipher->algorithm_auth & ssl->cert->mask_a) ||
- SSL_CIPHER_get_min_version(cipher) > ssl3_protocol_version(ssl) ||
- SSL_CIPHER_get_max_version(cipher) < ssl3_protocol_version(ssl) ||
- !sk_SSL_CIPHER_find(ssl_get_ciphers_by_id(ssl), NULL, cipher)) {
+ /* Check if the cipher is a TLS 1.3 cipher. */
+ if (SSL_CIPHER_get_min_version(cipher) > ssl3_protocol_version(ssl) ||
+ SSL_CIPHER_get_max_version(cipher) < ssl3_protocol_version(ssl)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CIPHER_RETURNED);
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index 574e6dd..14ebb81 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -17,6 +17,7 @@
#include <assert.h>
#include <string.h>
+#include <openssl/aead.h>
#include <openssl/bytestring.h>
#include <openssl/digest.h>
#include <openssl/err.h>
@@ -171,6 +172,48 @@
return ssl_hs_ok;
}
+static const SSL_CIPHER *choose_tls13_cipher(
+ const SSL *ssl, const struct ssl_early_callback_ctx *client_hello) {
+ if (client_hello->cipher_suites_len % 2 != 0) {
+ return NULL;
+ }
+
+ CBS cipher_suites;
+ CBS_init(&cipher_suites, client_hello->cipher_suites,
+ client_hello->cipher_suites_len);
+
+ const int aes_is_fine = EVP_has_aes_hardware();
+
+ const SSL_CIPHER *best = NULL;
+ while (CBS_len(&cipher_suites) > 0) {
+ uint16_t cipher_suite;
+ if (!CBS_get_u16(&cipher_suites, &cipher_suite)) {
+ return NULL;
+ }
+
+ const SSL_CIPHER *candidate = SSL_get_cipher_by_value(cipher_suite);
+ if (candidate == NULL || !ssl_is_valid_cipher(ssl, candidate)) {
+ continue;
+ }
+
+ /* TLS 1.3 removes legacy ciphers, so honor the client order, but prefer
+ * ChaCha20 if we do not have AES hardware. */
+ if (aes_is_fine) {
+ return candidate;
+ }
+
+ if (candidate->algorithm_enc == SSL_CHACHA20POLY1305) {
+ return candidate;
+ }
+
+ if (best == NULL) {
+ best = candidate;
+ }
+ }
+
+ return best;
+}
+
static enum ssl_hs_wait_t do_select_parameters(SSL *ssl, SSL_HANDSHAKE *hs) {
if (!ssl->s3->session_reused) {
/* Call |cert_cb| to update server certificates if required. */
@@ -197,8 +240,7 @@
}
if (!ssl->s3->session_reused) {
- const SSL_CIPHER *cipher =
- ssl3_choose_cipher(ssl, &client_hello, ssl_get_cipher_preferences(ssl));
+ const SSL_CIPHER *cipher = choose_tls13_cipher(ssl, &client_hello);
if (cipher == NULL) {
OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
