diff mbox series

Implement new functions for EAP:

Message ID 20210826092534.14538-1-juliusz@wolfssl.com
State Accepted
Headers show
Series Implement new functions for EAP: | expand

Commit Message

Juliusz Sosinowicz Aug. 26, 2021, 9:25 a.m. UTC
- `tls_get_tls_unique`
- `tls_connection_get_cipher_suite`
- `tls_connection_get_peer_subject`
- `tls_connection_get_own_cert_used`

The necessary wolfSSL changes are located in https://github.com/wolfSSL/wolfssl/pull/4205 .

Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com>
---
 src/crypto/crypto_wolfssl.c               | 12 ++++
 src/crypto/tls_wolfssl.c                  | 83 +++++++++++++++++++++--
 tests/hwsim/example-hostapd.config        |  3 +
 tests/hwsim/example-wpa_supplicant.config |  3 +
 tests/hwsim/test_ap_eap.py                |  8 +--
 tests/hwsim/test_dpp.py                   |  2 +-
 tests/hwsim/test_eap.py                   |  2 +-
 tests/hwsim/test_fils.py                  | 13 ++--
 8 files changed, 110 insertions(+), 16 deletions(-)

Comments

Jouni Malinen April 18, 2022, 2:09 p.m. UTC | #1
On Thu, Aug 26, 2021 at 11:25:34AM +0200, Juliusz Sosinowicz wrote:
> - `tls_get_tls_unique`
> - `tls_connection_get_cipher_suite`
> - `tls_connection_get_peer_subject`
> - `tls_connection_get_own_cert_used`
> 
> The necessary wolfSSL changes are located in https://github.com/wolfSSL/wolfssl/pull/4205 .

Thanks, applied with some cleanup in a number of separate commits.

While testing these with various wolfSSL versions, I noticed that there
were some regressions or old issues being revealed by additional
functionality getting enabled. I fixed some of these, but it looks like
there is still something wrong going on whenever TLS 1.3 is enabled and
the wpa_supplicant mechanism of disabling that by default not working
with wolfSSL. I ended up doing most of the testing with --disable-tls13
build of wolfSSL to verify previously working functionality.

As far as the now enabled TLS session resumption and caching is
concerned, it looks looks like there might be a memory leak in the
WOLFSSL_SESSION ex_data handling. tls_connection_set_success_data() adds
a heap allocated memory pointer with wolfSSL_SESSION_set_ex_data() and
that buffer is not always cleared. It looks like remove_session_cb() is
supposed to do that, but there is no mechanism to force that to happen
when exiting the process. It looks like there is
wolfSSL_flush_sessions(), but that is not called and even it it were,
the current implementation of it does not really do anything. I'm not
sure whether this is a real memory leak in the sense of each session
leaking one instance or whether this just shows up since the session
entry has not timed out and it would have been freed when processing the
eventual timeout.
Juliusz Sosinowicz April 20, 2022, 9:01 a.m. UTC | #2
Hi Jouni,

thank you for your feedback. We are aware of the issue with TLS 1.3 in 
wpa_supplicant with wolfSSL. We have put in work to fix this and I am 
preparing another patch that will add TLS 1.3 support along with a few 
other new features when using wolfSSL as the crypto/TLS backend.

This memory leak hasn't been reported before. I will fix it and include 
it in the upcoming patch.

Sincerely
Juliusz Sosinowicz

