Attached without the whitespace broken...
From cf89d424d5e51bc4f8b59aa29dc2d46ad7d1ce31 Mon Sep 17 00:00:00 2001 From: Nick Lowe <nick.lowe@xxxxxxxxxxxx> Date: Sun, 24 Jan 2016 01:02:41 +0000 Subject: [PATCH 1/3] Rework Acct-Session-Id and Acct-Multi-Session-Id to give better global and temporal uniqueness. Signed-off-by: Nick Lowe <nick.lowe@xxxxxxxxxxxx> --- src/ap/accounting.c | 41 ++++++++++++++++------------------------ src/ap/accounting.h | 5 +++-- src/ap/hostapd.c | 20 +++++++++----------- src/ap/hostapd.h | 1 - src/ap/ieee802_1x.c | 17 ++++++++--------- src/ap/pmksa_cache_auth.c | 15 ++++++--------- src/ap/pmksa_cache_auth.h | 3 +-- src/ap/sta_info.c | 3 ++- src/ap/sta_info.h | 3 +-- src/eapol_auth/eapol_auth_sm.c | 17 ++++++----------- src/eapol_auth/eapol_auth_sm_i.h | 6 +----- 11 files changed, 53 insertions(+), 78 deletions(-) diff --git a/src/ap/accounting.c b/src/ap/accounting.c index c60b3a6..95dc366 100644 --- a/src/ap/accounting.c +++ b/src/ap/accounting.c @@ -54,10 +54,9 @@ static struct radius_msg * accounting_msg(struct hostapd_data *hapd, if ((hapd->conf->wpa & 2) && !hapd->conf->disable_pmksa_caching && - sta->eapol_sm && sta->eapol_sm->acct_multi_session_id_hi) { - os_snprintf(buf, sizeof(buf), "%08X+%08X", - sta->eapol_sm->acct_multi_session_id_hi, - sta->eapol_sm->acct_multi_session_id_lo); + sta->eapol_sm && sta->eapol_sm->acct_multi_session_id) { + os_snprintf(buf, sizeof(buf), "%016lX", + sta->eapol_sm->acct_multi_session_id); if (!radius_msg_add_attr( msg, RADIUS_ATTR_ACCT_MULTI_SESSION_ID, (u8 *) buf, os_strlen(buf))) { @@ -226,8 +225,8 @@ void accounting_sta_start(struct hostapd_data *hapd, struct sta_info *sta) hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_RADIUS, HOSTAPD_LEVEL_INFO, - "starting accounting session %08X-%08X", - sta->acct_session_id_hi, sta->acct_session_id_lo); + "starting accounting session %016lX", + sta->acct_session_id); os_get_reltime(&sta->acct_session_start); sta->last_rx_bytes = sta->last_tx_bytes = 0; @@ -384,22 +383,25 @@ void accounting_sta_stop(struct hostapd_data *hapd, struct sta_info *sta) eloop_cancel_timeout(accounting_interim_update, hapd, sta); hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_RADIUS, HOSTAPD_LEVEL_INFO, - "stopped accounting session %08X-%08X", - sta->acct_session_id_hi, - sta->acct_session_id_lo); + "stopped accounting session %016lX", + sta->acct_session_id); sta->acct_session_started = 0; } } -void accounting_sta_get_id(struct hostapd_data *hapd, +int accounting_sta_get_id(struct hostapd_data *hapd, struct sta_info *sta) { - sta->acct_session_id_lo = hapd->acct_session_id_lo++; - if (hapd->acct_session_id_lo == 0) { - hapd->acct_session_id_hi++; + /* Acct-Session-Id should be globally and temporarily unique. + * A high quality random number is required therefore. + * This could be be improved by switching to a GUID. */ + if (os_get_random((u8 *) &sta->acct_session_id, + sizeof(sta->acct_session_id)) < 0) { + return -1; } - sta->acct_session_id_hi = hapd->acct_session_id_hi; + + return 0; } @@ -466,17 +468,6 @@ static void accounting_report_state(struct hostapd_data *hapd, int on) */ int accounting_init(struct hostapd_data *hapd) { - struct os_time now; - - /* Acct-Session-Id should be unique over reboots. Using a random number - * is preferred. If that is not available, take the current time. Mix - * in microseconds to make this more likely to be unique. */ - os_get_time(&now); - if (os_get_random((u8 *) &hapd->acct_session_id_hi, - sizeof(hapd->acct_session_id_hi)) < 0) - hapd->acct_session_id_hi = now.sec; - hapd->acct_session_id_hi ^= now.usec; - if (radius_client_register(hapd->radius, RADIUS_ACCT, accounting_receive, hapd)) return -1; diff --git a/src/ap/accounting.h b/src/ap/accounting.h index dcc54ee..59c3dd1 100644 --- a/src/ap/accounting.h +++ b/src/ap/accounting.h @@ -10,9 +10,10 @@ #define ACCOUNTING_H #ifdef CONFIG_NO_ACCOUNTING -static inline void accounting_sta_get_id(struct hostapd_data *hapd, +static inline int accounting_sta_get_id(struct hostapd_data *hapd, struct sta_info *sta) { + return 0; } static inline void accounting_sta_start(struct hostapd_data *hapd, @@ -34,7 +35,7 @@ static inline void accounting_deinit(struct hostapd_data *hapd) { } #else /* CONFIG_NO_ACCOUNTING */ -void accounting_sta_get_id(struct hostapd_data *hapd, struct sta_info *sta); +int accounting_sta_get_id(struct hostapd_data *hapd, struct sta_info *sta); void accounting_sta_start(struct hostapd_data *hapd, struct sta_info *sta); void accounting_sta_stop(struct hostapd_data *hapd, struct sta_info *sta); int accounting_init(struct hostapd_data *hapd); diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c index 2aa4f8c..3428697 100644 --- a/src/ap/hostapd.c +++ b/src/ap/hostapd.c @@ -673,7 +673,7 @@ static struct sta_info * hostapd_das_find_sta(struct hostapd_data *hapd, if (attr->acct_session_id) { num_attr++; - if (attr->acct_session_id_len != 17) { + if (attr->acct_session_id_len != 16) { wpa_printf(MSG_DEBUG, "RADIUS DAS: Acct-Session-Id cannot match"); return NULL; @@ -683,10 +683,9 @@ static struct sta_info * hostapd_das_find_sta(struct hostapd_data *hapd, for (sta = hapd->sta_list; sta; sta = sta->next) { if (!sta->radius_das_match) continue; - os_snprintf(buf, sizeof(buf), "%08X-%08X", - sta->acct_session_id_hi, - sta->acct_session_id_lo); - if (os_memcmp(attr->acct_session_id, buf, 17) != 0) + os_snprintf(buf, sizeof(buf), "%016lX", + sta->acct_session_id); + if (os_memcmp(attr->acct_session_id, buf, 16) != 0) sta->radius_das_match = 0; else count++; @@ -702,7 +701,7 @@ static struct sta_info * hostapd_das_find_sta(struct hostapd_data *hapd, if (attr->acct_multi_session_id) { num_attr++; - if (attr->acct_multi_session_id_len != 17) { + if (attr->acct_multi_session_id_len != 16) { wpa_printf(MSG_DEBUG, "RADIUS DAS: Acct-Multi-Session-Id cannot match"); return NULL; @@ -713,14 +712,13 @@ static struct sta_info * hostapd_das_find_sta(struct hostapd_data *hapd, if (!sta->radius_das_match) continue; if (!sta->eapol_sm || - !sta->eapol_sm->acct_multi_session_id_hi) { + !sta->eapol_sm->acct_multi_session_id) { sta->radius_das_match = 0; continue; } - os_snprintf(buf, sizeof(buf), "%08X+%08X", - sta->eapol_sm->acct_multi_session_id_hi, - sta->eapol_sm->acct_multi_session_id_lo); - if (os_memcmp(attr->acct_multi_session_id, buf, 17) != + os_snprintf(buf, sizeof(buf), "%016lX", + sta->eapol_sm->acct_multi_session_id); + if (os_memcmp(attr->acct_multi_session_id, buf, 16) != 0) sta->radius_das_match = 0; else diff --git a/src/ap/hostapd.h b/src/ap/hostapd.h index 7b59f80..72f8252 100644 --- a/src/ap/hostapd.h +++ b/src/ap/hostapd.h @@ -138,7 +138,6 @@ struct hostapd_data { void *msg_ctx_parent; /* parent interface ctx for wpa_msg() calls */ struct radius_client_data *radius; - u32 acct_session_id_hi, acct_session_id_lo; struct radius_das_data *radius_das; struct iapp_data *iapp; diff --git a/src/ap/ieee802_1x.c b/src/ap/ieee802_1x.c index 607f941..e153f08 100644 --- a/src/ap/ieee802_1x.c +++ b/src/ap/ieee802_1x.c @@ -438,9 +438,9 @@ static int add_common_radius_sta_attr(struct hostapd_data *hapd, return -1; } - if (sta->acct_session_id_hi || sta->acct_session_id_lo) { - os_snprintf(buf, sizeof(buf), "%08X-%08X", - sta->acct_session_id_hi, sta->acct_session_id_lo); + if (sta->acct_session_id) { + os_snprintf(buf, sizeof(buf), "%016lX", + sta->acct_session_id); if (!radius_msg_add_attr(msg, RADIUS_ATTR_ACCT_SESSION_ID, (u8 *) buf, os_strlen(buf))) { wpa_printf(MSG_ERROR, "Could not add Acct-Session-Id"); @@ -2495,12 +2495,12 @@ int ieee802_1x_get_mib_sta(struct hostapd_data *hapd, struct sta_info *sta, /* TODO: dot1xAuthSessionOctetsTx */ /* TODO: dot1xAuthSessionFramesRx */ /* TODO: dot1xAuthSessionFramesTx */ - "dot1xAuthSessionId=%08X-%08X\n" + "dot1xAuthSessionId=%016lX\n" "dot1xAuthSessionAuthenticMethod=%d\n" "dot1xAuthSessionTime=%u\n" "dot1xAuthSessionTerminateCause=999\n" "dot1xAuthSessionUserName=%s\n", - sta->acct_session_id_hi, sta->acct_session_id_lo, + sta->acct_session_id, (wpa_key_mgmt_wpa_ieee8021x( wpa_auth_sta_key_mgmt(sta->wpa_sm))) ? 1 : 2, @@ -2510,11 +2510,10 @@ int ieee802_1x_get_mib_sta(struct hostapd_data *hapd, struct sta_info *sta, return len; len += ret; - if (sm->acct_multi_session_id_hi) { + if (sm->acct_multi_session_id) { ret = os_snprintf(buf + len, buflen - len, - "authMultiSessionId=%08X+%08X\n", - sm->acct_multi_session_id_hi, - sm->acct_multi_session_id_lo); + "authMultiSessionId=%016lX\n", + sm->acct_multi_session_id); if (os_snprintf_error(buflen - len, ret)) return len; len += ret; diff --git a/src/ap/pmksa_cache_auth.c b/src/ap/pmksa_cache_auth.c index 83e4bda..b191e1c 100644 --- a/src/ap/pmksa_cache_auth.c +++ b/src/ap/pmksa_cache_auth.c @@ -148,8 +148,7 @@ static void pmksa_cache_from_eapol_data(struct rsn_pmksa_cache_entry *entry, entry->eap_type_authsrv = eapol->eap_type_authsrv; entry->vlan_id = ((struct sta_info *) eapol->sta)->vlan_id; - entry->acct_multi_session_id_hi = eapol->acct_multi_session_id_hi; - entry->acct_multi_session_id_lo = eapol->acct_multi_session_id_lo; + entry->acct_multi_session_id = eapol->acct_multi_session_id; } @@ -188,8 +187,7 @@ void pmksa_cache_to_eapol_data(struct rsn_pmksa_cache_entry *entry, eapol->eap_type_authsrv = entry->eap_type_authsrv; ((struct sta_info *) eapol->sta)->vlan_id = entry->vlan_id; - eapol->acct_multi_session_id_hi = entry->acct_multi_session_id_hi; - eapol->acct_multi_session_id_lo = entry->acct_multi_session_id_lo; + eapol->acct_multi_session_id = entry->acct_multi_session_id; } @@ -471,12 +469,11 @@ static int das_attr_match(struct rsn_pmksa_cache_entry *entry, if (attr->acct_multi_session_id) { char buf[20]; - if (attr->acct_multi_session_id_len != 17) + if (attr->acct_multi_session_id_len != 16) return 0; - os_snprintf(buf, sizeof(buf), "%08X+%08X", - entry->acct_multi_session_id_hi, - entry->acct_multi_session_id_lo); - if (os_memcmp(attr->acct_multi_session_id, buf, 17) != 0) + os_snprintf(buf, sizeof(buf), "%016lX", + entry->acct_multi_session_id); + if (os_memcmp(attr->acct_multi_session_id, buf, 16) != 0) return 0; match++; } diff --git a/src/ap/pmksa_cache_auth.h b/src/ap/pmksa_cache_auth.h index b2da379..4dc841f 100644 --- a/src/ap/pmksa_cache_auth.h +++ b/src/ap/pmksa_cache_auth.h @@ -31,8 +31,7 @@ struct rsn_pmksa_cache_entry { int vlan_id; int opportunistic; - u32 acct_multi_session_id_hi; - u32 acct_multi_session_id_lo; + u64 acct_multi_session_id; }; struct rsn_pmksa_cache; diff --git a/src/ap/sta_info.c b/src/ap/sta_info.c index 8bba73c..39b9d12 100644 --- a/src/ap/sta_info.c +++ b/src/ap/sta_info.c @@ -625,7 +625,8 @@ struct sta_info * ap_sta_add(struct hostapd_data *hapd, const u8 *addr) return NULL; } sta->acct_interim_interval = hapd->conf->acct_interim_interval; - accounting_sta_get_id(hapd, sta); + if (accounting_sta_get_id(hapd, sta) < 0) + return NULL; if (!(hapd->iface->drv_flags & WPA_DRIVER_FLAGS_INACTIVITY_TIMER)) { wpa_printf(MSG_DEBUG, "%s: register ap_handle_timer timeout " diff --git a/src/ap/sta_info.h b/src/ap/sta_info.h index e3b4915..359f480 100644 --- a/src/ap/sta_info.h +++ b/src/ap/sta_info.h @@ -101,8 +101,7 @@ struct sta_info { /* IEEE 802.1X related data */ struct eapol_state_machine *eapol_sm; - u32 acct_session_id_hi; - u32 acct_session_id_lo; + u64 acct_session_id; struct os_reltime acct_session_start; int acct_session_started; int acct_terminate_cause; /* Acct-Terminate-Cause */ diff --git a/src/eapol_auth/eapol_auth_sm.c b/src/eapol_auth/eapol_auth_sm.c index ff33d28..771ee23 100644 --- a/src/eapol_auth/eapol_auth_sm.c +++ b/src/eapol_auth/eapol_auth_sm.c @@ -866,10 +866,12 @@ eapol_auth_alloc(struct eapol_authenticator *eapol, const u8 *addr, sm->radius_cui = wpabuf_alloc_copy(radius_cui, os_strlen(radius_cui)); - sm->acct_multi_session_id_lo = eapol->acct_multi_session_id_lo++; - if (eapol->acct_multi_session_id_lo == 0) - eapol->acct_multi_session_id_hi++; - sm->acct_multi_session_id_hi = eapol->acct_multi_session_id_hi; + /* Acct-Multi-Session-Id should be globally and temporarily unique. + * A high quality random number is required therefore. + * This could be be improved by switching to a GUID. */ + if (os_get_random((u8 *) &sm->acct_multi_session_id, + sizeof(sm->acct_multi_session_id)) < 0) + return NULL; return sm; } @@ -1271,7 +1273,6 @@ struct eapol_authenticator * eapol_auth_init(struct eapol_auth_config *conf, struct eapol_auth_cb *cb) { struct eapol_authenticator *eapol; - struct os_time now; eapol = os_zalloc(sizeof(*eapol)); if (eapol == NULL) @@ -1300,12 +1301,6 @@ struct eapol_authenticator * eapol_auth_init(struct eapol_auth_config *conf, eapol->cb.erp_get_key = cb->erp_get_key; eapol->cb.erp_add_key = cb->erp_add_key; - /* Acct-Multi-Session-Id should be unique over reboots. If reliable - * clock is not available, this could be replaced with reboot counter, - * etc. */ - os_get_time(&now); - eapol->acct_multi_session_id_hi = now.sec; - return eapol; } diff --git a/src/eapol_auth/eapol_auth_sm_i.h b/src/eapol_auth/eapol_auth_sm_i.h index aa3e117..04386b2 100644 --- a/src/eapol_auth/eapol_auth_sm_i.h +++ b/src/eapol_auth/eapol_auth_sm_i.h @@ -30,9 +30,6 @@ struct eapol_authenticator { u8 *default_wep_key; u8 default_wep_key_idx; - - u32 acct_multi_session_id_hi; - u32 acct_multi_session_id_lo; }; @@ -173,8 +170,7 @@ struct eapol_state_machine { int remediation; - u32 acct_multi_session_id_hi; - u32 acct_multi_session_id_lo; + u64 acct_multi_session_id; }; #endif /* EAPOL_AUTH_SM_I_H */ -- 2.5.0
_______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap