Add server side support for creating the Hello Retry Request message
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2341)
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index a6d3412..705252c 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -329,6 +329,8 @@
{ERR_FUNC(SSL_F_TLS_CONSTRUCT_FINISHED), "tls_construct_finished"},
{ERR_FUNC(SSL_F_TLS_CONSTRUCT_HELLO_REQUEST),
"tls_construct_hello_request"},
+ {ERR_FUNC(SSL_F_TLS_CONSTRUCT_HELLO_RETRY_REQUEST),
+ "tls_construct_hello_retry_request"},
{ERR_FUNC(SSL_F_TLS_CONSTRUCT_NEW_SESSION_TICKET),
"tls_construct_new_session_ticket"},
{ERR_FUNC(SSL_F_TLS_CONSTRUCT_NEXT_PROTO), "tls_construct_next_proto"},
@@ -603,6 +605,7 @@
{ERR_REASON(SSL_R_NO_RENEGOTIATION), "no renegotiation"},
{ERR_REASON(SSL_R_NO_REQUIRED_DIGEST), "no required digest"},
{ERR_REASON(SSL_R_NO_SHARED_CIPHER), "no shared cipher"},
+ {ERR_REASON(SSL_R_NO_SHARED_GROUPS), "no shared groups"},
{ERR_REASON(SSL_R_NO_SHARED_SIGNATURE_ALGORITHMS),
"no shared signature algorithms"},
{ERR_REASON(SSL_R_NO_SRTP_PROFILES), "no srtp profiles"},
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 2a23007..099f8cc 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1005,6 +1005,9 @@
unsigned char cert_verify_hash[EVP_MAX_MD_SIZE];
size_t cert_verify_hash_len;
+ /* Flag to indicate whether we should send a HelloRetryRequest or not */
+ int hello_retry_request;
+
/*
* the session_id_context is used to ensure sessions are only reused in
* the appropriate context
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 4137f03..fd050f0 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -979,12 +979,18 @@
&& (!s->hit
|| (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0)) {
/* No suitable share */
- /* TODO(TLS1.3): Send a HelloRetryRequest */
+ if (s->server && s->hello_retry_request == 0 && sent) {
+ s->hello_retry_request = 1;
+ return 1;
+ }
+
+ /* Nothing left we can do - just fail */
*al = SSL_AD_HANDSHAKE_FAILURE;
SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE);
return 0;
}
+ s->hello_retry_request = 0;
/*
* For a client side resumption with no key_share we need to generate
* the handshake secret (otherwise this is done during key_share
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index efae916..0038745 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -164,7 +164,7 @@
}
/* Copy curve ID if supported */
for (i = 0; i < num_curves; i++, pcurvestmp += 2) {
- if (tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) {
+ if (tls_curve_allowed(s, pcurvestmp, SSL_SECOP_CURVE_SUPPORTED)) {
if (!WPACKET_put_bytes_u8(pkt, pcurvestmp[0])
|| !WPACKET_put_bytes_u8(pkt, pcurvestmp[1])) {
SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index c4d20e5..f7b7990 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -116,6 +116,8 @@
return 0;
}
+ OPENSSL_free(s->session->ext.hostname);
+ s->session->ext.hostname = NULL;
if (!PACKET_strndup(&hostname, &s->session->ext.hostname)) {
*al = TLS1_AD_INTERNAL_ERROR;
return 0;
@@ -361,6 +363,9 @@
}
} while (PACKET_remaining(&protocol_list) != 0);
+ OPENSSL_free(s->s3->alpn_proposed);
+ s->s3->alpn_proposed = NULL;
+ s->s3->alpn_proposed_len = 0;
if (!PACKET_memdup(&save_protocol_list,
&s->s3->alpn_proposed, &s->s3->alpn_proposed_len)) {
*al = TLS1_AD_INTERNAL_ERROR;
@@ -659,6 +664,9 @@
return 0;
}
+ OPENSSL_free(s->session->ext.supportedgroups);
+ s->session->ext.supportedgroups = NULL;
+ s->session->ext.supportedgroups_len = 0;
if (!PACKET_memdup(&supported_groups_list,
&s->session->ext.supportedgroups,
&s->session->ext.supportedgroups_len)) {
@@ -1024,7 +1032,65 @@
EVP_PKEY *ckey = s->s3->peer_tmp, *skey = NULL;
if (ckey == NULL) {
- /* No key_share received from client; must be resuming. */
+ /* No key_share received from client */
+ if (s->hello_retry_request) {
+ const unsigned char *pcurves, *pcurvestmp, *clntcurves;
+ size_t num_curves, clnt_num_curves, i;
+
+ /* Get the clients list of supported groups. */
+ if (!tls1_get_curvelist(s, 1, &clntcurves, &clnt_num_curves)) {
+ *al = SSL_AD_INTERNAL_ERROR;
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
+ ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+
+ /* Get our list of available groups */
+ if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
+ ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+
+ if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share)
+ || !WPACKET_start_sub_packet_u16(pkt)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
+ ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+
+ /* Find first group we allow that is also in client's list */
+ for (i = 0, pcurvestmp = pcurves; i < num_curves;
+ i++, pcurvestmp += 2) {
+ unsigned int group_id = pcurvestmp[0] << 8 | pcurvestmp[1];
+
+ if (check_in_list(s, group_id, clntcurves, clnt_num_curves,
+ 1)) {
+ if (!WPACKET_put_bytes_u16(pkt, group_id)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
+ ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+ break;
+ }
+ }
+ if (i == num_curves) {
+ /* No common groups */
+ *al = SSL_AD_HANDSHAKE_FAILURE;
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
+ SSL_R_NO_SHARED_GROUPS);
+ return 0;
+ }
+ if (!WPACKET_close(pkt)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
+ ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+
+ return 1;
+ }
+
+ /* Must be resuming. */
if (!s->hit || !tls13_generate_handshake_secret(s, NULL, 0)) {
*al = SSL_AD_INTERNAL_ERROR;
SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE, ERR_R_INTERNAL_ERROR);
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 8e7245b..13174ab 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1434,21 +1434,22 @@
switch (server_version) {
default:
+ if (!SSL_IS_TLS13(s)) {
+ if (version_cmp(s, client_version, s->version) < 0)
+ return SSL_R_WRONG_SSL_VERSION;
+ /*
+ * If this SSL handle is not from a version flexible method we don't
+ * (and never did) check min/max FIPS or Suite B constraints. Hope
+ * that's OK. It is up to the caller to not choose fixed protocol
+ * versions they don't want. If not, then easy to fix, just return
+ * ssl_method_error(s, s->method)
+ */
+ return 0;
+ }
/*
- * TODO(TLS1.3): This check will fail if someone attempts to do
- * renegotiation in TLS1.3 at the moment. We need to ensure we disable
- * renegotiation for TLS1.3
+ * Fall through if we are TLSv1.3 already (this means we must be after
+ * a HelloRetryRequest
*/
- if (version_cmp(s, client_version, s->version) < 0)
- return SSL_R_WRONG_SSL_VERSION;
- /*
- * If this SSL handle is not from a version flexible method we don't
- * (and never did) check min/max FIPS or Suite B constraints. Hope
- * that's OK. It is up to the caller to not choose fixed protocol
- * versions they don't want. If not, then easy to fix, just return
- * ssl_method_error(s, s->method)
- */
- return 0;
case TLS_ANY_VERSION:
table = tls_version_table;
break;
@@ -1503,6 +1504,15 @@
}
if (best_vers > 0) {
+ if (SSL_IS_TLS13(s)) {
+ /*
+ * We get here if this is after a HelloRetryRequest. In this
+ * case we just check that we still negotiated TLSv1.3
+ */
+ if (best_vers != TLS1_3_VERSION)
+ return SSL_R_UNSUPPORTED_PROTOCOL;
+ return 0;
+ }
s->version = best_vers;
s->method = best_method;
return 0;
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index de0fcc0..8aba669 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -62,6 +62,7 @@
#include <openssl/md5.h>
static int tls_construct_encrypted_extensions(SSL *s, WPACKET *pkt);
+static int tls_construct_hello_retry_request(SSL *s, WPACKET *pkt);
static STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
PACKET *cipher_suites,
STACK_OF(SSL_CIPHER)
@@ -82,11 +83,6 @@
OSSL_STATEM *st = &s->statem;
/*
- * TODO(TLS1.3): This is still based on the TLSv1.2 state machine. Over time
- * we will update this to look more like real TLSv1.3
- */
-
- /*
* Note: There is no case for TLS_ST_BEFORE because at that stage we have
* not negotiated TLSv1.3 yet, so that case is handled by
* ossl_statem_server_read_transition()
@@ -95,6 +91,13 @@
default:
break;
+ case TLS_ST_SW_HELLO_RETRY_REQUEST:
+ if (mt == SSL3_MT_CLIENT_HELLO) {
+ st->hand_state = TLS_ST_SR_CLNT_HELLO;
+ return 1;
+ }
+ break;
+
case TLS_ST_SW_FINISHED:
if (s->s3->tmp.cert_request) {
if (mt == SSL3_MT_CERTIFICATE) {
@@ -406,9 +409,15 @@
return WRITE_TRAN_ERROR;
case TLS_ST_SR_CLNT_HELLO:
- st->hand_state = TLS_ST_SW_SRVR_HELLO;
+ if (s->hello_retry_request)
+ st->hand_state = TLS_ST_SW_HELLO_RETRY_REQUEST;
+ else
+ st->hand_state = TLS_ST_SW_SRVR_HELLO;
return WRITE_TRAN_CONTINUE;
+ case TLS_ST_SW_HELLO_RETRY_REQUEST:
+ return WRITE_TRAN_FINISHED;
+
case TLS_ST_SW_SRVR_HELLO:
st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
return WRITE_TRAN_CONTINUE;
@@ -693,6 +702,11 @@
/* No post work to be done */
break;
+ case TLS_ST_SW_HELLO_RETRY_REQUEST:
+ if (statem_flush(s) != 1)
+ return WORK_MORE_A;
+ break;
+
case TLS_ST_SW_HELLO_REQ:
if (statem_flush(s) != 1)
return WORK_MORE_A;
@@ -904,6 +918,11 @@
*confunc = tls_construct_encrypted_extensions;
*mt = SSL3_MT_ENCRYPTED_EXTENSIONS;
break;
+
+ case TLS_ST_SW_HELLO_RETRY_REQUEST:
+ *confunc = tls_construct_hello_retry_request;
+ *mt = SSL3_MT_HELLO_RETRY_REQUEST;
+ break;
}
return 1;
@@ -1200,6 +1219,12 @@
if (clienthello.isv2) {
unsigned int mt;
+ if (!SSL_IS_FIRST_HANDSHAKE(s) || s->hello_retry_request) {
+ al = SSL_AD_HANDSHAKE_FAILURE;
+ SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_UNEXPECTED_MESSAGE);
+ goto f_err;
+ }
+
/*-
* An SSLv3/TLSv1 backwards-compatible CLIENT-HELLO in an SSLv2
* header is sent directly on the wire, not wrapped as a TLS
@@ -1402,7 +1427,7 @@
if (protverr) {
SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, protverr);
- if ((!s->enc_write_ctx && !s->write_hash)) {
+ if (SSL_IS_FIRST_HANDSHAKE(s)) {
/* like ssl3_get_record, send alert using remote version number */
s->version = s->client_version = clienthello.legacy_version;
}
@@ -3502,6 +3527,10 @@
return NULL;
}
+ OPENSSL_free(s->s3->tmp.ciphers_raw);
+ s->s3->tmp.ciphers_raw = NULL;
+ s->s3->tmp.ciphers_rawlen = 0;
+
if (sslv2format) {
size_t numciphers = PACKET_remaining(cipher_suites) / n;
PACKET sslv2ciphers = *cipher_suites;
@@ -3607,3 +3636,28 @@
sk_SSL_CIPHER_free(sk);
return NULL;
}
+
+static int tls_construct_hello_retry_request(SSL *s, WPACKET *pkt)
+{
+ int al;
+
+ /*
+ * TODO(TLS1.3): Remove the DRAFT version before release
+ * (should be s->version)
+ */
+ if (!WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT)
+ || !tls_construct_extensions(s, pkt, EXT_TLS1_3_HELLO_RETRY_REQUEST,
+ NULL, 0, &al)) {
+ ssl3_send_alert(s, SSL3_AL_FATAL, al);
+ SSLerr(SSL_F_TLS_CONSTRUCT_HELLO_RETRY_REQUEST, ERR_R_INTERNAL_ERROR);
+ ssl3_send_alert(s, SSL3_AL_FATAL, al);
+ return 0;
+ }
+
+ /* Ditch the session. We'll create a new one next time around */
+ SSL_SESSION_free(s->session);
+ s->session = NULL;
+ s->hit = 0;
+
+ return 1;
+}
