Message ID | CAFk-A4n9WyFC4hhTiAQaxZ6ct5QvWdL3rSyKKrB64YU0ysRHAg@mail.gmail.com |
---|---|
State | Changes Requested |
Headers | show |
On Mon, Oct 28, 2013 at 04:18:31PM +0900, Masashi Honma wrote: > diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c > + if (os_strcmp(name, "psk") == 0 && value[0] == '"' && ssid->ssid_len) { > + if (ssid->passphrase) { > + if (os_strlen(ssid->passphrase) != > + os_strlen(value) - 2 || > + os_memcmp(ssid->passphrase, &value[1], > + os_strlen(ssid->passphrase)) != 0) > + update_psk = 1; > + } else { > + update_psk = 1; > + } > + } ... > - if ((os_strcmp(name, "psk") == 0 && > - value[0] == '"' && ssid->ssid_len) || > - (os_strcmp(name, "ssid") == 0 && ssid->passphrase)) > + if (update_psk) > wpa_config_update_psk(ssid); Could you please clarify what is the use case that this is addressing? If I understood the implementation correctly, this skips wpa_config_update_psk() call in case someone tries to set psk or ssid to the existing value of the field. Why would such corner cases need to be handled? Wouldn't it be better to fix whatever is sending such pointless commands to not do that? In other words, I would need to get quite a bit better justification for this type of extra complexity in wpa_supplicant. PS. The patch was severely whitespace damaged and as such, would require manual editing from me to get it applied..
2013/11/10 Jouni Malinen <j@w1.fi>: > Could you please clarify what is the use case that this is addressing? This patch intends to skip PSK re-calculation. Because tiny device requires long time for PSK re-calculation. > Wouldn't it be better to fix whatever is sending such pointless commands to > not do that? As you say, our Android application can save PSK and check modification. But Android settings application can change the PSK also. Then our application can not recoginze the modification. And application can not get current PSK from wpa_supplicant via wpa_cli. So we want to check it on wpa_supplicant. > The patch was severely whitespace damaged I don't know why it occured... It is maybe mailer problem. I will send the patch as attachment. Regards, Masashi Honma.
diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c index d0c0a01..153611a 100644 --- a/wpa_supplicant/ctrl_iface.c +++ b/wpa_supplicant/ctrl_iface.c @@ -2332,9 +2332,11 @@ static int wpa_supplicant_ctrl_iface_remove_network( static int wpa_supplicant_ctrl_iface_set_network( struct wpa_supplicant *wpa_s, char *cmd) { - int id; + int id, update_psk = 0; struct wpa_ssid *ssid; char *name, *value; + u8 *old_ssid = NULL; + size_t old_ssid_len = 0; /* cmd: "<network id> <variable name> <value>" */ name = os_strchr(cmd, ' '); @@ -2360,12 +2362,44 @@ static int wpa_supplicant_ctrl_iface_set_network( return -1; } + if (os_strcmp(name, "psk") == 0 && value[0] == '"' && ssid->ssid_len) { + if (ssid->passphrase) { + if (os_strlen(ssid->passphrase) != + os_strlen(value) - 2 || + os_memcmp(ssid->passphrase, &value[1], + os_strlen(ssid->passphrase)) != 0) + update_psk = 1; + } else { + update_psk = 1; + } + } + + if (os_strcmp(name, "ssid") == 0 && ssid->passphrase) { + if (ssid->ssid_len) { + old_ssid = os_malloc(ssid->ssid_len); + if (old_ssid == NULL) + return -1; + os_memcpy(old_ssid, ssid->ssid, ssid->ssid_len); + old_ssid_len = ssid->ssid_len; + } else { + update_psk = 1; + } + } + if (wpa_config_set(ssid, name, value, 0) < 0) { wpa_printf(MSG_DEBUG, "CTRL_IFACE: Failed to set network " "variable '%s'", name); + os_free(old_ssid); return -1; } + if (old_ssid) { + if (old_ssid_len != ssid->ssid_len || + os_memcmp(old_ssid, ssid->ssid, ssid->ssid_len) != 0) + update_psk = 1; + os_free(old_ssid); + } + if (os_strcmp(name, "bssid") != 0 && os_strcmp(name, "priority") != 0) wpa_sm_pmksa_cache_flush(wpa_s->wpa, ssid); @@ -2378,9 +2412,7 @@ static int wpa_supplicant_ctrl_iface_set_network( eapol_sm_invalidate_cached_session(wpa_s->eapol); } - if ((os_strcmp(name, "psk") == 0 && - value[0] == '"' && ssid->ssid_len) || - (os_strcmp(name, "ssid") == 0 && ssid->passphrase)) + if (update_psk) wpa_config_update_psk(ssid); else if (os_strcmp(name, "priority") == 0) wpa_config_update_prio_list(wpa_s->conf);