On 18/04/2022 16:09, Jouni Malinen wrote:
> On Thu, Aug 26, 2021 at 11:25:34AM +0200, Juliusz Sosinowicz wrote:
>> - `tls_get_tls_unique`
>> - `tls_connection_get_cipher_suite`
>> - `tls_connection_get_peer_subject`
>> - `tls_connection_get_own_cert_used`
>>
>> The necessary wolfSSL changes are located in https://github.com/wolfSSL/wolfssl/pull/4205 .
> Thanks, applied with some cleanup in a number of separate commits.
>
> While testing these with various wolfSSL versions, I noticed that there
> were some regressions or old issues being revealed by additional
> functionality getting enabled. I fixed some of these, but it looks like
> there is still something wrong going on whenever TLS 1.3 is enabled and
> the wpa_supplicant mechanism of disabling that by default not working
> with wolfSSL. I ended up doing most of the testing with --disable-tls13
> build of wolfSSL to verify previously working functionality.
>
> As far as the now enabled TLS session resumption and caching is
> concerned, it looks looks like there might be a memory leak in the
> WOLFSSL_SESSION ex_data handling. tls_connection_set_success_data() adds
> a heap allocated memory pointer with wolfSSL_SESSION_set_ex_data() and
> that buffer is not always cleared. It looks like remove_session_cb() is
> supposed to do that, but there is no mechanism to force that to happen
> when exiting the process. It looks like there is
> wolfSSL_flush_sessions(), but that is not called and even it it were,
> the current implementation of it does not really do anything. I'm not
> sure whether this is a real memory leak in the sense of each session
> leaking one instance or whether this just shows up since the session
> entry has not timed out and it would have been freed when processing the
> eventual timeout.
>
Juliusz Sosinowicz May 6, 2022, 10:25 a.m. UTC | #3
Hi Jouni,

I believe that we have a proposed fix for the memory leak you found. The 
issue happens to be in our library. You can find it and test it here: 
https://github.com/wolfSSL/wolfssl/pull/5107

Sincerely
Juliusz

On 20/04/2022 11:01, Juliusz Sosinowicz wrote:
> Hi Jouni,
>
> thank you for your feedback. We are aware of the issue with TLS 1.3 in 
> wpa_supplicant with wolfSSL. We have put in work to fix this and I am 
> preparing another patch that will add TLS 1.3 support along with a few 
> other new features when using wolfSSL as the crypto/TLS backend.
>
> This memory leak hasn't been reported before. I will fix it and 
> include it in the upcoming patch.
>
> Sincerely
> Juliusz Sosinowicz
>
> On 18/04/2022 16:09, Jouni Malinen wrote:
>> On Thu, Aug 26, 2021 at 11:25:34AM +0200, Juliusz Sosinowicz wrote:
>>> - `tls_get_tls_unique`
>>> - `tls_connection_get_cipher_suite`
>>> - `tls_connection_get_peer_subject`
>>> - `tls_connection_get_own_cert_used`
>>>
>>> The necessary wolfSSL changes are located in 
>>> https://github.com/wolfSSL/wolfssl/pull/4205 .
>> Thanks, applied with some cleanup in a number of separate commits.
>>
>> While testing these with various wolfSSL versions, I noticed that there
>> were some regressions or old issues being revealed by additional
>> functionality getting enabled. I fixed some of these, but it looks like
>> there is still something wrong going on whenever TLS 1.3 is enabled and
>> the wpa_supplicant mechanism of disabling that by default not working
>> with wolfSSL. I ended up doing most of the testing with --disable-tls13
>> build of wolfSSL to verify previously working functionality.
>>
>> As far as the now enabled TLS session resumption and caching is
>> concerned, it looks looks like there might be a memory leak in the
>> WOLFSSL_SESSION ex_data handling. tls_connection_set_success_data() adds
>> a heap allocated memory pointer with wolfSSL_SESSION_set_ex_data() and
>> that buffer is not always cleared. It looks like remove_session_cb() is
>> supposed to do that, but there is no mechanism to force that to happen
>> when exiting the process. It looks like there is
>> wolfSSL_flush_sessions(), but that is not called and even it it were,
>> the current implementation of it does not really do anything. I'm not
>> sure whether this is a real memory leak in the sense of each session
>> leaking one instance or whether this just shows up since the session
>> entry has not timed out and it would have been freed when processing the
>> eventual timeout.
>>
diff mbox series

Patch

diff --git a/src/crypto/crypto_wolfssl.c b/src/crypto/crypto_wolfssl.c
index 2e4bf8962..afb8f40d4 100644
--- a/src/crypto/crypto_wolfssl.c
+++ b/src/crypto/crypto_wolfssl.c
@@ -409,8 +409,11 @@  int aes_128_cbc_decrypt(const u8 *key, const u8 *iv, u8 *data, size_t data_len)
 }
 
 
