| Message ID | 20221202121145.2424546-1-raphael.melotte@mind.be |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] Add SEND_ACTION control interface command for hostapd | expand |
On Fri, Dec 02, 2022 at 01:11:44PM +0100, Raphaël Mélotte wrote: > For userspace applications, sending custom action frames can be > cumbersome: many parameters need to be specified, while at the same > time those parameters are already known by hostapd (e.g. the BSSID, > current channel, etc). > > To make it easy to send custom action frames, add a new SEND_ACTION > control interface command. > It allows to send action frames (provided as a hex string) on the > current channel. I'm fine with doing this kind of things for testing purposes under the CONFIG_TESTING_OPTIONS build parameter, but providing a mechanism for sending practically any kind of Action frame in a production build feels problematic. There might be somewhat reduced privilege requirements for the hostapd control interfaces compared to being able to use NL80211_CMD_FRAME directly. In addition, sending some Action frames without hostapd having the internal context on the state can result in unexpected behavior when processing either the TX status callback or a response from a STA to such an Action frame.
Thanks for the review! On 12/2/22 14:08, Jouni Malinen wrote: > I'm fine with doing this kind of things for testing purposes under the > CONFIG_TESTING_OPTIONS build parameter, but providing a mechanism for > sending practically any kind of Action frame in a production build feels > problematic. There might be somewhat reduced privilege requirements for > the hostapd control interfaces compared to being able to use > NL80211_CMD_FRAME directly. Indeed, it makes sense.. In our case we needed it outside CONFIG_TESTING_OPTIONS, so I'll abandon this approach. > In addition, sending some Action frames > without hostapd having the internal context on the state can result in > unexpected behavior when processing either the TX status callback or a > response from a STA to such an Action frame. In our case we need to be able to send a DELBA frame to a station at an arbitrary point in time (the point is to tear down any existing block ack agreement for a very short period of time - on kernels that don't support NL80211_TID_CONFIG_ATTR_AMPDU_CTRL yet). Would a new command to send a DELBA frame to a specific station have more chances to be accepted? Kind regards, Raphaël
On Fri, Dec 02, 2022 at 05:02:41PM +0100, Raphaël Mélotte wrote: > In our case we need to be able to send a DELBA frame to a station at an arbitrary point in time (the point is to tear down any existing block ack agreement for a very short period of time - on kernels that don't support NL80211_TID_CONFIG_ATTR_AMPDU_CTRL yet). > > Would a new command to send a DELBA frame to a specific station have more chances to be accepted? The use case of sending a DELBA to control a block ack agreement sounds exactly like a case that I was worried about when injecting arbitrary frames without informing the local stack about that.. Isn't the appropriate approach for addressing that to provide such capability in the kernel? If this is just to have a temporary workaround for not wanting to update a kernel for some systems, it does not really sound like a convincing argument to complicate upstream hostapd with a feature that would then likely need to be maintained for significant amount of time in practice.
Hello, On 12/3/22 08:53, Jouni Malinen wrote: > On Fri, Dec 02, 2022 at 05:02:41PM +0100, Raphaël Mélotte wrote: > The use case of sending a DELBA to control a block ack agreement sounds > exactly like a case that I was worried about when injecting arbitrary > frames without informing the local stack about that.. Isn't the > appropriate approach for addressing that to provide such capability in > the kernel? If this is just to have a temporary workaround for not > wanting to update a kernel for some systems, it does not really sound > like a convincing argument to complicate upstream hostapd with a feature > that would then likely need to be maintained for significant amount of > time in practice. I didn't see it that way initially but following your explanation I agree it would make more sense to have that capability in the kernel instead. Thanks for the feedback! Raphaël
On Mon, 2022-12-05 at 10:07 +0100, Raphaël Mélotte wrote: > Hello, > > On 12/3/22 08:53, Jouni Malinen wrote: > > On Fri, Dec 02, 2022 at 05:02:41PM +0100, Raphaël Mélotte wrote: > > The use case of sending a DELBA to control a block ack agreement sounds > > exactly like a case that I was worried about when injecting arbitrary > > frames without informing the local stack about that.. Isn't the > > appropriate approach for addressing that to provide such capability in > > the kernel? If this is just to have a temporary workaround for not > > wanting to update a kernel for some systems, it does not really sound > > like a convincing argument to complicate upstream hostapd with a feature > > that would then likely need to be maintained for significant amount of > > time in practice. > > I didn't see it that way initially but following your explanation I > agree it would make more sense to have that capability in the kernel > instead. > There's already a debugfs file that can do it, at least if you use mac80211? johannes
diff --git a/hostapd/ctrl_iface.c b/hostapd/ctrl_iface.c index 464dfc8ee..63426ec98 100644 --- a/hostapd/ctrl_iface.c +++ b/hostapd/ctrl_iface.c @@ -3179,6 +3179,34 @@ static int hostapd_ctrl_iface_get_capability(struct hostapd_data *hapd, } +static int hostapd_ctrl_iface_send_action(struct hostapd_data *hapd, + const char *cmd) +{ + u8 addr[ETH_ALEN]; + const char *pos; + struct wpabuf *buf; + int ret; + + if (hwaddr_aton(cmd, addr)) + return -1; + + pos = cmd + 17; + if (*pos != ' ') + return -1; + pos++; + + buf = wpabuf_parse_bin(pos); + if (buf == NULL) + return -1; + + ret = hostapd_drv_send_action(hapd, hapd->iface->freq, 0, addr, + wpabuf_head(buf), wpabuf_len(buf)); + wpabuf_free(buf); + + return ret; +} + + #ifdef ANDROID static int hostapd_ctrl_iface_driver_cmd(struct hostapd_data *hapd, char *cmd, char *buf, size_t buflen) @@ -3293,6 +3321,9 @@ static int hostapd_ctrl_iface_receive_process(struct hostapd_data *hapd, } else if (os_strncmp(buf, "POLL_STA ", 9) == 0) { if (hostapd_ctrl_iface_poll_sta(hapd, buf + 9)) reply_len = -1; + } else if (os_strncmp(buf, "SEND_ACTION ", 12) == 0) { + if (hostapd_ctrl_iface_send_action(hapd, buf + 12)) + reply_len = -1; } else if (os_strcmp(buf, "STOP_AP") == 0) { if (hostapd_ctrl_iface_stop_ap(hapd)) reply_len = -1; diff --git a/hostapd/hostapd_cli.c b/hostapd/hostapd_cli.c index ec40dda77..6bd82eac5 100644 --- a/hostapd/hostapd_cli.c +++ b/hostapd/hostapd_cli.c @@ -1563,6 +1563,11 @@ static int hostapd_cli_cmd_reload_wpa_psk(struct wpa_ctrl *ctrl, int argc, return wpa_ctrl_command(ctrl, "RELOAD_WPA_PSK"); } +static int hostapd_cli_cmd_send_action(struct wpa_ctrl *ctrl, int argc, + char *argv[]) +{ + return hostapd_cli_cmd(ctrl, "SEND_ACTION", 2, argc, argv); +} #ifdef ANDROID static int hostapd_cli_cmd_driver(struct wpa_ctrl *ctrl, int argc, char *argv[]) @@ -1769,6 +1774,8 @@ static const struct hostapd_cli_cmd hostapd_cli_commands[] = { "<addr> = poll a STA to check connectivity with a QoS null frame" }, { "req_beacon", hostapd_cli_cmd_req_beacon, NULL, "<addr> [req_mode=] <measurement request hexdump> = send a Beacon report request to a station" }, + { "send_action", hostapd_cli_cmd_send_action, NULL, + "<addr> <hex formatted action frame> = send an action frame to a station" }, { "reload_wpa_psk", hostapd_cli_cmd_reload_wpa_psk, NULL, "= reload wpa_psk_file only" }, #ifdef ANDROID
For userspace applications, sending custom action frames can be cumbersome: many parameters need to be specified, while at the same time those parameters are already known by hostapd (e.g. the BSSID, current channel, etc). To make it easy to send custom action frames, add a new SEND_ACTION control interface command. It allows to send action frames (provided as a hex string) on the current channel. Signed-off-by: Raphaël Mélotte <raphael.melotte@mind.be> --- hostapd/ctrl_iface.c | 31 +++++++++++++++++++++++++++++++ hostapd/hostapd_cli.c | 7 +++++++ 2 files changed, 38 insertions(+)