| Message ID | 20231114171455.1108856-11-denkenz@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [01/11] netdev: Fix obtaining reason code from deauth frames | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-ci-gitlint | success | GitLint |
Hi Denis, On 11/14/23 9:14 AM, Denis Kenzior wrote: > There are generally three scenarios where iwd generates a disconnection > command to the kernel: > 1. Error conditions stemming from a connection related event. For > example if SAE/FT/FILS authentication fails during Authenticate or > Associate steps and the kernel doesn't disconnect properly. > 2. Deauthentication after the connection has been established and not > related to a connection attempt in progress. For example, SA Query > processing that triggers an disconnect. > 3. Disconnects that are triggered due to a handshake failure or if > setting keys resulting from the handshake fails. These disconnects > can be triggered as a result of a pending connection or when a > connection has been established (e.g. due to rekeying). > > Distinguish between 1 and 2/3 by having the disconnect procedure take > different paths. For now there are no functional changes since all > paths end up in netdev_connect_failed(), but this will change in the > future. I'm guessing the future changes would be to handle a CMD_DISCONNECT event via the connect callback? Thanks, James
Hi James, On 11/14/23 11:39, James Prestwood wrote: > Hi Denis, > > On 11/14/23 9:14 AM, Denis Kenzior wrote: >> There are generally three scenarios where iwd generates a disconnection >> command to the kernel: >> 1. Error conditions stemming from a connection related event. For >> example if SAE/FT/FILS authentication fails during Authenticate or >> Associate steps and the kernel doesn't disconnect properly. >> 2. Deauthentication after the connection has been established and not >> related to a connection attempt in progress. For example, SA Query >> processing that triggers an disconnect. >> 3. Disconnects that are triggered due to a handshake failure or if >> setting keys resulting from the handshake fails. These disconnects >> can be triggered as a result of a pending connection or when a >> connection has been established (e.g. due to rekeying). >> >> Distinguish between 1 and 2/3 by having the disconnect procedure take >> different paths. For now there are no functional changes since all >> paths end up in netdev_connect_failed(), but this will change in the >> future. > > I'm guessing the future changes would be to handle a CMD_DISCONNECT event via > the connect callback? Possibly. Have not made up my mind completely yet, but we do it both ways right now and we need to stick to one. Wanted to get the easy stuff out of the way first. Any comments or should I push this out? Regards, -Denis
On 11/14/23 12:09 PM, Denis Kenzior wrote: > Hi James, > > On 11/14/23 11:39, James Prestwood wrote: >> Hi Denis, >> >> On 11/14/23 9:14 AM, Denis Kenzior wrote: >>> There are generally three scenarios where iwd generates a disconnection >>> command to the kernel: >>> 1. Error conditions stemming from a connection related event. For >>> example if SAE/FT/FILS authentication fails during Authenticate or >>> Associate steps and the kernel doesn't disconnect properly. >>> 2. Deauthentication after the connection has been established and not >>> related to a connection attempt in progress. For example, SA >>> Query >>> processing that triggers an disconnect. >>> 3. Disconnects that are triggered due to a handshake failure or if >>> setting keys resulting from the handshake fails. These >>> disconnects >>> can be triggered as a result of a pending connection or when a >>> connection has been established (e.g. due to rekeying). >>> >>> Distinguish between 1 and 2/3 by having the disconnect procedure take >>> different paths. For now there are no functional changes since all >>> paths end up in netdev_connect_failed(), but this will change in the >>> future. >> >> I'm guessing the future changes would be to handle a CMD_DISCONNECT >> event via the connect callback? > > Possibly. Have not made up my mind completely yet, but we do it both > ways right now and we need to stick to one. Wanted to get the easy > stuff out of the way first. Any comments or should I push this out? Nope, it looks good to me. > > Regards, > -Denis >
Hi Denis, On Tue, Nov 14, 2023 at 11:14:34AM -0600, Denis Kenzior wrote: > There are generally three scenarios where iwd generates a disconnection > command to the kernel: > 1. Error conditions stemming from a connection related event. For > example if SAE/FT/FILS authentication fails during Authenticate or > Associate steps and the kernel doesn't disconnect properly. > 2. Deauthentication after the connection has been established and not > related to a connection attempt in progress. For example, SA Query > processing that triggers an disconnect. > 3. Disconnects that are triggered due to a handshake failure or if > setting keys resulting from the handshake fails. These disconnects > can be triggered as a result of a pending connection or when a > connection has been established (e.g. due to rekeying). > > Distinguish between 1 and 2/3 by having the disconnect procedure take > different paths. For now there are no functional changes since all > paths end up in netdev_connect_failed(), but this will change in the > future. Sorry for not following up on the issue I raised the other week. I am catching up with the mailing list now and see this patch. Am I right to assume this is some preperatory work for solving the issue I identified? I acknowledge your previous remark that the issue ought to be solved in netdev.c and not in station.c. Kind regards, Alvin
Hi Alvin, >> Distinguish between 1 and 2/3 by having the disconnect procedure take >> different paths. For now there are no functional changes since all >> paths end up in netdev_connect_failed(), but this will change in the >> future. > > Sorry for not following up on the issue I raised the other week. I am > catching up with the mailing list now and see this patch. Am I right to > assume this is some preperatory work for solving the issue I identified? Yep. I got side-tracked by some oFono work, so don't have a fix for your particular problem yet. Regards, -Denis
Hi Denis, On Fri, Nov 24, 2023 at 10:25:40AM -0600, Denis Kenzior wrote: > Hi Alvin, > > > > Distinguish between 1 and 2/3 by having the disconnect procedure take > > > different paths. For now there are no functional changes since all > > > paths end up in netdev_connect_failed(), but this will change in the > > > future. > > > > Sorry for not following up on the issue I raised the other week. I am > > catching up with the mailing list now and see this patch. Am I right to > > assume this is some preperatory work for solving the issue I identified? > > Yep. I got side-tracked by some oFono work, so don't have a fix for your > particular problem yet. Great, thanks a lot for following up on this. Sorry that I did not send out a more robust patch, but I have not had time these days. Please add me on CC if you work further on this and we can do some additional internal testing to see if it works. Kind regards, Alvin
diff --git a/src/netdev.c b/src/netdev.c index 418626d72579..d0ce0aaad5e7 100644 --- a/src/netdev.c +++ b/src/netdev.c @@ -858,7 +858,7 @@ static void netdev_connect_failed(struct netdev *netdev, } } -static void netdev_disconnect_cb(struct l_genl_msg *msg, void *user_data) +static void netdev_connect_failed_cb(struct l_genl_msg *msg, void *user_data) { struct netdev *netdev = user_data; @@ -866,6 +866,63 @@ static void netdev_disconnect_cb(struct l_genl_msg *msg, void *user_data) netdev_connect_failed(netdev, netdev->result, netdev->last_code); } +static void netdev_send_and_fail_connection(struct netdev *netdev, + enum netdev_result result, + uint16_t status_code, + struct l_genl_msg *msg) +{ + netdev->result = result; + netdev->last_code = status_code; + + netdev->disconnect_cmd_id = + l_genl_family_send(nl80211, msg, netdev_connect_failed_cb, + netdev, NULL); +} + +static void netdev_disconnect_and_fail_connection(struct netdev *netdev, + enum netdev_result result, + uint16_t status_code) +{ + struct l_genl_msg *msg = nl80211_build_disconnect(netdev->index, + MMPDU_REASON_CODE_UNSPECIFIED); + + netdev_send_and_fail_connection(netdev, result, status_code, msg); +} + +static void netdev_deauth_and_fail_connection(struct netdev *netdev, + enum netdev_result result, + uint16_t status_code) +{ + struct l_genl_msg *msg = nl80211_build_deauthenticate(netdev->index, + netdev->handshake->aa, + MMPDU_REASON_CODE_UNSPECIFIED); + + netdev_send_and_fail_connection(netdev, result, status_code, msg); +} + +static void netdev_disconnect_sme_cb(struct l_genl_msg *msg, void *user_data) +{ + struct netdev *netdev = user_data; + + netdev->disconnect_cmd_id = 0; + netdev_connect_failed(netdev, netdev->result, netdev->last_code); +} + +static void netdev_disconnect_by_sme(struct netdev *netdev, + enum netdev_result result, + uint16_t reason_code) +{ + struct l_genl_msg *msg = nl80211_build_disconnect(netdev->index, + reason_code); + + netdev->result = result; + netdev->last_code = reason_code; + + netdev->disconnect_cmd_id = l_genl_family_send(nl80211, msg, + netdev_disconnect_sme_cb, + netdev, NULL); +} + static void netdev_free(void *data) { struct netdev *netdev = data; @@ -1388,11 +1445,9 @@ static void netdev_setting_keys_failed(struct netdev_handshake_state *nhs, return; } - msg = nl80211_build_disconnect(netdev->index, - MMPDU_REASON_CODE_UNSPECIFIED); - netdev->disconnect_cmd_id = l_genl_family_send(nl80211, msg, - netdev_disconnect_cb, - netdev, NULL); + netdev_disconnect_by_sme(netdev, + NETDEV_RESULT_KEY_SETTING_FAILED, + MMPDU_REASON_CODE_UNSPECIFIED); break; case NL80211_IFTYPE_AP: if (err == -ENETDOWN) @@ -1407,7 +1462,6 @@ static void netdev_setting_keys_failed(struct netdev_handshake_state *nhs, break; } - netdev->result = NETDEV_RESULT_KEY_SETTING_FAILED; handshake_event(&nhs->super, HANDSHAKE_EVENT_SETTING_KEYS_FAILED, &err); } @@ -2118,16 +2172,11 @@ void netdev_handshake_failed(struct handshake_state *hs, uint16_t reason_code) netdev->sm = NULL; - netdev->result = NETDEV_RESULT_HANDSHAKE_FAILED; - netdev->last_code = reason_code; - switch (netdev->type) { case NL80211_IFTYPE_STATION: case NL80211_IFTYPE_P2P_CLIENT: - msg = nl80211_build_disconnect(netdev->index, reason_code); - netdev->disconnect_cmd_id = l_genl_family_send(nl80211, msg, - netdev_disconnect_cb, - netdev, NULL); + netdev_disconnect_by_sme(netdev, NETDEV_RESULT_HANDSHAKE_FAILED, + reason_code); break; case NL80211_IFTYPE_AP: case NL80211_IFTYPE_P2P_GO: @@ -2842,14 +2891,9 @@ error: return; deauth: - netdev->result = NETDEV_RESULT_ASSOCIATION_FAILED; - netdev->last_code = MMPDU_STATUS_CODE_UNSPECIFIED; - msg = nl80211_build_disconnect(netdev->index, - MMPDU_REASON_CODE_UNSPECIFIED); - netdev->disconnect_cmd_id = l_genl_family_send(nl80211, - msg, - netdev_disconnect_cb, - netdev, NULL); + netdev_disconnect_and_fail_connection(netdev, + NETDEV_RESULT_ASSOCIATION_FAILED, + MMPDU_STATUS_CODE_UNSPECIFIED); } static struct l_genl_msg *netdev_build_cmd_associate_common( @@ -2890,19 +2934,12 @@ static void netdev_cmd_ft_reassociate_cb(struct l_genl_msg *msg, netdev->connect_cmd_id = 0; - if (l_genl_msg_get_error(msg) < 0) { - struct l_genl_msg *cmd_deauth; + if (l_genl_msg_get_error(msg) >= 0) + return; - netdev->result = NETDEV_RESULT_ASSOCIATION_FAILED; - netdev->last_code = MMPDU_STATUS_CODE_UNSPECIFIED; - cmd_deauth = nl80211_build_deauthenticate(netdev->index, - netdev->handshake->aa, - MMPDU_REASON_CODE_UNSPECIFIED); - netdev->disconnect_cmd_id = l_genl_family_send(nl80211, - cmd_deauth, - netdev_disconnect_cb, - netdev, NULL); - } + netdev_deauth_and_fail_connection(netdev, + NETDEV_RESULT_ASSOCIATION_FAILED, + MMPDU_STATUS_CODE_UNSPECIFIED); } static bool kernel_will_retry_auth(uint16_t status_code, @@ -3027,17 +3064,9 @@ static void netdev_authenticate_event(struct l_genl_msg *msg, * to keep retrying, tell it to stop */ if (retry) { - struct l_genl_msg *cmd_deauth; - - netdev->result = NETDEV_RESULT_ASSOCIATION_FAILED; - netdev->last_code = MMPDU_STATUS_CODE_UNSPECIFIED; - cmd_deauth = nl80211_build_deauthenticate(netdev->index, - netdev->handshake->aa, - MMPDU_REASON_CODE_UNSPECIFIED); - netdev->disconnect_cmd_id = l_genl_family_send(nl80211, - cmd_deauth, - netdev_disconnect_cb, - netdev, NULL); + netdev_deauth_and_fail_connection(netdev, + NETDEV_RESULT_ASSOCIATION_FAILED, + MMPDU_STATUS_CODE_UNSPECIFIED); return; } } @@ -4545,17 +4574,14 @@ static void netdev_sa_query_timeout(struct l_timeout *timeout, void *user_data) { struct netdev *netdev = user_data; - struct l_genl_msg *msg; l_info("SA Query timed out, connection is invalid. Disconnecting..."); l_timeout_remove(netdev->sa_query_timeout); netdev->sa_query_timeout = NULL; - msg = nl80211_build_disconnect(netdev->index, - MMPDU_REASON_CODE_PREV_AUTH_NOT_VALID); - netdev->disconnect_cmd_id = l_genl_family_send(nl80211, msg, - netdev_disconnect_cb, netdev, NULL); + netdev_disconnect_by_sme(netdev, NETDEV_RESULT_ABORTED, + MMPDU_REASON_CODE_PREV_AUTH_NOT_VALID); } static void netdev_sa_query_req_cb(struct l_genl_msg *msg, void *user_data)
There are generally three scenarios where iwd generates a disconnection command to the kernel: 1. Error conditions stemming from a connection related event. For example if SAE/FT/FILS authentication fails during Authenticate or Associate steps and the kernel doesn't disconnect properly. 2. Deauthentication after the connection has been established and not related to a connection attempt in progress. For example, SA Query processing that triggers an disconnect. 3. Disconnects that are triggered due to a handshake failure or if setting keys resulting from the handshake fails. These disconnects can be triggered as a result of a pending connection or when a connection has been established (e.g. due to rekeying). Distinguish between 1 and 2/3 by having the disconnect procedure take different paths. For now there are no functional changes since all paths end up in netdev_connect_failed(), but this will change in the future. --- src/netdev.c | 126 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 50 deletions(-)