diff --git a/apps/apps.h b/apps/apps.h index bb89eaecf6cc8b13ed5309b6b376d15f26c152a7..321f6444da8a1e00333b80780c1103d5627b57cf 100644 --- a/apps/apps.h +++ b/apps/apps.h @@ -208,7 +208,7 @@ int set_cert_times(X509 *x, const char *startdate, const char *enddate, OPT_S_STRICT, OPT_S_SIGALGS, OPT_S_CLIENTSIGALGS, OPT_S_GROUPS, \ OPT_S_CURVES, OPT_S_NAMEDCURVE, OPT_S_CIPHER, \ OPT_S_RECORD_PADDING, OPT_S_DEBUGBROKE, OPT_S_COMP, \ - OPT_S_NO_RENEGOTIATION, OPT_S__LAST + OPT_S_NO_RENEGOTIATION, OPT_S_NO_MIDDLEBOX, OPT_S__LAST # define OPT_S_OPTIONS \ {"no_ssl3", OPT_S_NOSSL3, '-',"Just disable SSLv3" }, \ @@ -253,7 +253,8 @@ int set_cert_times(X509 *x, const char *startdate, const char *enddate, {"record_padding", OPT_S_RECORD_PADDING, 's', \ "Block size to pad TLS 1.3 records to."}, \ {"debug_broken_protocol", OPT_S_DEBUGBROKE, '-', \ - "Perform all sorts of protocol violations for testing purposes"} + "Perform all sorts of protocol violations for testing purposes"}, \ + {"no_middlebox", OPT_S_NO_MIDDLEBOX, '-', "Disable TLSv1.3 middlebox compat mode" } # define OPT_S_CASES \ @@ -283,7 +284,8 @@ int set_cert_times(X509 *x, const char *startdate, const char *enddate, case OPT_S_CIPHER: \ case OPT_S_RECORD_PADDING: \ case OPT_S_NO_RENEGOTIATION: \ - case OPT_S_DEBUGBROKE + case OPT_S_DEBUGBROKE: \ + case OPT_S_NO_MIDDLEBOX #define IS_NO_PROT_FLAG(o) \ (o == OPT_S_NOSSL3 || o == OPT_S_NOTLS1 || o == OPT_S_NOTLS1_1 \ diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 5c4ca2d6d0d3a13b37aaf770ba3fbd3cbfc2608d..4410314bc1facd2252185f191c4211a17682fa04 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -2463,6 +2463,7 @@ SSL_R_INVALID_MAX_EARLY_DATA:174:invalid max early data SSL_R_INVALID_NULL_CMD_NAME:385:invalid null cmd name SSL_R_INVALID_SEQUENCE_NUMBER:402:invalid sequence number SSL_R_INVALID_SERVERINFO_DATA:388:invalid serverinfo data +SSL_R_INVALID_SESSION_ID:232:invalid session id SSL_R_INVALID_SRP_USERNAME:357:invalid srp username SSL_R_INVALID_STATUS_RESPONSE:328:invalid status response SSL_R_INVALID_TICKET_KEYS_LENGTH:325:invalid ticket keys length diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index a5251b59cc9d07f11fdccbb8fed0f53c0555096d..48779fa6d2b98ab9175046a6096672a3170e2d30 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -338,9 +338,17 @@ typedef int (*SSL_verify_cb)(int preverify_ok, X509_STORE_CTX *x509_ctx); # define SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION 0x00040000U /* Disable encrypt-then-mac */ # define SSL_OP_NO_ENCRYPT_THEN_MAC 0x00080000U + +/* + * Enable TLSv1.3 Compatibility mode. This is on by default. A future version + * of OpenSSL may have this disabled by default. + */ +# define SSL_OP_ENABLE_MIDDLEBOX_COMPAT 0x00100000U + /* Prioritize Chacha20Poly1305 when client does. * Modifies SSL_OP_CIPHER_SERVER_PREFERENCE */ # define SSL_OP_PRIORITIZE_CHACHA 0x00200000U + /* * Set on servers to choose the cipher according to the server's preferences */ diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index 364b19809e1a20c341be9922ee35e6c62ceb5bbe..3199ab0ff420a429dce0084d024a4df1a1f1b2aa 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -543,6 +543,7 @@ int ERR_load_SSL_strings(void); # define SSL_R_INVALID_NULL_CMD_NAME 385 # define SSL_R_INVALID_SEQUENCE_NUMBER 402 # define SSL_R_INVALID_SERVERINFO_DATA 388 +# define SSL_R_INVALID_SESSION_ID 232 # define SSL_R_INVALID_SRP_USERNAME 357 # define SSL_R_INVALID_STATUS_RESPONSE 328 # define SSL_R_INVALID_TICKET_KEYS_LENGTH 325 diff --git a/ssl/ssl_conf.c b/ssl/ssl_conf.c index fe090ae40d1b995c8ed1981565938a0b5a385f48..0f53a470439120fea01468e7071a8dd461d82b9a 100644 --- a/ssl/ssl_conf.c +++ b/ssl/ssl_conf.c @@ -369,7 +369,8 @@ static int cmd_Options(SSL_CONF_CTX *cctx, const char *value) SSL_FLAG_TBL_INV("EncryptThenMac", SSL_OP_NO_ENCRYPT_THEN_MAC), SSL_FLAG_TBL("NoRenegotiation", SSL_OP_NO_RENEGOTIATION), SSL_FLAG_TBL("AllowNoDHEKEX", SSL_OP_ALLOW_NO_DHE_KEX), - SSL_FLAG_TBL("PrioritizeChaCha", SSL_OP_PRIORITIZE_CHACHA) + SSL_FLAG_TBL("PrioritizeChaCha", SSL_OP_PRIORITIZE_CHACHA), + SSL_FLAG_TBL("MiddleboxCompat", SSL_OP_ENABLE_MIDDLEBOX_COMPAT) }; if (value == NULL) return -3; @@ -591,6 +592,7 @@ static const ssl_conf_cmd_tbl ssl_conf_cmds[] = { SSL_CONF_CMD_SWITCH("allow_no_dhe_kex", 0), SSL_CONF_CMD_SWITCH("prioritize_chacha", SSL_CONF_FLAG_SERVER), SSL_CONF_CMD_SWITCH("strict", 0), + SSL_CONF_CMD_SWITCH("no_middlebox", SSL_CONF_FLAG_CLIENT), SSL_CONF_CMD_STRING(SignatureAlgorithms, "sigalgs", 0), SSL_CONF_CMD_STRING(ClientSignatureAlgorithms, "client_sigalgs", 0), SSL_CONF_CMD_STRING(Curves, "curves", 0), @@ -665,6 +667,8 @@ static const ssl_switch_tbl ssl_cmd_switches[] = { /* chacha reprioritization */ {SSL_OP_PRIORITIZE_CHACHA, 0}, {SSL_CERT_FLAG_TLS_STRICT, SSL_TFLAG_CERT}, /* strict */ + /* no_middlebox */ + {SSL_OP_ENABLE_MIDDLEBOX_COMPAT, SSL_TFLAG_INV}, }; static int ssl_conf_cmd_skip_prefix(SSL_CONF_CTX *cctx, const char **pcmd) diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index d608daff072454b98b1e61ce9775caf6aca8a2aa..fa02b21b9d23837c3260a11dcc9040d40418a34b 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -871,6 +871,7 @@ static const ERR_STRING_DATA SSL_str_reasons[] = { "invalid sequence number"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_INVALID_SERVERINFO_DATA), "invalid serverinfo data"}, + {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_INVALID_SESSION_ID), "invalid session id"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_INVALID_SRP_USERNAME), "invalid srp username"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_INVALID_STATUS_RESPONSE), diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 20073184eb007a3a80a05d4dee7de97fae02d8be..bba0291421e03f0efe0ceabbd17e6323d1bd2f39 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2894,9 +2894,11 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth) * Disable compression by default to prevent CRIME. Applications can * re-enable compression by configuring * SSL_CTX_clear_options(ctx, SSL_OP_NO_COMPRESSION); - * or by using the SSL_CONF library. + * or by using the SSL_CONF library. Similarly we also enable TLSv1.3 + * middlebox compatibility by default. This may be disabled by default in + * a later OpenSSL version. */ - ret->options |= SSL_OP_NO_COMPRESSION; + ret->options |= SSL_OP_NO_COMPRESSION | SSL_OP_ENABLE_MIDDLEBOX_COMPAT; ret->ext.status_type = TLSEXT_STATUSTYPE_nothing; diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index ed6b9a8fd43b5256d0d29e533dd6df4e3aef9374..c2d63fa100781499b6bc36bcca77499b048ddc36 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1132,6 +1132,12 @@ struct ssl_st { size_t psksession_id_len; /* Default generate session ID callback. */ GEN_SESSION_CB generate_session_id; + /* + * The temporary TLSv1.3 session id. This isn't really a session id at all + * but is a random value sent in the legacy session id field. + */ + unsigned char tmp_session_id[SSL_MAX_SSL_SESSION_ID_LENGTH]; + size_t tmp_session_id_len; /* Used in SSL3 */ /* * 0 don't care about verify failure. diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 3c628bdd99e42a29589fa668c56ba089efa605e1..473da7a73077d67b1283d7ec6bffaacf8e858eb7 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1028,6 +1028,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt) SSL_COMP *comp; #endif SSL_SESSION *sess = s->session; + unsigned char *session_id; if (!WPACKET_set_max_size(pkt, SSL3_RT_MAX_PLAIN_LENGTH)) { /* Should not happen */ @@ -1047,7 +1048,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt) if (sess == NULL || !ssl_version_supported(s, sess->ssl_version) || !SSL_SESSION_is_resumable(sess)) { - if (!ssl_get_new_session(s, 0)) { + if (!s->hello_retry_request && !ssl_get_new_session(s, 0)) { /* SSLfatal() already called */ return 0; } @@ -1121,13 +1122,34 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt) } /* Session ID */ - if (s->new_session || s->session->ssl_version == TLS1_3_VERSION) - sess_id_len = 0; - else + session_id = s->session->session_id; + if (s->new_session || s->session->ssl_version == TLS1_3_VERSION) { + if (s->version == TLS1_3_VERSION + && (s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0) { + sess_id_len = sizeof(s->tmp_session_id); + s->tmp_session_id_len = sess_id_len; + session_id = s->tmp_session_id; + if (!s->hello_retry_request + && ssl_randbytes(s, s->tmp_session_id, + sess_id_len) <= 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, + ERR_R_INTERNAL_ERROR); + return 0; + } + } else { + sess_id_len = 0; + } + } else { sess_id_len = s->session->session_id_length; + if (s->version == TLS1_3_VERSION) { + s->tmp_session_id_len = sess_id_len; + memcpy(s->tmp_session_id, s->session->session_id, sess_id_len); + } + } if (sess_id_len > sizeof(s->session->session_id) || !WPACKET_start_sub_packet_u8(pkt) - || (sess_id_len != 0 && !WPACKET_memcpy(pkt, s->session->session_id, + || (sess_id_len != 0 && !WPACKET_memcpy(pkt, session_id, sess_id_len)) || !WPACKET_close(pkt)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, @@ -1393,25 +1415,35 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) goto err; } - /* - * In TLSv1.3 a ServerHello message signals a key change so the end of the - * message must be on a record boundary. - */ - if (SSL_IS_TLS13(s) && RECORD_LAYER_processed_read_pending(&s->rlayer)) { - SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_TLS_PROCESS_SERVER_HELLO, - SSL_R_NOT_ON_RECORD_BOUNDARY); - goto err; - } - - if (SSL_IS_TLS13(s) && compression != 0) { - SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SERVER_HELLO, - SSL_R_INVALID_COMPRESSION_ALGORITHM); - goto err; - } - s->hit = 0; if (SSL_IS_TLS13(s)) { + /* + * In TLSv1.3 a ServerHello message signals a key change so the end of + * the message must be on a record boundary. + */ + if (RECORD_LAYER_processed_read_pending(&s->rlayer)) { + SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, + SSL_F_TLS_PROCESS_SERVER_HELLO, + SSL_R_NOT_ON_RECORD_BOUNDARY); + goto err; + } + + if (compression != 0) { + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, + SSL_F_TLS_PROCESS_SERVER_HELLO, + SSL_R_INVALID_COMPRESSION_ALGORITHM); + goto err; + } + + if (session_id_len != s->tmp_session_id_len + || memcmp(PACKET_data(&session_id), s->tmp_session_id, + session_id_len) != 0) { + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, + SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_INVALID_SESSION_ID); + goto err; + } + /* This will set s->hit if we are resuming */ if (!tls_parse_extension(s, TLSEXT_IDX_psk, SSL_EXT_TLS1_3_SERVER_HELLO, @@ -1493,11 +1525,19 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) } s->session->ssl_version = s->version; - s->session->session_id_length = session_id_len; - /* session_id_len could be 0 */ - if (session_id_len > 0) - memcpy(s->session->session_id, PACKET_data(&session_id), - session_id_len); + /* + * In TLSv1.2 and below we save the session id we were sent so we can + * resume it later. In TLSv1.3 the session id we were sent is just an + * echo of what we originally sent in the ClientHello and should not be + * used for resumption. + */ + if (!SSL_IS_TLS13(s)) { + s->session->session_id_length = session_id_len; + /* session_id_len could be 0 */ + if (session_id_len > 0) + memcpy(s->session->session_id, PACKET_data(&session_id), + session_id_len); + } } /* Session version and negotiated protocol version should match */ diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index e91bba4c0f82af71f4c2d52f5573f3004fdd9ffb..36083096b9dd1740633784c516848e169c40c942 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1686,6 +1686,12 @@ static int tls_early_post_process_client_hello(SSL *s) } } + if (SSL_IS_TLS13(s)) { + memcpy(s->tmp_session_id, s->clienthello->session_id, + s->clienthello->session_id_len); + s->tmp_session_id_len = s->clienthello->session_id_len; + } + /* * If it is a hit, check that the cipher is in the list. In TLSv1.3 we check * ciphersuite compatibility with the session as part of resumption. @@ -2192,6 +2198,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt) int compm; size_t sl, len; int version; + unsigned char *session_id; version = SSL_IS_TLS13(s) ? TLS1_2_VERSION : s->version; if (!WPACKET_put_bytes_u16(pkt, version) @@ -2217,6 +2224,8 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt) * session ID. * - However, if we want the new session to be single-use, * we send back a 0-length session ID. + * - In TLSv1.3 we echo back the session id sent to us by the client + * regardless * s->hit is non-zero in either case of session reuse, * so the following won't overwrite an ID that we're supposed * to send back. @@ -2226,17 +2235,20 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt) && !s->hit)) s->session->session_id_length = 0; - sl = s->session->session_id_length; + if (SSL_IS_TLS13(s)) { + sl = s->tmp_session_id_len; + session_id = s->tmp_session_id; + } else { + sl = s->session->session_id_length; + session_id = s->session->session_id; + } + if (sl > sizeof(s->session->session_id)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_SERVER_HELLO, ERR_R_INTERNAL_ERROR); return 0; } - /* Never send a session_id back in the ServerHello */ - if (SSL_IS_TLS13(s)) - sl = 0; - /* set up the compression method */ #ifdef OPENSSL_NO_COMP compm = 0; @@ -2247,7 +2259,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt) compm = s->s3->tmp.new_compression->id; #endif - if (!WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl) + if (!WPACKET_sub_memcpy_u8(pkt, session_id, sl) || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len) || !WPACKET_put_bytes_u8(pkt, compm) || !tls_construct_extensions(s, pkt, diff --git a/test/clienthellotest.c b/test/clienthellotest.c index 8ba65ce4fb476e9346a18c4bb72fccdc650de56a..88e0a1c66aa00932f9b8b9ad76d6c2428ea163d4 100644 --- a/test/clienthellotest.c +++ b/test/clienthellotest.c @@ -90,6 +90,8 @@ static int test_client_hello(int currtest) case TEST_ADD_PADDING: case TEST_PADDING_NOT_NEEDED: SSL_CTX_set_options(ctx, SSL_OP_TLSEXT_PADDING); + /* Make sure we get a consistent size across TLS versions */ + SSL_CTX_clear_options(ctx, SSL_OP_ENABLE_MIDDLEBOX_COMPAT); /* * Add some dummy ALPN protocols so that the ClientHello is at least * F5_WORKAROUND_MIN_MSG_LEN bytes long - meaning padding will be