Update to TLS 1.3 draft 18. This is the squash of the following CLs: https://boringssl-review.googlesource.com/c/12021/9 https://boringssl-review.googlesource.com/c/12022/9 https://boringssl-review.googlesource.com/c/12107/19 https://boringssl-review.googlesource.com/c/12141/22 https://boringssl-review.googlesource.com/c/12181/33 The Go portions were written by Nick Harper BUG=112 Change-Id: I375a1fcead493ec3e0282e231ccc8d7c4dde5063 Reviewed-on: https://boringssl-review.googlesource.com/12300 CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata index c25683e..3fb4d8f 100644 --- a/crypto/err/ssl.errordata +++ b/crypto/err/ssl.errordata
@@ -105,6 +105,7 @@ SSL,191,PATH_TOO_LONG SSL,192,PEER_DID_NOT_RETURN_A_CERTIFICATE SSL,193,PEER_ERROR_UNSUPPORTED_CERTIFICATE_TYPE +SSL,267,PRE_SHARED_KEY_MUST_BE_LAST SSL,194,PROTOCOL_IS_SHUTDOWN SSL,195,PSK_IDENTITY_NOT_FOUND SSL,196,PSK_NO_CLIENT_CB
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 1d56e75..827ae28 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -562,7 +562,7 @@ #define DTLS1_VERSION 0xfeff #define DTLS1_2_VERSION 0xfefd -#define TLS1_3_DRAFT_VERSION 0x7f10 +#define TLS1_3_DRAFT_VERSION 0x7f12 /* SSL_CTX_set_min_proto_version sets the minimum protocol version for |ctx| to * |version|. If |version| is zero, the default minimum version is used. It @@ -4536,6 +4536,7 @@ #define SSL_R_DUPLICATE_KEY_SHARE 264 #define SSL_R_NO_GROUPS_SPECIFIED 265 #define SSL_R_NO_SHARED_GROUP 266 +#define SSL_R_PRE_SHARED_KEY_MUST_BE_LAST 267 #define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000 #define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010 #define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h index 35d3145..dfaf78a 100644 --- a/include/openssl/tls1.h +++ b/include/openssl/tls1.h
@@ -205,13 +205,14 @@ /* ExtensionType value from RFC4507 */ #define TLSEXT_TYPE_session_ticket 35 -/* ExtensionType values from draft-ietf-tls-tls13-16 */ +/* ExtensionType values from draft-ietf-tls-tls13-18 */ #define TLSEXT_TYPE_supported_groups 10 #define TLSEXT_TYPE_key_share 40 #define TLSEXT_TYPE_pre_shared_key 41 #define TLSEXT_TYPE_early_data 42 #define TLSEXT_TYPE_supported_versions 43 #define TLSEXT_TYPE_cookie 44 +#define TLSEXT_TYPE_psk_key_exchange_modes 45 /* ExtensionType value from RFC5746 */ #define TLSEXT_TYPE_renegotiate 0xff01
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c index 27cd7ba..5161f13 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c
@@ -666,43 +666,66 @@ return CBB_flush(out); } -int ssl_add_client_hello_body(SSL *ssl, CBB *body) { +int ssl_write_client_hello(SSL *ssl) { uint16_t min_version, max_version; if (!ssl_get_version_range(ssl, &min_version, &max_version)) { return 0; } + CBB cbb, body; + if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CLIENT_HELLO)) { + goto err; + } + /* Renegotiations do not participate in session resumption. */ int has_session = ssl->session != NULL && !ssl->s3->initial_handshake_complete; CBB child; - if (!CBB_add_u16(body, ssl->client_version) || - !CBB_add_bytes(body, ssl->s3->client_random, SSL3_RANDOM_SIZE) || - !CBB_add_u8_length_prefixed(body, &child) || + if (!CBB_add_u16(&body, ssl->client_version) || + !CBB_add_bytes(&body, ssl->s3->client_random, SSL3_RANDOM_SIZE) || + !CBB_add_u8_length_prefixed(&body, &child) || (has_session && !CBB_add_bytes(&child, ssl->session->session_id, ssl->session->session_id_length))) { - return 0; + goto err; } if (SSL_is_dtls(ssl)) { - if (!CBB_add_u8_length_prefixed(body, &child) || + if (!CBB_add_u8_length_prefixed(&body, &child) || !CBB_add_bytes(&child, ssl->d1->cookie, ssl->d1->cookie_len)) { - return 0; + goto err; } } size_t header_len = SSL_is_dtls(ssl) ? DTLS1_HM_HEADER_LENGTH : SSL3_HM_HEADER_LENGTH; - if (!ssl_write_client_cipher_list(ssl, body, min_version, max_version) || - !CBB_add_u8(body, 1 /* one compression method */) || - !CBB_add_u8(body, 0 /* null compression */) || - !ssl_add_clienthello_tlsext(ssl, body, header_len + CBB_len(body))) { - return 0; + if (!ssl_write_client_cipher_list(ssl, &body, min_version, max_version) || + !CBB_add_u8(&body, 1 /* one compression method */) || + !CBB_add_u8(&body, 0 /* null compression */) || + !ssl_add_clienthello_tlsext(ssl, &body, header_len + CBB_len(&body))) { + goto err; } - return 1; + uint8_t *msg = NULL; + size_t len; + if (!ssl->method->finish_message(ssl, &cbb, &msg, &len)) { + goto err; + } + + /* Now that the length prefixes have been computed, fill in the placeholder + * PSK binder. */ + if (ssl->s3->hs->needs_psk_binder && + !tls13_write_psk_binder(ssl, msg, len)) { + OPENSSL_free(msg); + goto err; + } + + return ssl->method->queue_message(ssl, msg, len); + + err: + CBB_cleanup(&cbb); + return 0; } static int ssl3_send_client_hello(SSL *ssl) { @@ -717,12 +740,9 @@ return -1; } - CBB cbb; - CBB_zero(&cbb); - uint16_t min_version, max_version; if (!ssl_get_version_range(ssl, &min_version, &max_version)) { - goto err; + return -1; } assert(ssl->state == SSL3_ST_CW_CLNT_HELLO_A); @@ -758,22 +778,15 @@ * renegerate the client_random. The random must be reused. */ if ((!SSL_is_dtls(ssl) || !ssl->d1->send_cookie) && !RAND_bytes(ssl->s3->client_random, sizeof(ssl->s3->client_random))) { - goto err; + return -1; } - CBB body; - if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CLIENT_HELLO) || - !ssl_add_client_hello_body(ssl, &body) || - !ssl_complete_message(ssl, &cbb)) { - goto err; + if (!ssl_write_client_hello(ssl)) { + return -1; } ssl->state = SSL3_ST_CW_CLNT_HELLO_B; return ssl->method->write_message(ssl); - -err: - CBB_cleanup(&cbb); - return -1; } static int dtls1_get_hello_verify(SSL *ssl) {
diff --git a/ssl/internal.h b/ssl/internal.h index 861f8bd..830e76b 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -748,6 +748,10 @@ * configured and zero otherwise. */ int ssl_has_certificate(const SSL *ssl); +/* ssl_parse_x509 parses a X509 certificate from |cbs|. It returns NULL + * on error. */ +X509 *ssl_parse_x509(CBS *cbs); + /* ssl_parse_cert_chain parses a certificate list from |cbs| in the format used * by a TLS Certificate message. On success, it returns a newly-allocated * |X509| list and advances |cbs|. Otherwise, it returns NULL and sets @@ -792,21 +796,19 @@ /* TLS 1.3 key derivation. */ /* tls13_init_key_schedule initializes the handshake hash and key derivation - * state with the given resumption context. The cipher suite and PRF hash must - * have been selected at this point. It returns one on success and zero on - * error. */ -int tls13_init_key_schedule(SSL *ssl, const uint8_t *resumption_ctx, - size_t resumption_ctx_len); + * state. The cipher suite and PRF hash must have been selected at this point. + * It returns one on success and zero on error. */ +int tls13_init_key_schedule(SSL *ssl); /* tls13_advance_key_schedule incorporates |in| into the key schedule with * HKDF-Extract. It returns one on success and zero on error. */ int tls13_advance_key_schedule(SSL *ssl, const uint8_t *in, size_t len); -/* tls13_get_context_hashes writes Hash(Handshake Context) + - * Hash(resumption_context) to |out| which much have room for at least 2 * - * |EVP_MAX_MD_SIZE| bytes. On success, it returns one and sets |*out_len| to - * the number of bytes written. Otherwise, it returns zero. */ -int tls13_get_context_hashes(SSL *ssl, uint8_t *out, size_t *out_len); +/* tls13_get_context_hash writes Hash(Handshake Context) to |out| which must + * have room for at least |EVP_MAX_MD_SIZE| bytes. On success, it returns one + * and sets |*out_len| to the number of bytes written. Otherwise, it returns + * zero. */ +int tls13_get_context_hash(SSL *ssl, uint8_t *out, size_t *out_len); enum tls_record_type_t { type_early_handshake, @@ -815,11 +817,9 @@ type_data, }; -/* tls13_set_traffic_key sets the read or write traffic keys to |traffic_secret| - * for the given traffic phase |type|. It returns one on success and zero on - * error. */ -int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type, - enum evp_aead_direction_t direction, +/* tls13_set_traffic_key sets the read or write traffic keys to + * |traffic_secret|. It returns one on success and zero on error. */ +int tls13_set_traffic_key(SSL *ssl, enum evp_aead_direction_t direction, const uint8_t *traffic_secret, size_t traffic_secret_len); @@ -832,15 +832,15 @@ * returns one on success and zero on error. */ int tls13_rotate_traffic_key(SSL *ssl, enum evp_aead_direction_t direction); -/* tls13_derive_traffic_secret_0 derives the initial application data traffic - * secret based on the handshake transcripts and |master_secret|. It returns one - * on success and zero on error. */ -int tls13_derive_traffic_secret_0(SSL *ssl); +/* tls13_derive_application_secrets derives the initial application data traffic + * and exporter secrets based on the handshake transcripts and |master_secret|. + * It returns one on success and zero on error. */ +int tls13_derive_application_secrets(SSL *ssl); -/* tls13_finalize_keys derives the |exporter_secret| and |resumption_secret|. */ -int tls13_finalize_keys(SSL *ssl); +/* tls13_derive_resumption_secret derives the |resumption_secret|. */ +int tls13_derive_resumption_secret(SSL *ssl); -/* tls13_export_keying_material provides and exporter interface to use the +/* tls13_export_keying_material provides an exporter interface to use the * |exporter_secret|. */ int tls13_export_keying_material(SSL *ssl, uint8_t *out, size_t out_len, const char *label, size_t label_len, @@ -853,17 +853,15 @@ * 0 for the Client Finished. */ int tls13_finished_mac(SSL *ssl, uint8_t *out, size_t *out_len, int is_server); -/* tls13_resumption_psk calculates the PSK to use for the resumption of - * |session| and stores the result in |out|. It returns one on success, and - * zero on failure. */ -int tls13_resumption_psk(SSL *ssl, uint8_t *out, size_t out_len, - const SSL_SESSION *session); +/* tls13_write_psk_binder calculates the PSK binder value and replaces the last + * bytes of |msg| with the resulting value. It returns 1 on success, and 0 on + * failure. */ +int tls13_write_psk_binder(SSL *ssl, uint8_t *msg, size_t len); -/* tls13_resumption_context derives the context to be used for the handshake - * transcript on the resumption of |session|. It returns one on success, and - * zero on failure. */ -int tls13_resumption_context(SSL *ssl, uint8_t *out, size_t out_len, - const SSL_SESSION *session); +/* tls13_verify_psk_binder verifies that the handshake transcript, truncated + * up to the binders has a valid signature using the value of |session|'s + * resumption secret. It returns 1 on success, and 0 on failure. */ +int tls13_verify_psk_binder(SSL *ssl, SSL_SESSION *session, CBS *binders); /* Handshake functions. */ @@ -893,7 +891,6 @@ int state; size_t hash_len; - uint8_t resumption_hash[EVP_MAX_MD_SIZE]; uint8_t secret[EVP_MAX_MD_SIZE]; uint8_t client_traffic_secret_0[EVP_MAX_MD_SIZE]; uint8_t server_traffic_secret_0[EVP_MAX_MD_SIZE]; @@ -922,8 +919,19 @@ /* ecdh_ctx is the current ECDH instance. */ SSL_ECDH_CTX ecdh_ctx; + /* scts_requested is one if the SCT extension is in the ClientHello. */ + unsigned scts_requested:1; + + /* needs_psk_binder if the ClientHello has a placeholder PSK binder to be + * filled in. */ + unsigned needs_psk_binder:1; + unsigned received_hello_retry_request:1; + /* accept_psk_mode stores whether the client's PSK mode is compatible with our + * preferences. */ + unsigned accept_psk_mode:1; + /* retry_group is the group ID selected by the server in HelloRetryRequest in * TLS 1.3. */ uint16_t retry_group; @@ -1054,14 +1062,20 @@ uint8_t *out_alert, CBS *contents); int ssl_ext_key_share_add_serverhello(SSL *ssl, CBB *out); +int ssl_ext_pre_shared_key_add_clienthello(SSL *ssl, CBB *out); int ssl_ext_pre_shared_key_parse_serverhello(SSL *ssl, uint8_t *out_alert, CBS *contents); int ssl_ext_pre_shared_key_parse_clienthello(SSL *ssl, SSL_SESSION **out_session, + CBS *out_binders, uint8_t *out_alert, CBS *contents); int ssl_ext_pre_shared_key_add_serverhello(SSL *ssl, CBB *out); -int ssl_add_client_hello_body(SSL *ssl, CBB *body); +int ssl_ext_psk_key_exchange_modes_parse_clienthello(SSL *ssl, + uint8_t *out_alert, + CBS *contents); + +int ssl_write_client_hello(SSL *ssl); /* ssl_clear_tls13_state releases client state only needed for TLS 1.3. It * should be called once the version is known to be TLS 1.2 or earlier. */ @@ -1598,12 +1612,9 @@ extern const SSL3_ENC_METHOD TLSv1_enc_data; extern const SSL3_ENC_METHOD SSLv3_enc_data; -/* From draft-ietf-tls-tls13-16, used in determining PSK modes. */ -#define SSL_PSK_KE 0x0 -#define SSL_PSK_DHE_KE 0x1 - -#define SSL_PSK_AUTH 0x0 -#define SSL_PSK_SIGN_AUTH 0x1 +/* From draft-ietf-tls-tls13-18, used in determining PSK modes. */ +#define SSL_PSK_KE 0x0 +#define SSL_PSK_DHE_KE 0x1 /* From draft-ietf-tls-tls13-16, used in determining whether to respond with a * KeyUpdate. */
diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c index c1d931f..2a7b7d4 100644 --- a/ssl/ssl_asn1.c +++ b/ssl/ssl_asn1.c
@@ -511,20 +511,6 @@ return 1; } -static X509 *parse_x509(CBS *cbs) { - if (CBS_len(cbs) > LONG_MAX) { - OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION); - return NULL; - } - const uint8_t *ptr = CBS_data(cbs); - X509 *ret = d2i_X509(NULL, &ptr, (long)CBS_len(cbs)); - if (ret == NULL) { - return NULL; - } - CBS_skip(cbs, ptr - CBS_data(cbs)); - return ret; -} - static SSL_SESSION *SSL_SESSION_parse(CBS *cbs) { SSL_SESSION *ret = SSL_SESSION_new(); if (ret == NULL) { @@ -593,7 +579,7 @@ X509_free(ret->x509_peer); ret->x509_peer = NULL; if (has_peer) { - ret->x509_peer = parse_x509(&peer); + ret->x509_peer = ssl_parse_x509(&peer); if (ret->x509_peer == NULL) { goto err; } @@ -679,7 +665,7 @@ goto err; } while (CBS_len(&cert_chain) > 0) { - X509 *x509 = parse_x509(&cert_chain); + X509 *x509 = ssl_parse_x509(&cert_chain); if (x509 == NULL) { goto err; }
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index c3809ef..7f6cf63 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c
@@ -115,6 +115,7 @@ #include <openssl/ssl.h> #include <assert.h> +#include <limits.h> #include <string.h> #include <openssl/bn.h> @@ -446,6 +447,20 @@ return ssl->cert->x509_leaf != NULL && ssl_has_private_key(ssl); } +X509 *ssl_parse_x509(CBS *cbs) { + if (CBS_len(cbs) > LONG_MAX) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + return NULL; + } + const uint8_t *ptr = CBS_data(cbs); + X509 *ret = d2i_X509(NULL, &ptr, (long)CBS_len(cbs)); + if (ret == NULL) { + return NULL; + } + CBS_skip(cbs, ptr - CBS_data(cbs)); + return ret; +} + STACK_OF(X509) *ssl_parse_cert_chain(SSL *ssl, uint8_t *out_alert, uint8_t *out_leaf_sha256, CBS *cbs) { STACK_OF(X509) *ret = sk_X509_new_null(); @@ -476,10 +491,8 @@ SHA256(CBS_data(&certificate), CBS_len(&certificate), out_leaf_sha256); } - /* A u24 length cannot overflow a long. */ - const uint8_t *data = CBS_data(&certificate); - x = d2i_X509(NULL, &data, (long)CBS_len(&certificate)); - if (x == NULL || data != CBS_data(&certificate) + CBS_len(&certificate)) { + x = ssl_parse_x509(&certificate); + if (x == NULL || CBS_len(&certificate) != 0) { *out_alert = SSL_AD_DECODE_ERROR; goto err; }
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 5eede01..ac86baa 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -865,18 +865,22 @@ return true; } -// CreateSessionWithTicket returns a sample |SSL_SESSION| with the ticket -// replaced for one of length |ticket_len| or nullptr on failure. -static bssl::UniquePtr<SSL_SESSION> CreateSessionWithTicket(size_t ticket_len) { +// CreateSessionWithTicket returns a sample |SSL_SESSION| with the specified +// version and ticket length or nullptr on failure. +static bssl::UniquePtr<SSL_SESSION> CreateSessionWithTicket(uint16_t version, + size_t ticket_len) { std::vector<uint8_t> der; if (!DecodeBase64(&der, kOpenSSLSession)) { return nullptr; } - bssl::UniquePtr<SSL_SESSION> session(SSL_SESSION_from_bytes(der.data(), der.size())); + bssl::UniquePtr<SSL_SESSION> session( + SSL_SESSION_from_bytes(der.data(), der.size())); if (!session) { return nullptr; } + session->ssl_version = version; + // Swap out the ticket for a garbage one. OPENSSL_free(session->tlsext_tick); session->tlsext_tick = reinterpret_cast<uint8_t*>(OPENSSL_malloc(ticket_len)); @@ -915,27 +919,32 @@ return true; } -// GetClientHelloLen creates a client SSL connection with a ticket of length -// |ticket_len| and records the ClientHello. It returns the length of the -// ClientHello, not including the record header, on success and zero on error. -static size_t GetClientHelloLen(size_t ticket_len) { +// GetClientHelloLen creates a client SSL connection with the specified version +// and ticket length. It returns the length of the ClientHello, not including +// the record header, on success and zero on error. +static size_t GetClientHelloLen(uint16_t max_version, uint16_t session_version, + size_t ticket_len) { bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method())); - bssl::UniquePtr<SSL_SESSION> session = CreateSessionWithTicket(ticket_len); + bssl::UniquePtr<SSL_SESSION> session = + CreateSessionWithTicket(session_version, ticket_len); if (!ctx || !session) { return 0; } + + // Set a one-element cipher list so the baseline ClientHello is unpadded. bssl::UniquePtr<SSL> ssl(SSL_new(ctx.get())); - // Test at TLS 1.2. TLS 1.3 adds enough extensions that the ClientHello is - // longer than our test vectors. if (!ssl || !SSL_set_session(ssl.get(), session.get()) || - !SSL_set_max_proto_version(ssl.get(), TLS1_2_VERSION)) { + !SSL_set_cipher_list(ssl.get(), "ECDHE-RSA-AES128-GCM-SHA256") || + !SSL_set_max_proto_version(ssl.get(), max_version)) { return 0; } + std::vector<uint8_t> client_hello; if (!GetClientHello(ssl.get(), &client_hello) || client_hello.size() <= SSL3_RT_HEADER_LENGTH) { return 0; } + return client_hello.size() - SSL3_RT_HEADER_LENGTH; } @@ -964,28 +973,37 @@ {0x201, 0x201}, }; -static bool TestPaddingExtension() { +static bool TestPaddingExtension(uint16_t max_version, + uint16_t session_version) { // Sample a baseline length. - size_t base_len = GetClientHelloLen(1); + size_t base_len = GetClientHelloLen(max_version, session_version, 1); if (base_len == 0) { return false; } for (const PaddingTest &test : kPaddingTests) { if (base_len > test.input_len) { - fprintf(stderr, "Baseline ClientHello too long.\n"); + fprintf(stderr, + "Baseline ClientHello too long (max_version = %04x, " + "session_version = %04x).\n", + max_version, session_version); return false; } - size_t padded_len = GetClientHelloLen(1 + test.input_len - base_len); + size_t padded_len = GetClientHelloLen(max_version, session_version, + 1 + test.input_len - base_len); if (padded_len != test.padded_len) { - fprintf(stderr, "%u-byte ClientHello padded to %u bytes, not %u.\n", + fprintf(stderr, + "%u-byte ClientHello padded to %u bytes, not %u (max_version = " + "%04x, session_version = %04x).\n", static_cast<unsigned>(test.input_len), static_cast<unsigned>(padded_len), - static_cast<unsigned>(test.padded_len)); + static_cast<unsigned>(test.padded_len), max_version, + session_version); return false; } } + return true; } @@ -2584,7 +2602,14 @@ !TestDefaultVersion(TLS1_1_VERSION, TLS1_1_VERSION, &DTLSv1_method) || !TestDefaultVersion(TLS1_2_VERSION, TLS1_2_VERSION, &DTLSv1_2_method) || !TestCipherGetRFCName() || - !TestPaddingExtension() || + // Test the padding extension at TLS 1.2. + !TestPaddingExtension(TLS1_2_VERSION, TLS1_2_VERSION) || + // Test the padding extension at TLS 1.3 with a TLS 1.2 session, so there + // will be no PSK binder after the padding extension. + !TestPaddingExtension(TLS1_3_VERSION, TLS1_2_VERSION) || + // Test the padding extension at TLS 1.3 with a TLS 1.3 session, so there + // will be a PSK binder after the padding extension. + !TestPaddingExtension(TLS1_3_VERSION, TLS1_3_DRAFT_VERSION) || !TestClientCAList() || !TestInternalSessionCache() || !TestSequenceNumber() ||
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 3690548..24e4548 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c
@@ -1160,42 +1160,22 @@ return 1; } - if (ssl3_protocol_version(ssl) < TLS1_3_VERSION) { - /* OCSP stapling is forbidden on non-certificate ciphers. */ - if (CBS_len(contents) != 0 || - !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { - return 0; - } - - /* Note this does not check for resumption in TLS 1.2. Sending - * status_request here does not make sense, but OpenSSL does so and the - * specification does not say anything. Tolerate it but ignore it. */ - - ssl->s3->hs->certificate_status_expected = 1; - return 1; - } - - /* In TLS 1.3, OCSP stapling is forbidden on resumption. */ - if (ssl->s3->session_reused) { + /* TLS 1.3 OCSP responses are included in the Certificate extensions. */ + if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) { return 0; } - uint8_t status_type; - CBS ocsp_response; - if (!CBS_get_u8(contents, &status_type) || - status_type != TLSEXT_STATUSTYPE_ocsp || - !CBS_get_u24_length_prefixed(contents, &ocsp_response) || - CBS_len(&ocsp_response) == 0 || - CBS_len(contents) != 0) { + /* OCSP stapling is forbidden on non-certificate ciphers. */ + if (CBS_len(contents) != 0 || + !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { return 0; } - if (!CBS_stow(&ocsp_response, &ssl->s3->new_session->ocsp_response, - &ssl->s3->new_session->ocsp_response_length)) { - *out_alert = SSL_AD_INTERNAL_ERROR; - return 0; - } + /* Note this does not check for resumption in TLS 1.2. Sending + * status_request here does not make sense, but OpenSSL does so and the + * specification does not say anything. Tolerate it but ignore it. */ + ssl->s3->hs->certificate_status_expected = 1; return 1; } @@ -1218,34 +1198,18 @@ } static int ext_ocsp_add_serverhello(SSL *ssl, CBB *out) { - if (!ssl->s3->hs->ocsp_stapling_requested || + if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION || + !ssl->s3->hs->ocsp_stapling_requested || ssl->ctx->ocsp_response_length == 0 || ssl->s3->session_reused || - (ssl3_protocol_version(ssl) < TLS1_3_VERSION && - !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher))) { + !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { return 1; } - if (ssl3_protocol_version(ssl) < TLS1_3_VERSION) { - /* The extension shouldn't be sent when resuming sessions. */ - if (ssl->session != NULL) { - return 1; - } + ssl->s3->hs->certificate_status_expected = 1; - ssl->s3->hs->certificate_status_expected = 1; - - return CBB_add_u16(out, TLSEXT_TYPE_status_request) && - CBB_add_u16(out, 0 /* length */); - } - - CBB body, ocsp_response; return CBB_add_u16(out, TLSEXT_TYPE_status_request) && - CBB_add_u16_length_prefixed(out, &body) && - CBB_add_u8(&body, TLSEXT_STATUSTYPE_ocsp) && - CBB_add_u24_length_prefixed(&body, &ocsp_response) && - CBB_add_bytes(&ocsp_response, ssl->ctx->ocsp_response, - ssl->ctx->ocsp_response_length) && - CBB_flush(out); + CBB_add_u16(out, 0 /* length */); } @@ -1400,6 +1364,11 @@ return 1; } + /* TLS 1.3 SCTs are included in the Certificate extensions. */ + if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) { + return 0; + } + /* If this is false then we should never have sent the SCT extension in the * ClientHello and thus this function should never have been called. */ assert(ssl->signed_cert_timestamps_enabled); @@ -1428,12 +1397,14 @@ static int ext_sct_parse_clienthello(SSL *ssl, uint8_t *out_alert, CBS *contents) { + ssl->s3->hs->scts_requested = 1; return contents == NULL || CBS_len(contents) == 0; } static int ext_sct_add_serverhello(SSL *ssl, CBB *out) { /* The extension shouldn't be sent when resuming sessions. */ - if (ssl->s3->session_reused || + if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION || + ssl->s3->session_reused || ssl->ctx->signed_cert_timestamp_list_length == 0) { return 1; } @@ -1915,11 +1886,32 @@ return ext_ec_point_add_extension(ssl, out); } + /* Pre Shared Key * - * https://tools.ietf.org/html/draft-ietf-tls-tls13-16#section-4.2.6 */ + * https://tools.ietf.org/html/draft-ietf-tls-tls13-18#section-4.2.6 */ -static int ext_pre_shared_key_add_clienthello(SSL *ssl, CBB *out) { +static size_t ext_pre_shared_key_clienthello_length(SSL *ssl) { + uint16_t min_version, max_version; + if (!ssl_get_version_range(ssl, &min_version, &max_version)) { + return 0; + } + + uint16_t session_version; + if (max_version < TLS1_3_VERSION || ssl->session == NULL || + !ssl->method->version_from_wire(&session_version, + ssl->session->ssl_version) || + session_version < TLS1_3_VERSION) { + return 0; + } + + const EVP_MD *digest = + ssl_get_handshake_digest(ssl->session->cipher->algorithm_prf); + size_t binder_len = EVP_MD_size(digest); + return 15 + ssl->session->tlsext_ticklen + binder_len; +} + +int ssl_ext_pre_shared_key_add_clienthello(SSL *ssl, CBB *out) { uint16_t min_version, max_version; if (!ssl_get_version_range(ssl, &min_version, &max_version)) { return 0; @@ -1933,20 +1925,33 @@ return 1; } - CBB contents, identity, ke_modes, auth_modes, ticket; + struct timeval now; + ssl_get_current_time(ssl, &now); + uint32_t ticket_age = 1000 * (now.tv_sec - ssl->session->time); + uint32_t obfuscated_ticket_age = ticket_age + ssl->session->ticket_age_add; + + /* Fill in a placeholder zero binder of the appropriate length. It will be + * computed and filled in later after length prefixes are computed. */ + uint8_t zero_binder[EVP_MAX_MD_SIZE] = {0}; + const EVP_MD *digest = + ssl_get_handshake_digest(ssl->session->cipher->algorithm_prf); + size_t binder_len = EVP_MD_size(digest); + + CBB contents, identity, ticket, binders, binder; if (!CBB_add_u16(out, TLSEXT_TYPE_pre_shared_key) || !CBB_add_u16_length_prefixed(out, &contents) || !CBB_add_u16_length_prefixed(&contents, &identity) || - !CBB_add_u8_length_prefixed(&identity, &ke_modes) || - !CBB_add_u8(&ke_modes, SSL_PSK_DHE_KE) || - !CBB_add_u8_length_prefixed(&identity, &auth_modes) || - !CBB_add_u8(&auth_modes, SSL_PSK_AUTH) || !CBB_add_u16_length_prefixed(&identity, &ticket) || !CBB_add_bytes(&ticket, ssl->session->tlsext_tick, - ssl->session->tlsext_ticklen)) { + ssl->session->tlsext_ticklen) || + !CBB_add_u32(&identity, obfuscated_ticket_age) || + !CBB_add_u16_length_prefixed(&contents, &binders) || + !CBB_add_u8_length_prefixed(&binders, &binder) || + !CBB_add_bytes(&binder, zero_binder, binder_len)) { return 0; } + ssl->s3->hs->needs_psk_binder = 1; return CBB_flush(out); } @@ -1972,27 +1977,35 @@ int ssl_ext_pre_shared_key_parse_clienthello(SSL *ssl, SSL_SESSION **out_session, + CBS *out_binders, uint8_t *out_alert, CBS *contents) { /* We only process the first PSK identity since we don't support pure PSK. */ - CBS identity, ke_modes, auth_modes, ticket; + uint32_t obfuscated_ticket_age; + CBS identity, ticket, binders; if (!CBS_get_u16_length_prefixed(contents, &identity) || - !CBS_get_u8_length_prefixed(&identity, &ke_modes) || - !CBS_get_u8_length_prefixed(&identity, &auth_modes) || !CBS_get_u16_length_prefixed(&identity, &ticket) || + !CBS_get_u32(&identity, &obfuscated_ticket_age) || + !CBS_get_u16_length_prefixed(contents, &binders) || CBS_len(contents) != 0) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); *out_alert = SSL_AD_DECODE_ERROR; return 0; } - /* We only support tickets with PSK_DHE_KE and PSK_AUTH. */ - if (memchr(CBS_data(&ke_modes), SSL_PSK_DHE_KE, CBS_len(&ke_modes)) == NULL || - memchr(CBS_data(&auth_modes), SSL_PSK_AUTH, CBS_len(&auth_modes)) == - NULL) { - *out_session = NULL; - return 1; + *out_binders = binders; + + /* The PSK identity must have a corresponding binder. */ + CBS binder; + if (!CBS_get_u8_length_prefixed(&binders, &binder)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + *out_alert = SSL_AD_DECODE_ERROR; + return 0; } + /* TODO(svaldez): Check that the ticket_age is valid when attempting to use + * the PSK for 0-RTT. http://crbug.com/boringssl/113 */ + /* TLS 1.3 session tickets are renewed separately as part of the * NewSessionTicket. */ int renew; @@ -2018,6 +2031,49 @@ } +/* Pre-Shared Key Exchange Modes + * + * https://tools.ietf.org/html/draft-ietf-tls-tls13-18#section-4.2.7 */ +static int ext_psk_key_exchange_modes_add_clienthello(SSL *ssl, CBB *out) { + uint16_t min_version, max_version; + if (!ssl_get_version_range(ssl, &min_version, &max_version)) { + return 0; + } + + if (max_version < TLS1_3_VERSION) { + return 1; + } + + CBB contents, ke_modes; + if (!CBB_add_u16(out, TLSEXT_TYPE_psk_key_exchange_modes) || + !CBB_add_u16_length_prefixed(out, &contents) || + !CBB_add_u8_length_prefixed(&contents, &ke_modes) || + !CBB_add_u8(&ke_modes, SSL_PSK_DHE_KE)) { + return 0; + } + + return CBB_flush(out); +} + +int ssl_ext_psk_key_exchange_modes_parse_clienthello(SSL *ssl, + uint8_t *out_alert, + CBS *contents) { + CBS ke_modes; + if (!CBS_get_u8_length_prefixed(contents, &ke_modes) || + CBS_len(&ke_modes) == 0 || + CBS_len(contents) != 0) { + *out_alert = SSL_AD_DECODE_ERROR; + return 0; + } + + /* We only support tickets with PSK_DHE_KE. */ + ssl->s3->hs->accept_psk_mode = + memchr(CBS_data(&ke_modes), SSL_PSK_DHE_KE, CBS_len(&ke_modes)) != NULL; + + return 1; +} + + /* Key Share * * https://tools.ietf.org/html/draft-ietf-tls-tls13-16#section-4.2.5 */ @@ -2039,21 +2095,18 @@ return 0; } - uint16_t group_id; + uint16_t group_id = ssl->s3->hs->retry_group; if (ssl->s3->hs->received_hello_retry_request) { - /* Replay the old key shares. */ - if (!CBB_add_bytes(&kse_bytes, ssl->s3->hs->key_share_bytes, + /* We received a HelloRetryRequest without a new curve, so there is no new + * share to append. Leave |ecdh_ctx| as-is. */ + if (group_id == 0 && + !CBB_add_bytes(&kse_bytes, ssl->s3->hs->key_share_bytes, ssl->s3->hs->key_share_bytes_len)) { return 0; } OPENSSL_free(ssl->s3->hs->key_share_bytes); ssl->s3->hs->key_share_bytes = NULL; ssl->s3->hs->key_share_bytes_len = 0; - - group_id = ssl->s3->hs->retry_group; - - /* We received a HelloRetryRequest without a new curve, so there is no new - * share to append. Leave |ecdh_ctx| as-is. */ if (group_id == 0) { return CBB_flush(out); } @@ -2496,9 +2549,9 @@ dont_add_serverhello, }, { - TLSEXT_TYPE_pre_shared_key, + TLSEXT_TYPE_psk_key_exchange_modes, NULL, - ext_pre_shared_key_add_clienthello, + ext_psk_key_exchange_modes_add_clienthello, forbid_parse_serverhello, ignore_parse_clienthello, dont_add_serverhello, @@ -2627,7 +2680,8 @@ } if (!SSL_is_dtls(ssl)) { - header_len += 2 + CBB_len(&extensions); + size_t psk_extension_len = ext_pre_shared_key_clienthello_length(ssl); + header_len += 2 + CBB_len(&extensions) + psk_extension_len; if (header_len > 0xff && header_len < 0x200) { /* Add padding to workaround bugs in F5 terminators. See RFC 7685. * @@ -2655,6 +2709,11 @@ } } + /* The PSK extension must be last, including after the padding. */ + if (!ssl_ext_pre_shared_key_add_clienthello(ssl, &extensions)) { + goto err; + } + /* Discard empty extensions blocks. */ if (CBB_len(&extensions) == 0) { CBB_discard_child(out);
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 87bdb86..abfcf0e 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc
@@ -1904,6 +1904,10 @@ ERR_print_errors_fp(stderr); return 1; } + + if (config.resumption_delay != 0) { + g_clock.tv_sec += config.resumption_delay; + } } return 0;
diff --git a/ssl/test/runner/cipher_suites.go b/ssl/test/runner/cipher_suites.go index 656a3d0..a997016 100644 --- a/ssl/test/runner/cipher_suites.go +++ b/ssl/test/runner/cipher_suites.go
@@ -480,12 +480,16 @@ func mutualCipherSuite(have []uint16, want uint16) *cipherSuite { for _, id := range have { if id == want { - for _, suite := range cipherSuites { - if suite.id == want { - return suite - } - } - return nil + return cipherSuiteFromID(id) + } + } + return nil +} + +func cipherSuiteFromID(id uint16) *cipherSuite { + for _, suite := range cipherSuites { + if suite.id == id { + return suite } } return nil
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index b55467a..446c8dc 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -27,7 +27,7 @@ ) // A draft version of TLS 1.3 that is sent over the wire for the current draft. -const tls13DraftVersion = 0x7f10 +const tls13DraftVersion = 0x7f12 const ( maxPlaintext = 16384 // maximum plaintext payload length @@ -94,6 +94,7 @@ extensionEarlyData uint16 = 42 // draft-ietf-tls-tls13-16 extensionSupportedVersions uint16 = 43 // draft-ietf-tls-tls13-16 extensionCookie uint16 = 44 // draft-ietf-tls-tls13-16 + extensionPSKKeyExchangeModes uint16 = 45 // draft-ietf-tls-tls13-18 extensionCustom uint16 = 1234 // not IANA assigned extensionNextProtoNeg uint16 = 13172 // not IANA assigned extensionRenegotiationInfo uint16 = 0xff01 @@ -200,12 +201,6 @@ pskDHEKEMode = 1 ) -// PskAuthenticationMode values (see draft-ietf-tls-tls13-16) -const ( - pskAuthMode = 0 - pskSignAuthMode = 1 -) - // KeyUpdateRequest values (see draft-ietf-tls-tls13-16, section 4.5.3) const ( keyUpdateNotRequested = 0 @@ -259,6 +254,7 @@ ocspResponse []byte ticketCreationTime time.Time ticketExpiration time.Time + ticketAgeAdd uint32 } // ClientSessionCache is a cache of ClientSessionState objects that can be used @@ -947,12 +943,17 @@ // session ticket. SendEmptySessionTicket bool - // SnedPSKKeyExchangeModes, if present, determines the PSK key exchange modes + // SendPSKKeyExchangeModes, if present, determines the PSK key exchange modes // to send. SendPSKKeyExchangeModes []byte - // SendPSKAuthModes, if present, determines the PSK auth modes to send. - SendPSKAuthModes []byte + // ExpectNoNewSessionTicket, if present, means that the client will fail upon + // receipt of a NewSessionTicket message. + ExpectNoNewSessionTicket bool + + // ExpectTicketAge, if non-zero, is the expected age of the ticket that the + // server receives from the client. + ExpectTicketAge time.Duration // FailIfSessionOffered, if true, causes the server to fail any // connections where the client offers a non-empty session ID or session @@ -999,6 +1000,26 @@ // supplied stapled response. SendOCSPResponseOnResume []byte + // SendExtensionOnCertificate, if not nil, causes the runner to send the + // supplied bytes in the extensions on the Certificate message. + SendExtensionOnCertificate []byte + + // SendOCSPOnIntermediates, if not nil, causes the server to send the + // supplied OCSP on intermediate certificates in the Certificate message. + SendOCSPOnIntermediates []byte + + // SendSCTOnIntermediates, if not nil, causes the server to send the + // supplied SCT on intermediate certificates in the Certificate message. + SendSCTOnIntermediates []byte + + // SendDuplicateCertExtensions, if true, causes the server to send an extra + // copy of the OCSP/SCT extensions in the Certificate message. + SendDuplicateCertExtensions bool + + // ExpectNoExtensionsOnIntermediate, if true, causes the client to + // reject extensions on intermediate certificates. + ExpectNoExtensionsOnIntermediate bool + // CECPQ1BadX25519Part corrupts the X25519 part of a CECPQ1 key exchange, as // a trivial proof that it is actually used. CECPQ1BadX25519Part bool @@ -1064,14 +1085,6 @@ // identity. ExtraPSKIdentity bool - // OmitServerHelloSignatureAlgorithms, if true, causes the server to omit the - // signature_algorithms extension in the ServerHello. - OmitServerHelloSignatureAlgorithms bool - - // IncludeServerHelloSignatureAlgorithms, if true, causes the server to - // include the signature_algorithms extension in all ServerHellos. - IncludeServerHelloSignatureAlgorithms bool - // MissingKeyShare, if true, causes the TLS 1.3 implementation to skip // sending a key_share extension and use the zero ECDHE secret // instead. @@ -1168,6 +1181,21 @@ // ExpectGREASE, if true, causes messages without GREASE values to be // rejected. See draft-davidben-tls-grease-01. ExpectGREASE bool + + // SendShortPSKBinder, if true, causes the client to send a PSK binder + // that is one byte shorter than it should be. + SendShortPSKBinder bool + + // SendInvalidPSKBinder, if true, causes the client to send an invalid + // PSK binder. + SendInvalidPSKBinder bool + + // SendNoPSKBinder, if true, causes the client to send no PSK binders. + SendNoPSKBinder bool + + // PSKBinderFirst, if true, causes the client to send the PSK Binder + // extension as the first extension instead of the last extension. + PSKBinderFirst bool } func (c *Config) serverInit() {
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index bb036ad..39c2785 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go
@@ -208,9 +208,9 @@ } // useTrafficSecret sets the current cipher state for TLS 1.3. -func (hc *halfConn) useTrafficSecret(version uint16, suite *cipherSuite, secret, phase []byte, side trafficDirection) { +func (hc *halfConn) useTrafficSecret(version uint16, suite *cipherSuite, secret []byte, side trafficDirection) { hc.version = version - hc.cipher = deriveTrafficAEAD(version, suite, secret, phase, side) + hc.cipher = deriveTrafficAEAD(version, suite, secret, side) if hc.config.Bugs.NullAllCiphers { hc.cipher = nullCipher{} } @@ -223,7 +223,7 @@ if c.isClient == isOutgoing { side = clientWrite } - hc.useTrafficSecret(hc.version, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), hc.trafficSecret), applicationPhase, side) + hc.useTrafficSecret(hc.version, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), hc.trafficSecret), side) } // incSeq increments the sequence number. @@ -1404,27 +1404,14 @@ return errors.New("tls: no GREASE ticket extension found") } + if c.config.Bugs.ExpectNoNewSessionTicket { + return errors.New("tls: received unexpected NewSessionTicket") + } + if c.config.ClientSessionCache == nil || newSessionTicket.ticketLifetime == 0 { return nil } - var foundKE, foundAuth bool - for _, mode := range newSessionTicket.keModes { - if mode == pskDHEKEMode { - foundKE = true - } - } - for _, mode := range newSessionTicket.authModes { - if mode == pskAuthMode { - foundAuth = true - } - } - - // Ignore the ticket if the server preferences do not match a mode we implement. - if !foundKE || !foundAuth { - return nil - } - session := &ClientSessionState{ sessionTicket: newSessionTicket.ticket, vers: c.vers, @@ -1435,6 +1422,7 @@ ocspResponse: c.ocspResponse, ticketCreationTime: c.config.time(), ticketExpiration: c.config.time().Add(time.Duration(newSessionTicket.ticketLifetime) * time.Second), + ticketAgeAdd: newSessionTicket.ticketAgeAdd, } cacheKey := clientSessionCacheKey(c.conn.RemoteAddr(), c.config) @@ -1717,20 +1705,20 @@ peerCertificatesRaw = append(peerCertificatesRaw, cert.Raw) } + addBuffer := make([]byte, 4) + _, err := io.ReadFull(c.config.rand(), addBuffer) + if err != nil { + c.sendAlert(alertInternalError) + return errors.New("tls: short read from Rand: " + err.Error()) + } + ticketAgeAdd := uint32(addBuffer[3])<<24 | uint32(addBuffer[2])<<16 | uint32(addBuffer[1])<<8 | uint32(addBuffer[0]) + // TODO(davidben): Allow configuring these values. m := &newSessionTicketMsg{ version: c.vers, ticketLifetime: uint32(24 * time.Hour / time.Second), - keModes: []byte{pskDHEKEMode}, - authModes: []byte{pskAuthMode}, customExtension: c.config.Bugs.CustomTicketExtension, - } - - if len(c.config.Bugs.SendPSKKeyExchangeModes) != 0 { - m.keModes = c.config.Bugs.SendPSKKeyExchangeModes - } - if len(c.config.Bugs.SendPSKAuthModes) != 0 { - m.authModes = c.config.Bugs.SendPSKAuthModes + ticketAgeAdd: ticketAgeAdd, } state := sessionState{ @@ -1740,6 +1728,7 @@ certificates: peerCertificatesRaw, ticketCreationTime: c.config.time(), ticketExpiration: c.config.time().Add(time.Duration(m.ticketLifetime) * time.Second), + ticketAgeAdd: uint32(addBuffer[3])<<24 | uint32(addBuffer[2])<<16 | uint32(addBuffer[1])<<8 | uint32(addBuffer[0]), } if !c.config.Bugs.SendEmptySessionTicket { @@ -1752,7 +1741,7 @@ c.out.Lock() defer c.out.Unlock() - _, err := c.writeRecord(recordTypeHandshake, m.marshal()) + _, err = c.writeRecord(recordTypeHandshake, m.marshal()) return err }
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index dcae3e2..f8cf124 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -68,17 +68,19 @@ sctListSupported: true, serverName: c.config.ServerName, supportedCurves: c.config.curvePreferences(), + pskKEModes: []byte{pskDHEKEMode}, supportedPoints: []uint8{pointFormatUncompressed}, nextProtoNeg: len(c.config.NextProtos) > 0, secureRenegotiation: []byte{}, alpnProtocols: c.config.NextProtos, duplicateExtension: c.config.Bugs.DuplicateExtension, channelIDSupported: c.config.ChannelID != nil, - npnLast: c.config.Bugs.SwapNPNAndALPN, + npnAfterAlpn: c.config.Bugs.SwapNPNAndALPN, extendedMasterSecret: maxVersion >= VersionTLS10, srtpProtectionProfiles: c.config.SRTPProtectionProfiles, srtpMasterKeyIdentifier: c.config.Bugs.SRTPMasterKeyIdentifer, customExtension: c.config.Bugs.CustomExtension, + pskBinderFirst: c.config.Bugs.PSKBinderFirst, } disableEMS := c.config.Bugs.NoExtendedMasterSecret @@ -94,6 +96,10 @@ hello.supportedCurves = nil } + if len(c.config.Bugs.SendPSKKeyExchangeModes) != 0 { + hello.pskKEModes = c.config.Bugs.SendPSKKeyExchangeModes + } + if c.config.Bugs.SendCompressionMethods != nil { hello.compressionMethods = c.config.Bugs.SendCompressionMethods } @@ -232,6 +238,7 @@ } } + var pskCipherSuite *cipherSuite if session != nil && c.config.time().Before(session.ticketExpiration) { ticket := session.sessionTicket if c.config.Bugs.FilterTicket != nil && len(ticket) > 0 { @@ -246,20 +253,17 @@ } if session.vers >= VersionTLS13 || c.config.Bugs.SendBothTickets { + pskCipherSuite = cipherSuiteFromID(session.cipherSuite) + if pskCipherSuite == nil { + return errors.New("tls: client session cache has invalid cipher suite") + } // TODO(nharper): Support sending more // than one PSK identity. + ticketAge := uint32(c.config.time().Sub(session.ticketCreationTime) / time.Millisecond) psk := pskIdentity{ - keModes: []byte{pskDHEKEMode}, - authModes: []byte{pskAuthMode}, - ticket: ticket, + ticket: ticket, + obfuscatedTicketAge: session.ticketAgeAdd + ticketAge, } - if len(c.config.Bugs.SendPSKKeyExchangeModes) != 0 { - psk.keModes = c.config.Bugs.SendPSKKeyExchangeModes - } - if len(c.config.Bugs.SendPSKAuthModes) != 0 { - psk.authModes = c.config.Bugs.SendPSKAuthModes - } - hello.pskIdentities = []pskIdentity{psk} if c.config.Bugs.ExtraPSKIdentity { @@ -319,7 +323,11 @@ helloBytes = v2Hello.marshal() c.writeV2Record(helloBytes) } else { + if len(hello.pskIdentities) > 0 { + generatePSKBinders(hello, pskCipherSuite, session.masterSecret, []byte{}, c.config) + } helloBytes = hello.marshal() + if c.config.Bugs.PartialClientFinishedWithClientHello { // Include one byte of Finished. We can compute it // without completing the handshake. This assumes we @@ -424,10 +432,10 @@ return err } keyShares[group] = curve - hello.keyShares = append(hello.keyShares, keyShareEntry{ + hello.keyShares = []keyShareEntry{{ group: group, keyExchange: publicKey, - }) + }} } if c.config.Bugs.SecondClientHelloMissingKeyShare { @@ -438,6 +446,9 @@ hello.earlyDataContext = nil hello.raw = nil + if len(hello.pskIdentities) > 0 { + generatePSKBinders(hello, pskCipherSuite, session.masterSecret, append(helloBytes, helloRetryRequest.marshal()...), c.config) + } secondHelloBytes = hello.marshal() c.writeRecord(recordTypeHandshake, secondHelloBytes) c.flushHandshake() @@ -604,11 +615,6 @@ // 0-RTT is implemented. var psk []byte if hs.serverHello.hasPSKIdentity { - if hs.serverHello.useCertAuth || !hs.serverHello.hasKeyShare { - c.sendAlert(alertUnsupportedExtension) - return errors.New("tls: server omitted KeyShare or included SignatureAlgorithms on resumption.") - } - // We send at most one PSK identity. if hs.session == nil || hs.serverHello.pskIdentity != 0 { c.sendAlert(alertUnknownPSKIdentity) @@ -618,21 +624,19 @@ c.sendAlert(alertHandshakeFailure) return errors.New("tls: server sent invalid cipher suite") } - psk = deriveResumptionPSK(hs.suite, hs.session.masterSecret) - hs.finishedHash.setResumptionContext(deriveResumptionContext(hs.suite, hs.session.masterSecret)) + psk = hs.session.masterSecret c.didResume = true } else { - if !hs.serverHello.useCertAuth || !hs.serverHello.hasKeyShare { - c.sendAlert(alertUnsupportedExtension) - return errors.New("tls: server omitted KeyShare and SignatureAlgorithms on non-resumption.") - } - psk = zeroSecret - hs.finishedHash.setResumptionContext(zeroSecret) } earlySecret := hs.finishedHash.extractKey(zeroSecret, psk) + if !hs.serverHello.hasKeyShare { + c.sendAlert(alertUnsupportedExtension) + return errors.New("tls: server omitted KeyShare on resumption.") + } + // Resolve ECDHE and compute the handshake secret. var ecdheSecret []byte if !c.config.Bugs.MissingKeyShare && !c.config.Bugs.SecondClientHelloMissingKeyShare { @@ -657,9 +661,9 @@ // Switch to handshake traffic keys. clientHandshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, clientHandshakeTrafficLabel) - c.out.useTrafficSecret(c.vers, hs.suite, clientHandshakeTrafficSecret, handshakePhase, clientWrite) + c.out.useTrafficSecret(c.vers, hs.suite, clientHandshakeTrafficSecret, clientWrite) serverHandshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, serverHandshakeTrafficLabel) - c.in.useTrafficSecret(c.vers, hs.suite, serverHandshakeTrafficSecret, handshakePhase, serverWrite) + c.in.useTrafficSecret(c.vers, hs.suite, serverHandshakeTrafficSecret, serverWrite) msg, err := c.readHandshake() if err != nil { @@ -680,24 +684,12 @@ var chainToSend *Certificate var certReq *certificateRequestMsg - if !hs.serverHello.useCertAuth { - if encryptedExtensions.extensions.ocspResponse != nil { - c.sendAlert(alertUnsupportedExtension) - return errors.New("tls: server sent OCSP response without a certificate") - } - if encryptedExtensions.extensions.sctList != nil { - c.sendAlert(alertUnsupportedExtension) - return errors.New("tls: server sent SCT list without a certificate") - } - + if c.didResume { // Copy over authentication from the session. c.peerCertificates = hs.session.serverCertificates c.sctList = hs.session.sctList c.ocspResponse = hs.session.ocspResponse } else { - c.ocspResponse = encryptedExtensions.extensions.ocspResponse - c.sctList = encryptedExtensions.extensions.sctList - msg, err := c.readHandshake() if err != nil { return err @@ -738,6 +730,17 @@ return err } leaf := c.peerCertificates[0] + c.ocspResponse = certMsg.certificates[0].ocspResponse + c.sctList = certMsg.certificates[0].sctList + + if c.config.Bugs.ExpectNoExtensionsOnIntermediate { + for _, cert := range certMsg.certificates[1:] { + if cert.ocspResponse != nil || cert.sctList != nil { + c.sendAlert(alertUnsupportedExtension) + return errors.New("tls: unexpected extensions in the client certificate") + } + } + } msg, err = c.readHandshake() if err != nil { @@ -790,7 +793,12 @@ requestContext: certReq.requestContext, } if chainToSend != nil { - certMsg.certificates = chainToSend.Certificate + for _, certData := range chainToSend.Certificate { + certMsg.certificates = append(certMsg.certificates, certificateEntry{ + data: certData, + extraExtension: c.config.Bugs.SendExtensionOnCertificate, + }) + } } hs.writeClientHash(certMsg.marshal()) c.writeRecord(recordTypeHandshake, certMsg.marshal()) @@ -855,8 +863,8 @@ c.flushHandshake() // Switch to application data keys. - c.out.useTrafficSecret(c.vers, hs.suite, clientTrafficSecret, applicationPhase, clientWrite) - c.in.useTrafficSecret(c.vers, hs.suite, serverTrafficSecret, applicationPhase, serverWrite) + c.out.useTrafficSecret(c.vers, hs.suite, clientTrafficSecret, clientWrite) + c.in.useTrafficSecret(c.vers, hs.suite, serverTrafficSecret, serverWrite) c.exporterSecret = hs.finishedHash.deriveSecret(masterSecret, exporterLabel) c.resumptionSecret = hs.finishedHash.deriveSecret(masterSecret, resumptionLabel) @@ -969,7 +977,11 @@ } else if !c.config.Bugs.SkipClientCertificate { certMsg := new(certificateMsg) if chainToSend != nil { - certMsg.certificates = chainToSend.Certificate + for _, certData := range chainToSend.Certificate { + certMsg.certificates = append(certMsg.certificates, certificateEntry{ + data: certData, + }) + } } hs.writeClientHash(certMsg.marshal()) c.writeRecord(recordTypeHandshake, certMsg.marshal()) @@ -1057,8 +1069,8 @@ } certs := make([]*x509.Certificate, len(certMsg.certificates)) - for i, asn1Data := range certMsg.certificates { - cert, err := x509.ParseCertificate(asn1Data) + for i, certEntry := range certMsg.certificates { + cert, err := x509.ParseCertificate(certEntry.data) if err != nil { c.sendAlert(alertBadCertificate) return errors.New("tls: failed to parse certificate from server: " + err.Error()) @@ -1193,6 +1205,14 @@ return errors.New("tls: server advertised ticket extension over TLS 1.3") } + if serverExtensions.ocspStapling && c.vers >= VersionTLS13 { + return errors.New("tls: server advertised OCSP in ServerHello over TLS 1.3") + } + + if len(serverExtensions.sctList) > 0 && c.vers >= VersionTLS13 { + return errors.New("tls: server advertised SCTs in ServerHello over TLS 1.3") + } + if serverExtensions.srtpProtectionProfile != 0 { if serverExtensions.srtpMasterKeyIdentifier != "" { return errors.New("tls: server selected SRTP MKI value") @@ -1592,3 +1612,39 @@ xb := x.Bytes() copy(b[len(b)-len(xb):], xb) } + +func generatePSKBinders(hello *clientHelloMsg, pskCipherSuite *cipherSuite, psk, transcript []byte, config *Config) { + if config.Bugs.SendNoPSKBinder { + return + } + + binderLen := pskCipherSuite.hash().Size() + if config.Bugs.SendShortPSKBinder { + binderLen-- + } + + // Fill hello.pskBinders with appropriate length arrays of zeros so the + // length prefixes are correct when computing the binder over the truncated + // ClientHello message. + hello.pskBinders = make([][]byte, len(hello.pskIdentities)) + for i := range hello.pskIdentities { + hello.pskBinders[i] = make([]byte, binderLen) + } + + helloBytes := hello.marshal() + binderSize := len(hello.pskBinders)*(binderLen+1) + 2 + truncatedHello := helloBytes[:len(helloBytes)-binderSize] + binder := computePSKBinder(psk, resumptionPSKBinderLabel, pskCipherSuite, transcript, truncatedHello) + if config.Bugs.SendShortPSKBinder { + binder = binder[:binderLen] + } + if config.Bugs.SendInvalidPSKBinder { + binder[0] ^= 1 + } + + for i := range hello.pskBinders { + hello.pskBinders[i] = binder + } + + hello.raw = nil +}
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index 285587e..d2d7635 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go
@@ -125,9 +125,8 @@ } type pskIdentity struct { - keModes []byte - authModes []byte - ticket []uint8 + ticket []uint8 + obfuscatedTicketAge uint32 } type clientHelloMsg struct { @@ -148,6 +147,8 @@ keyShares []keyShareEntry trailingKeyShareData bool pskIdentities []pskIdentity + pskKEModes []byte + pskBinders [][]uint8 hasEarlyData bool earlyDataContext []byte tls13Cookie []byte @@ -159,13 +160,14 @@ alpnProtocols []string duplicateExtension bool channelIDSupported bool - npnLast bool + npnAfterAlpn bool extendedMasterSecret bool srtpProtectionProfiles []uint16 srtpMasterKeyIdentifier string sctListSupported bool customExtension string hasGREASEExtension bool + pskBinderFirst bool } func (m *clientHelloMsg) equal(i interface{}) bool { @@ -191,6 +193,8 @@ eqKeyShareEntryLists(m.keyShares, m1.keyShares) && m.trailingKeyShareData == m1.trailingKeyShareData && eqPSKIdentityLists(m.pskIdentities, m1.pskIdentities) && + bytes.Equal(m.pskKEModes, m1.pskKEModes) && + eqByteSlices(m.pskBinders, m1.pskBinders) && m.hasEarlyData == m1.hasEarlyData && bytes.Equal(m.earlyDataContext, m1.earlyDataContext) && bytes.Equal(m.tls13Cookie, m1.tls13Cookie) && @@ -203,13 +207,14 @@ eqStrings(m.alpnProtocols, m1.alpnProtocols) && m.duplicateExtension == m1.duplicateExtension && m.channelIDSupported == m1.channelIDSupported && - m.npnLast == m1.npnLast && + m.npnAfterAlpn == m1.npnAfterAlpn && m.extendedMasterSecret == m1.extendedMasterSecret && eqUint16s(m.srtpProtectionProfiles, m1.srtpProtectionProfiles) && m.srtpMasterKeyIdentifier == m1.srtpMasterKeyIdentifier && m.sctListSupported == m1.sctListSupported && m.customExtension == m1.customExtension && - m.hasGREASEExtension == m1.hasGREASEExtension + m.hasGREASEExtension == m1.hasGREASEExtension && + m.pskBinderFirst == m1.pskBinderFirst } func (m *clientHelloMsg) marshal() []byte { @@ -236,12 +241,26 @@ compressionMethods.addBytes(m.compressionMethods) extensions := hello.addU16LengthPrefixed() + if len(m.pskIdentities) > 0 && m.pskBinderFirst { + extensions.addU16(extensionPreSharedKey) + pskExtension := extensions.addU16LengthPrefixed() + + pskIdentities := pskExtension.addU16LengthPrefixed() + for _, psk := range m.pskIdentities { + pskIdentities.addU16LengthPrefixed().addBytes(psk.ticket) + pskIdentities.addU32(psk.obfuscatedTicketAge) + } + pskBinders := pskExtension.addU16LengthPrefixed() + for _, binder := range m.pskBinders { + pskBinders.addU8LengthPrefixed().addBytes(binder) + } + } if m.duplicateExtension { // Add a duplicate bogus extension at the beginning and end. extensions.addU16(0xffff) extensions.addU16(0) // 0-length for empty extension } - if m.nextProtoNeg && !m.npnLast { + if m.nextProtoNeg && !m.npnAfterAlpn { extensions.addU16(extensionNextProtoNeg) extensions.addU16(0) // The length is always 0 } @@ -316,16 +335,10 @@ keyShares.addU8(0) } } - if len(m.pskIdentities) > 0 { - extensions.addU16(extensionPreSharedKey) - pskExtension := extensions.addU16LengthPrefixed() - - pskIdentities := pskExtension.addU16LengthPrefixed() - for _, psk := range m.pskIdentities { - pskIdentities.addU8LengthPrefixed().addBytes(psk.keModes) - pskIdentities.addU8LengthPrefixed().addBytes(psk.authModes) - pskIdentities.addU16LengthPrefixed().addBytes(psk.ticket) - } + if len(m.pskKEModes) > 0 { + extensions.addU16(extensionPSKKeyExchangeModes) + pskModesExtension := extensions.addU16LengthPrefixed() + pskModesExtension.addU8LengthPrefixed().addBytes(m.pskKEModes) } if m.hasEarlyData { extensions.addU16(extensionEarlyData) @@ -383,7 +396,7 @@ extensions.addU16(extensionChannelID) extensions.addU16(0) // Length is always 0 } - if m.nextProtoNeg && m.npnLast { + if m.nextProtoNeg && m.npnAfterAlpn { extensions.addU16(extensionNextProtoNeg) extensions.addU16(0) // Length is always 0 } @@ -422,6 +435,21 @@ customExt := extensions.addU16LengthPrefixed() customExt.addBytes([]byte(m.customExtension)) } + // The PSK extension must be last (draft-ietf-tls-tls13-18 section 4.2.6). + if len(m.pskIdentities) > 0 && !m.pskBinderFirst { + extensions.addU16(extensionPreSharedKey) + pskExtension := extensions.addU16LengthPrefixed() + + pskIdentities := pskExtension.addU16LengthPrefixed() + for _, psk := range m.pskIdentities { + pskIdentities.addU16LengthPrefixed().addBytes(psk.ticket) + pskIdentities.addU32(psk.obfuscatedTicketAge) + } + pskBinders := pskExtension.addU16LengthPrefixed() + for _, binder := range m.pskBinders { + pskBinders.addU8LengthPrefixed().addBytes(binder) + } + } if extensions.len() == 0 { hello.discardChild() @@ -614,52 +642,69 @@ m.keyShares = append(m.keyShares, entry) } case extensionPreSharedKey: - // draft-ietf-tls-tls13 section 6.3.2.4 + // draft-ietf-tls-tls13-18 section 4.2.6 if length < 2 { return false } l := int(data[0])<<8 | int(data[1]) - if l != length-2 { - return false - } - d := data[2:length] + d := data[2 : l+2] + // Parse PSK identities. for len(d) > 0 { - var psk pskIdentity - - if len(d) < 1 { - return false - } - keModesLen := int(d[0]) - d = d[1:] - if len(d) < keModesLen { - return false - } - psk.keModes = d[:keModesLen] - d = d[keModesLen:] - - if len(d) < 1 { - return false - } - authModesLen := int(d[0]) - d = d[1:] - if len(d) < authModesLen { - return false - } - psk.authModes = d[:authModesLen] - d = d[authModesLen:] if len(d) < 2 { return false } pskLen := int(d[0])<<8 | int(d[1]) d = d[2:] - if len(d) < pskLen { + if len(d) < pskLen+4 { return false } - psk.ticket = d[:pskLen] + ticket := d[:pskLen] + obfuscatedTicketAge := uint32(d[pskLen])<<24 | uint32(d[pskLen+1])<<16 | uint32(d[pskLen+2])<<8 | uint32(d[pskLen+3]) + psk := pskIdentity{ + ticket: ticket, + obfuscatedTicketAge: obfuscatedTicketAge, + } m.pskIdentities = append(m.pskIdentities, psk) - d = d[pskLen:] + d = d[pskLen+4:] } + d = data[l+2:] + if len(d) < 2 { + return false + } + l = int(d[0])<<8 | int(d[1]) + d = d[2:] + if l != len(d) { + return false + } + // Parse PSK binders. + for len(d) > 0 { + if len(d) < 1 { + return false + } + binderLen := int(d[0]) + d = d[1:] + if binderLen > len(d) { + return false + } + m.pskBinders = append(m.pskBinders, d[:binderLen]) + d = d[binderLen:] + } + + // There must be the same number of identities as binders. + if len(m.pskIdentities) != len(m.pskBinders) { + return false + } + case extensionPSKKeyExchangeModes: + // draft-ietf-tls-tls13-18 section 4.2.7 + if length < 1 { + return false + } + l := int(data[0]) + if l != length-1 { + return false + } + m.pskKEModes = data[1:length] case extensionEarlyData: // draft-ietf-tls-tls13 section 6.3.2.5 if length < 1 { @@ -793,7 +838,6 @@ keyShare keyShareEntry hasPSKIdentity bool pskIdentity uint16 - useCertAuth bool earlyDataIndication bool compressionMethod uint8 customExtension string @@ -848,10 +892,6 @@ extensions.addU16(2) // Length extensions.addU16(m.pskIdentity) } - if m.useCertAuth { - extensions.addU16(extensionSignatureAlgorithms) - extensions.addU16(0) // Length - } if m.earlyDataIndication { extensions.addU16(extensionEarlyData) extensions.addU16(0) // Length @@ -870,7 +910,7 @@ protocolName.addBytes([]byte(m.unencryptedALPN)) } } else { - m.extensions.marshal(extensions, vers) + m.extensions.marshal(extensions) if extensions.len() == 0 { hello.discardChild() } @@ -962,11 +1002,6 @@ } m.pskIdentity = uint16(d[0])<<8 | uint16(d[1]) m.hasPSKIdentity = true - case extensionSignatureAlgorithms: - if len(d) != 0 { - return false - } - m.useCertAuth = true case extensionEarlyData: if len(d) != 0 { return false @@ -1001,7 +1036,7 @@ encryptedExtensions := encryptedExtensionsMsg.addU24LengthPrefixed() if !m.empty { extensions := encryptedExtensions.addU16LengthPrefixed() - m.extensions.marshal(extensions, VersionTLS13) + m.extensions.marshal(extensions) } m.raw = encryptedExtensionsMsg.finish() @@ -1033,7 +1068,6 @@ nextProtoNeg bool nextProtos []string ocspStapling bool - ocspResponse []byte ticketSupported bool secureRenegotiation []byte alpnProtocol string @@ -1045,18 +1079,18 @@ srtpMasterKeyIdentifier string sctList []byte customExtension string - npnLast bool + npnAfterAlpn bool hasKeyShare bool keyShare keyShareEntry } -func (m *serverExtensions) marshal(extensions *byteBuilder, version uint16) { +func (m *serverExtensions) marshal(extensions *byteBuilder) { if m.duplicateExtension { // Add a duplicate bogus extension at the beginning and end. extensions.addU16(0xffff) extensions.addU16(0) // length = 0 for empty extension } - if m.nextProtoNeg && !m.npnLast { + if m.nextProtoNeg && !m.npnAfterAlpn { extensions.addU16(extensionNextProtoNeg) extension := extensions.addU16LengthPrefixed() @@ -1068,19 +1102,9 @@ npn.addBytes([]byte(v)) } } - if version >= VersionTLS13 { - if m.ocspResponse != nil { - extensions.addU16(extensionStatusRequest) - body := extensions.addU16LengthPrefixed() - body.addU8(statusTypeOCSP) - response := body.addU24LengthPrefixed() - response.addBytes(m.ocspResponse) - } - } else { - if m.ocspStapling { - extensions.addU16(extensionStatusRequest) - extensions.addU16(0) - } + if m.ocspStapling { + extensions.addU16(extensionStatusRequest) + extensions.addU16(0) } if m.ticketSupported { extensions.addU16(extensionSessionTicket) @@ -1133,7 +1157,7 @@ customExt := extensions.addU16LengthPrefixed() customExt.addBytes([]byte(m.customExtension)) } - if m.nextProtoNeg && m.npnLast { + if m.nextProtoNeg && m.npnAfterAlpn { extensions.addU16(extensionNextProtoNeg) extension := extensions.addU16LengthPrefixed() @@ -1183,25 +1207,10 @@ d = d[l:] } case extensionStatusRequest: - if version >= VersionTLS13 { - if length < 4 { - return false - } - d := data[:length] - if d[0] != statusTypeOCSP { - return false - } - respLen := int(d[1])<<16 | int(d[2])<<8 | int(d[3]) - if respLen+4 != len(d) || respLen == 0 { - return false - } - m.ocspResponse = d[4:] - } else { - if length > 0 { - return false - } - m.ocspStapling = true + if length > 0 { + return false } + m.ocspStapling = true case extensionSessionTicket: if length > 0 { return false @@ -1377,11 +1386,19 @@ return true } +type certificateEntry struct { + data []byte + ocspResponse []byte + sctList []byte + duplicateExtensions bool + extraExtension []byte +} + type certificateMsg struct { raw []byte hasRequestContext bool requestContext []byte - certificates [][]byte + certificates []certificateEntry } func (m *certificateMsg) marshal() (x []byte) { @@ -1399,7 +1416,33 @@ certificateList := certificate.addU24LengthPrefixed() for _, cert := range m.certificates { certEntry := certificateList.addU24LengthPrefixed() - certEntry.addBytes(cert) + certEntry.addBytes(cert.data) + if m.hasRequestContext { + extensions := certificateList.addU16LengthPrefixed() + count := 1 + if cert.duplicateExtensions { + count = 2 + } + + for i := 0; i < count; i++ { + if cert.ocspResponse != nil { + extensions.addU16(extensionStatusRequest) + body := extensions.addU16LengthPrefixed() + body.addU8(statusTypeOCSP) + response := body.addU24LengthPrefixed() + response.addBytes(cert.ocspResponse) + } + + if cert.sctList != nil { + extensions.addU16(extensionSignedCertificateTimestamp) + extension := extensions.addU16LengthPrefixed() + extension.addBytes(cert.sctList) + } + } + if cert.extraExtension != nil { + extensions.addBytes(cert.extraExtension) + } + } } m.raw = certMsg.finish() @@ -1436,27 +1479,62 @@ return false } - numCerts := 0 - d := data - for certsLen > 0 { - if len(d) < 4 { + m.certificates = nil + for len(data) != 0 { + if len(data) < 3 { return false } - certLen := int(d[0])<<16 | int(d[1])<<8 | int(d[2]) - if len(d) < 3+certLen { + certLen := int(data[0])<<16 | int(data[1])<<8 | int(data[2]) + if len(data) < 3+certLen { return false } - d = d[3+certLen:] - certsLen -= 3 + certLen - numCerts++ - } + cert := certificateEntry{ + data: data[3 : 3+certLen], + } + data = data[3+certLen:] + if m.hasRequestContext { + if len(data) < 2 { + return false + } + extensionsLen := int(data[0])<<8 | int(data[1]) + if len(data) < 2+extensionsLen { + return false + } + extensions := data[2 : 2+extensionsLen] + data = data[2+extensionsLen:] + for len(extensions) != 0 { + if len(extensions) < 4 { + return false + } + extension := uint16(extensions[0])<<8 | uint16(extensions[1]) + length := int(extensions[2])<<8 | int(extensions[3]) + if len(extensions) < 4+length { + return false + } + contents := extensions[4 : 4+length] + extensions = extensions[4+length:] - m.certificates = make([][]byte, numCerts) - d = data - for i := 0; i < numCerts; i++ { - certLen := uint32(d[0])<<16 | uint32(d[1])<<8 | uint32(d[2]) - m.certificates[i] = d[3 : 3+certLen] - d = d[3+certLen:] + switch extension { + case extensionStatusRequest: + if length < 4 { + return false + } + if contents[0] != statusTypeOCSP { + return false + } + respLen := int(contents[1])<<16 | int(contents[2])<<8 | int(contents[3]) + if respLen+4 != len(contents) || respLen == 0 { + return false + } + cert.ocspResponse = contents[4:] + case extensionSignedCertificateTimestamp: + cert.sctList = contents + default: + return false + } + } + } + m.certificates = append(m.certificates, cert) } return true @@ -1904,8 +1982,7 @@ raw []byte version uint16 ticketLifetime uint32 - keModes []byte - authModes []byte + ticketAgeAdd uint32 ticket []byte customExtension string hasGREASEExtension bool @@ -1922,8 +1999,7 @@ body := ticketMsg.addU24LengthPrefixed() body.addU32(m.ticketLifetime) if m.version >= VersionTLS13 { - body.addU8LengthPrefixed().addBytes(m.keModes) - body.addU8LengthPrefixed().addBytes(m.authModes) + body.addU32(m.ticketAgeAdd) } ticket := body.addU16LengthPrefixed() @@ -1951,25 +2027,11 @@ data = data[8:] if m.version >= VersionTLS13 { - if len(data) < 1 { + if len(data) < 4 { return false } - keModesLength := int(data[0]) - if len(data)-1 < keModesLength { - return false - } - m.keModes = data[1 : 1+keModesLength] - data = data[1+keModesLength:] - - if len(data) < 1 { - return false - } - authModesLength := int(data[0]) - if len(data)-1 < authModesLength { - return false - } - m.authModes = data[1 : 1+authModesLength] - data = data[1+authModesLength:] + m.ticketAgeAdd = uint32(data[0])<<24 | uint32(data[1])<<16 | uint32(data[2])<<8 | uint32(data[3]) + data = data[4:] } if len(data) < 2 { @@ -2268,7 +2330,7 @@ return false } for i, v := range x { - if !bytes.Equal(y[i].keModes, v.keModes) || !bytes.Equal(y[i].authModes, v.authModes) || !bytes.Equal(y[i].ticket, v.ticket) { + if !bytes.Equal(y[i].ticket, v.ticket) || y[i].obfuscatedTicketAge != v.obfuscatedTicketAge { return false } }
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 4b3df2d..1e0d313 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -16,6 +16,7 @@ "fmt" "io" "math/big" + "time" ) // serverHandshakeState contains details of a server handshake in progress. @@ -404,78 +405,75 @@ } pskIdentities := hs.clientHello.pskIdentities + pskKEModes := hs.clientHello.pskKEModes + if len(pskIdentities) == 0 && len(hs.clientHello.sessionTicket) > 0 && c.config.Bugs.AcceptAnySession { psk := pskIdentity{ - keModes: []byte{pskDHEKEMode}, - authModes: []byte{pskAuthMode}, - ticket: hs.clientHello.sessionTicket, + ticket: hs.clientHello.sessionTicket, } pskIdentities = []pskIdentity{psk} + pskKEModes = []byte{pskDHEKEMode} } - for i, pskIdentity := range pskIdentities { - foundKE := false - foundAuth := false - for _, keMode := range pskIdentity.keModes { - if keMode == pskDHEKEMode { - foundKE = true - } - } - - for _, authMode := range pskIdentity.authModes { - if authMode == pskAuthMode { - foundAuth = true - } - } - - if !foundKE || !foundAuth { - continue - } - - sessionState, ok := c.decryptTicket(pskIdentity.ticket) - if !ok { - continue - } - if config.Bugs.AcceptAnySession { - // Replace the cipher suite with one known to work, to - // test cross-version resumption attempts. - sessionState.cipherSuite = TLS_AES_128_GCM_SHA256 - } else { - if sessionState.vers != c.vers && c.config.Bugs.AcceptAnySession { - continue - } - if sessionState.ticketExpiration.Before(c.config.time()) { + var pskIndex int + foundKEMode := bytes.IndexByte(pskKEModes, pskDHEKEMode) >= 0 + if foundKEMode { + for i, pskIdentity := range pskIdentities { + // TODO(svaldez): Check the obfuscatedTicketAge before accepting 0-RTT. + sessionState, ok := c.decryptTicket(pskIdentity.ticket) + if !ok { continue } - cipherSuiteOk := false - // Check that the client is still offering the ciphersuite in the session. - for _, id := range hs.clientHello.cipherSuites { - if id == sessionState.cipherSuite { - cipherSuiteOk = true - break + if config.Bugs.AcceptAnySession { + // Replace the cipher suite with one known to work, to + // test cross-version resumption attempts. + sessionState.cipherSuite = TLS_AES_128_GCM_SHA256 + } else { + if sessionState.vers != c.vers && c.config.Bugs.AcceptAnySession { + continue } + if sessionState.ticketExpiration.Before(c.config.time()) { + continue + } + + clientTicketAge := time.Duration(uint32(pskIdentity.obfuscatedTicketAge-sessionState.ticketAgeAdd)) * time.Millisecond + if config.Bugs.ExpectTicketAge != 0 && clientTicketAge != config.Bugs.ExpectTicketAge { + c.sendAlert(alertHandshakeFailure) + return errors.New("tls: invalid ticket age") + } + + cipherSuiteOk := false + // Check that the client is still offering the ciphersuite in the session. + for _, id := range hs.clientHello.cipherSuites { + if id == sessionState.cipherSuite { + cipherSuiteOk = true + break + } + } + if !cipherSuiteOk { + continue + } + } - if !cipherSuiteOk { + + // Check that we also support the ciphersuite from the session. + suite := c.tryCipherSuite(sessionState.cipherSuite, c.config.cipherSuites(), c.vers, true, true) + if suite == nil { continue } - } - // Check that we also support the ciphersuite from the session. - suite := c.tryCipherSuite(sessionState.cipherSuite, c.config.cipherSuites(), c.vers, true, true) - if suite == nil { - continue + hs.sessionState = sessionState + hs.suite = suite + hs.hello.hasPSKIdentity = true + hs.hello.pskIdentity = uint16(i) + pskIndex = i + if config.Bugs.SelectPSKIdentityOnResume != 0 { + hs.hello.pskIdentity = config.Bugs.SelectPSKIdentityOnResume + } + c.didResume = true + break } - - hs.sessionState = sessionState - hs.suite = suite - hs.hello.hasPSKIdentity = true - hs.hello.pskIdentity = uint16(i) - if config.Bugs.SelectPSKIdentityOnResume != 0 { - hs.hello.pskIdentity = config.Bugs.SelectPSKIdentityOnResume - } - c.didResume = true - break } if config.Bugs.AlwaysSelectPSKIdentity { @@ -483,6 +481,15 @@ hs.hello.pskIdentity = 0 } + // Verify the PSK binder. Note there may not be a PSK binder if + // AcceptAnyBinder is set. See https://crbug.com/boringssl/115. + if hs.sessionState != nil && !config.Bugs.AcceptAnySession { + binderToVerify := hs.clientHello.pskBinders[pskIndex] + if err := verifyPSKBinder(hs.clientHello, hs.sessionState, binderToVerify, []byte{}); err != nil { + return err + } + } + // If not resuming, select the cipher suite. if hs.suite == nil { var preferenceList, supportedList []uint16 @@ -515,26 +522,16 @@ hs.finishedHash.discardHandshakeBuffer() hs.writeClientHash(hs.clientHello.marshal()) - hs.hello.useCertAuth = hs.sessionState == nil - // Resolve PSK and compute the early secret. var psk []byte if hs.sessionState != nil { - psk = deriveResumptionPSK(hs.suite, hs.sessionState.masterSecret) - hs.finishedHash.setResumptionContext(deriveResumptionContext(hs.suite, hs.sessionState.masterSecret)) + psk = hs.sessionState.masterSecret } else { psk = hs.finishedHash.zeroSecret() - hs.finishedHash.setResumptionContext(hs.finishedHash.zeroSecret()) } earlySecret := hs.finishedHash.extractKey(hs.finishedHash.zeroSecret(), psk) - if config.Bugs.OmitServerHelloSignatureAlgorithms { - hs.hello.useCertAuth = false - } else if config.Bugs.IncludeServerHelloSignatureAlgorithms { - hs.hello.useCertAuth = true - } - hs.hello.hasKeyShare = true if hs.sessionState != nil && config.Bugs.NegotiatePSKResumption { hs.hello.hasKeyShare = false @@ -598,6 +595,7 @@ } if sendHelloRetryRequest { + oldClientHelloBytes := hs.clientHello.marshal() hs.writeServerHash(helloRetryRequest.marshal()) c.writeRecord(recordTypeHandshake, helloRetryRequest.marshal()) c.flushHandshake() @@ -627,11 +625,11 @@ if helloRetryRequest.hasSelectedGroup { newKeyShares := newClientHelloCopy.keyShares - if len(newKeyShares) == 0 || newKeyShares[len(newKeyShares)-1].group != helloRetryRequest.selectedGroup { - return errors.New("tls: KeyShare from HelloRetryRequest not present in new ClientHello") + if len(newKeyShares) != 1 || newKeyShares[0].group != helloRetryRequest.selectedGroup { + return errors.New("tls: KeyShare from HelloRetryRequest not in new ClientHello") } - selectedKeyShare = &newKeyShares[len(newKeyShares)-1] - newClientHelloCopy.keyShares = newKeyShares[:len(newKeyShares)-1] + selectedKeyShare = &newKeyShares[0] + newClientHelloCopy.keyShares = oldClientHelloCopy.keyShares } if len(helloRetryRequest.cookie) > 0 { @@ -640,6 +638,7 @@ } newClientHelloCopy.tls13Cookie = nil } + newClientHelloCopy.pskBinders = oldClientHelloCopy.pskBinders if !oldClientHelloCopy.equal(&newClientHelloCopy) { return errors.New("tls: new ClientHello does not match") @@ -649,6 +648,16 @@ firstHelloRetryRequest = false goto ResendHelloRetryRequest } + + // Verify the PSK binder. Note there may not be a PSK binder if + // AcceptAnyBinder is set. See https://crbug.com/115. + if hs.sessionState != nil && !config.Bugs.AcceptAnySession { + binderToVerify := newClientHello.pskBinders[pskIndex] + err := verifyPSKBinder(newClientHello, hs.sessionState, binderToVerify, append(oldClientHelloBytes, helloRetryRequest.marshal()...)) + if err != nil { + return err + } + } } // Resolve ECDHE and compute the handshake secret. @@ -728,25 +737,9 @@ // Switch to handshake traffic keys. serverHandshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, serverHandshakeTrafficLabel) - c.out.useTrafficSecret(c.vers, hs.suite, serverHandshakeTrafficSecret, handshakePhase, serverWrite) + c.out.useTrafficSecret(c.vers, hs.suite, serverHandshakeTrafficSecret, serverWrite) clientHandshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, clientHandshakeTrafficLabel) - c.in.useTrafficSecret(c.vers, hs.suite, clientHandshakeTrafficSecret, handshakePhase, clientWrite) - - if hs.hello.useCertAuth { - if hs.clientHello.ocspStapling { - encryptedExtensions.extensions.ocspResponse = hs.cert.OCSPStaple - } - if hs.clientHello.sctListSupported { - encryptedExtensions.extensions.sctList = hs.cert.SignedCertificateTimestampList - } - } else { - if config.Bugs.SendOCSPResponseOnResume != nil { - encryptedExtensions.extensions.ocspResponse = config.Bugs.SendOCSPResponseOnResume - } - if config.Bugs.SendSCTListOnResume != nil { - encryptedExtensions.extensions.sctList = config.Bugs.SendSCTListOnResume - } - } + c.in.useTrafficSecret(c.vers, hs.suite, clientHandshakeTrafficSecret, clientWrite) // Send EncryptedExtensions. hs.writeServerHash(encryptedExtensions.marshal()) @@ -757,7 +750,7 @@ c.writeRecord(recordTypeHandshake, encryptedExtensions.marshal()) } - if hs.hello.useCertAuth { + if hs.sessionState == nil { if config.ClientAuth >= RequestClientCert { // Request a client certificate certReq := &certificateRequestMsg{ @@ -785,7 +778,29 @@ hasRequestContext: true, } if !config.Bugs.EmptyCertificateList { - certMsg.certificates = hs.cert.Certificate + for i, certData := range hs.cert.Certificate { + cert := certificateEntry{ + data: certData, + } + if i == 0 { + if hs.clientHello.ocspStapling { + cert.ocspResponse = hs.cert.OCSPStaple + } + if hs.clientHello.sctListSupported { + cert.sctList = hs.cert.SignedCertificateTimestampList + } + cert.duplicateExtensions = config.Bugs.SendDuplicateCertExtensions + cert.extraExtension = config.Bugs.SendExtensionOnCertificate + } else { + if config.Bugs.SendOCSPOnIntermediates != nil { + cert.ocspResponse = config.Bugs.SendOCSPOnIntermediates + } + if config.Bugs.SendSCTOnIntermediates != nil { + cert.sctList = config.Bugs.SendSCTOnIntermediates + } + } + certMsg.certificates = append(certMsg.certificates, cert) + } } certMsgBytes := certMsg.marshal() hs.writeServerHash(certMsgBytes) @@ -844,10 +859,11 @@ masterSecret := hs.finishedHash.extractKey(handshakeSecret, hs.finishedHash.zeroSecret()) clientTrafficSecret := hs.finishedHash.deriveSecret(masterSecret, clientApplicationTrafficLabel) serverTrafficSecret := hs.finishedHash.deriveSecret(masterSecret, serverApplicationTrafficLabel) + c.exporterSecret = hs.finishedHash.deriveSecret(masterSecret, exporterLabel) // Switch to application data keys on write. In particular, any alerts // from the client certificate are sent over these keys. - c.out.useTrafficSecret(c.vers, hs.suite, serverTrafficSecret, applicationPhase, serverWrite) + c.out.useTrafficSecret(c.vers, hs.suite, serverTrafficSecret, serverWrite) // If we requested a client certificate, then the client must send a // certificate message, even if it's empty. @@ -873,7 +889,17 @@ } } - pub, err := hs.processCertsFromClient(certMsg.certificates) + var certs [][]byte + for _, cert := range certMsg.certificates { + certs = append(certs, cert.data) + // OCSP responses and SCT lists are not negotiated in + // client certificates. + if cert.ocspResponse != nil || cert.sctList != nil { + c.sendAlert(alertUnsupportedExtension) + return errors.New("tls: unexpected extensions in the client certificate") + } + } + pub, err := hs.processCertsFromClient(certs) if err != nil { return err } @@ -941,15 +967,14 @@ hs.writeClientHash(clientFinished.marshal()) // Switch to application data keys on read. - c.in.useTrafficSecret(c.vers, hs.suite, clientTrafficSecret, applicationPhase, clientWrite) + c.in.useTrafficSecret(c.vers, hs.suite, clientTrafficSecret, clientWrite) c.cipherSuite = hs.suite - c.exporterSecret = hs.finishedHash.deriveSecret(masterSecret, exporterLabel) c.resumptionSecret = hs.finishedHash.deriveSecret(masterSecret, resumptionLabel) // TODO(davidben): Allow configuring the number of tickets sent for // testing. - if !c.config.SessionTicketsDisabled { + if !c.config.SessionTicketsDisabled && foundKEMode { ticketCount := 2 for i := 0; i < ticketCount; i++ { c.SendNewSessionTicket() @@ -1134,7 +1159,7 @@ if hs.clientHello.nextProtoNeg && len(config.NextProtos) > 0 { serverExtensions.nextProtoNeg = true serverExtensions.nextProtos = config.NextProtos - serverExtensions.npnLast = config.Bugs.SwapNPNAndALPN + serverExtensions.npnAfterAlpn = config.Bugs.SwapNPNAndALPN } } } @@ -1340,7 +1365,11 @@ if !isPSK { certMsg := new(certificateMsg) if !config.Bugs.EmptyCertificateList { - certMsg.certificates = hs.cert.Certificate + for _, certData := range hs.cert.Certificate { + certMsg.certificates = append(certMsg.certificates, certificateEntry{ + data: certData, + }) + } } if !config.Bugs.UnauthenticatedECDH { certMsgBytes := certMsg.marshal() @@ -1428,7 +1457,9 @@ } hs.writeClientHash(certMsg.marshal()) - certificates = certMsg.certificates + for _, cert := range certMsg.certificates { + certificates = append(certificates, cert.data) + } } else if c.vers != VersionSSL30 { // In TLS, the Certificate message is required. In SSL // 3.0, the peer skips it when sending no certificates. @@ -1876,3 +1907,24 @@ func isGREASEValue(val uint16) bool { return val&0x0f0f == 0x0a0a && val&0xff == val>>8 } + +func verifyPSKBinder(clientHello *clientHelloMsg, sessionState *sessionState, binderToVerify, transcript []byte) error { + binderLen := 2 + for _, binder := range clientHello.pskBinders { + binderLen += 1 + len(binder) + } + + truncatedHello := clientHello.marshal() + truncatedHello = truncatedHello[:len(truncatedHello)-binderLen] + pskCipherSuite := cipherSuiteFromID(sessionState.cipherSuite) + if pskCipherSuite == nil { + return errors.New("tls: Unknown cipher suite for PSK in session") + } + + binder := computePSKBinder(sessionState.masterSecret, resumptionPSKBinderLabel, pskCipherSuite, transcript, truncatedHello) + if !bytes.Equal(binder, binderToVerify) { + return errors.New("tls: PSK binder does not verify") + } + + return nil +}
diff --git a/ssl/test/runner/prf.go b/ssl/test/runner/prf.go index dff8534..c311e99 100644 --- a/ssl/test/runner/prf.go +++ b/ssl/test/runner/prf.go
@@ -230,10 +230,6 @@ // full buffer is required. buffer []byte - // TLS 1.3 has a resumption context which is carried over on PSK - // resumption. - resumptionContextHash []byte - version uint16 prf func(result, secret, label, seed []byte) } @@ -374,13 +370,6 @@ return make([]byte, h.hash.Size()) } -// setResumptionContext sets the TLS 1.3 resumption context. -func (h *finishedHash) setResumptionContext(resumptionContext []byte) { - hash := h.hash.New() - hash.Write(resumptionContext) - h.resumptionContextHash = hash.Sum(nil) -} - // extractKey combines two secrets together with HKDF-Expand in the TLS 1.3 key // derivation schedule. func (h *finishedHash) extractKey(salt, ikm []byte) []byte { @@ -412,12 +401,13 @@ // resumption context hash, as used in TLS 1.3. func (h *finishedHash) appendContextHashes(b []byte) []byte { b = h.client.Sum(b) - b = append(b, h.resumptionContextHash...) return b } // The following are labels for traffic secret derivation in TLS 1.3. var ( + externalPSKBinderLabel = []byte("external psk binder key") + resumptionPSKBinderLabel = []byte("resumption psk binder key") earlyTrafficLabel = []byte("client early traffic secret") clientHandshakeTrafficLabel = []byte("client handshake traffic secret") serverHandshakeTrafficLabel = []byte("server handshake traffic secret") @@ -431,10 +421,6 @@ // deriveSecret implements TLS 1.3's Derive-Secret function, as defined in // section 7.1 of draft ietf-tls-tls13-16. func (h *finishedHash) deriveSecret(secret, label []byte) []byte { - if h.resumptionContextHash == nil { - panic("Resumption context not set.") - } - return hkdfExpandLabel(h.hash, secret, label, h.appendContextHashes(nil), h.hash.Size()) } @@ -459,14 +445,6 @@ return b } -// The following are phase values for traffic key derivation in TLS 1.3. -var ( - earlyHandshakePhase = []byte("early handshake key expansion") - earlyApplicationPhase = []byte("early application data key expansion") - handshakePhase = []byte("handshake key expansion") - applicationPhase = []byte("application data key expansion") -) - type trafficDirection int const ( @@ -474,17 +452,16 @@ serverWrite ) +var ( + keyTLS13 = []byte("key") + ivTLS13 = []byte("iv") +) + // deriveTrafficAEAD derives traffic keys and constructs an AEAD given a traffic // secret. -func deriveTrafficAEAD(version uint16, suite *cipherSuite, secret, phase []byte, side trafficDirection) interface{} { - label := make([]byte, 0, len(phase)+2+16) - label = append(label, phase...) - label = append(label, []byte(", key")...) - key := hkdfExpandLabel(suite.hash(), secret, label, nil, suite.keyLen) - - label = label[:len(label)-3] // Remove "key" from the end. - label = append(label, []byte("iv")...) - iv := hkdfExpandLabel(suite.hash(), secret, label, nil, suite.ivLen(version)) +func deriveTrafficAEAD(version uint16, suite *cipherSuite, secret []byte, side trafficDirection) interface{} { + key := hkdfExpandLabel(suite.hash(), secret, keyTLS13, nil, suite.keyLen) + iv := hkdfExpandLabel(suite.hash(), secret, ivTLS13, nil, suite.ivLen(version)) return suite.aead(version, key, iv) } @@ -493,10 +470,11 @@ return hkdfExpandLabel(hash, secret, applicationTrafficLabel, nil, hash.Size()) } -func deriveResumptionPSK(suite *cipherSuite, resumptionSecret []byte) []byte { - return hkdfExpandLabel(suite.hash(), resumptionSecret, []byte("resumption psk"), nil, suite.hash().Size()) -} - -func deriveResumptionContext(suite *cipherSuite, resumptionSecret []byte) []byte { - return hkdfExpandLabel(suite.hash(), resumptionSecret, []byte("resumption context"), nil, suite.hash().Size()) +func computePSKBinder(psk, label []byte, cipherSuite *cipherSuite, transcript, truncatedHello []byte) []byte { + finishedHash := newFinishedHash(VersionTLS13, cipherSuite) + earlySecret := finishedHash.extractKey(finishedHash.zeroSecret(), psk) + binderKey := finishedHash.deriveSecret(earlySecret, label) + finishedHash.Write(transcript) + finishedHash.Write(truncatedHello) + return finishedHash.clientSum(binderKey) }
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 396d5f2..19ec131 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -170,6 +170,9 @@ var testOCSPResponse = []byte{1, 2, 3, 4} var testSCTList = []byte{5, 6, 7, 8} +var testOCSPExtension = append([]byte{byte(extensionStatusRequest) >> 8, byte(extensionStatusRequest), 0, 8, statusTypeOCSP, 0, 0, 4}, testOCSPResponse...) +var testSCTExtension = append([]byte{byte(extensionSignedCertificateTimestamp) >> 8, byte(extensionSignedCertificateTimestamp), 0, 4}, testSCTList...) + func initCertificates() { for i := range testCerts { cert, err := LoadX509KeyPair(path.Join(*resourceDir, testCerts[i].certFile), path.Join(*resourceDir, testCerts[i].keyFile)) @@ -5313,23 +5316,139 @@ resumeSession: true, }) - // Beginning TLS 1.3, enforce this does not happen. testCases = append(testCases, testCase{ - name: "SendOCSPResponseOnResume-TLS13", + name: "SendUnsolicitedOCSPOnCertificate-TLS13", config: Config{ MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ - SendOCSPResponseOnResume: []byte("bogus"), + SendExtensionOnCertificate: testOCSPExtension, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + }) + + testCases = append(testCases, testCase{ + name: "SendUnsolicitedSCTOnCertificate-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendExtensionOnCertificate: testSCTExtension, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + }) + + // Test that extensions on client certificates are never accepted. + testCases = append(testCases, testCase{ + name: "SendExtensionOnClientCertificate-TLS13", + testType: serverTest, + config: Config{ + MaxVersion: VersionTLS13, + Certificates: []Certificate{rsaCertificate}, + Bugs: ProtocolBugs{ + SendExtensionOnCertificate: testOCSPExtension, + }, + }, + flags: []string{ + "-enable-ocsp-stapling", + "-require-any-client-certificate", + }, + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + }) + + testCases = append(testCases, testCase{ + name: "SendUnknownExtensionOnCertificate-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendExtensionOnCertificate: []byte{0x00, 0x7f, 0, 0}, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + }) + + // Test that extensions on intermediates are allowed but ignored. + testCases = append(testCases, testCase{ + name: "IgnoreExtensionsOnIntermediates-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + Certificates: []Certificate{rsaChainCertificate}, + Bugs: ProtocolBugs{ + // Send different values on the intermediate. This tests + // the intermediate's extensions do not override the + // leaf's. + SendOCSPOnIntermediates: []byte{1, 3, 3, 7}, + SendSCTOnIntermediates: []byte{1, 3, 3, 7}, }, }, flags: []string{ "-enable-ocsp-stapling", "-expect-ocsp-response", base64.StdEncoding.EncodeToString(testOCSPResponse), + "-enable-signed-cert-timestamps", + "-expect-signed-cert-timestamps", + base64.StdEncoding.EncodeToString(testSCTList), + }, + resumeSession: true, + }) + + // Test that extensions are not sent on intermediates when configured + // only for a leaf. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "SendNoExtensionsOnIntermediate-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + ExpectNoExtensionsOnIntermediate: true, + }, + }, + flags: []string{ + "-cert-file", path.Join(*resourceDir, rsaChainCertificateFile), + "-key-file", path.Join(*resourceDir, rsaChainKeyFile), + "-ocsp-response", + base64.StdEncoding.EncodeToString(testOCSPResponse), + "-signed-cert-timestamps", + base64.StdEncoding.EncodeToString(testSCTList), + }, + }) + + // Test that extensions are not sent on client certificates. + testCases = append(testCases, testCase{ + name: "SendNoClientCertificateExtensions-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + ClientAuth: RequireAnyClientCert, + }, + flags: []string{ + "-cert-file", path.Join(*resourceDir, rsaCertificateFile), + "-key-file", path.Join(*resourceDir, rsaKeyFile), + "-ocsp-response", + base64.StdEncoding.EncodeToString(testOCSPResponse), + "-signed-cert-timestamps", + base64.StdEncoding.EncodeToString(testSCTList), + }, + }) + + testCases = append(testCases, testCase{ + name: "SendDuplicateExtensionsOnCerts-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendDuplicateCertExtensions: true, + }, + }, + flags: []string{ + "-enable-ocsp-stapling", + "-enable-signed-cert-timestamps", }, resumeSession: true, shouldFail: true, - expectedError: ":ERROR_PARSING_EXTENSION:", + expectedError: ":DUPLICATE_EXTENSION:", }) } @@ -5596,6 +5715,66 @@ shouldFail: true, expectedError: ":OLD_SESSION_CIPHER_NOT_RETURNED:", }) + + testCases = append(testCases, testCase{ + testType: serverTest, + name: "Resume-Server-BinderWrongLength", + resumeSession: true, + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendShortPSKBinder: true, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: error decrypting message", + expectedError: ":DIGEST_CHECK_FAILED:", + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + name: "Resume-Server-NoPSKBinder", + resumeSession: true, + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendNoPSKBinder: true, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: error decoding message", + expectedError: ":DECODE_ERROR:", + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + name: "Resume-Server-InvalidPSKBinder", + resumeSession: true, + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendInvalidPSKBinder: true, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: error decrypting message", + expectedError: ":DIGEST_CHECK_FAILED:", + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + name: "Resume-Server-PSKBinderFirstExtension", + resumeSession: true, + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + PSKBinderFirst: true, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: illegal parameter", + expectedError: ":PRE_SHARED_KEY_MUST_BE_LAST:", + }) } func addRenegotiationTests() { @@ -7860,19 +8039,34 @@ MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ SendPSKKeyExchangeModes: []byte{0x1a, pskDHEKEMode, 0x2a}, - SendPSKAuthModes: []byte{0x1a, pskAuthMode, 0x2a}, }, }, resumeSession: true, expectedResumeVersion: VersionTLS13, }) - // Test that the server declines sessions with no matching key exchange mode. + // Test that the server does not send session tickets with no matching key exchange mode. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "TLS13-ExpectNoSessionTicketOnBadKEMode-Server", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendPSKKeyExchangeModes: []byte{0x1a}, + ExpectNoNewSessionTicket: true, + }, + }, + }) + + // Test that the server does not accept a session with no matching key exchange mode. testCases = append(testCases, testCase{ testType: serverTest, name: "TLS13-SendBadKEModeSessionTicket-Server", config: Config{ MaxVersion: VersionTLS13, + }, + resumeConfig: &Config{ + MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ SendPSKKeyExchangeModes: []byte{0x1a}, }, @@ -7881,60 +8075,37 @@ expectResumeRejected: true, }) - // Test that the server declines sessions with no matching auth mode. - testCases = append(testCases, testCase{ - testType: serverTest, - name: "TLS13-SendBadAuthModeSessionTicket-Server", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - SendPSKAuthModes: []byte{0x1a}, - }, - }, - resumeSession: true, - expectResumeRejected: true, - }) - - // Test that the client ignores unknown PSK modes. + // Test that the client ticket age is sent correctly. testCases = append(testCases, testCase{ testType: clientTest, - name: "TLS13-SendUnknownModeSessionTicket-Client", + name: "TLS13-TestValidTicketAge-Client", config: Config{ MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ - SendPSKKeyExchangeModes: []byte{0x1a, pskDHEKEMode, 0x2a}, - SendPSKAuthModes: []byte{0x1a, pskAuthMode, 0x2a}, + ExpectTicketAge: 10 * time.Second, }, }, - resumeSession: true, - expectedResumeVersion: VersionTLS13, + resumeSession: true, + flags: []string{ + "-resumption-delay", "10", + }, }) - // Test that the client ignores tickets with no matching key exchange mode. + // Test that the client ticket age is enforced. testCases = append(testCases, testCase{ testType: clientTest, - name: "TLS13-SendBadKEModeSessionTicket-Client", + name: "TLS13-TestBadTicketAge-Client", config: Config{ MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ - SendPSKKeyExchangeModes: []byte{0x1a}, + ExpectTicketAge: 1000 * time.Second, }, }, - flags: []string{"-expect-no-session"}, + resumeSession: true, + shouldFail: true, + expectedLocalError: "tls: invalid ticket age", }) - // Test that the client ignores tickets with no matching auth mode. - testCases = append(testCases, testCase{ - testType: clientTest, - name: "TLS13-SendBadAuthModeSessionTicket-Client", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - SendPSKAuthModes: []byte{0x1a}, - }, - }, - flags: []string{"-expect-no-session"}, - }) } func addChangeCipherSpecTests() { @@ -8593,33 +8764,6 @@ testCases = append(testCases, testCase{ testType: clientTest, - name: "OmitServerHelloSignatureAlgorithms", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - OmitServerHelloSignatureAlgorithms: true, - }, - }, - shouldFail: true, - expectedError: ":UNEXPECTED_EXTENSION:", - }) - - testCases = append(testCases, testCase{ - testType: clientTest, - name: "IncludeServerHelloSignatureAlgorithms", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - IncludeServerHelloSignatureAlgorithms: true, - }, - }, - resumeSession: true, - shouldFail: true, - expectedError: ":UNEXPECTED_EXTENSION:", - }) - - testCases = append(testCases, testCase{ - testType: clientTest, name: "MissingKeyShare-Client", config: Config{ MaxVersion: VersionTLS13,
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index c0fe3c4..470f034 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -161,6 +161,7 @@ { "-max-cert-list", &TestConfig::max_cert_list }, { "-expect-cipher-aes", &TestConfig::expect_cipher_aes }, { "-expect-cipher-no-aes", &TestConfig::expect_cipher_no_aes }, + { "-resumption-delay", &TestConfig::resumption_delay }, }; const Flag<std::vector<int>> kIntVectorFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index f4028cd..806edf1 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -120,6 +120,7 @@ int expect_cipher_aes = 0; int expect_cipher_no_aes = 0; std::string expect_peer_cert_file; + int resumption_delay = 0; }; bool ParseConfig(int argc, char **argv, TestConfig *out_config);
diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c index 56ca9fc..242aa03 100644 --- a/ssl/tls13_both.c +++ b/ssl/tls13_both.c
@@ -147,10 +147,10 @@ goto err; } - uint8_t context_hashes[2 * EVP_MAX_MD_SIZE]; - size_t context_hashes_len; - if (!tls13_get_context_hashes(ssl, context_hashes, &context_hashes_len) || - !CBB_add_bytes(&cbb, context_hashes, context_hashes_len) || + uint8_t context_hash[EVP_MAX_MD_SIZE]; + size_t context_hash_len; + if (!tls13_get_context_hash(ssl, context_hash, &context_hash_len) || + !CBB_add_bytes(&cbb, context_hash, context_hash_len) || !CBB_finish(&cbb, out, out_len)) { goto err; } @@ -164,7 +164,7 @@ } int tls13_process_certificate(SSL *ssl, int allow_anonymous) { - CBS cbs, context; + CBS cbs, context, certificate_list; CBS_init(&cbs, ssl->init_msg, ssl->init_num); if (!CBS_get_u8_length_prefixed(&cbs, &context) || CBS_len(&context) != 0) { @@ -176,15 +176,138 @@ const int retain_sha256 = ssl->server && ssl->ctx->retain_only_sha256_of_client_certs; int ret = 0; - uint8_t alert; - STACK_OF(X509) *chain = ssl_parse_cert_chain( - ssl, &alert, retain_sha256 ? ssl->s3->new_session->peer_sha256 : NULL, - &cbs); + + STACK_OF(X509) *chain = sk_X509_new_null(); if (chain == NULL) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); goto err; } + if (!CBS_get_u24_length_prefixed(&cbs, &certificate_list)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + goto err; + } + + while (CBS_len(&certificate_list) > 0) { + CBS certificate, extensions; + if (!CBS_get_u24_length_prefixed(&certificate_list, &certificate) || + !CBS_get_u16_length_prefixed(&certificate_list, &extensions)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_LENGTH_MISMATCH); + goto err; + } + + /* Retain the hash of the leaf certificate if requested. */ + if (sk_X509_num(chain) == 0 && retain_sha256) { + SHA256(CBS_data(&certificate), CBS_len(&certificate), + ssl->s3->new_session->peer_sha256); + } + + X509 *x = ssl_parse_x509(&certificate); + if (x == NULL || CBS_len(&certificate) != 0) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + X509_free(x); + goto err; + } + if (!sk_X509_push(chain, x)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + X509_free(x); + goto err; + } + + /* Parse out the extensions. */ + int have_status_request = 0, have_sct = 0; + CBS status_request, sct; + while (CBS_len(&extensions) != 0) { + uint16_t type; + CBS extension; + if (!CBS_get_u16(&extensions, &type) || + !CBS_get_u16_length_prefixed(&extensions, &extension)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_PARSE_TLSEXT); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + goto err; + } + + switch (type) { + case TLSEXT_TYPE_status_request: + if (have_status_request) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + goto err; + } + status_request = extension; + have_status_request = 1; + break; + case TLSEXT_TYPE_certificate_timestamp: + if (have_sct) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + goto err; + } + sct = extension; + have_sct = 1; + break; + default: + OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION); + goto err; + } + } + + /* All Certificate extensions are parsed, but only the leaf extensions are + * stored. */ + if (have_status_request) { + if (ssl->server || !ssl->ocsp_stapling_enabled) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION); + goto err; + } + + uint8_t status_type; + CBS ocsp_response; + if (!CBS_get_u8(&status_request, &status_type) || + status_type != TLSEXT_STATUSTYPE_ocsp || + !CBS_get_u24_length_prefixed(&status_request, &ocsp_response) || + CBS_len(&ocsp_response) == 0 || + CBS_len(&status_request) != 0) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + goto err; + } + + if (sk_X509_num(chain) == 1 && + !CBS_stow(&ocsp_response, &ssl->s3->new_session->ocsp_response, + &ssl->s3->new_session->ocsp_response_length)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + goto err; + } + } + + if (have_sct) { + if (ssl->server || !ssl->signed_cert_timestamps_enabled) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION); + goto err; + } + + if (CBS_len(&sct) == 0) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + goto err; + } + + if (sk_X509_num(chain) == 1 && + !CBS_stow(&sct, + &ssl->s3->new_session->tlsext_signed_cert_timestamp_list, + &ssl->s3->new_session + ->tlsext_signed_cert_timestamp_list_length)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + goto err; + } + } + } + if (CBS_len(&cbs) != 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); @@ -324,17 +447,77 @@ } int tls13_prepare_certificate(SSL *ssl) { - CBB cbb, body; + CBB cbb, body, certificate_list; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CERTIFICATE) || /* The request context is always empty in the handshake. */ !CBB_add_u8(&body, 0) || - !ssl_add_cert_chain(ssl, &body) || - !ssl_complete_message(ssl, &cbb)) { - CBB_cleanup(&cbb); - return 0; + !CBB_add_u24_length_prefixed(&body, &certificate_list)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + goto err; + } + + if (!ssl_has_certificate(ssl)) { + if (!ssl_complete_message(ssl, &cbb)) { + goto err; + } + + return 1; + } + + CERT *cert = ssl->cert; + CBB leaf, extensions; + if (!CBB_add_u24_length_prefixed(&certificate_list, &leaf) || + !ssl_add_cert_to_cbb(&leaf, cert->x509_leaf) || + !CBB_add_u16_length_prefixed(&certificate_list, &extensions)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + goto err; + } + + if (ssl->s3->hs->scts_requested && + ssl->ctx->signed_cert_timestamp_list_length != 0) { + CBB contents; + if (!CBB_add_u16(&extensions, TLSEXT_TYPE_certificate_timestamp) || + !CBB_add_u16_length_prefixed(&extensions, &contents) || + !CBB_add_bytes(&contents, ssl->ctx->signed_cert_timestamp_list, + ssl->ctx->signed_cert_timestamp_list_length)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + goto err; + } + } + + if (ssl->s3->hs->ocsp_stapling_requested && + ssl->ctx->ocsp_response_length != 0) { + CBB contents, ocsp_response; + if (!CBB_add_u16(&extensions, TLSEXT_TYPE_status_request) || + !CBB_add_u16_length_prefixed(&extensions, &contents) || + !CBB_add_u8(&contents, TLSEXT_STATUSTYPE_ocsp) || + !CBB_add_u24_length_prefixed(&contents, &ocsp_response) || + !CBB_add_bytes(&ocsp_response, ssl->ctx->ocsp_response, + ssl->ctx->ocsp_response_length)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + goto err; + } + } + + for (size_t i = 0; i < sk_X509_num(cert->x509_chain); i++) { + CBB child; + if (!CBB_add_u24_length_prefixed(&certificate_list, &child) || + !ssl_add_cert_to_cbb(&child, sk_X509_value(cert->x509_chain, i)) || + !CBB_add_u16(&certificate_list, 0 /* no extensions */)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + goto err; + } + } + + if (!ssl_complete_message(ssl, &cbb)) { + goto err; } return 1; + +err: + CBB_cleanup(&cbb); + return 0; } enum ssl_private_key_result_t tls13_prepare_certificate_verify(
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c index a5b2e85..e540b77 100644 --- a/ssl/tls13_client.c +++ b/ssl/tls13_client.c
@@ -165,11 +165,7 @@ static enum ssl_hs_wait_t do_send_second_client_hello(SSL *ssl, SSL_HANDSHAKE *hs) { - CBB cbb, body; - if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CLIENT_HELLO) || - !ssl_add_client_hello_body(ssl, &body) || - !ssl_complete_message(ssl, &cbb)) { - CBB_cleanup(&cbb); + if (!ssl_write_client_hello(ssl)) { return ssl_hs_error; } @@ -227,8 +223,8 @@ } /* Parse out the extensions. */ - int have_key_share = 0, have_pre_shared_key = 0, have_sigalgs = 0; - CBS key_share, pre_shared_key, sigalgs; + int have_key_share = 0, have_pre_shared_key = 0; + CBS key_share, pre_shared_key; while (CBS_len(&extensions) != 0) { uint16_t type; CBS extension; @@ -258,15 +254,6 @@ pre_shared_key = extension; have_pre_shared_key = 1; break; - case TLSEXT_TYPE_signature_algorithms: - if (have_sigalgs) { - OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); - return ssl_hs_error; - } - sigalgs = extension; - have_sigalgs = 1; - break; default: OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION); @@ -274,8 +261,8 @@ } } - /* We only support PSK_AUTH and PSK_DHE_KE. */ - if (!have_key_share || have_sigalgs == have_pre_shared_key) { + /* We only support PSK_DHE_KE. */ + if (!have_key_share) { OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); return ssl_hs_error; @@ -337,20 +324,17 @@ EVP_MD_size(ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl))); /* Derive resumption material. */ - uint8_t resumption_ctx[EVP_MAX_MD_SIZE] = {0}; uint8_t psk_secret[EVP_MAX_MD_SIZE] = {0}; if (ssl->s3->session_reused) { - if (!tls13_resumption_context(ssl, resumption_ctx, hash_len, - ssl->s3->new_session) || - !tls13_resumption_psk(ssl, psk_secret, hash_len, - ssl->s3->new_session)) { + if (hash_len != (size_t) ssl->s3->new_session->master_key_length) { return ssl_hs_error; } + memcpy(psk_secret, ssl->s3->new_session->master_key, hash_len); } /* Set up the key schedule, hash in the ClientHello, and incorporate the PSK * into the running secret. */ - if (!tls13_init_key_schedule(ssl, resumption_ctx, hash_len) || + if (!tls13_init_key_schedule(ssl) || !tls13_advance_key_schedule(ssl, psk_secret, hash_len)) { return ssl_hs_error; } @@ -370,12 +354,6 @@ } OPENSSL_free(dhe_secret); - if (have_sigalgs && - CBS_len(&sigalgs) != 0) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); - return ssl_hs_error; - } - /* If there was no HelloRetryRequest, the version negotiation logic has * already hashed the message. */ if (hs->received_hello_retry_request && @@ -505,7 +483,7 @@ !ssl_hash_current_message(ssl) || /* Update the secret to the master secret and derive traffic keys. */ !tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len) || - !tls13_derive_traffic_secret_0(ssl)) { + !tls13_derive_application_secrets(ssl)) { return ssl_hs_error; } @@ -621,11 +599,11 @@ } static enum ssl_hs_wait_t do_flush(SSL *ssl, SSL_HANDSHAKE *hs) { - if (!tls13_set_traffic_key(ssl, type_data, evp_aead_open, - hs->server_traffic_secret_0, hs->hash_len) || - !tls13_set_traffic_key(ssl, type_data, evp_aead_seal, - hs->client_traffic_secret_0, hs->hash_len) || - !tls13_finalize_keys(ssl)) { + if (!tls13_set_traffic_key(ssl, evp_aead_open, hs->server_traffic_secret_0, + hs->hash_len) || + !tls13_set_traffic_key(ssl, evp_aead_seal, hs->client_traffic_secret_0, + hs->hash_len) || + !tls13_derive_resumption_secret(ssl)) { return ssl_hs_error; } @@ -711,13 +689,10 @@ ssl_session_refresh_time(ssl, session); - CBS cbs, ke_modes, auth_modes, ticket, extensions; + CBS cbs, ticket, extensions; CBS_init(&cbs, ssl->init_msg, ssl->init_num); if (!CBS_get_u32(&cbs, &session->tlsext_tick_lifetime_hint) || - !CBS_get_u8_length_prefixed(&cbs, &ke_modes) || - CBS_len(&ke_modes) == 0 || - !CBS_get_u8_length_prefixed(&cbs, &auth_modes) || - CBS_len(&auth_modes) == 0 || + !CBS_get_u32(&cbs, &session->ticket_age_add) || !CBS_get_u16_length_prefixed(&cbs, &ticket) || !CBS_stow(&ticket, &session->tlsext_tick, &session->tlsext_ticklen) || !CBS_get_u16_length_prefixed(&cbs, &extensions) || @@ -728,13 +703,10 @@ return 0; } + session->ticket_age_add_valid = 1; session->not_resumable = 0; - /* Ignore the ticket unless the server preferences are compatible with us. */ - if (memchr(CBS_data(&ke_modes), SSL_PSK_DHE_KE, CBS_len(&ke_modes)) != NULL && - memchr(CBS_data(&auth_modes), SSL_PSK_AUTH, CBS_len(&auth_modes)) != - NULL && - ssl->ctx->new_session_cb != NULL && + if (ssl->ctx->new_session_cb != NULL && ssl->ctx->new_session_cb(ssl, session)) { /* |new_session_cb|'s return value signals that it took ownership. */ return 1;
diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 95132cc..c846d9c 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c
@@ -20,27 +20,19 @@ #include <openssl/aead.h> #include <openssl/bytestring.h> #include <openssl/digest.h> -#include <openssl/hmac.h> #include <openssl/hkdf.h> +#include <openssl/hmac.h> #include <openssl/mem.h> #include "internal.h" -int tls13_init_key_schedule(SSL *ssl, const uint8_t *resumption_ctx, - size_t resumption_ctx_len) { +int tls13_init_key_schedule(SSL *ssl) { SSL_HANDSHAKE *hs = ssl->s3->hs; const EVP_MD *digest = ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl)); hs->hash_len = EVP_MD_size(digest); - /* Save the hash of the resumption context. */ - unsigned resumption_hash_len; - if (!EVP_Digest(resumption_ctx, resumption_ctx_len, hs->resumption_hash, - &resumption_hash_len, digest, NULL)) { - return 0; - } - /* Initialize the secret to the zero key. */ memset(hs->secret, 0, hs->hash_len); @@ -89,22 +81,17 @@ return ret; } -int tls13_get_context_hashes(SSL *ssl, uint8_t *out, size_t *out_len) { - SSL_HANDSHAKE *hs = ssl->s3->hs; - +int tls13_get_context_hash(SSL *ssl, uint8_t *out, size_t *out_len) { EVP_MD_CTX ctx; EVP_MD_CTX_init(&ctx); unsigned handshake_len = 0; int ok = EVP_MD_CTX_copy_ex(&ctx, &ssl->s3->handshake_hash) && EVP_DigestFinal_ex(&ctx, out, &handshake_len); EVP_MD_CTX_cleanup(&ctx); - if (!ok) { - return 0; + if (ok) { + *out_len = handshake_len; } - - memcpy(out + handshake_len, hs->resumption_hash, hs->hash_len); - *out_len = handshake_len + hs->hash_len; - return 1; + return ok; } /* derive_secret derives a secret of length |len| and writes the result in |out| @@ -115,18 +102,17 @@ SSL_HANDSHAKE *hs = ssl->s3->hs; const EVP_MD *digest = ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl)); - uint8_t context_hashes[2 * EVP_MAX_MD_SIZE]; - size_t context_hashes_len; - if (!tls13_get_context_hashes(ssl, context_hashes, &context_hashes_len)) { + uint8_t context_hash[EVP_MAX_MD_SIZE]; + size_t context_hash_len; + if (!tls13_get_context_hash(ssl, context_hash, &context_hash_len)) { return 0; } return hkdf_expand_label(out, digest, hs->secret, hs->hash_len, label, - label_len, context_hashes, context_hashes_len, len); + label_len, context_hash, context_hash_len, len); } -int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type, - enum evp_aead_direction_t direction, +int tls13_set_traffic_key(SSL *ssl, enum evp_aead_direction_t direction, const uint8_t *traffic_secret, size_t traffic_secret_len) { if (traffic_secret_len > 0xff) { @@ -134,28 +120,6 @@ return 0; } - const char *key_label, *iv_label; - switch (type) { - case type_early_handshake: - key_label = "early handshake key expansion, key"; - iv_label = "early handshake key expansion, iv"; - break; - case type_early_data: - key_label = "early application data key expansion, key"; - iv_label = "early application data key expansion, iv"; - break; - case type_handshake: - key_label = "handshake key expansion, key"; - iv_label = "handshake key expansion, iv"; - break; - case type_data: - key_label = "application data key expansion, key"; - iv_label = "application data key expansion, iv"; - break; - default: - return 0; - } - /* Look up cipher suite properties. */ const EVP_AEAD *aead; const EVP_MD *digest = ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl)); @@ -170,8 +134,7 @@ size_t key_len = EVP_AEAD_key_length(aead); uint8_t key[EVP_AEAD_MAX_KEY_LENGTH]; if (!hkdf_expand_label(key, digest, traffic_secret, traffic_secret_len, - (const uint8_t *)key_label, strlen(key_label), NULL, 0, - key_len)) { + (const uint8_t *)"key", 3, NULL, 0, key_len)) { return 0; } @@ -179,8 +142,7 @@ size_t iv_len = EVP_AEAD_nonce_length(aead); uint8_t iv[EVP_AEAD_MAX_NONCE_LENGTH]; if (!hkdf_expand_label(iv, digest, traffic_secret, traffic_secret_len, - (const uint8_t *)iv_label, strlen(iv_label), NULL, 0, - iv_len)) { + (const uint8_t *)"iv", 2, NULL, 0, iv_len)) { return 0; } @@ -241,26 +203,29 @@ } if (ssl->server) { - if (!tls13_set_traffic_key(ssl, type_handshake, evp_aead_open, - client_traffic_secret, hs->hash_len) || - !tls13_set_traffic_key(ssl, type_handshake, evp_aead_seal, - server_traffic_secret, hs->hash_len)) { + if (!tls13_set_traffic_key(ssl, evp_aead_open, client_traffic_secret, + hs->hash_len) || + !tls13_set_traffic_key(ssl, evp_aead_seal, server_traffic_secret, + hs->hash_len)) { return 0; } } else { - if (!tls13_set_traffic_key(ssl, type_handshake, evp_aead_open, - server_traffic_secret, hs->hash_len) || - !tls13_set_traffic_key(ssl, type_handshake, evp_aead_seal, - client_traffic_secret, hs->hash_len)) { + if (!tls13_set_traffic_key(ssl, evp_aead_open, server_traffic_secret, + hs->hash_len) || + !tls13_set_traffic_key(ssl, evp_aead_seal, client_traffic_secret, + hs->hash_len)) { return 0; } } return 1; } -int tls13_derive_traffic_secret_0(SSL *ssl) { +static const char kTLS13LabelExporter[] = "exporter master secret"; + +int tls13_derive_application_secrets(SSL *ssl) { SSL_HANDSHAKE *hs = ssl->s3->hs; + ssl->s3->exporter_secret_len = hs->hash_len; return derive_secret(ssl, hs->client_traffic_secret_0, hs->hash_len, (const uint8_t *)kTLS13LabelClientApplicationTraffic, strlen(kTLS13LabelClientApplicationTraffic)) && @@ -270,7 +235,10 @@ (const uint8_t *)kTLS13LabelServerApplicationTraffic, strlen(kTLS13LabelServerApplicationTraffic)) && ssl_log_secret(ssl, "SERVER_TRAFFIC_SECRET_0", - hs->server_traffic_secret_0, hs->hash_len); + hs->server_traffic_secret_0, hs->hash_len) && + derive_secret(ssl, ssl->s3->exporter_secret, hs->hash_len, + (const uint8_t *)kTLS13LabelExporter, + strlen(kTLS13LabelExporter)); } static const char kTLS13LabelApplicationTraffic[] = @@ -296,27 +264,36 @@ return 0; } - return tls13_set_traffic_key(ssl, type_data, direction, secret, secret_len); + return tls13_set_traffic_key(ssl, direction, secret, secret_len); } -static const char kTLS13LabelExporter[] = "exporter master secret"; static const char kTLS13LabelResumption[] = "resumption master secret"; -int tls13_finalize_keys(SSL *ssl) { - SSL_HANDSHAKE *hs = ssl->s3->hs; +int tls13_derive_resumption_secret(SSL *ssl) { + ssl->s3->new_session->master_key_length = ssl->s3->hs->hash_len; + return derive_secret(ssl, ssl->s3->new_session->master_key, + ssl->s3->new_session->master_key_length, + (const uint8_t *)kTLS13LabelResumption, + strlen(kTLS13LabelResumption)); +} - ssl->s3->exporter_secret_len = hs->hash_len; - ssl->s3->new_session->master_key_length = hs->hash_len; - if (!derive_secret( - ssl, ssl->s3->exporter_secret, ssl->s3->exporter_secret_len, - (const uint8_t *)kTLS13LabelExporter, strlen(kTLS13LabelExporter)) || - !derive_secret(ssl, ssl->s3->new_session->master_key, - ssl->s3->new_session->master_key_length, - (const uint8_t *)kTLS13LabelResumption, - strlen(kTLS13LabelResumption))) { +static const char kTLS13LabelFinished[] = "finished"; + +/* tls13_verify_data sets |out| to be the HMAC of |context| using a derived + * Finished key for both Finished messages and the PSK binder. */ +static int tls13_verify_data(const EVP_MD *digest, uint8_t *out, + size_t *out_len, const uint8_t *secret, + size_t hash_len, uint8_t *context, + size_t context_len) { + uint8_t key[EVP_MAX_MD_SIZE]; + unsigned len; + if (!hkdf_expand_label(key, digest, secret, hash_len, + (const uint8_t *)kTLS13LabelFinished, + strlen(kTLS13LabelFinished), NULL, 0, hash_len) || + HMAC(digest, key, hash_len, context, context_len, out, &len) == NULL) { return 0; } - + *out_len = len; return 1; } @@ -324,54 +301,23 @@ SSL_HANDSHAKE *hs = ssl->s3->hs; const EVP_MD *digest = ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl)); - uint8_t key[EVP_MAX_MD_SIZE]; - size_t key_len = EVP_MD_size(digest); - const uint8_t *traffic_secret; - const char *label = "finished"; if (is_server == ssl->server) { traffic_secret = ssl->s3->write_traffic_secret; } else { traffic_secret = ssl->s3->read_traffic_secret; } - uint8_t context_hashes[2 * EVP_MAX_MD_SIZE]; - size_t context_hashes_len; - unsigned len; - if (!hkdf_expand_label(key, digest, traffic_secret, hs->hash_len, - (const uint8_t *)label, strlen(label), NULL, 0, - hs->hash_len) || - !tls13_get_context_hashes(ssl, context_hashes, &context_hashes_len) || - HMAC(digest, key, key_len, context_hashes, context_hashes_len, out, - &len) == NULL) { + uint8_t context_hash[EVP_MAX_MD_SIZE]; + size_t context_hash_len; + if (!tls13_get_context_hash(ssl, context_hash, &context_hash_len) || + !tls13_verify_data(digest, out, out_len, traffic_secret, hs->hash_len, + context_hash, context_hash_len)) { return 0; } - *out_len = len; return 1; } -static const char kTLS13LabelResumptionPSK[] = "resumption psk"; -static const char kTLS13LabelResumptionContext[] = "resumption context"; - -int tls13_resumption_psk(SSL *ssl, uint8_t *out, size_t out_len, - const SSL_SESSION *session) { - const EVP_MD *digest = ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl)); - return hkdf_expand_label(out, digest, session->master_key, - session->master_key_length, - (const uint8_t *)kTLS13LabelResumptionPSK, - strlen(kTLS13LabelResumptionPSK), NULL, 0, out_len); -} - -int tls13_resumption_context(SSL *ssl, uint8_t *out, size_t out_len, - const SSL_SESSION *session) { - const EVP_MD *digest = ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl)); - return hkdf_expand_label(out, digest, session->master_key, - session->master_key_length, - (const uint8_t *)kTLS13LabelResumptionContext, - strlen(kTLS13LabelResumptionContext), NULL, 0, - out_len); -} - int tls13_export_keying_material(SSL *ssl, uint8_t *out, size_t out_len, const char *label, size_t label_len, const uint8_t *context, size_t context_len, @@ -388,3 +334,115 @@ ssl->s3->exporter_secret_len, (const uint8_t *)label, label_len, hash, hash_len, out_len); } + +static const char kTLS13LabelPSKBinder[] = "resumption psk binder key"; + +static int tls13_psk_binder(SSL *ssl, uint8_t *out, const EVP_MD *digest, + uint8_t *psk, size_t psk_len, uint8_t *context, + size_t context_len, size_t hash_len) { + uint8_t binder_context[EVP_MAX_MD_SIZE]; + unsigned binder_context_len; + if (!EVP_Digest(NULL, 0, binder_context, &binder_context_len, digest, NULL)) { + return 0; + } + + uint8_t early_secret[EVP_MAX_MD_SIZE] = {0}; + size_t early_secret_len; + if (!HKDF_extract(early_secret, &early_secret_len, digest, psk, hash_len, + NULL, 0)) { + return 0; + } + + uint8_t binder_key[EVP_MAX_MD_SIZE] = {0}; + size_t len; + if (!hkdf_expand_label(binder_key, digest, early_secret, hash_len, + (const uint8_t *)kTLS13LabelPSKBinder, + strlen(kTLS13LabelPSKBinder), binder_context, + binder_context_len, hash_len) || + !tls13_verify_data(digest, out, &len, binder_key, hash_len, context, + context_len)) { + return 0; + } + + return 1; +} + +int tls13_write_psk_binder(SSL *ssl, uint8_t *msg, size_t len) { + const EVP_MD *digest = + ssl_get_handshake_digest(ssl->session->cipher->algorithm_prf); + size_t hash_len = EVP_MD_size(digest); + + if (len < hash_len + 3) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return 0; + } + + EVP_MD_CTX ctx; + EVP_MD_CTX_init(&ctx); + uint8_t context[EVP_MAX_MD_SIZE]; + unsigned context_len; + if (!EVP_DigestInit_ex(&ctx, digest, NULL) || + !EVP_DigestUpdate(&ctx, ssl->s3->handshake_buffer->data, + ssl->s3->handshake_buffer->length) || + !EVP_DigestUpdate(&ctx, msg, len - hash_len - 3) || + !EVP_DigestFinal_ex(&ctx, context, &context_len)) { + EVP_MD_CTX_cleanup(&ctx); + return 0; + } + + EVP_MD_CTX_cleanup(&ctx); + + uint8_t verify_data[EVP_MAX_MD_SIZE] = {0}; + if (!tls13_psk_binder(ssl, verify_data, digest, ssl->session->master_key, + ssl->session->master_key_length, context, + context_len, hash_len)) { + return 0; + } + + memcpy(msg + len - hash_len, verify_data, hash_len); + return 1; +} + +int tls13_verify_psk_binder(SSL *ssl, SSL_SESSION *session, + CBS *binders) { + const EVP_MD *digest = + ssl_get_handshake_digest(session->cipher->algorithm_prf); + size_t hash_len = EVP_MD_size(digest); + + /* Get the full ClientHello, including message header. It must be large enough + * to exclude the binders. */ + CBS message; + ssl->method->get_current_message(ssl, &message); + if (CBS_len(&message) < CBS_len(binders) + 2) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return 0; + } + + /* Hash a ClientHello prefix up to the binders. For now, this assumes we only + * ever verify PSK binders on initial ClientHellos. */ + uint8_t context[EVP_MAX_MD_SIZE]; + unsigned context_len; + if (!EVP_Digest(CBS_data(&message), CBS_len(&message) - CBS_len(binders) - 2, + context, &context_len, digest, NULL)) { + return 0; + } + + uint8_t verify_data[EVP_MAX_MD_SIZE] = {0}; + CBS binder; + if (!tls13_psk_binder(ssl, verify_data, digest, session->master_key, + session->master_key_length, context, context_len, + hash_len) || + /* We only consider the first PSK, so compare against the first binder. */ + !CBS_get_u8_length_prefixed(binders, &binder)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return 0; + } + + if (CBS_len(&binder) != hash_len || + CRYPTO_memcmp(CBS_data(&binder), verify_data, hash_len) != 0) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DIGEST_CHECK_FAILED); + return 0; + } + + return 1; +}
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index ca7b17e..0db16d9 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c
@@ -110,16 +110,38 @@ memcpy(ssl->s3->client_random, client_hello.random, client_hello.random_len); uint8_t alert = SSL_AD_DECODE_ERROR; - SSL_SESSION *session = NULL; - CBS pre_shared_key; - if (ssl_early_callback_get_extension(&client_hello, &pre_shared_key, - TLSEXT_TYPE_pre_shared_key) && - !ssl_ext_pre_shared_key_parse_clienthello(ssl, &session, &alert, - &pre_shared_key)) { + CBS psk_key_exchange_modes; + if (ssl_early_callback_get_extension(&client_hello, &psk_key_exchange_modes, + TLSEXT_TYPE_psk_key_exchange_modes) && + !ssl_ext_psk_key_exchange_modes_parse_clienthello( + ssl, &alert, &psk_key_exchange_modes)) { ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); return 0; } + SSL_SESSION *session = NULL; + CBS binders; + if (hs->accept_psk_mode) { + CBS pre_shared_key; + if (ssl_early_callback_get_extension(&client_hello, &pre_shared_key, + TLSEXT_TYPE_pre_shared_key)) { + /* Verify that the pre_shared_key extension is the last extension in + * ClientHello. */ + if (CBS_data(&pre_shared_key) + CBS_len(&pre_shared_key) != + client_hello.extensions + client_hello.extensions_len) { + OPENSSL_PUT_ERROR(SSL, SSL_R_PRE_SHARED_KEY_MUST_BE_LAST); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + return 0; + } + + if (!ssl_ext_pre_shared_key_parse_clienthello(ssl, &session, &binders, + &alert, &pre_shared_key)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); + return 0; + } + } + } + if (session != NULL && /* Only resume if the session's version matches. */ (session->ssl_version != ssl->version || @@ -136,6 +158,13 @@ return ssl_hs_error; } } else { + /* Check the PSK binder. */ + if (!tls13_verify_psk_binder(ssl, session, &binders)) { + SSL_SESSION_free(session); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECRYPT_ERROR); + return ssl_hs_error; + } + /* Only authentication information carries over in TLS 1.3. */ ssl->s3->new_session = SSL_SESSION_dup(session, SSL_SESSION_DUP_AUTH_ONLY); if (ssl->s3->new_session == NULL) { @@ -266,20 +295,17 @@ EVP_MD_size(ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl))); /* Derive resumption material. */ - uint8_t resumption_ctx[EVP_MAX_MD_SIZE] = {0}; uint8_t psk_secret[EVP_MAX_MD_SIZE] = {0}; if (ssl->s3->session_reused) { - if (!tls13_resumption_context(ssl, resumption_ctx, hash_len, - ssl->s3->new_session) || - !tls13_resumption_psk(ssl, psk_secret, hash_len, - ssl->s3->new_session)) { + if (hash_len != (size_t) ssl->s3->new_session->master_key_length) { return ssl_hs_error; } + memcpy(psk_secret, ssl->s3->new_session->master_key, hash_len); } /* Set up the key schedule, hash in the ClientHello, and incorporate the PSK * into the running secret. */ - if (!tls13_init_key_schedule(ssl, resumption_ctx, hash_len) || + if (!tls13_init_key_schedule(ssl) || !tls13_advance_key_schedule(ssl, psk_secret, hash_len)) { return ssl_hs_error; } @@ -367,18 +393,8 @@ !CBB_add_u16(&body, ssl_cipher_get_value(ssl->s3->tmp.new_cipher)) || !CBB_add_u16_length_prefixed(&body, &extensions) || !ssl_ext_pre_shared_key_add_serverhello(ssl, &extensions) || - !ssl_ext_key_share_add_serverhello(ssl, &extensions)) { - goto err; - } - - if (!ssl->s3->session_reused) { - if (!CBB_add_u16(&extensions, TLSEXT_TYPE_signature_algorithms) || - !CBB_add_u16(&extensions, 0)) { - goto err; - } - } - - if (!ssl_complete_message(ssl, &cbb)) { + !ssl_ext_key_share_add_serverhello(ssl, &extensions) || + !ssl_complete_message(ssl, &cbb)) { goto err; } @@ -509,9 +525,9 @@ static enum ssl_hs_wait_t do_flush(SSL *ssl, SSL_HANDSHAKE *hs) { /* Update the secret to the master secret and derive traffic keys. */ if (!tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len) || - !tls13_derive_traffic_secret_0(ssl) || - !tls13_set_traffic_key(ssl, type_data, evp_aead_seal, - hs->server_traffic_secret_0, hs->hash_len)) { + !tls13_derive_application_secrets(ssl) || + !tls13_set_traffic_key(ssl, evp_aead_seal, hs->server_traffic_secret_0, + hs->hash_len)) { return ssl_hs_error; } @@ -590,9 +606,9 @@ !tls13_process_finished(ssl) || !ssl_hash_current_message(ssl) || /* evp_aead_seal keys have already been switched. */ - !tls13_set_traffic_key(ssl, type_data, evp_aead_open, - hs->client_traffic_secret_0, hs->hash_len) || - !tls13_finalize_keys(ssl)) { + !tls13_set_traffic_key(ssl, evp_aead_open, hs->client_traffic_secret_0, + hs->hash_len) || + !tls13_derive_resumption_secret(ssl)) { return ssl_hs_error; } @@ -611,19 +627,26 @@ static enum ssl_hs_wait_t do_send_new_session_ticket(SSL *ssl, SSL_HANDSHAKE *hs) { + /* If the client doesn't accept resumption with PSK_DHE_KE, don't send a + * session ticket. */ + if (!hs->accept_psk_mode) { + hs->state = state_done; + return ssl_hs_ok; + } + SSL_SESSION *session = ssl->s3->new_session; + if (!RAND_bytes((uint8_t *)&session->ticket_age_add, 4)) { + goto err; + } /* TODO(svaldez): Add support for sending 0RTT through TicketEarlyDataInfo * extension. */ - CBB cbb, body, ke_modes, auth_modes, ticket, extensions; + CBB cbb, body, ticket, extensions; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_NEW_SESSION_TICKET) || !CBB_add_u32(&body, session->timeout) || - !CBB_add_u8_length_prefixed(&body, &ke_modes) || - !CBB_add_u8(&ke_modes, SSL_PSK_DHE_KE) || - !CBB_add_u8_length_prefixed(&body, &auth_modes) || - !CBB_add_u8(&auth_modes, SSL_PSK_AUTH) || + !CBB_add_u32(&body, session->ticket_age_add) || !CBB_add_u16_length_prefixed(&body, &ticket) || !ssl_encrypt_ticket(ssl, &ticket, session) || !CBB_add_u16_length_prefixed(&body, &extensions)) {