[openssl-commits] [openssl] OpenSSL_1_1_1-stable update
Matt Caswell
matt at openssl.org
Wed Dec 5 11:02:39 UTC 2018
The branch OpenSSL_1_1_1-stable has been updated
via db860ea3dcf56a1993c66da22bd44460d7ac4914 (commit)
via 56e0f123dc17cb99f50efbae4bbbab77f360818f (commit)
from 8df98cd98812dff67aa93a3f760860ac91f7f24a (commit)
- Log -----------------------------------------------------------------
commit db860ea3dcf56a1993c66da22bd44460d7ac4914
Author: Matt Caswell <matt at openssl.org>
Date: Tue Dec 4 08:37:04 2018 +0000
Fix some SSL_export_keying_material() issues
Fix some issues in tls13_hkdf_expand() which impact the above function
for TLSv1.3. In particular test that we can use the maximum label length
in TLSv1.3.
Reviewed-by: Tim Hudson <tjh at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7755)
(cherry picked from commit 0fb2815b873304d145ed00283454fc9f3bd35e6b)
commit 56e0f123dc17cb99f50efbae4bbbab77f360818f
Author: Matt Caswell <matt at openssl.org>
Date: Mon Dec 3 18:14:57 2018 +0000
Revert "Reduce stack usage in tls13_hkdf_expand"
This reverts commit ec0c5f5693e39c5a013f81e6dd9dfd09ec65162d.
SSL_export_keying_material() may use longer label lengths.
Fixes #7712
Reviewed-by: Tim Hudson <tjh at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7755)
(cherry picked from commit ed371b8cbac0d0349667558c061c1ae380cf75eb)
-----------------------------------------------------------------------
Summary of changes:
doc/man3/SSL_export_keying_material.pod | 3 +-
ssl/ssl_locl.h | 2 +-
ssl/statem/extensions.c | 2 +-
ssl/statem/statem_clnt.c | 2 +-
ssl/statem/statem_srvr.c | 2 +-
ssl/tls13_enc.c | 73 ++++++++++++++++++++-------------
test/sslapitest.c | 48 ++++++++++++++++------
test/tls13secretstest.c | 2 +-
8 files changed, 88 insertions(+), 46 deletions(-)
diff --git a/doc/man3/SSL_export_keying_material.pod b/doc/man3/SSL_export_keying_material.pod
index abebf91..4c81a60 100644
--- a/doc/man3/SSL_export_keying_material.pod
+++ b/doc/man3/SSL_export_keying_material.pod
@@ -59,7 +59,8 @@ B<label> and should be B<llen> bytes long. Typically this will be a value from
the IANA Exporter Label Registry
(L<https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#exporter-labels>).
Alternatively labels beginning with "EXPERIMENTAL" are permitted by the standard
-to be used without registration.
+to be used without registration. TLSv1.3 imposes a maximum label length of
+249 bytes.
Note that this function is only defined for TLSv1.0 and above, and DTLSv1.0 and
above. Attempting to use it in SSLv3 will result in an error.
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 70e5a17..307131d 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -2461,7 +2461,7 @@ __owur int tls13_hkdf_expand(SSL *s, const EVP_MD *md,
const unsigned char *secret,
const unsigned char *label, size_t labellen,
const unsigned char *data, size_t datalen,
- unsigned char *out, size_t outlen);
+ unsigned char *out, size_t outlen, int fatal);
__owur int tls13_derive_key(SSL *s, const EVP_MD *md,
const unsigned char *secret, unsigned char *key,
size_t keylen);
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 63e61c6..716d6d2 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1506,7 +1506,7 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
/* Generate the binder key */
if (!tls13_hkdf_expand(s, md, early_secret, label, labelsize, hash,
- hashsize, binderkey, hashsize)) {
+ hashsize, binderkey, hashsize, 1)) {
/* SSLfatal() already called */
goto err;
}
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 5a8f116..a0e495d 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -2740,7 +2740,7 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
PACKET_data(&nonce),
PACKET_remaining(&nonce),
s->session->master_key,
- hashlen)) {
+ hashlen, 1)) {
/* SSLfatal() already called */
goto err;
}
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index e7c11c4..a8e862c 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -4099,7 +4099,7 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
tick_nonce,
TICKET_NONCE_SIZE,
s->session->master_key,
- hashlen)) {
+ hashlen, 1)) {
/* SSLfatal() already called */
goto err;
}
diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c
index b6825d2..c3021d1 100644
--- a/ssl/tls13_enc.c
+++ b/ssl/tls13_enc.c
@@ -13,14 +13,7 @@
#include <openssl/evp.h>
#include <openssl/kdf.h>
-/*
- * RFC 8446, 7.1 Key Schedule, says:
- * Note: With common hash functions, any label longer than 12 characters
- * requires an additional iteration of the hash function to compute.
- * The labels in this specification have all been chosen to fit within
- * this limit.
- */
-#define TLS13_MAX_LABEL_LEN 12
+#define TLS13_MAX_LABEL_LEN 249
/* Always filled with zeros */
static const unsigned char default_zeros[EVP_MAX_MD_SIZE];
@@ -29,12 +22,13 @@ static const unsigned char default_zeros[EVP_MAX_MD_SIZE];
* Given a |secret|; a |label| of length |labellen|; and |data| of length
* |datalen| (e.g. typically a hash of the handshake messages), derive a new
* secret |outlen| bytes long and store it in the location pointed to be |out|.
- * The |data| value may be zero length. Returns 1 on success 0 on failure.
+ * The |data| value may be zero length. Any errors will be treated as fatal if
+ * |fatal| is set. Returns 1 on success 0 on failure.
*/
int tls13_hkdf_expand(SSL *s, const EVP_MD *md, const unsigned char *secret,
const unsigned char *label, size_t labellen,
const unsigned char *data, size_t datalen,
- unsigned char *out, size_t outlen)
+ unsigned char *out, size_t outlen, int fatal)
{
static const unsigned char label_prefix[] = "tls13 ";
EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, NULL);
@@ -48,12 +42,27 @@ int tls13_hkdf_expand(SSL *s, const EVP_MD *md, const unsigned char *secret,
*/
unsigned char hkdflabel[sizeof(uint16_t) + sizeof(uint8_t) +
+ sizeof(label_prefix) + TLS13_MAX_LABEL_LEN
- + EVP_MAX_MD_SIZE];
+ + 1 + EVP_MAX_MD_SIZE];
WPACKET pkt;
if (pctx == NULL)
return 0;
+ if (labellen > TLS13_MAX_LABEL_LEN) {
+ if (fatal) {
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_HKDF_EXPAND,
+ ERR_R_INTERNAL_ERROR);
+ } else {
+ /*
+ * Probably we have been called from SSL_export_keying_material(),
+ * or SSL_export_keying_material_early().
+ */
+ SSLerr(SSL_F_TLS13_HKDF_EXPAND, SSL_R_TLS_ILLEGAL_EXPORTER_LABEL);
+ }
+ EVP_PKEY_CTX_free(pctx);
+ return 0;
+ }
+
hashlen = EVP_MD_size(md);
if (!WPACKET_init_static_len(&pkt, hkdflabel, sizeof(hkdflabel), 0)
@@ -67,8 +76,11 @@ int tls13_hkdf_expand(SSL *s, const EVP_MD *md, const unsigned char *secret,
|| !WPACKET_finish(&pkt)) {
EVP_PKEY_CTX_free(pctx);
WPACKET_cleanup(&pkt);
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_HKDF_EXPAND,
- ERR_R_INTERNAL_ERROR);
+ if (fatal)
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_HKDF_EXPAND,
+ ERR_R_INTERNAL_ERROR);
+ else
+ SSLerr(SSL_F_TLS13_HKDF_EXPAND, ERR_R_INTERNAL_ERROR);
return 0;
}
@@ -82,9 +94,13 @@ int tls13_hkdf_expand(SSL *s, const EVP_MD *md, const unsigned char *secret,
EVP_PKEY_CTX_free(pctx);
- if (ret != 0)
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_HKDF_EXPAND,
- ERR_R_INTERNAL_ERROR);
+ if (ret != 0) {
+ if (fatal)
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_HKDF_EXPAND,
+ ERR_R_INTERNAL_ERROR);
+ else
+ SSLerr(SSL_F_TLS13_HKDF_EXPAND, ERR_R_INTERNAL_ERROR);
+ }
return ret == 0;
}
@@ -99,7 +115,7 @@ int tls13_derive_key(SSL *s, const EVP_MD *md, const unsigned char *secret,
static const unsigned char keylabel[] = "key";
return tls13_hkdf_expand(s, md, secret, keylabel, sizeof(keylabel) - 1,
- NULL, 0, key, keylen);
+ NULL, 0, key, keylen, 1);
}
/*
@@ -112,7 +128,7 @@ int tls13_derive_iv(SSL *s, const EVP_MD *md, const unsigned char *secret,
static const unsigned char ivlabel[] = "iv";
return tls13_hkdf_expand(s, md, secret, ivlabel, sizeof(ivlabel) - 1,
- NULL, 0, iv, ivlen);
+ NULL, 0, iv, ivlen, 1);
}
int tls13_derive_finishedkey(SSL *s, const EVP_MD *md,
@@ -122,7 +138,7 @@ int tls13_derive_finishedkey(SSL *s, const EVP_MD *md,
static const unsigned char finishedlabel[] = "finished";
return tls13_hkdf_expand(s, md, secret, finishedlabel,
- sizeof(finishedlabel) - 1, NULL, 0, fin, finlen);
+ sizeof(finishedlabel) - 1, NULL, 0, fin, finlen, 1);
}
/*
@@ -185,7 +201,7 @@ int tls13_generate_secret(SSL *s, const EVP_MD *md,
if (!tls13_hkdf_expand(s, md, prevsecret,
(unsigned char *)derived_secret_label,
sizeof(derived_secret_label) - 1, hash, mdlen,
- preextractsec, mdlen)) {
+ preextractsec, mdlen, 1)) {
/* SSLfatal() already called */
EVP_PKEY_CTX_free(pctx);
return 0;
@@ -345,7 +361,7 @@ static int derive_secret_key_and_iv(SSL *s, int sending, const EVP_MD *md,
hashlen = (size_t)hashleni;
if (!tls13_hkdf_expand(s, md, insecret, label, labellen, hash, hashlen,
- secret, hashlen)) {
+ secret, hashlen, 1)) {
/* SSLfatal() already called */
goto err;
}
@@ -525,7 +541,8 @@ int tls13_change_cipher_state(SSL *s, int which)
early_exporter_master_secret,
sizeof(early_exporter_master_secret) - 1,
hashval, hashlen,
- s->early_exporter_master_secret, hashlen)) {
+ s->early_exporter_master_secret, hashlen,
+ 1)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR,
SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR);
goto err;
@@ -612,7 +629,7 @@ int tls13_change_cipher_state(SSL *s, int which)
resumption_master_secret,
sizeof(resumption_master_secret) - 1,
hashval, hashlen, s->resumption_master_secret,
- hashlen)) {
+ hashlen, 1)) {
/* SSLfatal() already called */
goto err;
}
@@ -632,7 +649,7 @@ int tls13_change_cipher_state(SSL *s, int which)
exporter_master_secret,
sizeof(exporter_master_secret) - 1,
hash, hashlen, s->exporter_master_secret,
- hashlen)) {
+ hashlen, 1)) {
/* SSLfatal() already called */
goto err;
}
@@ -746,10 +763,10 @@ int tls13_export_keying_material(SSL *s, unsigned char *out, size_t olen,
|| EVP_DigestFinal_ex(ctx, data, &datalen) <= 0
|| !tls13_hkdf_expand(s, md, s->exporter_master_secret,
(const unsigned char *)label, llen,
- data, datalen, exportsecret, hashsize)
+ data, datalen, exportsecret, hashsize, 0)
|| !tls13_hkdf_expand(s, md, exportsecret, exporterlabel,
sizeof(exporterlabel) - 1, hash, hashsize,
- out, olen))
+ out, olen, 0))
goto err;
ret = 1;
@@ -805,10 +822,10 @@ int tls13_export_keying_material_early(SSL *s, unsigned char *out, size_t olen,
|| EVP_DigestFinal_ex(ctx, data, &datalen) <= 0
|| !tls13_hkdf_expand(s, md, s->early_exporter_master_secret,
(const unsigned char *)label, llen,
- data, datalen, exportsecret, hashsize)
+ data, datalen, exportsecret, hashsize, 0)
|| !tls13_hkdf_expand(s, md, exportsecret, exporterlabel,
sizeof(exporterlabel) - 1, hash, hashsize,
- out, olen))
+ out, olen, 0))
goto err;
ret = 1;
diff --git a/test/sslapitest.c b/test/sslapitest.c
index 108d57e..a4bbb4f 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -4028,20 +4028,25 @@ static int test_serverinfo(int tst)
* no test vectors so all we do is test that both sides of the communication
* produce the same results for different protocol versions.
*/
+#define SMALL_LABEL_LEN 10
+#define LONG_LABEL_LEN 249
static int test_export_key_mat(int tst)
{
int testresult = 0;
SSL_CTX *cctx = NULL, *sctx = NULL, *sctx2 = NULL;
SSL *clientssl = NULL, *serverssl = NULL;
- const char label[] = "test label";
+ const char label[LONG_LABEL_LEN + 1] = "test label";
const unsigned char context[] = "context";
const unsigned char *emptycontext = NULL;
unsigned char ckeymat1[80], ckeymat2[80], ckeymat3[80];
unsigned char skeymat1[80], skeymat2[80], skeymat3[80];
+ size_t labellen;
const int protocols[] = {
TLS1_VERSION,
TLS1_1_VERSION,
TLS1_2_VERSION,
+ TLS1_3_VERSION,
+ TLS1_3_VERSION,
TLS1_3_VERSION
};
@@ -4058,7 +4063,7 @@ static int test_export_key_mat(int tst)
return 1;
#endif
#ifdef OPENSSL_NO_TLS1_3
- if (tst == 3)
+ if (tst >= 3)
return 1;
#endif
if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), TLS_client_method(),
@@ -4076,33 +4081,52 @@ static int test_export_key_mat(int tst)
SSL_ERROR_NONE)))
goto end;
+ if (tst == 5) {
+ /*
+ * TLSv1.3 imposes a maximum label len of 249 bytes. Check we fail if we
+ * go over that.
+ */
+ if (!TEST_int_le(SSL_export_keying_material(clientssl, ckeymat1,
+ sizeof(ckeymat1), label,
+ LONG_LABEL_LEN + 1, context,
+ sizeof(context) - 1, 1), 0))
+ goto end;
+
+ testresult = 1;
+ goto end;
+ } else if (tst == 4) {
+ labellen = LONG_LABEL_LEN;
+ } else {
+ labellen = SMALL_LABEL_LEN;
+ }
+
if (!TEST_int_eq(SSL_export_keying_material(clientssl, ckeymat1,
sizeof(ckeymat1), label,
- sizeof(label) - 1, context,
+ labellen, context,
sizeof(context) - 1, 1), 1)
|| !TEST_int_eq(SSL_export_keying_material(clientssl, ckeymat2,
sizeof(ckeymat2), label,
- sizeof(label) - 1,
+ labellen,
emptycontext,
0, 1), 1)
|| !TEST_int_eq(SSL_export_keying_material(clientssl, ckeymat3,
sizeof(ckeymat3), label,
- sizeof(label) - 1,
+ labellen,
NULL, 0, 0), 1)
|| !TEST_int_eq(SSL_export_keying_material(serverssl, skeymat1,
sizeof(skeymat1), label,
- sizeof(label) - 1,
+ labellen,
context,
sizeof(context) -1, 1),
1)
|| !TEST_int_eq(SSL_export_keying_material(serverssl, skeymat2,
sizeof(skeymat2), label,
- sizeof(label) - 1,
+ labellen,
emptycontext,
0, 1), 1)
|| !TEST_int_eq(SSL_export_keying_material(serverssl, skeymat3,
sizeof(skeymat3), label,
- sizeof(label) - 1,
+ labellen,
NULL, 0, 0), 1)
/*
* Check that both sides created the same key material with the
@@ -4131,10 +4155,10 @@ static int test_export_key_mat(int tst)
* Check that an empty context and no context produce different results in
* protocols less than TLSv1.3. In TLSv1.3 they should be the same.
*/
- if ((tst != 3 && !TEST_mem_ne(ckeymat2, sizeof(ckeymat2), ckeymat3,
+ if ((tst < 3 && !TEST_mem_ne(ckeymat2, sizeof(ckeymat2), ckeymat3,
sizeof(ckeymat3)))
- || (tst ==3 && !TEST_mem_eq(ckeymat2, sizeof(ckeymat2), ckeymat3,
- sizeof(ckeymat3))))
+ || (tst >= 3 && !TEST_mem_eq(ckeymat2, sizeof(ckeymat2), ckeymat3,
+ sizeof(ckeymat3))))
goto end;
testresult = 1;
@@ -5909,7 +5933,7 @@ int setup_tests(void)
ADD_ALL_TESTS(test_custom_exts, 3);
#endif
ADD_ALL_TESTS(test_serverinfo, 8);
- ADD_ALL_TESTS(test_export_key_mat, 4);
+ ADD_ALL_TESTS(test_export_key_mat, 6);
#ifndef OPENSSL_NO_TLS1_3
ADD_ALL_TESTS(test_export_key_mat_early, 3);
#endif
diff --git a/test/tls13secretstest.c b/test/tls13secretstest.c
index 724c170..66a0582 100644
--- a/test/tls13secretstest.c
+++ b/test/tls13secretstest.c
@@ -236,7 +236,7 @@ static int test_secret(SSL *s, unsigned char *prk,
}
if (!tls13_hkdf_expand(s, md, prk, label, labellen, hash, hashsize,
- gensecret, hashsize)) {
+ gensecret, hashsize, 1)) {
TEST_error("Secret generation failed");
return 0;
}
More information about the openssl-commits
mailing list