On Tue, May 10, 2016 at 1:28 PM, David Benjamin <davidben@xxxxxxxxxx> wrote:
> 1. I can condition the EAP-FAST-only parts of the key derivation
> conditioned on EAP_FAST #defines. With
> df5bde83daaf3c820b6ac1f8d383b16a64ebd2db, we're finally no longer
> defining those symbols, so this would work. I would probably, to make
> this cleaner, split the tls_connection_prf entry-point into two
> functions, one for the reasonable RFC 5705 mechanisms and one for
> EAP-FAST's crazy thing. (Right now the API supports the RFC 5705
> style, but with skip_keyblock set, which is a little odd.)
I've attached the cleanup and #ifdefs I was thinking for this option.
Note: I've only compile-tested this against Android thus far. It's
mostly to clarify what I mean, and I'm sure I messed up somewhere.
I think the first of these two patches is worthwhile in itself. The
EAP-FAST key derivation is very different compared to how new
protocols are supposed to do it (RFC 5705). The split is also more
conducive to, say, OpenSSL maybe providing an API for extracting
EAP-FAST style keys, either by adding BoringSSL's
SSL_generate_key_block API to OpenSSL or perhaps they add an
EAP-FAST-specific key derivation function. I don't think it makes
sense that hostap needs to reimplement the TLS PRF.
You can also see awkwardness in the merged tls_connection_prf; the
skip_keyblock parameter makes no sense unless server_random_first and
label align with the native TLS key derivation. Whereas when
skip_keyblock and server_random_first is 0, this is just a call to
SSL_export_keying_material in OpenSSL. This suggests they should be
two distinct operations.
The second one I don't have strong opinions on. For my purposes, I
would be happy with it, with making BoringSSL use the OpenSSL 1.1.x
path (I'd add the missing functions), or with using the
BoringSSL-specific functions. I'm happy to put together whichever
version you all prefer.
Thoughts?
> 2. For OpenSSL 1.1.x, you all use SSL_CIPHER_get_cipher_nid and
> SSL_CIPHER_get_digest_nid. BoringSSL doesn't have these, but I have no
> problems adding them. (I would not have used NIDs for
> SSL_CIPHER_get_kx_nid and SSL_CIPHER_get_auth_nid but whatever.) It's
> a little odd because the computation is wrong for TLS 1.1+ and we
> don't use EVP_CIPHER in our SSL code anymore. But the code will never
> run anyway, and if it were to later, I could make it match OpenSSL.
>
> 3. For other reasons, we added a pair of functions,
> SSL_get_key_block_len and SSL_generate_key_block which are actually
> exactly what EAP-FAST wants here. I could make
> openssl_get_keyblock_size call SSL_get_key_block_len and even make
> openssl_tls_prf use SSL_generate_key_block. (Probably this would come
> with a similar tls_connection_prf split as in option #1.) But this
> would be BoringSSL-only code to support a feature that doesn't work in
> BoringSSL anyway, so this is of questionable use.
> https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_get_key_block_len
>
> Do you all have a preference? #2 would be the least invasive option,
> but splitting tls_connection_prf in two for #1 might be preferable, at
> which point you may as well condition on EAP-FAST. In doing so, we can
> probably do away with the fallback from SSL_export_keying_material to
> openssl_tls_prf if SSL_export_keying_material fails (looks like that's
> a remnant of how the #ifdefs worked out while 1.0.0 was supported) and
> not even ship a custom TLS PRF in the no-EAP-FAST config. I'm happy to
> do a bit of cleanup for you along the way here.
>
> David
From 699e48df4e45d80011169d96088bb441ddee9930 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@xxxxxxxxxx>
Date: Mon, 16 May 2016 11:47:37 -0400
Subject: [PATCH 1/2] TLS: Split tls_connection_prf into two functions.
Most protocols extracting keys from TLS use RFC 5705 exporters which is
commonly implemented in TLS libraries. This is the mechanism used by EAP-TLS.
(EAP-TLS actually predates RFC 5705, but RFC 5705 was defined to be compatible
with it.)
EAP-FAST, however, uses a legacy mechanism. It reuses the TLS internal key
block derivation and derives key material after the key block. This is uncommon
and a misuse of TLS internals, so not all TLS libraries support this. Instead,
we reimplement the PRF for the OpenSSL backend and don't support it at all in
the GnuTLS one.
Since these two are very different operations, split tls_connection_prf in two.
tls_connection_export_key implements the standard RFC 5705 mechanism that we
expect most TLS libraries to support. tls_connection_get_eap_fast_key
implements the EAP-FAST-specific legacy mechanism which may not be implemented
on all backends but is only used by EAP-FAST.
Signed-Off-By: David Benjamin <davidben@xxxxxxxxxx>
---
src/crypto/tls.h | 37 +++++++++++--------
src/crypto/tls_gnutls.c | 15 +++++---
src/crypto/tls_internal.c | 21 +++++++++--
src/crypto/tls_none.c | 12 +++++--
src/crypto/tls_openssl.c | 77 ++++++++++++++++------------------------
src/eap_common/eap_fast_common.c | 5 ++-
src/eap_common/eap_fast_common.h | 2 +-
src/eap_peer/eap_fast.c | 3 +-
src/eap_peer/eap_tls_common.c | 4 +--
src/eap_server/eap_server_fast.c | 3 +-
10 files changed, 97 insertions(+), 82 deletions(-)
diff --git a/src/crypto/tls.h b/src/crypto/tls.h
index 15a3bcf..c959f26 100644
--- a/src/crypto/tls.h
+++ b/src/crypto/tls.h
@@ -336,29 +336,36 @@ int __must_check tls_connection_get_random(void *tls_ctx,
struct tls_random *data);
/**
- * tls_connection_prf - Use TLS-PRF to derive keying material
+ * tls_connection_export_key - Derive keying material from a TLS connection
* @tls_ctx: TLS context data from tls_init()
* @conn: Connection context data from tls_connection_init()
* @label: Label (e.g., description of the key) for PRF
- * @server_random_first: seed is 0 = client_random|server_random,
- * 1 = server_random|client_random
- * @skip_keyblock: Skip TLS key block from the beginning of PRF output
* @out: Buffer for output data from TLS-PRF
* @out_len: Length of the output buffer
* Returns: 0 on success, -1 on failure
*
- * tls_connection_prf() is required so that further keying material can be
- * derived from the master secret. Example implementation of this function is in
- * tls_prf_sha1_md5() when it is called with seed set to
- * client_random|server_random (or server_random|client_random). For TLSv1.2 and
- * newer, a different PRF is needed, though.
+ * Exports keying material using the mechanism described in RFC 5705.
*/
-int __must_check tls_connection_prf(void *tls_ctx,
- struct tls_connection *conn,
- const char *label,
- int server_random_first,
- int skip_keyblock,
- u8 *out, size_t out_len);
+int __must_check tls_connection_export_key(void *tls_ctx,
+ struct tls_connection *conn,
+ const char *label,
+ u8 *out, size_t out_len);
+
+/**
+ * tls_connection_get_eap_fast_key - Derive key material for EAP-FAST
+ * @tls_ctx: TLS context data from tls_init()
+ * @conn: Connection context data from tls_connection_init()
+ * @out: Buffer for output data from TLS-PRF
+ * @out_len: Length of the output buffer
+ * Returns: 0 on success, -1 on failure
+ *
+ * Exports key material after the normal TLS key block for use with
+ * EAP-FAST. Most callers will want tls_connection_export_key(), but EAP-FAST
+ * uses a different legacy mechanism.
+ */
+int __must_check tls_connection_get_eap_fast_key(void *tls_ctx,
+ struct tls_connection *conn,
+ u8 *out, size_t out_len);
/**
* tls_connection_handshake - Process TLS handshake (client side)
diff --git a/src/crypto/tls_gnutls.c b/src/crypto/tls_gnutls.c
index c4cd3c1..6c1e31d 100644
--- a/src/crypto/tls_gnutls.c
+++ b/src/crypto/tls_gnutls.c
@@ -810,15 +810,22 @@ int tls_connection_get_random(void *ssl_ctx, struct tls_connection *conn,
}
-int tls_connection_prf(void *tls_ctx, struct tls_connection *conn,
- const char *label, int server_random_first,
- int skip_keyblock, u8 *out, size_t out_len)
+int tls_connection_export_key(void *tls_ctx, struct tls_connection *conn,
+ const char *label, u8 *out, size_t out_len)
{
if (conn == NULL || conn->session == NULL || skip_keyblock)
return -1;
return gnutls_prf(conn->session, os_strlen(label), label,
- server_random_first, 0, NULL, out_len, (char *) out);
+ 0 /* client_random first */, 0, NULL, out_len,
+ (char *) out);
+}
+
+
+int tls_connection_get_eap_fast_key(void *tls_ctx, struct tls_connection *conn,
+ u8 *out, size_t out_len)
+{
+ return -1;
}
diff --git a/src/crypto/tls_internal.c b/src/crypto/tls_internal.c
index 01a7c97..f567e96 100644
--- a/src/crypto/tls_internal.c
+++ b/src/crypto/tls_internal.c
@@ -394,9 +394,9 @@ static int tls_get_keyblock_size(struct tls_connection *conn)
}
-int tls_connection_prf(void *tls_ctx, struct tls_connection *conn,
- const char *label, int server_random_first,
- int skip_keyblock, u8 *out, size_t out_len)
+static int tls_connection_prf(void *tls_ctx, struct tls_connection *conn,
+ const char *label, int server_random_first,
+ int skip_keyblock, u8 *out, size_t out_len)
{
int ret = -1, skip = 0;
u8 *tmp_out = NULL;
@@ -434,6 +434,21 @@ int tls_connection_prf(void *tls_ctx, struct tls_connection *conn,
}
+int tls_connection_export_key(void *tls_ctx, struct tls_connection *conn,
+ const char *label, u8 *out, size_t out_len)
+{
+ return tls_connection_prf(tls_ctx, conn, label, 0, 0, out, out_len);
+}
+
+
+int tls_connection_get_eap_fast_key(void *tls_ctx, struct tls_connection *conn,
+ u8 *out, size_t out_len)
+{
+ return tls_connection_prf(tls_ctx, conn, "key expansion", 1, 1, out,
+ out_len);
+}
+
+
struct wpabuf * tls_connection_handshake(void *tls_ctx,
struct tls_connection *conn,
const struct wpabuf *in_data,
diff --git a/src/crypto/tls_none.c b/src/crypto/tls_none.c
index ae392ad..7519475 100644
--- a/src/crypto/tls_none.c
+++ b/src/crypto/tls_none.c
@@ -86,9 +86,15 @@ int tls_connection_get_random(void *tls_ctx, struct tls_connection *conn,
}
-int tls_connection_prf(void *tls_ctx, struct tls_connection *conn,
- const char *label, int server_random_first,
- int skip_keyblock, u8 *out, size_t out_len)
+int tls_connection_export_key(void *tls_ctx, struct tls_connection *conn,
+ const char *label, u8 *out, size_t out_len)
+{
+ return -1;
+}
+
+
+int tls_connection_get_eap_fast_key(void *tls_ctx, struct tls_connection *conn,
+ u8 *out, size_t out_len)
{
return -1;
}
diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
index 8c9ecc5..e53cf9d 100644
--- a/src/crypto/tls_openssl.c
+++ b/src/crypto/tls_openssl.c
@@ -3146,9 +3146,21 @@ static int openssl_get_keyblock_size(SSL *ssl)
#endif /* CONFIG_FIPS */
-static int openssl_tls_prf(struct tls_connection *conn,
- const char *label, int server_random_first,
- int skip_keyblock, u8 *out, size_t out_len)
+int tls_connection_export_key(void *tls_ctx, struct tls_connection *conn,
+ const char *label, u8 *out, size_t out_len)
+{
+ if (conn == NULL)
+ return -1;
+ if (SSL_export_keying_material(conn->ssl, out, out_len, label,
+ os_strlen(label), NULL, 0, 0) != 1) {
+ return -1;
+ }
+ return 0;
+}
+
+
+int tls_connection_get_eap_fast_key(void *tls_ctx, struct tls_connection *conn,
+ u8 *out, size_t out_len)
{
#ifdef CONFIG_FIPS
wpa_printf(MSG_ERROR, "OpenSSL: TLS keys cannot be exported in FIPS "
@@ -3169,9 +3181,9 @@ static int openssl_tls_prf(struct tls_connection *conn,
const char *ver;
/*
- * TLS library did not support key generation, so get the needed TLS
- * session parameters and use an internal implementation of TLS PRF to
- * derive the key.
+ * TLS library did not support EAP-FAST key generation, so get the
+ * needed TLS session parameters and use an internal implementation of
+ * TLS PRF to derive the key.
*/
if (conn == NULL)
@@ -3184,15 +3196,13 @@ static int openssl_tls_prf(struct tls_connection *conn,
if (!ver || !sess)
return -1;
- if (skip_keyblock) {
- skip = openssl_get_keyblock_size(ssl);
- if (skip < 0)
- return -1;
- tmp_out = os_malloc(skip + out_len);
- if (!tmp_out)
- return -1;
- _out = tmp_out;
- }
+ skip = openssl_get_keyblock_size(ssl);
+ if (skip < 0)
+ return -1;
+ tmp_out = os_malloc(skip + out_len);
+ if (!tmp_out)
+ return -1;
+ _out = tmp_out;
rnd = os_malloc(2 * SSL3_RANDOM_SIZE);
if (!rnd) {
@@ -3205,29 +3215,22 @@ static int openssl_tls_prf(struct tls_connection *conn,
master_key_len = SSL_SESSION_get_master_key(sess, master_key,
sizeof(master_key));
- if (server_random_first) {
- os_memcpy(rnd, server_random, SSL3_RANDOM_SIZE);
- os_memcpy(rnd + SSL3_RANDOM_SIZE, client_random,
- SSL3_RANDOM_SIZE);
- } else {
- os_memcpy(rnd, client_random, SSL3_RANDOM_SIZE);
- os_memcpy(rnd + SSL3_RANDOM_SIZE, server_random,
- SSL3_RANDOM_SIZE);
- }
+ os_memcpy(rnd, server_random, SSL3_RANDOM_SIZE);
+ os_memcpy(rnd + SSL3_RANDOM_SIZE, client_random, SSL3_RANDOM_SIZE);
if (os_strcmp(ver, "TLSv1.2") == 0) {
tls_prf_sha256(master_key, master_key_len,
- label, rnd, 2 * SSL3_RANDOM_SIZE,
+ "key expansion", rnd, 2 * SSL3_RANDOM_SIZE,
_out, skip + out_len);
ret = 0;
} else if (tls_prf_sha1_md5(master_key, master_key_len,
- label, rnd, 2 * SSL3_RANDOM_SIZE,
+ "key expansion", rnd, 2 * SSL3_RANDOM_SIZE,
_out, skip + out_len) == 0) {
ret = 0;
}
os_memset(master_key, 0, sizeof(master_key));
os_free(rnd);
- if (ret == 0 && skip_keyblock)
+ if (ret == 0)
os_memcpy(out, _out + skip, out_len);
bin_clear_free(tmp_out, skip);
@@ -3236,26 +3239,6 @@ static int openssl_tls_prf(struct tls_connection *conn,
}
-int tls_connection_prf(void *tls_ctx, struct tls_connection *conn,
- const char *label, int server_random_first,
- int skip_keyblock, u8 *out, size_t out_len)
-{
- if (conn == NULL)
- return -1;
- if (server_random_first || skip_keyblock)
- return openssl_tls_prf(conn, label,
- server_random_first, skip_keyblock,
- out, out_len);
- if (SSL_export_keying_material(conn->ssl, out, out_len, label,
- os_strlen(label), NULL, 0, 0) == 1) {
- wpa_printf(MSG_DEBUG, "OpenSSL: Using internal PRF");
- return 0;
- }
- return openssl_tls_prf(conn, label, server_random_first,
- skip_keyblock, out, out_len);
-}
-
-
static struct wpabuf *
openssl_handshake(struct tls_connection *conn, const struct wpabuf *in_data,
int server)
diff --git a/src/eap_common/eap_fast_common.c b/src/eap_common/eap_fast_common.c
index e8587fd..9ef671c 100644
--- a/src/eap_common/eap_fast_common.c
+++ b/src/eap_common/eap_fast_common.c
@@ -93,8 +93,7 @@ void eap_fast_derive_master_secret(const u8 *pac_key, const u8 *server_random,
}
-u8 * eap_fast_derive_key(void *ssl_ctx, struct tls_connection *conn,
- const char *label, size_t len)
+u8 * eap_fast_derive_key(void *ssl_ctx, struct tls_connection *conn, size_t len)
{
u8 *out;
@@ -102,7 +101,7 @@ u8 * eap_fast_derive_key(void *ssl_ctx, struct tls_connection *conn,
if (out == NULL)
return NULL;
- if (tls_connection_prf(ssl_ctx, conn, label, 1, 1, out, len)) {
+ if (tls_connection_get_eap_fast_key(ssl_ctx, conn, out, len)) {
os_free(out);
return NULL;
}
diff --git a/src/eap_common/eap_fast_common.h b/src/eap_common/eap_fast_common.h
index 6756dd2..724204c 100644
--- a/src/eap_common/eap_fast_common.h
+++ b/src/eap_common/eap_fast_common.h
@@ -98,7 +98,7 @@ struct wpabuf * eap_fast_tlv_eap_payload(struct wpabuf *buf);
void eap_fast_derive_master_secret(const u8 *pac_key, const u8 *server_random,
const u8 *client_random, u8 *master_secret);
u8 * eap_fast_derive_key(void *ssl_ctx, struct tls_connection *conn,
- const char *label, size_t len);
+ size_t len);
int eap_fast_derive_eap_msk(const u8 *simck, u8 *msk);
int eap_fast_derive_eap_emsk(const u8 *simck, u8 *emsk);
int eap_fast_parse_tlv(struct eap_fast_tlv_parse *tlv,
diff --git a/src/eap_peer/eap_fast.c b/src/eap_peer/eap_fast.c
index f03cd4a..7702033 100644
--- a/src/eap_peer/eap_fast.c
+++ b/src/eap_peer/eap_fast.c
@@ -275,7 +275,7 @@ static int eap_fast_derive_key_auth(struct eap_sm *sm,
* Extra key material after TLS key_block: session_key_seed[40]
*/
- sks = eap_fast_derive_key(sm->ssl_ctx, data->ssl.conn, "key expansion",
+ sks = eap_fast_derive_key(sm->ssl_ctx, data->ssl.conn,
EAP_FAST_SKS_LEN);
if (sks == NULL) {
wpa_printf(MSG_DEBUG, "EAP-FAST: Failed to derive "
@@ -303,7 +303,6 @@ static int eap_fast_derive_key_provisioning(struct eap_sm *sm,
os_free(data->key_block_p);
data->key_block_p = (struct eap_fast_key_block_provisioning *)
eap_fast_derive_key(sm->ssl_ctx, data->ssl.conn,
- "key expansion",
sizeof(*data->key_block_p));
if (data->key_block_p == NULL) {
wpa_printf(MSG_DEBUG, "EAP-FAST: Failed to derive key block");
diff --git a/src/eap_peer/eap_tls_common.c b/src/eap_peer/eap_tls_common.c
index 406c162..0dcb9c1 100644
--- a/src/eap_peer/eap_tls_common.c
+++ b/src/eap_peer/eap_tls_common.c
@@ -328,8 +328,8 @@ u8 * eap_peer_tls_derive_key(struct eap_sm *sm, struct eap_ssl_data *data,
if (out == NULL)
return NULL;
- if (tls_connection_prf(data->ssl_ctx, data->conn, label, 0, 0,
- out, len)) {
+ if (tls_connection_export_key(data->ssl_ctx, data->conn, label, out,
+ len)) {
os_free(out);
return NULL;
}
diff --git a/src/eap_server/eap_server_fast.c b/src/eap_server/eap_server_fast.c
index 6993159..2049172 100644
--- a/src/eap_server/eap_server_fast.c
+++ b/src/eap_server/eap_server_fast.c
@@ -278,7 +278,7 @@ static void eap_fast_derive_key_auth(struct eap_sm *sm,
* Extra key material after TLS key_block: session_key_seed[40]
*/
- sks = eap_fast_derive_key(sm->ssl_ctx, data->ssl.conn, "key expansion",
+ sks = eap_fast_derive_key(sm->ssl_ctx, data->ssl.conn,
EAP_FAST_SKS_LEN);
if (sks == NULL) {
wpa_printf(MSG_DEBUG, "EAP-FAST: Failed to derive "
@@ -305,7 +305,6 @@ static void eap_fast_derive_key_provisioning(struct eap_sm *sm,
os_free(data->key_block_p);
data->key_block_p = (struct eap_fast_key_block_provisioning *)
eap_fast_derive_key(sm->ssl_ctx, data->ssl.conn,
- "key expansion",
sizeof(*data->key_block_p));
if (data->key_block_p == NULL) {
wpa_printf(MSG_DEBUG, "EAP-FAST: Failed to derive key block");
--
2.8.0.rc3.226.g39d4020
From 415c00bdbad6498137300230bfb9597c70ff288e Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@xxxxxxxxxx>
Date: Tue, 17 May 2016 13:24:43 -0400
Subject: [PATCH 2/2] OpenSSL: Don't implement tls_connection_get_eap_fast_key
if EAP-FAST is disabled.
This avoids internal access of structs and also removes the dependency on the
reimplemented TLS PRF functions when EAP-FAST support is not enabled. Notably,
BoringSSL doesn't support EAP-FAST, so there is no need access its internals
with openssl_get_keyblock_size.
Signed-Off-By: David Benjamin <davidben@xxxxxxxxxx>
---
src/crypto/tls_openssl.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
index e53cf9d..f880ea7 100644
--- a/src/crypto/tls_openssl.c
+++ b/src/crypto/tls_openssl.c
@@ -3087,8 +3087,9 @@ int tls_connection_get_random(void *ssl_ctx, struct tls_connection *conn,
return 0;
}
-
-#ifndef CONFIG_FIPS
+#if !defined(CONFIG_FIPS) || \
+ (!defined(EAP_FAST) && !defined(EAP_FAST_DYNAMIC) && \
+ !defined(EAP_SERVER_FAST))
static int openssl_get_keyblock_size(SSL *ssl)
{
#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
@@ -3143,7 +3144,7 @@ static int openssl_get_keyblock_size(SSL *ssl)
EVP_CIPHER_iv_length(c));
#endif
}
-#endif /* CONFIG_FIPS */
+#endif /* CONFIG_FIPS || !EAP_FAST */
int tls_connection_export_key(void *tls_ctx, struct tls_connection *conn,
@@ -3162,11 +3163,13 @@ int tls_connection_export_key(void *tls_ctx, struct tls_connection *conn,
int tls_connection_get_eap_fast_key(void *tls_ctx, struct tls_connection *conn,
u8 *out, size_t out_len)
{
-#ifdef CONFIG_FIPS
+#if !defined(EAP_FAST) && !defined(EAP_FAST_DYNAMIC) && !defined(EAP_SERVER_FAST)
+ return -1;
+#elif defined(CONFIG_FIPS)
wpa_printf(MSG_ERROR, "OpenSSL: TLS keys cannot be exported in FIPS "
"mode");
return -1;
-#else /* CONFIG_FIPS */
+#else
SSL *ssl;
SSL_SESSION *sess;
u8 *rnd;
@@ -3235,7 +3238,7 @@ int tls_connection_get_eap_fast_key(void *tls_ctx, struct tls_connection *conn,
bin_clear_free(tmp_out, skip);
return ret;
-#endif /* CONFIG_FIPS */
+#endif
}
--
2.8.0.rc3.226.g39d4020
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
