diff mbox series

[RESEND] Update PMK parameters in WPA state machine before association

Message ID 20220923134807.31484-1-bgalvani@redhat.com
State Changes Requested
Headers show
Series [RESEND] Update PMK parameters in WPA state machine before association | expand

Commit Message

Beniamino Galvani Sept. 23, 2022, 1:48 p.m. UTC 
Currently, PMK parameters in the WPA state machine are set from
configuration only when the interface is initialized.

If those parameters are changed later (for example, via D-Bus) and
then a new association starts, the new values will not have effect.

Update the parameters also before a new association.

Signed-off-by: Beniamino Galvani <bgalvani@redhat.com>
---
 wpa_supplicant/wpa_supplicant.c | 55 ++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 22 deletions(-)

Comments

Jouni Malinen Sept. 29, 2022, 10:30 a.m. UTC | #1 
On Fri, Sep 23, 2022 at 03:48:07PM +0200, Beniamino Galvani wrote:
> Currently, PMK parameters in the WPA state machine are set from
> configuration only when the interface is initialized.
> 
> If those parameters are changed later (for example, via D-Bus) and
> then a new association starts, the new values will not have effect.
> 
> Update the parameters also before a new association.

This does not feel correct.. The control interface for updating these
parameters is already calling wap_sm_set_param() immediately when the
value is updated. This might have an impact even for the current
association, so forcing an update when starting a new one would not
cover that. Shouldn't the D-Bus interface handler be updated to call
wpa_sm_set_param() instead to match the design in
wpa_supplicant/ctrl_iface.c?
Beniamino Galvani Sept. 30, 2022, 2:35 p.m. UTC | #2 
On Thu, Sep 29, 2022 at 01:30:45PM +0300, Jouni Malinen wrote:
> On Fri, Sep 23, 2022 at 03:48:07PM +0200, Beniamino Galvani wrote:
> > Currently, PMK parameters in the WPA state machine are set from
> > configuration only when the interface is initialized.
> > 
> > If those parameters are changed later (for example, via D-Bus) and
> > then a new association starts, the new values will not have effect.
> > 
> > Update the parameters also before a new association.
> 
> This does not feel correct.. The control interface for updating these
> parameters is already calling wap_sm_set_param() immediately when the
> value is updated. This might have an impact even for the current
> association, so forcing an update when starting a new one would not
> cover that. Shouldn't the D-Bus interface handler be updated to call
> wpa_sm_set_param() instead to match the design in
> wpa_supplicant/ctrl_iface.c?

Makes sense, I'll send a new patch.

Thanks,
Beniamino
diff mbox series

Patch

diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 761017248..d782291bb 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -2338,6 +2338,35 @@  int wpas_restore_permanent_mac_addr(struct wpa_supplicant *wpa_s)
 }
 
 
+static int wpa_supplicant_init_wpa_sm_pmk_params(struct wpa_supplicant *wpa_s)
+{
+	if (wpa_s->conf->dot11RSNAConfigPMKLifetime &&
+	    wpa_sm_set_param(wpa_s->wpa, RSNA_PMK_LIFETIME,
+			     wpa_s->conf->dot11RSNAConfigPMKLifetime)) {
+		wpa_msg(wpa_s, MSG_ERROR, "Invalid WPA parameter value for "
+			"dot11RSNAConfigPMKLifetime");
+		return -1;
+	}
+
+	if (wpa_s->conf->dot11RSNAConfigPMKReauthThreshold &&
+	    wpa_sm_set_param(wpa_s->wpa, RSNA_PMK_REAUTH_THRESHOLD,
+			     wpa_s->conf->dot11RSNAConfigPMKReauthThreshold)) {
+		wpa_msg(wpa_s, MSG_ERROR, "Invalid WPA parameter value for "
+			"dot11RSNAConfigPMKReauthThreshold");
+		return -1;
+	}
+
+	if (wpa_s->conf->dot11RSNAConfigSATimeout &&
+	    wpa_sm_set_param(wpa_s->wpa, RSNA_SA_TIMEOUT,
+			     wpa_s->conf->dot11RSNAConfigSATimeout)) {
+		wpa_msg(wpa_s, MSG_ERROR, "Invalid WPA parameter value for "
+			"dot11RSNAConfigSATimeout");
+		return -1;
+	}
+
+	return 0;
+}
+
 static void wpas_start_assoc_cb(struct wpa_radio_work *work, int deinit);
 
 /**
@@ -2470,6 +2499,9 @@  void wpa_supplicant_associate(struct wpa_supplicant *wpa_s,
 	 */
 	wpa_supplicant_rsn_supp_set_config(wpa_s, ssid);
 
+	if (wpa_supplicant_init_wpa_sm_pmk_params(wpa_s))
+		return;
+
 #ifdef CONFIG_DPP
 	if (wpas_dpp_check_connect(wpa_s, ssid, bss) != 0)
 		return;
@@ -6820,29 +6852,8 @@  static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
 			  NULL);
 	wpa_sm_set_fast_reauth(wpa_s->wpa, wpa_s->conf->fast_reauth);
 
-	if (wpa_s->conf->dot11RSNAConfigPMKLifetime &&
-	    wpa_sm_set_param(wpa_s->wpa, RSNA_PMK_LIFETIME,
-			     wpa_s->conf->dot11RSNAConfigPMKLifetime)) {
-		wpa_msg(wpa_s, MSG_ERROR, "Invalid WPA parameter value for "
-			"dot11RSNAConfigPMKLifetime");
-		return -1;
-	}
-
-	if (wpa_s->conf->dot11RSNAConfigPMKReauthThreshold &&
-	    wpa_sm_set_param(wpa_s->wpa, RSNA_PMK_REAUTH_THRESHOLD,
-			     wpa_s->conf->dot11RSNAConfigPMKReauthThreshold)) {
-		wpa_msg(wpa_s, MSG_ERROR, "Invalid WPA parameter value for "
-			"dot11RSNAConfigPMKReauthThreshold");
-		return -1;
-	}
-
-	if (wpa_s->conf->dot11RSNAConfigSATimeout &&
-	    wpa_sm_set_param(wpa_s->wpa, RSNA_SA_TIMEOUT,
-			     wpa_s->conf->dot11RSNAConfigSATimeout)) {
-		wpa_msg(wpa_s, MSG_ERROR, "Invalid WPA parameter value for "
-			"dot11RSNAConfigSATimeout");
+	if (wpa_supplicant_init_wpa_sm_pmk_params(wpa_s))
 		return -1;
-	}
 
 	wpa_s->hw.modes = wpa_drv_get_hw_feature_data(wpa_s,
 						      &wpa_s->hw.num_modes,