iwd.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] storage: Add TLS session cache file read/write utils
@ 2022-11-09 17:04 Andrew Zaborowski
  2022-11-09 17:04 ` [PATCH 2/2] eap-tls: Add session caching Andrew Zaborowski
  2022-11-09 20:32 ` [PATCH 1/2] storage: Add TLS session cache file read/write utils Denis Kenzior
  0 siblings, 2 replies; 8+ messages in thread
From: Andrew Zaborowski @ 2022-11-09 17:04 UTC (permalink / raw)
  To: iwd

Add storage_tls_session_cache_{load,sync} similar to
storage_known_frequencies_{load,sync}.
---
 src/storage.c | 35 +++++++++++++++++++++++++++++++++++
 src/storage.h |  3 +++
 2 files changed, 38 insertions(+)

diff --git a/src/storage.c b/src/storage.c
index d6e478bd..b2c5ed48 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -53,6 +53,7 @@
 #define STORAGE_FILE_MODE (S_IRUSR | S_IWUSR)
 
 #define KNOWN_FREQ_FILENAME ".known_network.freq"
+#define TLS_CACHE_FILENAME ".tls-session-cache"
 
 static char *storage_path = NULL;
 static char *storage_hotspot_path = NULL;
@@ -701,6 +702,40 @@ void storage_known_frequencies_sync(struct l_settings *known_freqs)
 	l_free(known_freq_file_path);
 }
 
