wbrawner/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix some SSL_export_keying_material() issues

Browse source 
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@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7755)
This commit is contained in:
Matt Caswell 2018-12-04 08:37:04 +00:00
parent ed371b8cba
commit 0fb2815b87
8 changed files with 92 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
doc/man3/SSL_export_keying_material.pod
View file

@ -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.

2
ssl/ssl_locl.h
View file

@ -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);

2
ssl/statem/extensions.c
View file

@ -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;
}

2
ssl/statem/statem_clnt.c
View file

@ -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;
}

2
ssl/statem/statem_srvr.c
View file

@ -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;
}

73
ssl/tls13_enc.c
View file

@ -13,7 +13,7 @@
#include <openssl/evp.h>
#include <openssl/kdf.h>
#define TLS13_MAX_LABEL_LEN 246
#define TLS13_MAX_LABEL_LEN 249
/* Always filled with zeros */
static const unsigned char default_zeros[EVP_MAX_MD_SIZE];
@ -22,30 +22,47 @@ 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)
{
const unsigned char label_prefix[] = "tls13 ";
static const unsigned char label_prefix[] = "tls13 ";
EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, NULL);
int ret;
size_t hkdflabellen;
size_t hashlen;
/*
* 2 bytes for length of whole HkdfLabel + 1 byte for length of combined
* prefix and label + bytes for the label itself + bytes for the hash
* 2 bytes for length of derived secret + 1 byte for length of combined
* prefix and label + bytes for the label itself + 1 byte length of hash
* + bytes for the hash itself
*/
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)
@ -59,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;
}
@ -74,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;
}
@ -91,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);
}
/*
@ -104,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,
@ -114,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);
}
/*
@ -177,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;
@ -337,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;
}
@ -517,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;
@ -604,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;
}
@ -624,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;
}
@ -738,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;
@ -797,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;

48
test/sslapitest.c
View file

@ -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

2
test/tls13secretstest.c
View file

@ -226,7 +226,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;
}