[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Mon Oct 16 14:58:15 UTC 2017
The branch master has been updated
via beb30941d6b2d663144d74dc3846d5d49c127454 (commit)
via 61278ff3f952570a3ca06d02b07502069cd78f55 (commit)
via a2b97bdf3dbbd255ee24aa8a74cf88d4f7034898 (commit)
from aeb3e4abb6abab64342df44b2a7dc407f6508a11 (commit)
- Log -----------------------------------------------------------------
commit beb30941d6b2d663144d74dc3846d5d49c127454
Author: Matt Caswell <matt at openssl.org>
Date: Mon Oct 16 11:19:03 2017 +0100
Tweak the comment regarding record version check with respect to TLSv1.3
Reviewed-by: Tim Hudson <tjh at openssl.org>
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4527)
commit 61278ff3f952570a3ca06d02b07502069cd78f55
Author: Matt Caswell <matt at openssl.org>
Date: Fri Oct 13 14:36:32 2017 +0100
Sanity check the HRR version field
The previous commit removed version negotiation on an HRR. However we should
still sanity check the contents of the version field.
Reviewed-by: Tim Hudson <tjh at openssl.org>
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4527)
commit a2b97bdf3dbbd255ee24aa8a74cf88d4f7034898
Author: Matt Caswell <matt at openssl.org>
Date: Fri Oct 13 11:41:50 2017 +0100
Don't do version neg on an HRR
Previously if a client received an HRR then we would do version negotiation
immediately - because we know we are going to get TLSv1.3. However this
causes a problem when we emit the 2nd ClientHello because we start changing
a whole load of stuff to ommit things that aren't relevant for < TLSv1.3.
The spec requires that the 2nd ClientHello is the same except for changes
required from the HRR. Therefore the simplest thing to do is to defer the
version negotiation until we receive the ServerHello.
Fixes #4292
Reviewed-by: Tim Hudson <tjh at openssl.org>
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4527)
-----------------------------------------------------------------------
Summary of changes:
ssl/record/ssl3_record.c | 8 +++++++-
ssl/statem/extensions.c | 16 +++++++++++++---
ssl/statem/statem_clnt.c | 23 +++++++++++------------
test/recipes/70-test_tls13kexmodes.t | 8 ++++++++
test/recipes/70-test_tls13messages.t | 8 ++++++++
5 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index ee02622..518e7a8 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -271,8 +271,14 @@ int ssl3_get_record(SSL *s)
thisrr->type = type;
thisrr->rec_version = version;
- /* Lets check version. In TLSv1.3 we ignore this field */
+ /*
+ * Lets check version. In TLSv1.3 we ignore this field. For the
+ * ServerHello after an HRR we haven't actually selected TLSv1.3
+ * yet, but we still treat it as TLSv1.3, so we must check for
+ * that explicitly
+ */
if (!s->first_packet && !SSL_IS_TLS13(s)
+ && !s->hello_retry_request
&& version != (unsigned int)s->version) {
SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_WRONG_VERSION_NUMBER);
if ((s->version & 0xFF00) == (version & 0xFF00)
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index b5091ac..e16b75f 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -408,13 +408,23 @@ static int verify_extension(SSL *s, unsigned int context, unsigned int type,
*/
int extension_is_relevant(SSL *s, unsigned int extctx, unsigned int thisctx)
{
+ int is_tls13;
+
+ /*
+ * For HRR we haven't selected the version yet but we know it will be
+ * TLSv1.3
+ */
+ if ((thisctx & SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) != 0)
+ is_tls13 = 1;
+ else
+ is_tls13 = SSL_IS_TLS13(s);
+
if ((SSL_IS_DTLS(s)
&& (extctx & SSL_EXT_TLS_IMPLEMENTATION_ONLY) != 0)
|| (s->version == SSL3_VERSION
&& (extctx & SSL_EXT_SSL3_ALLOWED) == 0)
- || (SSL_IS_TLS13(s)
- && (extctx & SSL_EXT_TLS1_2_AND_BELOW_ONLY) != 0)
- || (!SSL_IS_TLS13(s) && (extctx & SSL_EXT_TLS1_3_ONLY) != 0)
+ || (is_tls13 && (extctx & SSL_EXT_TLS1_2_AND_BELOW_ONLY) != 0)
+ || (!is_tls13 && (extctx & SSL_EXT_TLS1_3_ONLY) != 0)
|| (s->hit && (extctx & SSL_EXT_IGNORE_ON_RESUMPTION) != 0))
return 0;
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 88c0889..338325f 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -391,10 +391,6 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)
/* We only hit this in the case of HelloRetryRequest */
return WRITE_TRAN_FINISHED;
- case TLS_ST_CR_HELLO_RETRY_REQUEST:
- st->hand_state = TLS_ST_CW_CLNT_HELLO;
- return WRITE_TRAN_CONTINUE;
-
case TLS_ST_CR_FINISHED:
if (s->early_data_state == SSL_EARLY_DATA_WRITE_RETRY
|| s->early_data_state == SSL_EARLY_DATA_FINISHED_WRITING)
@@ -500,6 +496,10 @@ WRITE_TRAN ossl_statem_client_write_transition(SSL *s)
*/
return WRITE_TRAN_FINISHED;
+ case TLS_ST_CR_HELLO_RETRY_REQUEST:
+ st->hand_state = TLS_ST_CW_CLNT_HELLO;
+ return WRITE_TRAN_CONTINUE;
+
case TLS_ST_EARLY_DATA:
return WRITE_TRAN_FINISHED;
@@ -1558,7 +1558,6 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
{
unsigned int sversion;
- int errorcode;
const unsigned char *cipherchars;
RAW_EXTENSION *extensions = NULL;
int al;
@@ -1570,6 +1569,13 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
goto f_err;
}
+ /* TODO(TLS1.3): Remove the TLS1_3_VERSION_DRAFT clause before release */
+ if (sversion != TLS1_3_VERSION && sversion != TLS1_3_VERSION_DRAFT) {
+ SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, SSL_R_WRONG_SSL_VERSION);
+ al = SSL_AD_PROTOCOL_VERSION;
+ goto f_err;
+ }
+
s->hello_retry_request = 1;
/*
@@ -1579,13 +1585,6 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
EVP_CIPHER_CTX_free(s->enc_write_ctx);
s->enc_write_ctx = NULL;
- /* This will fail if it doesn't choose TLSv1.3+ */
- errorcode = ssl_choose_client_version(s, sversion, 0, &al);
- if (errorcode != 0) {
- SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, errorcode);
- goto f_err;
- }
-
if (!PACKET_get_bytes(pkt, &cipherchars, TLS_CIPHER_LEN)) {
SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, SSL_R_LENGTH_MISMATCH);
al = SSL_AD_DECODE_ERROR;
diff --git a/test/recipes/70-test_tls13kexmodes.t b/test/recipes/70-test_tls13kexmodes.t
index fe7415a..3f3cfaf 100644
--- a/test/recipes/70-test_tls13kexmodes.t
+++ b/test/recipes/70-test_tls13kexmodes.t
@@ -99,12 +99,20 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
checkhandshake::STATUS_REQUEST_CLI_EXTENSION],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SUPPORTED_GROUPS,
checkhandshake::DEFAULT_EXTENSIONS],
+ [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_EC_POINT_FORMATS,
+ checkhandshake::DEFAULT_EXTENSIONS],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SIG_ALGS,
checkhandshake::DEFAULT_EXTENSIONS],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_ALPN,
checkhandshake::ALPN_CLI_EXTENSION],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SCT,
checkhandshake::SCT_CLI_EXTENSION],
+ [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_ENCRYPT_THEN_MAC,
+ checkhandshake::DEFAULT_EXTENSIONS],
+ [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_EXTENDED_MASTER_SECRET,
+ checkhandshake::DEFAULT_EXTENSIONS],
+ [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SESSION_TICKET,
+ checkhandshake::DEFAULT_EXTENSIONS],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_KEY_SHARE,
checkhandshake::DEFAULT_EXTENSIONS],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t
index 24ffb80..239eabf 100644
--- a/test/recipes/70-test_tls13messages.t
+++ b/test/recipes/70-test_tls13messages.t
@@ -99,12 +99,20 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
checkhandshake::STATUS_REQUEST_CLI_EXTENSION],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SUPPORTED_GROUPS,
checkhandshake::DEFAULT_EXTENSIONS],
+ [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_EC_POINT_FORMATS,
+ checkhandshake::DEFAULT_EXTENSIONS],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SIG_ALGS,
checkhandshake::DEFAULT_EXTENSIONS],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_ALPN,
checkhandshake::ALPN_CLI_EXTENSION],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SCT,
checkhandshake::SCT_CLI_EXTENSION],
+ [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_ENCRYPT_THEN_MAC,
+ checkhandshake::DEFAULT_EXTENSIONS],
+ [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_EXTENDED_MASTER_SECRET,
+ checkhandshake::DEFAULT_EXTENSIONS],
+ [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SESSION_TICKET,
+ checkhandshake::DEFAULT_EXTENSIONS],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_KEY_SHARE,
checkhandshake::DEFAULT_EXTENSIONS],
[TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
More information about the openssl-commits
mailing list