[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Wed Mar 14 10:00:03 UTC 2018
The branch master has been updated
via 5b68d1792021463b7cd5d76c82b251d61a56d869 (commit)
via 27e462f1b0c8d6295c745611e36beb5774de6688 (commit)
via 3295d2423889496e0933b3f9af6dc692c9f9a8f2 (commit)
from 95ea8da1768bf457b021f07cde9a6330827dc8a1 (commit)
- Log -----------------------------------------------------------------
commit 5b68d1792021463b7cd5d76c82b251d61a56d869
Author: Matt Caswell <matt at openssl.org>
Date: Tue Mar 13 13:13:33 2018 +0000
Update version numbers for TLSv1.3 draft-26
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5604)
commit 27e462f1b0c8d6295c745611e36beb5774de6688
Author: Matt Caswell <matt at openssl.org>
Date: Tue Mar 13 10:36:03 2018 +0000
Only allow supported_versions in a TLSv1.3 ServerHello
As per the latest text in TLSv1.3 draft-26
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5604)
commit 3295d2423889496e0933b3f9af6dc692c9f9a8f2
Author: Matt Caswell <matt at openssl.org>
Date: Mon Feb 26 12:26:14 2018 +0000
Use the TLSv1.3 record header as AAD
As of TLSv1.3 draft-25 the record header data must be used as AAD
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5604)
-----------------------------------------------------------------------
Summary of changes:
engines/e_ossltest.c | 2 +-
include/openssl/tls1.h | 6 +++---
ssl/record/rec_layer_s3.c | 4 +++-
ssl/record/record_locl.h | 1 +
ssl/record/ssl3_record.c | 24 +++++++++++++++---------
ssl/record/ssl3_record_tls13.c | 31 ++++++++++++++++++++++++++++---
ssl/statem/extensions.c | 5 ++---
ssl/statem/extensions_clnt.c | 24 ++++++++++++------------
ssl/statem/extensions_srvr.c | 8 ++++++--
test/recipes/70-test_sslrecords.t | 6 +++---
test/recordlentest.c | 6 +-----
test/tls13encryptiontest.c | 24 +++++++++++++++---------
util/perl/TLSProxy/Record.pm | 2 +-
13 files changed, 91 insertions(+), 52 deletions(-)
diff --git a/engines/e_ossltest.c b/engines/e_ossltest.c
index 2bda610..6437624 100644
--- a/engines/e_ossltest.c
+++ b/engines/e_ossltest.c
@@ -637,7 +637,7 @@ int ossltest_aes128_gcm_cipher(EVP_CIPHER_CTX *ctx, unsigned char *out,
EVP_CIPHER_meth_get_do_cipher(EVP_aes_128_gcm())(ctx, out, in, inl);
/* Throw it all away and just use the plaintext as the output */
- if (tmpbuf != NULL)
+ if (tmpbuf != NULL && out != NULL)
memcpy(out, tmpbuf, inl);
OPENSSL_free(tmpbuf);
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index f167856..0d5b9f8 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -30,9 +30,9 @@ extern "C" {
# define TLS1_3_VERSION 0x0304
# define TLS_MAX_VERSION TLS1_3_VERSION
-/* TODO(TLS1.3) REMOVE ME: Version indicator for draft -23 */
-# define TLS1_3_VERSION_DRAFT 0x7f17
-# define TLS1_3_VERSION_DRAFT_TXT "TLS 1.3 (draft 23)"
+/* TODO(TLS1.3) REMOVE ME: Version indicator for draft -26 */
+# define TLS1_3_VERSION_DRAFT 0x7f1a
+# define TLS1_3_VERSION_DRAFT_TXT "TLS 1.3 (draft 26)"
/* Special value for method supporting multiple versions */
# define TLS_ANY_VERSION 0x10000
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 0953d2b..61010f4 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -825,7 +825,6 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
thispkt = &pkt[j];
thiswr = &wr[j];
- SSL3_RECORD_set_type(thiswr, type);
/*
* In TLSv1.3, once encrypting, we always use application data for the
* record type
@@ -834,6 +833,8 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
rectype = SSL3_RT_APPLICATION_DATA;
else
rectype = type;
+ SSL3_RECORD_set_type(thiswr, rectype);
+
/*
* Some servers hang if initial client hello is larger than 256 bytes
* and record version number > TLS 1.0
@@ -843,6 +844,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
&& TLS1_get_version(s) > TLS1_VERSION
&& s->hello_retry_request == SSL_HRR_NONE)
version = TLS1_VERSION;
+ SSL3_RECORD_set_rec_version(thiswr, version);
maxcomplen = pipelens[j];
if (s->compress != NULL)
diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
index c20f5fe..1782a4f 100644
--- a/ssl/record/record_locl.h
+++ b/ssl/record/record_locl.h
@@ -80,6 +80,7 @@ int ssl3_release_write_buffer(SSL *s);
#define SSL3_RECORD_get_type(r) ((r)->type)
#define SSL3_RECORD_set_type(r, t) ((r)->type = (t))
+#define SSL3_RECORD_set_rec_version(r, v) ((r)->rec_version = (v))
#define SSL3_RECORD_get_length(r) ((r)->length)
#define SSL3_RECORD_set_length(r, l) ((r)->length = (l))
#define SSL3_RECORD_add_length(r, l) ((r)->length += (l))
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index fda918a..5bfbaf9 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -270,7 +270,8 @@ int ssl3_get_record(SSL *s)
thisrr->rec_version = version;
/*
- * Lets check version. In TLSv1.3 we ignore this field. For the
+ * Lets check version. In TLSv1.3 we only check this field
+ * when encryption is occurring (see later check). 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
@@ -333,14 +334,19 @@ int ssl3_get_record(SSL *s)
}
}
- if (SSL_IS_TLS13(s)
- && s->enc_read_ctx != NULL
- && thisrr->type != SSL3_RT_APPLICATION_DATA
- && (thisrr->type != SSL3_RT_CHANGE_CIPHER_SPEC
- || !SSL_IS_FIRST_HANDSHAKE(s))) {
- SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE,
- SSL_F_SSL3_GET_RECORD, SSL_R_BAD_RECORD_TYPE);
- return -1;
+ if (SSL_IS_TLS13(s) && s->enc_read_ctx != NULL) {
+ if (thisrr->type != SSL3_RT_APPLICATION_DATA
+ && (thisrr->type != SSL3_RT_CHANGE_CIPHER_SPEC
+ || !SSL_IS_FIRST_HANDSHAKE(s))) {
+ SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE,
+ SSL_F_SSL3_GET_RECORD, SSL_R_BAD_RECORD_TYPE);
+ return -1;
+ }
+ if (thisrr->rec_version != TLS1_2_VERSION) {
+ SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_SSL3_GET_RECORD,
+ SSL_R_WRONG_VERSION_NUMBER);
+ return -1;
+ }
}
if (thisrr->length >
diff --git a/ssl/record/ssl3_record_tls13.c b/ssl/record/ssl3_record_tls13.c
index f1e1667..21073b6 100644
--- a/ssl/record/ssl3_record_tls13.c
+++ b/ssl/record/ssl3_record_tls13.c
@@ -25,13 +25,14 @@
int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
{
EVP_CIPHER_CTX *ctx;
- unsigned char iv[EVP_MAX_IV_LENGTH];
- size_t ivlen, taglen, offset, loop;
+ unsigned char iv[EVP_MAX_IV_LENGTH], recheader[SSL3_RT_HEADER_LENGTH];
+ size_t ivlen, taglen, offset, loop, hdrlen;
unsigned char *staticiv;
unsigned char *seq;
int lenu, lenf;
SSL3_RECORD *rec = &recs[0];
uint32_t alg_enc;
+ WPACKET wpkt;
if (n_recs != 1) {
/* Should not happen */
@@ -143,7 +144,31 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
if (EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv, sending) <= 0
|| (!sending && EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG,
taglen,
- rec->data + rec->length) <= 0)
+ rec->data + rec->length) <= 0)) {
+ return -1;
+ }
+
+ /* Set up the AAD */
+ if (!WPACKET_init_static_len(&wpkt, recheader, sizeof(recheader), 0)
+ || !WPACKET_put_bytes_u8(&wpkt, rec->type)
+ || !WPACKET_put_bytes_u16(&wpkt, rec->rec_version)
+ || !WPACKET_put_bytes_u16(&wpkt, rec->length + taglen)
+ || !WPACKET_get_total_written(&wpkt, &hdrlen)
+ || hdrlen != SSL3_RT_HEADER_LENGTH
+ || !WPACKET_finish(&wpkt)) {
+ WPACKET_cleanup(&wpkt);
+ return -1;
+ }
+
+ /*
+ * For CCM we must explicitly set the total plaintext length before we add
+ * any AAD.
+ */
+ if (((alg_enc & SSL_AESCCM) != 0
+ && EVP_CipherUpdate(ctx, NULL, &lenu, NULL,
+ (unsigned int)rec->length) <= 0)
+ || EVP_CipherUpdate(ctx, NULL, &lenu, recheader,
+ sizeof(recheader)) <= 0
|| EVP_CipherUpdate(ctx, rec->data, &lenu, rec->input,
(unsigned int)rec->length) <= 0
|| EVP_CipherFinal_ex(ctx, rec->data + lenu, &lenf) <= 0
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 0641a25..3dc4e8e 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -307,9 +307,8 @@ static const EXTENSION_DEFINITION ext_defs[] = {
},
{
TLSEXT_TYPE_supported_versions,
- SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO
- | SSL_EXT_TLS1_3_SERVER_HELLO | SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST
- | SSL_EXT_TLS_IMPLEMENTATION_ONLY,
+ SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_SERVER_HELLO
+ | SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST | SSL_EXT_TLS_IMPLEMENTATION_ONLY,
NULL,
/* Processed inline as part of version selection */
NULL, tls_parse_stoc_supported_versions,
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 9bf2d1c..bd025d7 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -1780,20 +1780,20 @@ int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context,
if (version == TLS1_3_VERSION_DRAFT)
version = TLS1_3_VERSION;
+ /*
+ * The only protocol version we support which is valid in this extension in
+ * a ServerHello is TLSv1.3 therefore we shouldn't be getting anything else.
+ */
+ if (version != TLS1_3_VERSION) {
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+ SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS,
+ SSL_R_BAD_PROTOCOL_VERSION_NUMBER);
+ return 0;
+ }
+
/* We ignore this extension for HRRs except to sanity check it */
- if (context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) {
- /*
- * The only protocol version we support which has an HRR message is
- * TLSv1.3, therefore we shouldn't be getting an HRR for anything else.
- */
- if (version != TLS1_3_VERSION) {
- SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
- SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS,
- SSL_R_BAD_HRR_VERSION);
- return 0;
- }
+ if (context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST)
return 1;
- }
/* We just set it here. We validate it in ssl_choose_client_version */
s->version = version;
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 425cd80..a1f92b0 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -1572,8 +1572,12 @@ 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 (!ossl_assert(SSL_IS_TLS13(s))) {
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+ SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS,
+ ERR_R_INTERNAL_ERROR);
+ return EXT_RETURN_FAIL;
+ }
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)
|| !WPACKET_start_sub_packet_u16(pkt)
diff --git a/test/recipes/70-test_sslrecords.t b/test/recipes/70-test_sslrecords.t
index 10c559e..88cb022 100644
--- a/test/recipes/70-test_sslrecords.t
+++ b/test/recipes/70-test_sslrecords.t
@@ -149,11 +149,11 @@ ok(TLSProxy::Message->fail(), "Changed record version in TLS1.2");
SKIP: {
skip "TLSv1.3 disabled", 6 if disabled("tls1_3");
- #Test 13: Sending a different record version in TLS1.3 should succeed
+ #Test 13: Sending a different record version in TLS1.3 should fail
$proxy->clear();
$proxy->filter(\&change_version);
$proxy->start();
- ok(TLSProxy::Message->success(), "Changed record version in TLS1.3");
+ ok(TLSProxy::Message->fail(), "Changed record version in TLS1.3");
#Test 14: Sending an unrecognised record type in TLS1.3 should fail
$proxy->clear();
@@ -464,7 +464,7 @@ sub change_version
my $proxy = shift;
# We'll change a version after the initial version neg has taken place
- if ($proxy->flight != 2) {
+ if ($proxy->flight != 1) {
return;
}
diff --git a/test/recordlentest.c b/test/recordlentest.c
index d0941ca..824c09f 100644
--- a/test/recordlentest.c
+++ b/test/recordlentest.c
@@ -157,11 +157,7 @@ static int test_record_overflow(int idx)
overf_expected = 0;
}
- if (idx == TEST_ENCRYPTED_OVERFLOW_TLS1_3_OK
- || idx == TEST_ENCRYPTED_OVERFLOW_TLS1_3_NOT_OK)
- recversion = TLS1_VERSION;
- else
- recversion = TLS1_2_VERSION;
+ recversion = TLS1_2_VERSION;
if (!TEST_true(write_record(serverbio, len, SSL3_RT_APPLICATION_DATA,
recversion)))
diff --git a/test/tls13encryptiontest.c b/test/tls13encryptiontest.c
index 6e4b889..4d6595a 100644
--- a/test/tls13encryptiontest.c
+++ b/test/tls13encryptiontest.c
@@ -92,7 +92,7 @@ static RECORD_DATA refdata[] = {
"83dd29f64508b2ec3e635a2134fc0e1a39d3ecb51dcddfcf8382c88ffe2a7378"
"42ad1de7fe505b6c4d1673870f6fc2a0f2f7972acaee368a1599d64ba18798f1"
"0333f9779bd5b05f9b084d03dab2f3d80c2eb74ec70c9866ea31c18b491cd597"
- "aae3e941205fcc38a3a10ce8c0269f02ccc9c51278e25f1a0f0731a9"
+ "aae3e941205fcc38a3a10ce8f2e230d97e3406b77ee53d84d89ca548"
},
"d2dd45f87ad87801a85ac38187f9023b",
"f0a14f808692cef87a3daf70",
@@ -106,7 +106,7 @@ static RECORD_DATA refdata[] = {
},
{
"fa15e92daa21cd05d8f9c3152a61748d9aaf049da559718e583f95aacecad657"
- "b52a6562da09a5819e864d86ac2989360a1eb22795","",""
+ "b52a6562da66864fd14969acc30dc04a78c38283c5","",""
},
"40e1201d75d419627f04c88530a15c9d",
"a0f073f3b35e18f96969696b",
@@ -128,7 +128,7 @@ static RECORD_DATA refdata[] = {
"836905229eac811c4ef8b2faa89867e9ffc586f7f03c216591aa5e620eac3c62"
"dfe60f846036bd7ecc4464b584af184e9644e94ee1d7834dba408a51cbe42480"
"04796ed9c558e0f5f96115a6f6ba487e17d16a2e20a3d3a650a9a070fb53d9da"
- "82864b5621d77650bd0c7947e9889917b53d0515627c72b0ded521","",""
+ "82864b5621d77650bd0c7972f592aa8546de09b8e46921fab4d876","",""
},
"3381f6b3f94500f16226de440193e858",
"4f1d73cc1d465eb30021c41f",
@@ -142,8 +142,8 @@ static RECORD_DATA refdata[] = {
},
{
"e306178ad97f74bb64f35eaf3c39846b83aef8472cbc9046749b81a949dfb12c"
- "fbc65cbabd20ade92c1f944605892ceeb12fdee8a927bce77c83036ac5a794a8"
- "f54a69","",""
+ "fbc65cbabd20ade92c1f944605892ceeb12fde5781d40e2ca080fc921b750b8c"
+ "21bd8d","",""
},
"eb23a804904b80ba4fe8399e09b1ce42",
"efa8c50c06b9c9b8c483e174",
@@ -157,8 +157,8 @@ static RECORD_DATA refdata[] = {
},
{
"467d99a807dbf778e6ffd8be52456c70665f890811ef2f3c495d5bbe983feeda"
- "b0c251dde596bc7e2b135909ec9f9166fb0152e8c16a84e4b1039256467f9538"
- "be4463","",""
+ "b0c251dde596bc7e2b135909ec9f9166fb01526c70c7e42b6df52d63b0000222"
+ "cb2047","",""
},
"3381f6b3f94500f16226de440193e858",
"4f1d73cc1d465eb30021c41f",
@@ -170,7 +170,7 @@ static RECORD_DATA refdata[] = {
"010015","",""
},
{
- "6bdf60847ba6fb650da36e872adc684a4af2e8","",""
+ "6bdf609107610cff95d70387a67b89e2494f0d","",""
},
"eb23a804904b80ba4fe8399e09b1ce42",
"efa8c50c06b9c9b8c483e174",
@@ -182,7 +182,7 @@ static RECORD_DATA refdata[] = {
"010015","",""
},
{
- "621b7cc1962cd8a70109fee68a52efedf87d2e","",""
+ "621b7c60d32528b149b36a78c8891a8d2f65ad","",""
},
"3381f6b3f94500f16226de440193e858",
"4f1d73cc1d465eb30021c41f",
@@ -306,7 +306,13 @@ static int test_tls13_encryption(void)
int ret = 0;
size_t ivlen, ctr;
+ /*
+ * Encrypted TLSv1.3 records always have an outer content type of
+ * application data, and a record version of TLSv1.2.
+ */
rec.data = NULL;
+ rec.type = SSL3_RT_APPLICATION_DATA;
+ rec.rec_version = TLS1_2_VERSION;
ctx = SSL_CTX_new(TLS_method());
if (!TEST_ptr(ctx)) {
diff --git a/util/perl/TLSProxy/Record.pm b/util/perl/TLSProxy/Record.pm
index 773f759..b52e344 100644
--- a/util/perl/TLSProxy/Record.pm
+++ b/util/perl/TLSProxy/Record.pm
@@ -36,7 +36,7 @@ my %record_type = (
use constant {
VERS_TLS_1_4 => 0x0305,
- VERS_TLS_1_3_DRAFT => 0x7f17,
+ VERS_TLS_1_3_DRAFT => 0x7f1a,
VERS_TLS_1_3 => 0x0304,
VERS_TLS_1_2 => 0x0303,
VERS_TLS_1_1 => 0x0302,
More information about the openssl-commits
mailing list