+#ifndef CONFIG_FIPS
+#ifndef CONFIG_OPENSSL_INTERNAL_AES_WRAP
 int aes_wrap(const u8 *kek, size_t kek_len, int n, const u8 *plain, u8 *cipher)
 {
+#ifdef HAVE_AES_KEYWRAP
 	int ret;
 
 	if (TEST_FAIL())
@@ -419,12 +422,16 @@  int aes_wrap(const u8 *kek, size_t kek_len, int n, const u8 *plain, u8 *cipher)
 	ret = wc_AesKeyWrap(kek, kek_len, plain, n * 8, cipher, (n + 1) * 8,
 			    NULL);
 	return ret != (n + 1) * 8 ? -1 : 0;
+#else
+	return -1;
+#endif /* HAVE_AES_KEYWRAP */
 }
 
 
 int aes_unwrap(const u8 *kek, size_t kek_len, int n, const u8 *cipher,
 	       u8 *plain)
 {
+#ifdef HAVE_AES_KEYWRAP
 	int ret;
 
 	if (TEST_FAIL())
@@ -433,7 +440,12 @@  int aes_unwrap(const u8 *kek, size_t kek_len, int n, const u8 *cipher,
 	ret = wc_AesKeyUnWrap(kek, kek_len, cipher, (n + 1) * 8, plain, n * 8,
 			      NULL);
 	return ret != n * 8 ? -1 : 0;
+#else
+    return -1;
+#endif /* HAVE_AES_KEYWRAP */
 }
+#endif /* CONFIG_OPENSSL_INTERNAL_AES_WRAP */
+#endif /* CONFIG_FIPS */
 
 
 #ifndef CONFIG_NO_RC4
diff --git a/src/crypto/tls_wolfssl.c b/src/crypto/tls_wolfssl.c
index cf482bfc3..1f695985f 100644
--- a/src/crypto/tls_wolfssl.c
+++ b/src/crypto/tls_wolfssl.c
@@ -58,6 +58,7 @@  struct tls_context {
 	void *cb_ctx;
 	int cert_in_cb;
 	char *ocsp_stapling_response;
+    unsigned int tls_session_lifetime;
 };
 
 static struct tls_context *tls_global = NULL;
@@ -94,6 +95,7 @@  struct tls_connection {
 	WOLFSSL_X509 *peer_cert;
 	WOLFSSL_X509 *peer_issuer;
 	WOLFSSL_X509 *peer_issuer_issuer;
+    char *peer_subject; /* peer subject info for authenticated peer */
 };
 
 
@@ -189,6 +191,11 @@  static void remove_session_cb(WOLFSSL_CTX *ctx, WOLFSSL_SESSION *sess)
 	wolfSSL_SESSION_set_ex_data(sess, tls_ex_idx_session, NULL);
 }
 
+void wolfSSL_logging_cb(const int logLevel, const char *const logMessage)
+{
+	(void)logLevel;
+	wpa_printf(MSG_DEBUG, "wolfSSL log:%s", logMessage);
+}
 
 void * tls_init(const struct tls_config *conf)
 {
@@ -197,6 +204,7 @@  void * tls_init(const struct tls_config *conf)
 	const char *ciphers;
 
 #ifdef DEBUG_WOLFSSL
+	wolfSSL_SetLoggingCb(wolfSSL_logging_cb);
 	wolfSSL_Debugging_ON();
 #endif /* DEBUG_WOLFSSL */
 
@@ -227,17 +235,20 @@  void * tls_init(const struct tls_config *conf)
 	}
 	wolfSSL_SetIORecv(ssl_ctx, wolfssl_receive_cb);
 	wolfSSL_SetIOSend(ssl_ctx, wolfssl_send_cb);