+struct l_settings *storage_tls_session_cache_load(void)
+{
+	_auto_(l_settings_free) struct l_settings *cache = l_settings_new();
+	_auto_(l_free) char *tls_cache_file_path =
+		storage_get_path("%s", TLS_CACHE_FILENAME);
+
+	if (unlikely(!l_settings_load_from_file(cache, tls_cache_file_path)))
+		return NULL;
+
+	return l_steal_ptr(cache);
+}
+
+void storage_tls_session_cache_sync(struct l_settings *cache)
+{
+	_auto_(l_free) char *tls_cache_file_path = NULL;
+	_auto_(l_free) char *data = NULL;
+	size_t len;
+
+	if (!cache)
+		return;
+
+	tls_cache_file_path = storage_get_path("%s", TLS_CACHE_FILENAME);
+	data = l_settings_to_data(cache, &len);
+
+	/*
+	 * Note this data contains cryptographic secrets.  write_file()
+	 * happens to set the right permissions on the file.
+	 *
+	 * TODO: consider encrypting with system_key.
+	 */
+	write_file(data, len, false, "%s", tls_cache_file_path);
+	explicit_bzero(data, len);
+}
+
 bool storage_is_file(const char *filename)
 {
 	char *path;
diff --git a/src/storage.h b/src/storage.h
index 6877fb65..fe6ddbf5 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -51,6 +51,9 @@ int storage_network_remove(enum security type, const char *ssid);
 struct l_settings *storage_known_frequencies_load(void);
 void storage_known_frequencies_sync(struct l_settings *known_freqs);
 
+struct l_settings *storage_tls_session_cache_load(void);
+void storage_tls_session_cache_sync(struct l_settings *cache);
+
 int __storage_decrypt(struct l_settings *settings, const char *ssid,
 				bool *changed);
 char *__storage_encrypt(const struct l_settings *settings, const char *ssid,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] eap-tls: Add session caching
  2022-11-09 17:04 [PATCH 1/2] storage: Add TLS session cache file read/write utils Andrew Zaborowski
@ 2022-11-09 17:04 ` Andrew Zaborowski
  2022-11-09 20:45   ` Denis Kenzior
  2022-11-09 20:32 ` [PATCH 1/2] storage: Add TLS session cache file read/write utils Denis Kenzior
  1 sibling, 1 reply; 8+ messages in thread
From: Andrew Zaborowski @ 2022-11-09 17:04 UTC (permalink / raw)
  To: iwd

Use l_tls_set_session_cache() to enable session cache/resume in the
TLS-based EAP methods.  Sessions for all 802.1x networks are stored in
one file, /var/lib/iwd/.tls-session-cache, indexed by the EAP method
name and authenticator's MAC/BSSID (handshake->aa) because that's
ultimately what identifies the authenticator which will usually be the
TLS server as also.  We may want to add the SSID to the index.  Multiple
BSSes and even multiple ESSes (SSIDs) may be backed by a single
authentication server but it's hard for us to know that.

eap_{get,set}_peer_id() API is added for eapol to set the identifier of
the authenticator (or the supplicant if we're the authenticator, if
there's ever a use case for that).

eap-tls-common.c can't call storage_tls_session_cache_{load,sync}()
directly because it's linked into ead and some unit tests without
storage.c, so station.c sets pointers to the load & sync functions using
the new eap_tls_set_session_cache_ops().
---
 src/eap-tls-common.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
 src/eap-tls-common.h |  6 +++++
 src/eap.c            | 13 +++++++++++
 src/eap.h            |  3 +++
 src/eapol.c          |  4 ++++
 src/station.c        |  6 +++++
 6 files changed, 84 insertions(+)

diff --git a/src/eap-tls-common.c b/src/eap-tls-common.c
index acc5b387..232163af 100644
--- a/src/eap-tls-common.c
+++ b/src/eap-tls-common.c
@@ -28,7 +28,9 @@
 #include <errno.h>
 #include <ell/ell.h>
 
+#include "ell/useful.h"
 #include "src/missing.h"
+#include "src/module.h"
 #include "src/eap.h"
 #include "src/eap-private.h"
 #include "src/eap-tls-common.h"
@@ -123,6 +125,10 @@ struct eap_tls_state {
 	void *variant_data;
 };
 
+static struct l_settings *eap_tls_session_cache;
+static eap_tls_session_cache_load_func_t eap_tls_session_cache_load;
+static eap_tls_session_cache_sync_func_t eap_tls_session_cache_sync;
+
 static void __eap_tls_common_state_reset(struct eap_tls_state *eap_tls)
 {
 	eap_tls->version_negotiated = EAP_TLS_VERSION_NOT_NEGOTIATED;
@@ -571,9 +577,15 @@ static int eap_tls_handle_fragmented_request(struct eap_state *eap,
 	return r;
 }
 
+static void eap_tls_session_cache_update(void *user_data)
+{
+	eap_tls_session_cache_sync(eap_tls_session_cache);
+}
+
 static bool eap_tls_tunnel_init(struct eap_state *eap)
 {
 	struct eap_tls_state *eap_tls = eap_get_data(eap);
+	_auto_(l_free) char *cache_group_name = NULL;
 
 	if (eap_tls->tunnel)
 		goto start;
@@ -633,6 +645,26 @@ static bool eap_tls_tunnel_init(struct eap_state *eap)
 	if (eap_tls->domain_mask)
 		l_tls_set_domain_mask(eap_tls->tunnel, eap_tls->domain_mask);
 
+	if (!eap_tls_session_cache_load)
+		goto start;
+
+	if (!eap_tls_session_cache) {
+		eap_tls_session_cache = eap_tls_session_cache_load();
+
+		if (!eap_tls_session_cache) {
+			eap_tls_session_cache = l_settings_new();
+			l_debug("No session cache loaded, starting with an "
+				"empty cache");
+		}
+	}
+
+	cache_group_name = l_strdup_printf("EAP-%s-%s",
+						eap_get_method_name(eap),
+						eap_get_peer_id(eap));
+	l_tls_set_session_cache(eap_tls->tunnel, eap_tls_session_cache,
+				cache_group_name, 24 * 3600 * L_USEC_PER_SEC, 0,
+				eap_tls_session_cache_update, NULL);
+
 start:
 	if (!l_tls_start(eap_tls->tunnel)) {
 		l_error("%s: Failed to start the TLS client",
@@ -1085,3 +1117,23 @@ void eap_tls_common_tunnel_close(struct eap_state *eap)
 
 	l_tls_close(eap_tls->tunnel);
 }
+
+void eap_tls_set_session_cache_ops(eap_tls_session_cache_load_func_t load,
+				eap_tls_session_cache_sync_func_t sync)
+{
+	eap_tls_session_cache_load = load;
+	eap_tls_session_cache_sync = sync;
+}
+
+static int eap_tls_common_init(void)
+{
+	return 0;
+}
+
+static void eap_tls_common_exit(void)
+{
+	l_settings_free(eap_tls_session_cache);
+	eap_tls_session_cache = NULL;
+}
+
+IWD_MODULE(eap_tls_common, eap_tls_common_init, eap_tls_common_exit);
diff --git a/src/eap-tls-common.h b/src/eap-tls-common.h
index 174770c0..748c8d15 100644
--- a/src/eap-tls-common.h
+++ b/src/eap-tls-common.h
@@ -20,6 +20,9 @@
  *
  */
 
+typedef struct l_settings *(*eap_tls_session_cache_load_func_t)(void);
+typedef void (*eap_tls_session_cache_sync_func_t)(struct l_settings *);
+
 enum eap_tls_version {
 	EAP_TLS_VERSION_0               = 0x00,
 	EAP_TLS_VERSION_1               = 0x01,
@@ -81,3 +84,6 @@ bool eap_tls_common_tunnel_prf_get_bytes(struct eap_state *eap,
 void eap_tls_common_tunnel_send(struct eap_state *eap, const uint8_t *data,
 							size_t data_len);
 void eap_tls_common_tunnel_close(struct eap_state *eap);
+
+void eap_tls_set_session_cache_ops(eap_tls_session_cache_load_func_t load,
+				eap_tls_session_cache_sync_func_t sync);
diff --git a/src/eap.c b/src/eap.c
index 6f523f2f..981b6388 100644
--- a/src/eap.c
+++ b/src/eap.c
@@ -59,6 +59,7 @@ struct eap_state {
 	char *identity;
 	char *identity_setting;
 	bool authenticator;
+	char *peer_id;
 
 	int last_id;
 	void *method_state;
@@ -154,6 +155,7 @@ void eap_free(struct eap_state *eap)
 
 	eap_free_common(eap);
 	l_timeout_remove(eap->complete_timeout);
+	eap_set_peer_id(eap, NULL);
 
 	l_free(eap);
 }
@@ -837,6 +839,17 @@ err:
 	return false;
 }
 
+void eap_set_peer_id(struct eap_state *eap, const char *id)
+{
+	l_free(eap->peer_id);
+	eap->peer_id = l_strdup(id);
+}
+
+const char *eap_get_peer_id(struct eap_state *eap)
+{
+	return eap->peer_id;
+}
+
 void eap_set_data(struct eap_state *eap, void *data)
 {
 	eap->method_state = data;
diff --git a/src/eap.h b/src/eap.h
index f1e867f5..702caf24 100644
--- a/src/eap.h
+++ b/src/eap.h
@@ -94,6 +94,9 @@ const char *eap_get_identity(struct eap_state *eap);
 
 void eap_rx_packet(struct eap_state *eap, const uint8_t *pkt, size_t len);
 
+void eap_set_peer_id(struct eap_state *eap, const char *id);
+const char *eap_get_peer_id(struct eap_state *eap);
+
 void __eap_set_config(struct l_settings *config);
 
 int eap_init(void);
diff --git a/src/eapol.c b/src/eapol.c
index 4a1abd28..8e599394 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -2770,6 +2770,9 @@ void eapol_register(struct eapol_sm *sm)
 bool eapol_start(struct eapol_sm *sm)
 {
 	if (sm->handshake->settings_8021x) {
+		const uint8_t *addr = sm->handshake->authenticator ?
+			sm->handshake->spa : sm->handshake->aa;
+
 		sm->eap = eap_new(eapol_eap_msg_cb, eapol_eap_complete_cb, sm);
 
 		if (!sm->eap)
@@ -2785,6 +2788,7 @@ bool eapol_start(struct eapol_sm *sm)
 
 		eap_set_key_material_func(sm->eap, eapol_eap_results_cb);
 		eap_set_event_func(sm->eap, eapol_eap_event_cb);
+		eap_set_peer_id(sm->eap, util_address_to_string(addr));
 	}
 
 	sm->started = true;
diff --git a/src/station.c b/src/station.c
index eab16eff..08d62870 100644
--- a/src/station.c
+++ b/src/station.c
@@ -60,6 +60,9 @@
 #include "src/sysfs.h"
 #include "src/band.h"
 #include "src/ft.h"
+#include "src/eap.h"
+#include "src/eap-tls-common.h"
+#include "src/storage.h"
 
 static struct l_queue *station_list;
 static uint32_t netdev_watch;
@@ -5139,6 +5142,9 @@ static int station_init(void)
 
 	watchlist_init(&event_watches, NULL);
 
+	eap_tls_set_session_cache_ops(storage_tls_session_cache_load,
+					storage_tls_session_cache_sync);
+
 	return 0;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] storage: Add TLS session cache file read/write utils
  2022-11-09 17:04 [PATCH 1/2] storage: Add TLS session cache file read/write utils Andrew Zaborowski
  2022-11-09 17:04 ` [PATCH 2/2] eap-tls: Add session caching Andrew Zaborowski
@ 2022-11-09 20:32 ` Denis Kenzior
  1 sibling, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2022-11-09 20:32 UTC (permalink / raw)
  To: Andrew Zaborowski, iwd

Hi Andrew,

On 11/9/22 11:04, Andrew Zaborowski wrote:
> Add storage_tls_session_cache_{load,sync} similar to
> storage_known_frequencies_{load,sync}.
> ---
>   src/storage.c | 35 +++++++++++++++++++++++++++++++++++
>   src/storage.h |  3 +++
>   2 files changed, 38 insertions(+)
> 

Applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] eap-tls: Add session caching
  2022-11-09 17:04 ` [PATCH 2/2] eap-tls: Add session caching Andrew Zaborowski
@ 2022-11-09 20:45   ` Denis Kenzior
  2022-11-09 21:28     ` Andrew Zaborowski
  0 siblings, 1 reply; 8+ messages in thread
From: Denis Kenzior @ 2022-11-09 20:45 UTC (permalink / raw)
  To: Andrew Zaborowski, iwd

Hi Andrew,

On 11/9/22 11:04, Andrew Zaborowski wrote:
> Use l_tls_set_session_cache() to enable session cache/resume in the
> TLS-based EAP methods.  Sessions for all 802.1x networks are stored in
> one file, /var/lib/iwd/.tls-session-cache, indexed by the EAP method
> name and authenticator's MAC/BSSID (handshake->aa) because that's
> ultimately what identifies the authenticator which will usually be the
> TLS server as also.  We may want to add the SSID to the index.  Multiple
> BSSes and even multiple ESSes (SSIDs) may be backed by a single
> authentication server but it's hard for us to know that.

But isn't the TLS session ultimately out to the RADIUS server?  So wouldn't the 
session id be identical between the entire ESS/SSID?  I also don't see the need 
to store the EAP type here as well.  We can't have SSIDs with multiple EAP types.

This does however bring up another point.  Shouldn't you clear out the cache 
when a network is forgotten?  In which case, how do you reload the cache in the 
EAP module?

> 
> eap_{get,set}_peer_id() API is added for eapol to set the identifier of
> the authenticator (or the supplicant if we're the authenticator, if
> there's ever a use case for that).

Okay

> 
> eap-tls-common.c can't call storage_tls_session_cache_{load,sync}()
> directly because it's linked into ead and some unit tests without
> storage.c, so station.c sets pointers to the load & sync functions using
> the new eap_tls_set_session_cache_ops().

ead should probably start using storage.c anyway since that does profile 
encryption and we ultimately want that inside ead as well.  But we can solve 
this later.

> ---
>   src/eap-tls-common.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
>   src/eap-tls-common.h |  6 +++++
>   src/eap.c            | 13 +++++++++++
>   src/eap.h            |  3 +++
>   src/eapol.c          |  4 ++++
>   src/station.c        |  6 +++++
>   6 files changed, 84 insertions(+)
> 

<snip>

> @@ -633,6 +645,26 @@ static bool eap_tls_tunnel_init(struct eap_state *eap)
>   	if (eap_tls->domain_mask)
>   		l_tls_set_domain_mask(eap_tls->tunnel, eap_tls->domain_mask);
>   
> +	if (!eap_tls_session_cache_load)
> +		goto start;
> +
> +	if (!eap_tls_session_cache) {
> +		eap_tls_session_cache = eap_tls_session_cache_load();
> +
> +		if (!eap_tls_session_cache) {
> +			eap_tls_session_cache = l_settings_new();
> +			l_debug("No session cache loaded, starting with an "
> +				"empty cache");
> +		}

Can we run into a situation where we have two TLS sessions running with the same 
cache entry and it is invalidated somehow?  Do we need to notify the other TLS 
sessions somehow?

Perhaps if we're running a multi WLAN setup and both are connected to the same SSID?

> +	}
> +

Regards,
-Denis

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] eap-tls: Add session caching
  2022-11-09 20:45   ` Denis Kenzior
@ 2022-11-09 21:28     ` Andrew Zaborowski
  2022-11-10 15:18       ` Denis Kenzior
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Zaborowski @ 2022-11-09 21:28 UTC (permalink / raw)
  To: Denis Kenzior; +Cc: iwd

Hi Denis,

On Wed, 9 Nov 2022 at 21:45, Denis Kenzior <denkenz@gmail.com> wrote:
> On 11/9/22 11:04, Andrew Zaborowski wrote:
> > Use l_tls_set_session_cache() to enable session cache/resume in the
> > TLS-based EAP methods.  Sessions for all 802.1x networks are stored in
> > one file, /var/lib/iwd/.tls-session-cache, indexed by the EAP method
> > name and authenticator's MAC/BSSID (handshake->aa) because that's
> > ultimately what identifies the authenticator which will usually be the
> > TLS server as also.  We may want to add the SSID to the index.  Multiple
> > BSSes and even multiple ESSes (SSIDs) may be backed by a single
> > authentication server but it's hard for us to know that.
>
> But isn't the TLS session ultimately out to the RADIUS server?  So wouldn't the
> session id be identical between the entire ESS/SSID?

So like I said it may be but we can't know this.  There may be no
RADIUS server: looking at 802.1x everytime it mentions RADIUS it's an
"example" AAA and also mentions Diameter.  But there may be no central
AAA server in the first place.

For example 6.3.1 says: "The Authenticator and an Authentication
Server can be co-located within the same system, allowing that system
to perform the authentication function without the need for
communication with an external server. However most port-based network
access control applications use a separate Authentication Server, to
allow centralized administration (...) throughout a network.", similar
wording in other places.

And then multiple ESSes (different SSIDs) can use the same AAA but
it's the same situation because we can't know for sure (unless the
BSSID is the same)

I think indexing by BSSID should work well but can also change this to
use just the SSID, or both.

> I also don't see the need
> to store the EAP type here as well.

Ok.

> We can't have SSIDs with multiple EAP types.

I think some of the eduroam networks do support fallback EAP types but
I guess the TLS cache may be independent of the EAP type.

>
> This does however bring up another point.  Shouldn't you clear out the cache
> when a network is forgotten?

Good point, if we include the SSID as the (or one of) primary keys we
can drop entries related to that SSID.  Or we could expire old entries
with time.

> In which case, how do you reload the cache in the
> EAP module?

We don't need to notify the l_tls instances about the change.  They
share one l_settings object but they only look at it at one time in
the handshake and don't assume that the cache is unchanged between
handshakes.

>
> >
> > eap_{get,set}_peer_id() API is added for eapol to set the identifier of
> > the authenticator (or the supplicant if we're the authenticator, if
> > there's ever a use case for that).
>
> Okay
>
> >
> > eap-tls-common.c can't call storage_tls_session_cache_{load,sync}()
> > directly because it's linked into ead and some unit tests without
> > storage.c, so station.c sets pointers to the load & sync functions using
> > the new eap_tls_set_session_cache_ops().
>
> ead should probably start using storage.c anyway since that does profile
> encryption and we ultimately want that inside ead as well.  But we can solve
> this later.
>
> > ---
> >   src/eap-tls-common.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
> >   src/eap-tls-common.h |  6 +++++
> >   src/eap.c            | 13 +++++++++++
> >   src/eap.h            |  3 +++
> >   src/eapol.c          |  4 ++++
> >   src/station.c        |  6 +++++
> >   6 files changed, 84 insertions(+)
> >
>
> <snip>
>
> > @@ -633,6 +645,26 @@ static bool eap_tls_tunnel_init(struct eap_state *eap)
> >       if (eap_tls->domain_mask)
> >               l_tls_set_domain_mask(eap_tls->tunnel, eap_tls->domain_mask);
> >
> > +     if (!eap_tls_session_cache_load)
> > +             goto start;
> > +
> > +     if (!eap_tls_session_cache) {
> > +             eap_tls_session_cache = eap_tls_session_cache_load();
> > +
> > +             if (!eap_tls_session_cache) {
> > +                     eap_tls_session_cache = l_settings_new();
> > +                     l_debug("No session cache loaded, starting with an "
> > +                             "empty cache");
> > +             }
>
> Can we run into a situation where we have two TLS sessions running with the same
> cache entry and it is invalidated somehow?  Do we need to notify the other TLS
> sessions somehow?
>
> Perhaps if we're running a multi WLAN setup and both are connected to the same SSID?

This should be fine, once a new session is ready it's immediately
added to the cache and other instances can resume the same session
even while it's active.  The spec says if the session is being
removed, active instances that use it can keep going, only no new
resumptions can happen.

Best regards

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] eap-tls: Add session caching
  2022-11-09 21:28     ` Andrew Zaborowski
@ 2022-11-10 15:18       ` Denis Kenzior
  2022-11-10 18:15         ` Andrew Zaborowski
  0 siblings, 1 reply; 8+ messages in thread
From: Denis Kenzior @ 2022-11-10 15:18 UTC (permalink / raw)
  To: Andrew Zaborowski; +Cc: iwd

Hi Andrew,

>> But isn't the TLS session ultimately out to the RADIUS server?  So wouldn't the
>> session id be identical between the entire ESS/SSID?
> 
> So like I said it may be but we can't know this.  There may be no
> RADIUS server: looking at 802.1x everytime it mentions RADIUS it's an
> "example" AAA and also mentions Diameter.  But there may be no central
> AAA server in the first place.
> 
> For example 6.3.1 says: "The Authenticator and an Authentication
> Server can be co-located within the same system, allowing that system
> to perform the authentication function without the need for
> communication with an external server. However most port-based network
> access control applications use a separate Authentication Server, to
> allow centralized administration (...) throughout a network.", similar
> wording in other places.
> 

Right.  A centralized RADIUS server is the general case.

> And then multiple ESSes (different SSIDs) can use the same AAA but
> it's the same situation because we can't know for sure (unless the
> BSSID is the same)

Sure, but why even bring this up?  We can't know whether different SSIDs would 
use the same RADIUS server.  But it is pretty much expected that a within an 
ESS, the same RADIUS server is used.

> 
> I think indexing by BSSID should work well but can also change this to
> use just the SSID, or both.

Ultimately I don't see why we would want to limit this to just the BSSID?  Lets 
say you logged into a WPA-Enterprise network on 'AP A'.  You suspend your laptop 
and move within the building.  Resume and re-connect to 'AP B'.  There is a high 
likelihood that AP A & B share the RADIUS server and thus the TLS session.

But even if 'AP B' doesn't share the TLS session, what is the harm in trying? 
Worst case the TLS server will reject our session ID and we'd have to start from 
scratch.

So if we index by
	a) SSID -> there's a good chance that we can reuse the session across routers, 
