tls-crypto: Simplify handshake/application key derivation and rename methods

Also consistently change the ciphers outside of tls_crypto_t and
simplify key derivation in tls_peer_t and fix a memory leak.
This commit is contained in:
Tobias Brunner 2020-08-25 13:22:04 +02:00
parent fff1974012
commit 2921f43705
3 changed files with 85 additions and 153 deletions

View File

@ -1244,10 +1244,6 @@ static bool create_aead(private_tls_crypto_t *this, suite_algs_t *algs)
{
this->aead_in = tls_aead_create_seq(algs->encr, algs->encr_size);
this->aead_out = tls_aead_create_seq(algs->encr, algs->encr_size);
this->protection->set_cipher(this->protection, TRUE, this->aead_in);
this->protection->set_cipher(this->protection, FALSE, this->aead_out);
}
if (!this->aead_in || !this->aead_out)
{
@ -1982,148 +1978,79 @@ METHOD(tls_crypto_t, derive_secrets, bool,
expand_keys(this, client_random, server_random);
}
METHOD(tls_crypto_t, derive_handshake_secret, bool,
private_tls_crypto_t *this, chunk_t shared_secret)
/**
* Derive and configure the keys/IVs using the given labels.
*/
static bool derive_labeled_keys(private_tls_crypto_t *this,
tls_hkdf_label_t client_label,
tls_hkdf_label_t server_label)
{
chunk_t c_key, c_iv, s_key, s_iv;
this->hkdf->set_shared_secret(this->hkdf, shared_secret);
/* client key material */
if (!this->hkdf->generate_secret(this->hkdf, TLS_HKDF_C_HS_TRAFFIC,
this->handshake, NULL) ||
!this->hkdf->derive_key(this->hkdf, FALSE,
this->aead_out->
get_encr_key_size(this->aead_out), &c_key) ||
!this->hkdf->derive_iv(this->hkdf, FALSE,
this->aead_out->
get_iv_size(this->aead_out), &c_iv))
{
DBG1(DBG_TLS, "deriving client key material failed");
chunk_clear(&c_key);
chunk_clear(&c_iv);
return FALSE;
}
/* server key material */
if (!this->hkdf->generate_secret(this->hkdf, TLS_HKDF_S_HS_TRAFFIC,
this->handshake, NULL) ||
!this->hkdf->derive_key(this->hkdf, TRUE, this->aead_in->
get_encr_key_size(this->aead_in), &s_key) ||
!this->hkdf->derive_iv(this->hkdf, TRUE, this->aead_in->
get_iv_size(this->aead_in), &s_iv))
{
DBG1(DBG_TLS, "deriving server key material failed");
chunk_clear(&c_key);
chunk_clear(&c_iv);
chunk_clear(&s_key);
chunk_clear(&s_iv);
return FALSE;
}
chunk_t c_key = chunk_empty, c_iv = chunk_empty;
chunk_t s_key = chunk_empty, s_iv = chunk_empty;
tls_aead_t *aead_c = this->aead_out, *aead_s = this->aead_in;
bool success = FALSE;
if (this->tls->is_server(this->tls))
{
if (!this->aead_in->set_keys(this->aead_in, chunk_empty, s_key, s_iv) ||
!this->aead_out->set_keys(this->aead_out, chunk_empty, c_key, c_iv))
{
DBG1(DBG_TLS, "setting aead server key material failed");
chunk_clear(&c_key);
chunk_clear(&c_iv);
chunk_clear(&s_key);
chunk_clear(&s_iv);
return FALSE;
}
}
else
{
if (!this->aead_in->set_keys(this->aead_in, chunk_empty, s_key, s_iv) ||
!this->aead_out->set_keys(this->aead_out, chunk_empty, c_key, c_iv))
{
DBG1(DBG_TLS, "setting aead client key material failed");
chunk_clear(&c_key);
chunk_clear(&c_iv);
chunk_clear(&s_key);
chunk_clear(&s_iv);
return FALSE;
}
aead_c = this->aead_in;
aead_s = this->aead_out;
}
if (!this->hkdf->generate_secret(this->hkdf, client_label, this->handshake,
NULL) ||
!this->hkdf->derive_key(this->hkdf, FALSE,
this->aead_out->get_encr_key_size(this->aead_out),
&c_key) ||
!this->hkdf->derive_iv(this->hkdf, FALSE,
this->aead_out->get_iv_size(this->aead_out),
&c_iv))
{
DBG1(DBG_TLS, "deriving client key material failed");
goto out;
}
if (!this->hkdf->generate_secret(this->hkdf, server_label, this->handshake,
NULL) ||
!this->hkdf->derive_key(this->hkdf, TRUE,
this->aead_in->get_encr_key_size(this->aead_in),
&s_key) ||
!this->hkdf->derive_iv(this->hkdf, TRUE,
this->aead_in->get_iv_size(this->aead_in),
&s_iv))
{
DBG1(DBG_TLS, "deriving server key material failed");
goto out;
}
if (!aead_c->set_keys(aead_c, chunk_empty, c_key, c_iv) ||
!aead_s->set_keys(aead_s, chunk_empty, s_key, s_iv))
{
DBG1(DBG_TLS, "setting AEAD key material failed");
goto out;
}
success = TRUE;
out:
chunk_clear(&c_key);
chunk_clear(&c_iv);
chunk_clear(&s_key);
chunk_clear(&s_iv);
return TRUE;
return success;
}
METHOD(tls_crypto_t, derive_app_secret, bool,
METHOD(tls_crypto_t, derive_handshake_keys, bool,
private_tls_crypto_t *this, chunk_t shared_secret)
{
this->hkdf->set_shared_secret(this->hkdf, shared_secret);
return derive_labeled_keys(this, TLS_HKDF_C_HS_TRAFFIC,
TLS_HKDF_S_HS_TRAFFIC);
}
METHOD(tls_crypto_t, derive_app_keys, bool,
private_tls_crypto_t *this)
{
chunk_t c_key, c_iv, s_key, s_iv;
/* client key material */
if (!this->hkdf->generate_secret(this->hkdf, TLS_HKDF_C_AP_TRAFFIC,
this->handshake, NULL) ||
!this->hkdf->derive_key(this->hkdf, FALSE,
this->aead_out->
get_encr_key_size(this->aead_out), &c_key) ||
!this->hkdf->derive_iv(this->hkdf, FALSE,
this->aead_out->
get_iv_size(this->aead_out), &c_iv))
{
DBG1(DBG_TLS, "deriving client key material failed");
chunk_clear(&c_key);
chunk_clear(&c_iv);
return FALSE;
}
/* server key material */
if (!this->hkdf->generate_secret(this->hkdf, TLS_HKDF_S_AP_TRAFFIC,
this->handshake, NULL) ||
!this->hkdf->derive_key(this->hkdf, TRUE, this->aead_in->
get_encr_key_size(this->aead_in), &s_key) ||
!this->hkdf->derive_iv(this->hkdf, TRUE, this->aead_in->
get_iv_size(this->aead_in), &s_iv))
{
DBG1(DBG_TLS, "deriving server key material failed");
chunk_clear(&c_key);
chunk_clear(&c_iv);
chunk_clear(&s_key);
chunk_clear(&s_iv);
return FALSE;
}
if (this->tls->is_server(this->tls))
{
if (!this->aead_in->set_keys(this->aead_in, chunk_empty, s_key, s_iv) ||
!this->aead_out->set_keys(this->aead_out, chunk_empty, c_key, c_iv))
{
DBG1(DBG_TLS, "setting aead server key material failed");
chunk_clear(&c_key);
chunk_clear(&c_iv);
chunk_clear(&s_key);
chunk_clear(&s_iv);
return FALSE;
}
}
else
{
if (!this->aead_in->set_keys(this->aead_in, chunk_empty, s_key, s_iv) ||
!this->aead_out->set_keys(this->aead_out, chunk_empty, c_key, c_iv))
{
DBG1(DBG_TLS, "setting aead client key material failed");
chunk_clear(&c_key);
chunk_clear(&c_iv);
chunk_clear(&s_key);
chunk_clear(&s_iv);
return FALSE;
}
}
chunk_clear(&c_key);
chunk_clear(&c_iv);
chunk_clear(&s_key);
chunk_clear(&s_iv);
return TRUE;
return derive_labeled_keys(this, TLS_HKDF_C_AP_TRAFFIC,
TLS_HKDF_S_AP_TRAFFIC);
}
METHOD(tls_crypto_t, resume_session, tls_cipher_suite_t,
@ -2223,8 +2150,8 @@ tls_crypto_t *tls_crypto_create(tls_t *tls, tls_cache_t *cache)
.calculate_finished = _calculate_finished,
.calculate_finished_tls13 = _calculate_finished_tls13,
.derive_secrets = _derive_secrets,
.derive_handshake_secret = _derive_handshake_secret,
.derive_app_secret = _derive_app_secret,
.derive_handshake_keys = _derive_handshake_keys,
.derive_app_keys = _derive_app_keys,
.resume_session = _resume_session,
.get_session = _get_session,
.change_cipher = _change_cipher,

View File

@ -1,4 +1,9 @@
/*
* Copyright (C) 2020 Tobias Brunner
* Copyright (C) 2020 Pascal Knecht
* Copyright (C) 2020 Méline Sieber
* HSR Hochschule fuer Technik Rapperswil
*
* Copyright (C) 2010 Martin Willi
* Copyright (C) 2010 revosec AG
*
@ -514,7 +519,7 @@ struct tls_crypto_t {
bio_reader_t *reader);
/**
* Calculate the data of a legacyTLS finished message.
* Calculate the data of a legacy TLS finished message.
*
* @param label ASCII label to use for calculation
* @param out buffer to write finished data to
@ -551,14 +556,14 @@ struct tls_crypto_t {
* @param shared_secret input key material
* @return TRUE if secret derived successfully
*/
bool (*derive_handshake_secret)(tls_crypto_t *this, chunk_t shared_secret);
bool (*derive_handshake_keys)(tls_crypto_t *this, chunk_t shared_secret);
/**
* Derive the application keys.
*
* @return TRUE if secret derived successfully
*/
bool (*derive_app_secret)(tls_crypto_t *this);
bool (*derive_app_keys)(tls_crypto_t *this);
/**
* Try to resume a TLS session, derive key material.

View File

@ -262,22 +262,21 @@ static status_t process_server_hello(private_tls_peer_t *this,
if (this->tls->get_version_max(this->tls) == TLS_1_3)
{
chunk_t shared_secret;
chunk_t shared_secret = chunk_empty;
if (key_type != CURVE_25519 &&
!this->dh->set_other_public_value(this->dh, ext_key_share))
if (!this->dh->set_other_public_value(this->dh, ext_key_share) ||
!this->dh->get_shared_secret(this->dh, &shared_secret) ||
!this->crypto->derive_handshake_keys(this->crypto, shared_secret))
{
DBG2(DBG_TLS, "server key share unable to save");
}
if (!this->dh->get_shared_secret(this->dh, &shared_secret))
{
DBG2(DBG_TLS, "No shared secret key found");
DBG1(DBG_TLS, "DH key derivation failed");
this->alert->add(this->alert, TLS_FATAL, TLS_HANDSHAKE_FAILURE);
chunk_clear(&shared_secret);
return NEED_MORE;
}
chunk_clear(&shared_secret);
if (!this->crypto->derive_handshake_secret(this->crypto, shared_secret))
{
DBG2(DBG_TLS, "derive handshake traffic secret failed");
}
this->crypto->change_cipher(this->crypto, TRUE);
this->crypto->change_cipher(this->crypto, FALSE);
}
this->state = STATE_HELLO_RECEIVED;
@ -1537,14 +1536,15 @@ METHOD(tls_handshake_t, build, status_t,
case STATE_INIT:
return send_client_hello(this, type, writer);
case STATE_HELLO_DONE:
/* otherwise fall through to next state */
case STATE_CIPHERSPEC_CHANGED_OUT:
case STATE_FINISHED_RECEIVED:
/* fall through since legacy TLS and TLS 1.3
* expect the same message */
return send_finished(this, type, writer);
case STATE_FINISHED_SENT:
this->crypto->derive_app_secret(this->crypto);
if (!this->crypto->derive_app_keys(this->crypto))
{
this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
return NEED_MORE;
}
this->crypto->change_cipher(this->crypto, TRUE);
this->crypto->change_cipher(this->crypto, FALSE);
this->state = STATE_FINISHED_SENT_KEY_SWITCHED;