+	context->tls_session_lifetime = conf->tls_session_lifetime;
 	wolfSSL_CTX_set_ex_data(ssl_ctx, 0, context);
 
 	if (conf->tls_session_lifetime > 0) {
+	    wolfSSL_CTX_set_session_id_context(ssl_ctx,
+	            (const unsigned char*)"hostapd", 7);
 		wolfSSL_CTX_set_quiet_shutdown(ssl_ctx, 1);
 		wolfSSL_CTX_set_session_cache_mode(ssl_ctx,
-						   SSL_SESS_CACHE_SERVER);
+		        WOLFSSL_SESS_CACHE_SERVER);
 		wolfSSL_CTX_set_timeout(ssl_ctx, conf->tls_session_lifetime);
 		wolfSSL_CTX_sess_set_remove_cb(ssl_ctx, remove_session_cb);
 	} else {
 		wolfSSL_CTX_set_session_cache_mode(ssl_ctx,
-						   SSL_SESS_CACHE_CLIENT);
+		        WOLFSSL_SESS_CACHE_OFF);
 	}
 
 	if (conf && conf->openssl_ciphers)
@@ -336,6 +347,7 @@  void tls_connection_deinit(void *tls_ctx, struct tls_connection *conn)
 	os_free(conn->alt_subject_match);
 	os_free(conn->suffix_match);
 	os_free(conn->domain_match);
+    os_free(conn->peer_subject);
 
 	/* self */
 	os_free(conn);
@@ -1134,6 +1146,11 @@  static int tls_verify_cb(int preverify_ok, WOLFSSL_X509_STORE_CTX *x509_ctx)
 		context->event_cb(context->cb_ctx,
 				  TLS_CERT_CHAIN_SUCCESS, NULL);
 
+    if (depth == 0 && preverify_ok) {
+        os_free(conn->peer_subject);
+        conn->peer_subject = os_strdup(buf);
+    }
+
 	return preverify_ok;
 }
 