even if that fails in some cases
	b) SSID + BSS -> We never try the session ID unless we have been connected to 
the same BSS before.  In which case there's no advantage over PMK caching?

<snip>

>> We can't have SSIDs with multiple EAP types.
> 
> I think some of the eduroam networks do support fallback EAP types but
> I guess the TLS cache may be independent of the EAP type.
> 

We also can't use multiple method types within our profile either.

>>
>> This does however bring up another point.  Shouldn't you clear out the cache
>> when a network is forgotten?
> 
> Good point, if we include the SSID as the (or one of) primary keys we
> can drop entries related to that SSID.  Or we could expire old entries
> with time.

I would think we need to explicitly drop anything related to the SSID of the 
network we just removed via KnownNetworks.Forget()

> 
>> In which case, how do you reload the cache in the
>> EAP module?
> 
> We don't need to notify the l_tls instances about the change.  They
> share one l_settings object but they only look at it at one time in
> the handshake and don't assume that the cache is unchanged between
> handshakes.

Yes, but how does eap remove the relevant entries from the cache?

<snip>

>>
>> Can we run into a situation where we have two TLS sessions running with the same
>> cache entry and it is invalidated somehow?  Do we need to notify the other TLS
>> sessions somehow?
>>
>> Perhaps if we're running a multi WLAN setup and both are connected to the same SSID?
> 
> This should be fine, once a new session is ready it's immediately
> added to the cache and other instances can resume the same session
> even while it's active.  The spec says if the session is being
> removed, active instances that use it can keep going, only no new
> resumptions can happen.
> 

