Update ServerHello to new draft-22 format
The new ServerHello format is essentially now the same as the old TLSv1.2
one, but it must additionally include supported_versions. The version
field is fixed at TLSv1.2, and the version negotiation happens solely via
supported_versions.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 464a5ef..988e919 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -9,6 +9,7 @@
#include <string.h>
#include "internal/nelem.h"
+#include "internal/cryptlib.h"
#include "../ssl_locl.h"
#include "statem_locl.h"
@@ -261,11 +262,13 @@
},
{
TLSEXT_TYPE_supported_versions,
- SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS_IMPLEMENTATION_ONLY
- | SSL_EXT_TLS1_3_ONLY,
+ SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO
+ | SSL_EXT_TLS1_3_SERVER_HELLO | SSL_EXT_TLS_IMPLEMENTATION_ONLY,
NULL,
/* Processed inline as part of version selection */
- NULL, NULL, NULL, tls_construct_ctos_supported_versions, NULL
+ NULL, tls_parse_stoc_supported_versions,
+ tls_construct_stoc_supported_versions,
+ tls_construct_ctos_supported_versions, NULL
},
{
TLSEXT_TYPE_psk_kex_modes,
@@ -357,6 +360,44 @@
return 1;
}
+int tls_validate_all_contexts(SSL *s, unsigned int thisctx, RAW_EXTENSION *exts)
+{
+ size_t i, num_exts, builtin_num = OSSL_NELEM(ext_defs), offset;
+ RAW_EXTENSION *thisext;
+ unsigned int context;
+ ENDPOINT role = ENDPOINT_BOTH;
+
+ if ((thisctx & SSL_EXT_CLIENT_HELLO) != 0)
+ role = ENDPOINT_SERVER;
+ else if ((thisctx & SSL_EXT_TLS1_2_SERVER_HELLO) != 0)
+ role = ENDPOINT_CLIENT;
+
+ /* Calculate the number of extensions in the extensions list */
+ num_exts = builtin_num + s->cert->custext.meths_count;
+
+ for (thisext = exts, i = 0; i < num_exts; i++, thisext++) {
+ if (!thisext->present)
+ continue;
+
+ if (i < builtin_num) {
+ context = ext_defs[i].context;
+ } else {
+ custom_ext_method *meth = NULL;
+
+ meth = custom_ext_find(&s->cert->custext, role, thisext->type,
+ &offset);
+ if (!ossl_assert(meth != NULL))
+ return 0;
+ context = meth->context;
+ }
+
+ if (!validate_context(s, context, thisctx))
+ return 0;
+ }
+
+ return 1;
+}
+
/*
* Verify whether we are allowed to use the extension |type| in the current
* |context|. Returns 1 to indicate the extension is allowed or unknown or 0 to
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index b7ef54e..2640756 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -507,6 +507,20 @@
{
int currv, min_version, max_version, reason;
+ reason = ssl_get_min_max_version(s, &min_version, &max_version);
+ if (reason != 0) {
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+ SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason);
+ return EXT_RETURN_FAIL;
+ }
+
+ /*
+ * Don't include this if we can't negotiate TLSv1.3. We can do a straight
+ * comparison here because we will never be called in DTLS.
+ */
+ if (max_version < TLS1_3_VERSION)
+ return EXT_RETURN_NOT_SENT;
+
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)
|| !WPACKET_start_sub_packet_u16(pkt)
|| !WPACKET_start_sub_packet_u8(pkt)) {
@@ -516,13 +530,6 @@
return EXT_RETURN_FAIL;
}
- reason = ssl_get_min_max_version(s, &min_version, &max_version);
- if (reason != 0) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR,
- SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason);
- return EXT_RETURN_FAIL;
- }
-
/*
* TODO(TLS1.3): There is some discussion on the TLS list as to whether
* we should include versions <TLS1.2. For the moment we do. To be
@@ -1633,6 +1640,29 @@
return 1;
}
+int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context,
+ X509 *x, size_t chainidx)
+{
+ unsigned int version;
+
+ if (!PACKET_get_net_2(pkt, &version)
+ || PACKET_remaining(pkt) != 0) {
+ SSLfatal(s, SSL_AD_DECODE_ERROR,
+ SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS,
+ SSL_R_LENGTH_MISMATCH);
+ return 0;
+ }
+
+ /* TODO(TLS1.3): Remove this before release */
+ if (version == TLS1_3_VERSION_DRAFT)
+ version = TLS1_3_VERSION;
+
+ /* We just set it here. We validate it in ssl_choose_client_version */
+ s->version = version;
+
+ return 1;
+}
+
int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
size_t chainidx)
{
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index b07376f..93ac98f 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -1213,6 +1213,27 @@
return EXT_RETURN_SENT;
}
+EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt,
+ unsigned int context, X509 *x,
+ size_t chainidx)
+{
+ if (!SSL_IS_TLS13(s))
+ return EXT_RETURN_NOT_SENT;
+
+ if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)
+ || !WPACKET_start_sub_packet_u16(pkt)
+ /* TODO(TLS1.3): Update to remove the TLSv1.3 draft indicator */
+ || !WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT)
+ || !WPACKET_close(pkt)) {
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+ SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS,
+ ERR_R_INTERNAL_ERROR);
+ return EXT_RETURN_FAIL;
+ }
+
+ return EXT_RETURN_SENT;
+}
+
EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt,
unsigned int context, X509 *x,
size_t chainidx)
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index a7a5a17..3c628bd 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1332,16 +1332,67 @@
goto err;
}
- /*
- * We do this immediately so we know what format the ServerHello is in.
- * Must be done after reading the random data so we can check for the
- * TLSv1.3 downgrade sentinels
- */
- if (!ssl_choose_client_version(s, sversion, 1)) {
+ /* Get the session-id. */
+ if (!PACKET_get_length_prefixed_1(pkt, &session_id)) {
+ SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
+ SSL_R_LENGTH_MISMATCH);
+ goto err;
+ }
+ session_id_len = PACKET_remaining(&session_id);
+ if (session_id_len > sizeof(s->session->session_id)
+ || session_id_len > SSL3_SESSION_ID_SIZE) {
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SERVER_HELLO,
+ SSL_R_SSL3_SESSION_ID_TOO_LONG);
+ goto err;
+ }
+
+ if (!PACKET_get_bytes(pkt, &cipherchars, TLS_CIPHER_LEN)) {
+ SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
+ SSL_R_LENGTH_MISMATCH);
+ goto err;
+ }
+
+ if (!PACKET_get_1(pkt, &compression)) {
+ SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
+ SSL_R_LENGTH_MISMATCH);
+ goto err;
+ }
+
+ /* TLS extensions */
+ if (PACKET_remaining(pkt) == 0) {
+ PACKET_null_init(&extpkt);
+ } else if (!PACKET_as_length_prefixed_2(pkt, &extpkt)
+ || PACKET_remaining(pkt) != 0) {
+ SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
+ SSL_R_BAD_LENGTH);
+ goto err;
+ }
+
+ if (!tls_collect_extensions(s, &extpkt,
+ SSL_EXT_TLS1_2_SERVER_HELLO
+ | SSL_EXT_TLS1_3_SERVER_HELLO,
+ &extensions, NULL, 1)) {
/* SSLfatal() already called */
goto err;
}
+ if (!ssl_choose_client_version(s, sversion, extensions)) {
+ /* SSLfatal() already called */
+ goto err;
+ }
+
+ /*
+ * Now we have chosen the version we need to check again that the extensions
+ * are appropriate for this version.
+ */
+ context = SSL_IS_TLS13(s) ? SSL_EXT_TLS1_3_SERVER_HELLO
+ : SSL_EXT_TLS1_2_SERVER_HELLO;
+ if (!tls_validate_all_contexts(s, context, extensions)) {
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SERVER_HELLO,
+ SSL_R_BAD_EXTENSION);
+ goto err;
+ }
+
/*
* In TLSv1.3 a ServerHello message signals a key change so the end of the
* message must be on a record boundary.
@@ -1352,56 +1403,9 @@
goto err;
}
- /* Get the session-id. */
- if (!SSL_IS_TLS13(s)) {
- if (!PACKET_get_length_prefixed_1(pkt, &session_id)) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
- SSL_R_LENGTH_MISMATCH);
- goto err;
- }
- session_id_len = PACKET_remaining(&session_id);
- if (session_id_len > sizeof(s->session->session_id)
- || session_id_len > SSL3_SESSION_ID_SIZE) {
- SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
- SSL_F_TLS_PROCESS_SERVER_HELLO,
- SSL_R_SSL3_SESSION_ID_TOO_LONG);
- goto err;
- }
- } else {
- PACKET_null_init(&session_id);
- session_id_len = 0;
- }
-
- if (!PACKET_get_bytes(pkt, &cipherchars, TLS_CIPHER_LEN)) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
- SSL_R_LENGTH_MISMATCH);
- goto err;
- }
-
- if (!SSL_IS_TLS13(s)) {
- if (!PACKET_get_1(pkt, &compression)) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
- SSL_R_LENGTH_MISMATCH);
- goto err;
- }
- } else {
- compression = 0;
- }
-
- /* TLS extensions */
- if (PACKET_remaining(pkt) == 0) {
- PACKET_null_init(&extpkt);
- } else if (!PACKET_as_length_prefixed_2(pkt, &extpkt)
- || PACKET_remaining(pkt) != 0) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
- SSL_R_BAD_LENGTH);
- goto err;
- }
-
- context = SSL_IS_TLS13(s) ? SSL_EXT_TLS1_3_SERVER_HELLO
- : SSL_EXT_TLS1_2_SERVER_HELLO;
- if (!tls_collect_extensions(s, &extpkt, context, &extensions, NULL, 1)) {
- /* SSLfatal() already called */
+ 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;
}
@@ -1411,7 +1415,7 @@
/* This will set s->hit if we are resuming */
if (!tls_parse_extension(s, TLSEXT_IDX_psk,
SSL_EXT_TLS1_3_SERVER_HELLO,
- extensions, NULL, 0l)) {
+ extensions, NULL, 0)) {
/* SSLfatal() already called */
goto err;
}
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index b8e094b..7b18115 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1739,21 +1739,31 @@
*
* @s: client SSL handle.
* @version: The proposed version from the server's HELLO.
- * @checkdgrd: Whether to check the downgrade sentinels in the server_random
+ * @extensions: The extensions received
*
* Returns 1 on success or 0 on error.
*/
-int ssl_choose_client_version(SSL *s, int version, int checkdgrd)
+int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
{
const version_info *vent;
const version_info *table;
int highver = 0;
+ int origv;
- /* TODO(TLS1.3): Remove this before release */
- if (version == TLS1_3_VERSION_DRAFT)
- version = TLS1_3_VERSION;
+ origv = s->version;
+ s->version = version;
- if (s->hello_retry_request && version != TLS1_3_VERSION) {
+ /* This will overwrite s->version if the extension is present */
+ if (!tls_parse_extension(s, TLSEXT_IDX_supported_versions,
+ SSL_EXT_TLS1_2_SERVER_HELLO
+ | SSL_EXT_TLS1_3_SERVER_HELLO, extensions,
+ NULL, 0)) {
+ s->version = origv;
+ return 0;
+ }
+
+ if (s->hello_retry_request && s->version != TLS1_3_VERSION) {
+ s->version = origv;
SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,
SSL_R_WRONG_SSL_VERSION);
return 0;
@@ -1761,7 +1771,8 @@
switch (s->method->version) {
default:
- if (version != s->version) {
+ if (s->version != s->method->version) {
+ s->version = origv;
SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
SSL_F_SSL_CHOOSE_CLIENT_VERSION,
SSL_R_WRONG_SSL_VERSION);
@@ -1790,13 +1801,14 @@
if (vent->cmeth == NULL)
continue;
- if (highver != 0 && version != vent->version)
+ if (highver != 0 && s->version != vent->version)
continue;
method = vent->cmeth();
err = ssl_method_error(s, method);
if (err != 0) {
- if (version == vent->version) {
+ if (s->version == vent->version) {
+ s->version = origv;
SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
SSL_F_SSL_CHOOSE_CLIENT_VERSION, err);
return 0;
@@ -1807,43 +1819,43 @@
if (highver == 0)
highver = vent->version;
- if (version != vent->version)
+ if (s->version != vent->version)
continue;
#ifndef OPENSSL_NO_TLS13DOWNGRADE
/* Check for downgrades */
- if (checkdgrd) {
- if (version == TLS1_2_VERSION && highver > version) {
- if (memcmp(tls12downgrade,
- s->s3->server_random + SSL3_RANDOM_SIZE
- - sizeof(tls12downgrade),
- sizeof(tls12downgrade)) == 0) {
- SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
- SSL_F_SSL_CHOOSE_CLIENT_VERSION,
- SSL_R_INAPPROPRIATE_FALLBACK);
- return 0;
- }
- } else if (!SSL_IS_DTLS(s)
- && version < TLS1_2_VERSION
- && highver > version) {
- if (memcmp(tls11downgrade,
- s->s3->server_random + SSL3_RANDOM_SIZE
- - sizeof(tls11downgrade),
- sizeof(tls11downgrade)) == 0) {
- SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
- SSL_F_SSL_CHOOSE_CLIENT_VERSION,
- SSL_R_INAPPROPRIATE_FALLBACK);
- return 0;
- }
+ if (s->version == TLS1_2_VERSION && highver > s->version) {
+ if (memcmp(tls12downgrade,
+ s->s3->server_random + SSL3_RANDOM_SIZE
+ - sizeof(tls12downgrade),
+ sizeof(tls12downgrade)) == 0) {
+ s->version = origv;
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+ SSL_F_SSL_CHOOSE_CLIENT_VERSION,
+ SSL_R_INAPPROPRIATE_FALLBACK);
+ return 0;
+ }
+ } else if (!SSL_IS_DTLS(s)
+ && s->version < TLS1_2_VERSION
+ && highver > s->version) {
+ if (memcmp(tls11downgrade,
+ s->s3->server_random + SSL3_RANDOM_SIZE
+ - sizeof(tls11downgrade),
+ sizeof(tls11downgrade)) == 0) {
+ s->version = origv;
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+ SSL_F_SSL_CHOOSE_CLIENT_VERSION,
+ SSL_R_INAPPROPRIATE_FALLBACK);
+ return 0;
}
}
#endif
s->method = method;
- s->version = version;
return 1;
}
+ s->version = origv;
SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,
SSL_R_UNSUPPORTED_PROTOCOL);
return 0;
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 888c0b5..ad5b028 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -161,6 +161,8 @@
EXT_RETURN_NOT_SENT
} EXT_RETURN;
+__owur int tls_validate_all_contexts(SSL *s, unsigned int thisctx,
+ RAW_EXTENSION *exts);
__owur int extension_is_relevant(SSL *s, unsigned int extctx,
unsigned int thisctx);
__owur int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
@@ -271,6 +273,9 @@
X509 *x, size_t chainidx);
EXT_RETURN tls_construct_stoc_ems(SSL *s, WPACKET *pkt, unsigned int context,
X509 *x, size_t chainidx);
+EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt,
+ unsigned int context, X509 *x,
+ size_t chainidx);
EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt,
unsigned int context, X509 *x,
size_t chainidx);
@@ -388,6 +393,8 @@
size_t chainidx);
int tls_parse_stoc_ems(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
size_t chainidx);
+int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context,
+ X509 *x, size_t chainidx);
int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
size_t chainidx);
int tls_parse_stoc_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index f95c19b..e91bba4 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -2193,8 +2193,7 @@
size_t sl, len;
int version;
- /* TODO(TLS1.3): Remove the DRAFT conditional before release */
- version = SSL_IS_TLS13(s) ? TLS1_3_VERSION_DRAFT : s->version;
+ version = SSL_IS_TLS13(s) ? TLS1_2_VERSION : s->version;
if (!WPACKET_put_bytes_u16(pkt, version)
/*
* Random stuff. Filling of the server_random takes place in
@@ -2234,21 +2233,23 @@
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;
#else
- if (s->s3->tmp.new_compression == NULL)
+ if (SSL_IS_TLS13(s) || s->s3->tmp.new_compression == NULL)
compm = 0;
else
compm = s->s3->tmp.new_compression->id;
#endif
- if ((!SSL_IS_TLS13(s)
- && !WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl))
+ if (!WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl)
|| !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len)
- || (!SSL_IS_TLS13(s)
- && !WPACKET_put_bytes_u8(pkt, compm))
+ || !WPACKET_put_bytes_u8(pkt, compm)
|| !tls_construct_extensions(s, pkt,
SSL_IS_TLS13(s)
? SSL_EXT_TLS1_3_SERVER_HELLO