@@ -1238,10 +1255,8 @@  static int tls_connection_ca_cert(void *tls_ctx, struct tls_connection *conn,
 static void tls_set_conn_flags(WOLFSSL *ssl, unsigned int flags)
 {
 #ifdef HAVE_SESSION_TICKET
-#if 0
 	if (!(flags & TLS_CONN_DISABLE_SESSION_TICKET))
 		wolfSSL_UseSessionTicket(ssl);
-#endif
 #endif /* HAVE_SESSION_TICKET */
 
 	if (flags & TLS_CONN_DISABLE_TLSv1_0)
@@ -1590,6 +1605,8 @@  int tls_connection_set_verify(void *ssl_ctx, struct tls_connection *conn,
 			      int verify_peer, unsigned int flags,
 			      const u8 *session_ctx, size_t session_ctx_len)
 {
+    static int counter = 0;
+    struct tls_context *context;
 	if (!conn)
 		return -1;
 
@@ -1607,6 +1624,23 @@  int tls_connection_set_verify(void *ssl_ctx, struct tls_connection *conn,
 
 	wolfSSL_set_accept_state(conn->ssl);
 
+	context = wolfSSL_CTX_get_ex_data((WOLFSSL_CTX*)ssl_ctx, 0);
+	if (context && context->tls_session_lifetime == 0) {
+        /*
+         * Set session id context to a unique value to make sure
+         * session resumption cannot be used either through session
+         * caching or TLS ticket extension.
+         */
+        counter++;
+        wolfSSL_set_session_id_context(conn->ssl,
+                       (const unsigned char *) &counter,
+                       sizeof(counter));
+	}
+	else
+        wolfSSL_set_session_id_context(conn->ssl, session_ctx,
+                session_ctx_len);
+	(void)context;
+
 	/* TODO: do we need to fake a session like OpenSSL does here? */
 
 	return 0;
@@ -2160,6 +2194,39 @@  void tls_connection_remove_session(struct tls_connection *conn)
 }
 
 
+int tls_get_tls_unique(struct tls_connection *conn, u8 *buf, size_t max_len)
+{
+    size_t len;
+    int reused;
+
+    reused = wolfSSL_session_reused(conn->ssl);
+    if ((wolfSSL_is_server(conn->ssl) && !reused) ||
+            (!wolfSSL_is_server(conn->ssl) && reused))
+        len = wolfSSL_get_peer_finished(conn->ssl, buf, max_len);
+    else
+        len = wolfSSL_get_finished(conn->ssl, buf, max_len);
+
+    if (len == 0 || len > max_len)
+        return -1;
+
+    return len;
+}
+
+
+u16 tls_connection_get_cipher_suite(struct tls_connection *conn)
+{
+    return (u16)wolfSSL_get_current_cipher_suite(conn->ssl);
+}
+
+
+const char * tls_connection_get_peer_subject(struct tls_connection *conn)
+{
+    if (conn)
+        return conn->peer_subject;
+    return NULL;
+}
+
+
 void tls_connection_set_success_data(struct tls_connection *conn,
 				     struct wpabuf *data)
 {
@@ -2206,3 +2273,11 @@  tls_connection_get_success_data(struct tls_connection *conn)
 		return NULL;
 	return wolfSSL_SESSION_get_ex_data(sess, tls_ex_idx_session);
 }
+
+
+bool tls_connection_get_own_cert_used(struct tls_connection *conn)
+{
+    if (conn)
+        return wolfSSL_get_certificate(conn->ssl) != NULL;
+    return false;
+}
diff --git a/tests/hwsim/example-hostapd.config b/tests/hwsim/example-hostapd.config
index d01a1d2ed..5b7130fdc 100644
--- a/tests/hwsim/example-hostapd.config
+++ b/tests/hwsim/example-hostapd.config
@@ -36,6 +36,9 @@  CONFIG_EAP_UNAUTH_TLS=y
 ifeq ($(CONFIG_TLS), openssl)
 CONFIG_EAP_PWD=y
 endif
+ifeq ($(CONFIG_TLS), wolfssl)
+CONFIG_EAP_PWD=y
+endif
 CONFIG_EAP_EKE=y
 CONFIG_PKCS12=y
 CONFIG_RADIUS_SERVER=y
diff --git a/tests/hwsim/example-wpa_supplicant.config b/tests/hwsim/example-wpa_supplicant.config
index 5e5acd695..ea6ef7d27 100644
--- a/tests/hwsim/example-wpa_supplicant.config
+++ b/tests/hwsim/example-wpa_supplicant.config
@@ -38,6 +38,9 @@  CONFIG_EAP_IKEV2=y
 ifeq ($(CONFIG_TLS), openssl)
 CONFIG_EAP_PWD=y
 endif
+ifeq ($(CONFIG_TLS), wolfssl)
+CONFIG_EAP_PWD=y
+endif
 
 CONFIG_USIM_SIMULATOR=y
 CONFIG_SIM_SIMULATOR=y
diff --git a/tests/hwsim/test_ap_eap.py b/tests/hwsim/test_ap_eap.py
index 269500a93..7cf5d9242 100644
--- a/tests/hwsim/test_ap_eap.py
+++ b/tests/hwsim/test_ap_eap.py
@@ -50,7 +50,7 @@  def check_subject_match_support(dev):
 
 def check_check_cert_subject_support(dev):
     tls = dev.request("GET tls_library")
-    if not tls.startswith("OpenSSL"):
+    if not tls.startswith("OpenSSL") and not tls.startswith("wolfSSL"):
         raise HwsimSkip("check_cert_subject not supported with this TLS library: " + tls)
 
 def check_altsubject_match_support(dev):
@@ -3881,7 +3881,7 @@  def test_ap_wpa2_eap_fast_prf_oom(dev, apdev):
     """WPA2-Enterprise connection using EAP-FAST and OOM in PRF"""
     check_eap_capa(dev[0], "FAST")
     tls = dev[0].request("GET tls_library")
-    if tls.startswith("OpenSSL"):
+    if tls.startswith("OpenSSL") or tls.startswith("wolfSSL"):
         func = "tls_connection_get_eap_fast_key"
         count = 2
     elif tls.startswith("internal"):
@@ -6161,11 +6161,11 @@  def test_rsn_ie_proto_eap_sta(dev, apdev):
 
 def check_tls_session_resumption_capa(dev, hapd):
     tls = hapd.request("GET tls_library")
-    if not tls.startswith("OpenSSL"):
+    if not tls.startswith("OpenSSL") and not tls.startswith("wolfSSL"):
         raise HwsimSkip("hostapd TLS library is not OpenSSL or wolfSSL: " + tls)
 
     tls = dev.request("GET tls_library")
-    if not tls.startswith("OpenSSL"):
+    if not tls.startswith("OpenSSL") and not tls.startswith("wolfSSL"):
         raise HwsimSkip("Session resumption not supported with this TLS library: " + tls)
 
 def test_eap_ttls_pap_session_resumption(dev, apdev):
diff --git a/tests/hwsim/test_dpp.py b/tests/hwsim/test_dpp.py
index 71df7fc64..dba30bd0a 100644
--- a/tests/hwsim/test_dpp.py
+++ b/tests/hwsim/test_dpp.py
@@ -38,7 +38,7 @@  def check_dpp_capab(dev, brainpool=False, min_ver=1):
         raise HwsimSkip("DPP not supported")
     if brainpool:
         tls = dev.request("GET tls_library")
-        if not tls.startswith("OpenSSL") or "run=BoringSSL" in tls:
+        if (not tls.startswith("OpenSSL") or "run=BoringSSL" in tls) and not tls.startswith("wolfSSL"):
             raise HwsimSkip("Crypto library does not support Brainpool curves: " + tls)
     capa = dev.request("GET_CAPABILITY dpp")
     ver = 1
diff --git a/tests/hwsim/test_eap.py b/tests/hwsim/test_eap.py
index 144e4d314..d3bbec3d4 100644
--- a/tests/hwsim/test_eap.py
+++ b/tests/hwsim/test_eap.py
@@ -440,7 +440,7 @@  def test_eap_teap_tls_cs_sha384(dev, apdev):
 def run_eap_teap_tls_cs(dev, apdev, cipher):
     check_eap_capa(dev[0], "TEAP")
     tls = dev[0].request("GET tls_library")
-    if not tls.startswith("OpenSSL"):
+    if not tls.startswith("OpenSSL") and not tls.startswith("wolfSSL"):
         raise HwsimSkip("TLS library not supported for TLS CS configuration: " + tls)
     params = int_teap_server_params(eap_teap_auth="1")
     params['openssl_ciphers'] = cipher
diff --git a/tests/hwsim/test_fils.py b/tests/hwsim/test_fils.py
index 4d4ddc39a..ffb063ba4 100644
--- a/tests/hwsim/test_fils.py
+++ b/tests/hwsim/test_fils.py
@@ -1419,12 +1419,13 @@  def run_fils_sk_pfs(dev, apdev, group, params):
     check_erp_capa(dev[0])
 
     tls = dev[0].request("GET tls_library")
-    if int(group) in [25]:
-        if not (tls.startswith("OpenSSL") and ("build=OpenSSL 1.0.2" in tls or "build=OpenSSL 1.1" in tls) and ("run=OpenSSL 1.0.2" in tls or "run=OpenSSL 1.1" in tls)):
-            raise HwsimSkip("EC group not supported")
-    if int(group) in [27, 28, 29, 30]:
-        if not (tls.startswith("OpenSSL") and ("build=OpenSSL 1.0.2" in tls or "build=OpenSSL 1.1" in tls) and ("run=OpenSSL 1.0.2" in tls or "run=OpenSSL 1.1" in tls)):
-            raise HwsimSkip("Brainpool EC group not supported")
+    if not tls.startswith("wolfSSL"):
+        if int(group) in [25]:
+            if not (tls.startswith("OpenSSL") and ("build=OpenSSL 1.0.2" in tls or "build=OpenSSL 1.1" in tls) and ("run=OpenSSL 1.0.2" in tls or "run=OpenSSL 1.1" in tls)):
+                raise HwsimSkip("EC group not supported")
+        if int(group) in [27, 28, 29, 30]:
+            if not (tls.startswith("OpenSSL") and ("build=OpenSSL 1.0.2" in tls or "build=OpenSSL 1.1" in tls) and ("run=OpenSSL 1.0.2" in tls or "run=OpenSSL 1.1" in tls)):
+                raise HwsimSkip("Brainpool EC group not supported")
 
     start_erp_as(msk_dump=os.path.join(params['logdir'], "msk.lst"))