[PATCH v3 1/2] eap-tls-common: update to new ELL TLS APIs

[PATCH v3 1/2] eap-tls-common: update to new ELL TLS APIs

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

---
 src/eap-tls-common.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

-v3:
 * Fixed the ca_cert cleanup, l_certchain_free was being used rather
   than l_cert_free.

diff --git a/src/eap-tls-common.c b/src/eap-tls-common.c
index b069fcfd..b7a9b674 100644
--- a/src/eap-tls-common.c
+++ b/src/eap-tls-common.c
@@ -523,6 +523,9 @@ static int eap_tls_handle_fragmented_request(struct eap_state *eap,
 static bool eap_tls_tunnel_init(struct eap_state *eap)
 {
 	struct eap_tls_state *eap_tls = eap_get_data(eap);
+	struct l_certchain *client_cert;
+	struct l_key *client_key;
+	struct l_queue *ca_cert;
 
 	if (eap_tls->tunnel)
 		return false;
@@ -543,14 +546,19 @@ static bool eap_tls_tunnel_init(struct eap_state *eap)
 		l_tls_set_debug(eap_tls->tunnel, eap_tls_tunnel_debug, eap,
 									NULL);
 
-	if (!l_tls_set_auth_data(eap_tls->tunnel, eap_tls->client_cert,
-							eap_tls->client_key,
-							eap_tls->passphrase) ||
-			(eap_tls->ca_cert &&
-				!l_tls_set_cacert(eap_tls->tunnel,
-							eap_tls->ca_cert))) {
+	client_cert = l_pem_load_certificate_chain(eap_tls->client_cert);
+	client_key = l_pem_load_private_key(eap_tls->client_key,
+						eap_tls->passphrase, NULL);
+	ca_cert = l_pem_load_certificate_list(eap_tls->ca_cert);
+
+	if (!l_tls_set_auth_data(eap_tls->tunnel, client_cert, client_key) ||
+			(ca_cert &&
+				!l_tls_set_cacert(eap_tls->tunnel, ca_cert))) {
 		l_error("%s: Error loading TLS keys or certificates.",
 						eap_get_method_name(eap));
+		l_certchain_free(client_cert);
+		l_key_free(client_key);
+		l_queue_destroy(ca_cert, (l_queue_destroy_func_t)l_cert_free);
 		return false;
 	}
 
-- 
2.17.1

[PATCH v3 2/2] unit: update test-eapol to new ELL APIs

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

---
 unit/test-eapol.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/unit/test-eapol.c b/unit/test-eapol.c
index c43fe278..109c93bb 100644
--- a/unit/test-eapol.c
+++ b/unit/test-eapol.c
@@ -2906,6 +2906,9 @@ static void eapol_sm_test_tls(struct eapol_8021x_tls_test_state *s,
 	struct eapol_key *step1, *step2, *step3, *step4;
 	uint8_t ptk_buf[64];
 	uint8_t *ptk;
+	struct l_certchain *server_cert;
+	struct l_key *server_key;
+	struct l_queue *ca_cert;
 
 	aa = ap_address;
 	spa = sta_address;
@@ -2955,9 +2958,13 @@ static void eapol_sm_test_tls(struct eapol_8021x_tls_test_state *s,
 	s->tx_buf_len = 0;
 	s->tx_buf_offset = 0;
 
-	assert(l_tls_set_auth_data(s->tls, CERTDIR "cert-server.pem",
-				CERTDIR "cert-server-key-pkcs8.pem", NULL));
-	assert(l_tls_set_cacert(s->tls, CERTDIR "cert-ca.pem"));
+	server_cert = l_pem_load_certificate_chain(CERTDIR "cert-server.pem");
+	server_key = l_pem_load_private_key(CERTDIR "cert-server-key-pkcs8.pem",
+						NULL, NULL);
+	ca_cert = l_pem_load_certificate_list(CERTDIR "cert-ca.pem");
+
+	assert(l_tls_set_auth_data(s->tls, server_cert, server_key));
+	assert(l_tls_set_cacert(s->tls, ca_cert));
 	assert(l_tls_start(s->tls));
 
 	ths->handshake_failed = false;
-- 
2.17.1

Re: [PATCH v3 2/2] unit: update test-eapol to new ELL APIs

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 256 bytes --]

Hi James,

On 10/1/19 4:38 PM, James Prestwood wrote:
> ---
>   unit/test-eapol.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 

I applied this one with a few extra asserts thrown in.  Please review.

Regards,
-Denis

Re: [PATCH v3 1/2] eap-tls-common: update to new ELL TLS APIs

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]

Hi James,

On 10/1/19 4:38 PM, James Prestwood wrote:
> ---
>   src/eap-tls-common.c | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
> 
> -v3:
>   * Fixed the ca_cert cleanup, l_certchain_free was being used rather
>     than l_cert_free.
> 

I ended up pushing my own version of this that was extra paranoid.  The 
issue is that since the certificates are on disk, we can't simply assume 
that they will be loaded successfully (as they might have been 
inadvertently or maliciously changed between the time check_settings and 
tunnel_init are called).

It might actually make sense to load the keys at load_settings time or 
(or even earlier) to avoid extra latency during connection setup time.

Please review.

Regards,
-Denis