Ah, nice.  That makes this not a concern then.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] eap-tls: Add session caching
  2022-11-10 15:18       ` Denis Kenzior
@ 2022-11-10 18:15         ` Andrew Zaborowski
  2022-11-10 18:31           ` Denis Kenzior
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Zaborowski @ 2022-11-10 18:15 UTC (permalink / raw)
  To: Denis Kenzior; +Cc: iwd

On Thu, 10 Nov 2022 at 16:18, Denis Kenzior <denkenz@gmail.com> wrote:
> >> But isn't the TLS session ultimately out to the RADIUS server?  So wouldn't the
> >> session id be identical between the entire ESS/SSID?
> >
> > So like I said it may be but we can't know this.  There may be no
> > RADIUS server: looking at 802.1x everytime it mentions RADIUS it's an
> > "example" AAA and also mentions Diameter.  But there may be no central
> > AAA server in the first place.
> >
> > For example 6.3.1 says: "The Authenticator and an Authentication
> > Server can be co-located within the same system, allowing that system
> > to perform the authentication function without the need for
> > communication with an external server. However most port-based network
> > access control applications use a separate Authentication Server, to
> > allow centralized administration (...) throughout a network.", similar
> > wording in other places.
> >
>
> Right.  A centralized RADIUS server is the general case.
>
> > And then multiple ESSes (different SSIDs) can use the same AAA but
> > it's the same situation because we can't know for sure (unless the
> > BSSID is the same)
>
> Sure, but why even bring this up?  We can't know whether different SSIDs would
> use the same RADIUS server.  But it is pretty much expected that a within an
> ESS, the same RADIUS server is used.

I don't have actual statistics but the other case ("co-located AA
server") also exists so to me this question is similar in that IWD has
no data to assume either option.

But I'm fine just ignoring the co-located AA case if that's what we want to do.

>
> >
> > I think indexing by BSSID should work well but can also change this to
> > use just the SSID, or both.
>
> Ultimately I don't see why we would want to limit this to just the BSSID?  Lets
> say you logged into a WPA-Enterprise network on 'AP A'.  You suspend your laptop
> and move within the building.  Resume and re-connect to 'AP B'.  There is a high
> likelihood that AP A & B share the RADIUS server and thus the TLS session.
>
> But even if 'AP B' doesn't share the TLS session, what is the harm in trying?
> Worst case the TLS server will reject our session ID and we'd have to start from
> scratch.
>
> So if we index by
>         a) SSID -> there's a good chance that we can reuse the session across routers,
> even if that fails in some cases
>         b) SSID + BSS -> We never try the session ID unless we have been connected to
> the same BSS before.  In which case there's no advantage over PMK caching?

The flipside of this is:
   a) SSID -> since we only store one session so everytime we switch
BSSes we overwrite the cached session in the co-located AA case and
rarely see the benefit of the cache.
   b) BSSID -> we store one session for each BSS and after some number
of reconnects every next one is going to be fast.

That said the RFC-recommended session lifetime being only 24h makes
this less practical.  Since we can't know how long each server's
session lifetime is I wonder if IWD should set it to a higher value in
case we're lucky.

>
> <snip>
>
> >> We can't have SSIDs with multiple EAP types.
> >
> > I think some of the eduroam networks do support fallback EAP types but
> > I guess the TLS cache may be independent of the EAP type.
> >
>
> We also can't use multiple method types within our profile either.

Right, let me remove it from the group name.

I put the full method name in there partly to make the contents of the
file more self explanatory to somebody who happens to open it in vim.

>
> >>
> >> This does however bring up another point.  Shouldn't you clear out the cache
> >> when a network is forgotten?
> >
> > Good point, if we include the SSID as the (or one of) primary keys we
> > can drop entries related to that SSID.  Or we could expire old entries
> > with time.
>
> I would think we need to explicitly drop anything related to the SSID of the
> network we just removed via KnownNetworks.Forget()

Ok.  I don't believe we do this for known frequencies though.

>
> >
> >> In which case, how do you reload the cache in the
> >> EAP module?
> >
> > We don't need to notify the l_tls instances about the change.  They
> > share one l_settings object but they only look at it at one time in
> > the handshake and don't assume that the cache is unchanged between
> > handshakes.
>
> Yes, but how does eap remove the relevant entries from the cache?

If we want to do this I'll probably move the cache singleton to
knownnetworks.c and drop the relevant group and not notify eap.
eap-tls-common.c would use something like
known_networks_get_tls_session_cache().

Best regards

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] eap-tls: Add session caching
  2022-11-10 18:15         ` Andrew Zaborowski
