Serialize SSL configuration in handoff and check it on application.
A split SSL handshake may involve 2 binaries, potentially built at
different versions: call them the "handoff/handback" binary and the
"handshake" binary. We would like to guarantee that the
handoff/handback binary does not make any promises that the handshake
binary cannot keep.
As a start, this commit serializes |kCiphers| to the handoff message.
When the handoff message is applied to an |SSL|, any configured
ciphers not listed in the handoff message will be removed, in order to
prevent them from being negotiated.
Subsequent commits will apply the same approach to other lists of features.
Change-Id: Idf6dbeadb750c076ab0509c09b9d3f22eb162b9c
Reviewed-on: https://boringssl-review.googlesource.com/c/29264
Reviewed-by: Matt Braithwaite <mab@google.com>
diff --git a/ssl/handoff.cc b/ssl/handoff.cc
index a47b7c1..a49d289 100644
--- a/ssl/handoff.cc
+++ b/ssl/handoff.cc
@@ -24,6 +24,22 @@
constexpr int kHandoffVersion = 0;
constexpr int kHandbackVersion = 0;
+// serialize_features adds a description of features supported by this binary to
+// |out|. Returns true on success and false on error.
+static bool serialize_features(CBB *out) {
+ CBB ciphers;
+ if (!CBB_add_asn1(out, &ciphers, CBS_ASN1_OCTETSTRING)) {
+ return false;
+ }
+ Span<const SSL_CIPHER> all_ciphers = AllCiphers();
+ for (const SSL_CIPHER& cipher : all_ciphers) {
+ if (!CBB_add_u16(&ciphers, static_cast<uint16_t>(cipher.id))) {
+ return false;
+ }
+ }
+ return CBB_flush(out);
+}
+
bool SSL_serialize_handoff(const SSL *ssl, CBB *out) {
const SSL3_STATE *const s3 = ssl->s3;
if (!ssl->server ||
@@ -40,6 +56,7 @@
!CBB_add_asn1_octet_string(&seq,
reinterpret_cast<uint8_t *>(s3->hs_buf->data),
s3->hs_buf->length) ||
+ !serialize_features(&seq) ||
!CBB_flush(out)) {
return false;
}
@@ -59,6 +76,53 @@
return true;
}
+// apply_remote_features reads a list of supported features from |in| and
+// (possibly) reconfigures |ssl| to disallow the negotation of features whose
+// support has not been indicated. (This prevents the the handshake from
+// committing to features that are not supported on the handoff/handback side.)
+static bool apply_remote_features(SSL *ssl, CBS *in) {
+ CBS ciphers;
+ if (!CBS_get_asn1(in, &ciphers, CBS_ASN1_OCTETSTRING)) {
+ return false;
+ }
+ bssl::UniquePtr<STACK_OF(SSL_CIPHER)> supported(sk_SSL_CIPHER_new_null());
+ while (CBS_len(&ciphers)) {
+ uint16_t id;
+ if (!CBS_get_u16(&ciphers, &id)) {
+ return false;
+ }
+ const SSL_CIPHER *cipher = SSL_get_cipher_by_value(id);
+ if (!cipher) {
+ continue;
+ }
+ if (!sk_SSL_CIPHER_push(supported.get(), cipher)) {
+ return false;
+ }
+ }
+ STACK_OF(SSL_CIPHER) *configured =
+ ssl->config->cipher_list ? ssl->config->cipher_list->ciphers.get()
+ : ssl->ctx->cipher_list->ciphers.get();
+ bssl::UniquePtr<STACK_OF(SSL_CIPHER)> unsupported(sk_SSL_CIPHER_new_null());
+ for (const SSL_CIPHER *configured_cipher : configured) {
+ if (sk_SSL_CIPHER_find(supported.get(), nullptr, configured_cipher)) {
+ continue;
+ }
+ if (!sk_SSL_CIPHER_push(unsupported.get(), configured_cipher)) {
+ return false;
+ }
+ }
+ if (sk_SSL_CIPHER_num(unsupported.get()) && !ssl->config->cipher_list) {
+ ssl->config->cipher_list = bssl::MakeUnique<SSLCipherPreferenceList>();
+ if (!ssl->config->cipher_list->Init(*ssl->ctx->cipher_list)) {
+ return false;
+ }
+ }
+ for (const SSL_CIPHER *unsupported_cipher : unsupported.get()) {
+ ssl->config->cipher_list->Remove(unsupported_cipher);
+ }
+ return sk_SSL_CIPHER_num(SSL_get_ciphers(ssl)) > 0;
+}
+
bool SSL_apply_handoff(SSL *ssl, Span<const uint8_t> handoff) {
if (ssl->method->is_dtls) {
return false;
@@ -74,7 +138,8 @@
CBS transcript, hs_buf;
if (!CBS_get_asn1(&seq, &transcript, CBS_ASN1_OCTETSTRING) ||
- !CBS_get_asn1(&seq, &hs_buf, CBS_ASN1_OCTETSTRING)) {
+ !CBS_get_asn1(&seq, &hs_buf, CBS_ASN1_OCTETSTRING) ||
+ !apply_remote_features(ssl, &seq)) {
return false;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index a036a17..2e05ee0 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -509,11 +509,17 @@
bool Init(UniquePtr<STACK_OF(SSL_CIPHER)> ciphers,
Span<const bool> in_group_flags);
+ bool Init(const SSLCipherPreferenceList &);
+
+ void Remove(const SSL_CIPHER *cipher);
UniquePtr<STACK_OF(SSL_CIPHER)> ciphers;
bool *in_group_flags = nullptr;
};
+// AllCiphers returns an array of all supported ciphers, sorted by id.
+Span<const SSL_CIPHER> AllCiphers();
+
// ssl_cipher_get_evp_aead sets |*out_aead| to point to the correct EVP_AEAD
// object for |cipher| protocol version |version|. It sets |*out_mac_secret_len|
// and |*out_fixed_iv_len| to the MAC key length and fixed IV length,
diff --git a/ssl/ssl_cipher.cc b/ssl/ssl_cipher.cc
index 0ed91d6..a420f4d 100644
--- a/ssl/ssl_cipher.cc
+++ b/ssl/ssl_cipher.cc
@@ -156,7 +156,6 @@
BSSL_NAMESPACE_BEGIN
-// kCiphers is an array of all supported ciphers, sorted by id.
static constexpr SSL_CIPHER kCiphers[] = {
// The RSA ciphers
// Cipher 02
@@ -464,7 +463,9 @@
};
-static const size_t kCiphersLen = OPENSSL_ARRAY_SIZE(kCiphers);
+Span<const SSL_CIPHER> AllCiphers() {
+ return MakeConstSpan(kCiphers, OPENSSL_ARRAY_SIZE(kCiphers));
+}
#define CIPHER_ADD 1
#define CIPHER_KILL 2
@@ -707,7 +708,7 @@
CIPHER_ORDER **out_head,
CIPHER_ORDER **out_tail) {
Array<CIPHER_ORDER> co_list;
- if (!co_list.Init(kCiphersLen)) {
+ if (!co_list.Init(OPENSSL_ARRAY_SIZE(kCiphers))) {
return false;
}
@@ -772,6 +773,31 @@
return true;
}
+bool SSLCipherPreferenceList::Init(const SSLCipherPreferenceList& other) {
+ size_t size = sk_SSL_CIPHER_num(other.ciphers.get());
+ Span<const bool> other_flags(other.in_group_flags, size);
+ UniquePtr<STACK_OF(SSL_CIPHER)> other_ciphers(sk_SSL_CIPHER_dup(
+ other.ciphers.get()));
+ if (!other_ciphers) {
+ return false;
+ }
+ return Init(std::move(other_ciphers), other_flags);
+}
+
+void SSLCipherPreferenceList::Remove(const SSL_CIPHER *cipher) {
+ size_t index;
+ if (!sk_SSL_CIPHER_find(ciphers.get(), &index, cipher)) {
+ return;
+ }
+ if (!in_group_flags[index] /* last element of group */ && index > 0) {
+ in_group_flags[index-1] = false;
+ }
+ for (size_t i = index; i < sk_SSL_CIPHER_num(ciphers.get()) - 1; ++i) {
+ in_group_flags[i] = in_group_flags[i+1];
+ }
+ sk_SSL_CIPHER_delete(ciphers.get(), index);
+}
+
// ssl_cipher_apply_rule applies the rule type |rule| to ciphers matching its
// parameters in the linked list from |*head_p| to |*tail_p|. It writes the new
// head and tail of the list to |*head_p| and |*tail_p|, respectively.
@@ -1051,7 +1077,7 @@
// Look for a matching exact cipher. These aren't allowed in multipart
// rules.
if (!multi && ch != '+') {
- for (j = 0; j < kCiphersLen; j++) {
+ for (j = 0; j < OPENSSL_ARRAY_SIZE(kCiphers); j++) {
const SSL_CIPHER *cipher = &kCiphers[j];
if (rule_equals(cipher->name, buf, buf_len) ||
rule_equals(cipher->standard_name, buf, buf_len)) {
@@ -1217,7 +1243,7 @@
UniquePtr<STACK_OF(SSL_CIPHER)> cipherstack(sk_SSL_CIPHER_new_null());
Array<bool> in_group_flags;
if (cipherstack == nullptr ||
- !in_group_flags.Init(kCiphersLen)) {
+ !in_group_flags.Init(OPENSSL_ARRAY_SIZE(kCiphers))) {
return false;
}
@@ -1345,7 +1371,8 @@
c.id = 0x03000000L | value;
return reinterpret_cast<const SSL_CIPHER *>(bsearch(
- &c, kCiphers, kCiphersLen, sizeof(SSL_CIPHER), ssl_cipher_id_cmp));
+ &c, kCiphers, OPENSSL_ARRAY_SIZE(kCiphers), sizeof(SSL_CIPHER),
+ ssl_cipher_id_cmp));
}
uint32_t SSL_CIPHER_get_id(const SSL_CIPHER *cipher) { return cipher->id; }
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 4792560..f7b299a 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -4278,6 +4278,35 @@
}
}
+TEST(SSLTest, ApplyHandoffRemovesUnsupportedCiphers) {
+ bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
+ bssl::UniquePtr<SSL> server(SSL_new(server_ctx.get()));
+
+ // handoff is a handoff message that has been artificially modified to pretend
+ // that only cipher 0x0A is supported. When it is applied to |server|, all
+ // ciphers but that one should be removed.
+ uint8_t handoff[] = {
+ 0x30, 0x81, 0x8e, 0x02, 0x01, 0x00, 0x04, 0x00, 0x04, 0x81, 0x82, 0x01,
+ 0x00, 0x00, 0x7e, 0x03, 0x03, 0x77, 0x62, 0x00, 0x9a, 0x13, 0x48, 0x23,
+ 0x46, 0x11, 0x6c, 0x0b, 0x1c, 0x91, 0x4e, 0xbc, 0x1c, 0xff, 0x54, 0xb9,
+ 0xe6, 0x3f, 0xa8, 0x8d, 0x49, 0x37, 0x7a, 0x9e, 0xbf, 0x36, 0xd5, 0x08,
+ 0x24, 0x00, 0x00, 0x1e, 0xc0, 0x2b, 0xc0, 0x2f, 0xc0, 0x2c, 0xc0, 0x30,
+ 0xcc, 0xa9, 0xcc, 0xa8, 0xc0, 0x09, 0xc0, 0x13, 0xc0, 0x0a, 0xc0, 0x14,
+ 0x00, 0x9c, 0x00, 0x9d, 0x00, 0x2f, 0x00, 0x35, 0x00, 0x0a, 0x01, 0x00,
+ 0x00, 0x37, 0xff, 0x01, 0x00, 0x01, 0x00, 0x00, 0x17, 0x00, 0x00, 0x00,
+ 0x23, 0x00, 0x00, 0x00, 0x0d, 0x00, 0x14, 0x00, 0x12, 0x04, 0x03, 0x08,
+ 0x04, 0x04, 0x01, 0x05, 0x03, 0x08, 0x05, 0x05, 0x01, 0x08, 0x06, 0x06,
+ 0x01, 0x02, 0x01, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x0a, 0x00,
+ 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, 0x04, 0x02, 0x00,
+ 0x0a,
+ };
+
+ EXPECT_EQ(20u, sk_SSL_CIPHER_num(SSL_get_ciphers(server.get())));
+ ASSERT_TRUE(
+ SSL_apply_handoff(server.get(), {handoff, OPENSSL_ARRAY_SIZE(handoff)}));
+ EXPECT_EQ(1u, sk_SSL_CIPHER_num(SSL_get_ciphers(server.get())));
+}
+
TEST_P(SSLVersionTest, VerifyBeforeCertRequest) {
// Configure the server to request client certificates.
SSL_CTX_set_custom_verify(