@ 2022-11-10 18:31           ` Denis Kenzior
  0 siblings, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2022-11-10 18:31 UTC (permalink / raw)
  To: Andrew Zaborowski; +Cc: iwd

Hi Andrew,

<snip>

>> So if we index by
>>          a) SSID -> there's a good chance that we can reuse the session across routers,
>> even if that fails in some cases
>>          b) SSID + BSS -> We never try the session ID unless we have been connected to
>> the same BSS before.  In which case there's no advantage over PMK caching?
> 
> The flipside of this is:
>     a) SSID -> since we only store one session so everytime we switch
> BSSes we overwrite the cached session in the co-located AA case and
> rarely see the benefit of the cache.
>     b) BSSID -> we store one session for each BSS and after some number
> of reconnects every next one is going to be fast.
> 

So for case b, I would argue PMK caching would be even better since we skip 
8021x completely.  As I mentioned earlier, I would think that RADIUS server is 
the 'general' case and we should optimize for that.  Maybe I'm wrong here, but 
I've never seen a colocated option in any commercial APs.  It is always a RADIUS 
server.

> That said the RFC-recommended session lifetime being only 24h makes
> this less practical.  Since we can't know how long each server's
> session lifetime is I wonder if IWD should set it to a higher value in
> case we're lucky.

I'd just default to what the RFC recommends, but maybe we can add something to 
the profile to make this configurable.

<snip>

>> I would think we need to explicitly drop anything related to the SSID of the
>> network we just removed via KnownNetworks.Forget()
> 
> Ok.  I don't believe we do this for known frequencies though.
> 

I think we do?  See known_networks_remove()

<snip>

>>
>> Yes, but how does eap remove the relevant entries from the cache?
> 
> If we want to do this I'll probably move the cache singleton to
> knownnetworks.c and drop the relevant group and not notify eap.
> eap-tls-common.c would use something like
> known_networks_get_tls_session_cache().

That might be the way to go then.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-11-10 18:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-09 17:04 [PATCH 1/2] storage: Add TLS session cache file read/write utils Andrew Zaborowski
2022-11-09 17:04 ` [PATCH 2/2] eap-tls: Add session caching Andrew Zaborowski
2022-11-09 20:45   ` Denis Kenzior
2022-11-09 21:28     ` Andrew Zaborowski
2022-11-10 15:18       ` Denis Kenzior
2022-11-10 18:15         ` Andrew Zaborowski
2022-11-10 18:31           ` Denis Kenzior
2022-11-09 20:32 ` [PATCH 1/2] storage: Add TLS session cache file read/write utils Denis Kenzior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).