Date: Wed, 5 Nov 2008 18:30:09 GMT From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/128582: [patch] activate readline(3) support in wpa_cli Message-ID: <200811051830.mA5IU9VA074406@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/128582; it has been noted by GNATS. From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> To: Sam Leffler <sam@freebsd.org> Cc: FreeBSD-gnats-submit@freebsd.org Subject: Re: bin/128582: [patch] activate readline(3) support in wpa_cli Date: Wed, 5 Nov 2008 20:58:53 +0300 --i0/AhcQY5QxfSsSZ Content-Type: multipart/mixed; boundary="NzB8fVQJ5HfG6fxh" Content-Disposition: inline --NzB8fVQJ5HfG6fxh Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Sam, good day. Tue, Nov 04, 2008 at 11:37:22AM -0800, Sam Leffler wrote: > readline support in wpa_cli is not enabled by default because it bloats= =20 > the system with very little gain. It is trivial to have a local=20 > configuration that adds it w/o touching the source code but once added=20 > in the way proposed it is not possible to disable it w/o modification. OK, no problems: what about the attached patch? > I believe the original PR was submitted by brix@. I recently asked him= =20 > to review it and decide whether he wanted to commit the change;=20 > otherwise close the PR. Yes, it is bin/116606. I had spotted from that patch that ncurses is needed too and had explicitely added it to my patch. The main difference between my and Henrik patches is that I am defining additional knob WPA_CLI_WITH_READLINE that enables the realine support (but only when MK_GNU_SUPPORT is defined too). So, by-default the support won't be included and people will be able to turn it on without resorting the the make trickery. And I had also described this in the manual page. As a bonus, I had written proper support for the history command validation for the potentially sensitive commands that shouldn't be written to the disk. The original version was faulty and incomplete. The patch is attached too. May be it should go upstream, I'll try to submit it there too. --=20 Eygene _ ___ _.--. # \`.|\..----...-'` `-._.-'_.-'` # Remember that it is hard / ' ` , __.--' # to read the on-line manual =20 )/' _/ \ `-_, / # while single-stepping the kernel. `-'" `"\_ ,_.-;_.-\_ ', fsc/as # _.-'_./ {_.' ; / # -- FreeBSD Developers handbook=20 {_.-``-' {_/ # --NzB8fVQJ5HfG6fxh Content-Type: text/plain; charset=koi8-r Content-Disposition: attachment; filename=patch-wpa_cli-readline-support Content-Transfer-Encoding: quoted-printable =46rom dfcf8eb229255ff0d28e7afc4bfd49cf3afb573f Mon Sep 17 00:00:00 2001 =46rom: Eygene Ryabinkin <rea-fbsd@codelabs.ru> Date: Tue, 4 Nov 2008 16:00:51 +0300 Subject: [PATCH] WPA supplicant: activate support for readline and history = browsing The support is present since wpa_cli.c, rev. 1.1, but it was not turned on. Since it is rather useful to have readline/history support for the interactive mode, I am enabling this functionality, but it is done conditionally. Sam Leffler told me [1] that it will be the system bloat when there will be unconditional support for readline(3). So, one should set the macro WPA_CLI_WITH_READLINE=3D"yes" to enable this. readline(3) support won't be enabled when GNU support is globally disabled. [1] http://www.freebsd.org/cgi/query-pr.cgi?pr=3D128582 Signed-off-by: Eygene Ryabinkin <rea-fbsd@codelabs.ru> --- usr.sbin/wpa/wpa_cli/Makefile | 10 ++++++++++ usr.sbin/wpa/wpa_cli/wpa_cli.8 | 25 ++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletions(-) diff --git a/usr.sbin/wpa/wpa_cli/Makefile b/usr.sbin/wpa/wpa_cli/Makefile index 75c3c02..4531598 100644 --- a/usr.sbin/wpa/wpa_cli/Makefile +++ b/usr.sbin/wpa/wpa_cli/Makefile @@ -11,4 +11,14 @@ MAN=3D wpa_cli.8 CFLAGS+=3D -DCONFIG_CTRL_IFACE CFLAGS+=3D -DCONFIG_CTRL_IFACE_UNIX =20 +.include <bsd.own.mk> + +.if ${MK_GNU_SUPPORT} =3D=3D "no" +# Do nothing, we don't want readline(3) support: GNU support it disabled +.elif defined(WPA_CLI_WITH_READLINE) && ${WPA_CLI_WITH_READLINE} =3D=3D "y= es" +CFLAGS+=3D -DCONFIG_READLINE +DPADD+=3D ${LIBREADLINE} ${LIBNCURSES} +LDADD+=3D -lreadline -lncurses +.endif + .include <bsd.prog.mk> diff --git a/usr.sbin/wpa/wpa_cli/wpa_cli.8 b/usr.sbin/wpa/wpa_cli/wpa_cli.8 index e9a22ee..3ed857c 100644 --- a/usr.sbin/wpa/wpa_cli/wpa_cli.8 +++ b/usr.sbin/wpa/wpa_cli/wpa_cli.8 @@ -202,9 +202,32 @@ to terminate. Exit .Nm . .El +.Sh IMPLEMENTATION DETAILS +.Pp +One can enable +.Xr readline 3 +support for the +.Nm +by setting the macro=20 +.Dv WPA_CLI_WITH_READLINE +to +.Qq yes +either on the compile time, in the +.Pa /etc/make.conf +or +.Pa /etc/src.conf . +This option will be effective only when GNU tools support is enabled: +the macro +.Dv WITHOUT_GNU_SUPPORT +should be unset. +.Xr readline 3 +support will add history browsing, command editing and command completion +for the interactive operation mode. .Sh SEE ALSO .Xr wpa_supplicant.conf 5 , -.Xr wpa_supplicant 8 +.Xr wpa_supplicant 8 , +.Xr src.conf 5 , +.Xr make.conf 5 .Sh HISTORY The .Nm --=20 1.6.0.3 --NzB8fVQJ5HfG6fxh Content-Type: text/plain; charset=koi8-r Content-Disposition: attachment; filename=patch-wpa_cli-properly-clean-history Content-Transfer-Encoding: quoted-printable =46rom fbd42c715af5d894a48b45db3abf12ac8eaf36b0 Mon Sep 17 00:00:00 2001 =46rom: Eygene Ryabinkin <rea-fbsd@codelabs.ru> Date: Wed, 5 Nov 2008 20:29:56 +0300 Subject: [PATCH] WPA CLI tool: properly clean history before writing it to = the disk First of all, the history had not been written to the disk, since almost all commands were cleaned up due to the error in the history cleaning: the return value of the last os_strncasecmp() call was not compared to zero, but was rather used as is. So the condition was almost always true and most commands were removed from the history. The second problem was that the evaluation of the potentially sensitive commands was started at the entry number 1, instead of very first entry. I had added flags to the every command description: just now the only meaningful flag tells that this command has sensitive arguments and it shouldn't be written to the disk. I rewrote the logics for the search for the sensitive commands: special procedure is now loops over all commands and tries to see if command has sensitive data. And I had rewritten the command checking loop to properly check all commands. Signed-off-by: Eygene Ryabinkin <rea-fbsd@codelabs.ru> --- contrib/wpa_supplicant/wpa_cli.c | 124 ++++++++++++++++++++++------------= ---- 1 files changed, 72 insertions(+), 52 deletions(-) diff --git a/contrib/wpa_supplicant/wpa_cli.c b/contrib/wpa_supplicant/wpa_= cli.c index 7176c95..f3e90ef 100644 --- a/contrib/wpa_supplicant/wpa_cli.c +++ b/contrib/wpa_supplicant/wpa_cli.c @@ -973,57 +973,81 @@ static int wpa_cli_cmd_interface_remove(struct wpa_ct= rl *ctrl, int argc, return wpa_ctrl_command(ctrl, cmd); } =20 +enum wpa_cli_cmd_flags { + cli_cmd_flag_none =3D 0x00, + cli_cmd_flag_sensitive =3D 0x01 +}; =20 struct wpa_cli_cmd { const char *cmd; int (*handler)(struct wpa_ctrl *ctrl, int argc, char *argv[]); + enum wpa_cli_cmd_flags flags; }; =20 static struct wpa_cli_cmd wpa_cli_commands[] =3D { - { "status", wpa_cli_cmd_status }, - { "ping", wpa_cli_cmd_ping }, - { "mib", wpa_cli_cmd_mib }, - { "help", wpa_cli_cmd_help }, - { "interface", wpa_cli_cmd_interface }, - { "level", wpa_cli_cmd_level }, - { "license", wpa_cli_cmd_license }, - { "quit", wpa_cli_cmd_quit }, - { "set", wpa_cli_cmd_set }, - { "logon", wpa_cli_cmd_logon }, - { "logoff", wpa_cli_cmd_logoff }, - { "pmksa", wpa_cli_cmd_pmksa }, - { "reassociate", wpa_cli_cmd_reassociate }, - { "preauthenticate", wpa_cli_cmd_preauthenticate }, - { "identity", wpa_cli_cmd_identity }, - { "password", wpa_cli_cmd_password }, - { "new_password", wpa_cli_cmd_new_password }, - { "pin", wpa_cli_cmd_pin }, - { "otp", wpa_cli_cmd_otp }, - { "passphrase", wpa_cli_cmd_passphrase }, - { "bssid", wpa_cli_cmd_bssid }, - { "list_networks", wpa_cli_cmd_list_networks }, - { "select_network", wpa_cli_cmd_select_network }, - { "enable_network", wpa_cli_cmd_enable_network }, - { "disable_network", wpa_cli_cmd_disable_network }, - { "add_network", wpa_cli_cmd_add_network }, - { "remove_network", wpa_cli_cmd_remove_network }, - { "set_network", wpa_cli_cmd_set_network }, - { "get_network", wpa_cli_cmd_get_network }, - { "save_config", wpa_cli_cmd_save_config }, - { "disconnect", wpa_cli_cmd_disconnect }, - { "reconnect", wpa_cli_cmd_reconnect }, - { "scan", wpa_cli_cmd_scan }, - { "scan_results", wpa_cli_cmd_scan_results }, - { "get_capability", wpa_cli_cmd_get_capability }, - { "reconfigure", wpa_cli_cmd_reconfigure }, - { "terminate", wpa_cli_cmd_terminate }, - { "interface_add", wpa_cli_cmd_interface_add }, - { "interface_remove", wpa_cli_cmd_interface_remove }, - { "ap_scan", wpa_cli_cmd_ap_scan }, - { "stkstart", wpa_cli_cmd_stkstart }, - { NULL, NULL } + { "status", wpa_cli_cmd_status, cli_cmd_flag_none }, + { "ping", wpa_cli_cmd_ping, cli_cmd_flag_none }, + { "mib", wpa_cli_cmd_mib, cli_cmd_flag_none }, + { "help", wpa_cli_cmd_help, cli_cmd_flag_none }, + { "interface", wpa_cli_cmd_interface, cli_cmd_flag_none }, + { "level", wpa_cli_cmd_level, cli_cmd_flag_none }, + { "license", wpa_cli_cmd_license, cli_cmd_flag_none }, + { "quit", wpa_cli_cmd_quit, cli_cmd_flag_none }, + { "set", wpa_cli_cmd_set, cli_cmd_flag_none }, + { "logon", wpa_cli_cmd_logon, cli_cmd_flag_none }, + { "logoff", wpa_cli_cmd_logoff, cli_cmd_flag_none }, + { "pmksa", wpa_cli_cmd_pmksa, cli_cmd_flag_none }, + { "reassociate", wpa_cli_cmd_reassociate, cli_cmd_flag_none }, + { "preauthenticate", wpa_cli_cmd_preauthenticate, cli_cmd_flag_none }, + { "identity", wpa_cli_cmd_identity, cli_cmd_flag_none }, + { "password", wpa_cli_cmd_password, cli_cmd_flag_sensitive }, + { "new_password", wpa_cli_cmd_new_password, cli_cmd_flag_sensitive }, + { "pin", wpa_cli_cmd_pin, cli_cmd_flag_sensitive }, + { "otp", wpa_cli_cmd_otp, cli_cmd_flag_sensitive }, + { "passphrase", wpa_cli_cmd_passphrase, cli_cmd_flag_sensitive }, + { "bssid", wpa_cli_cmd_bssid, cli_cmd_flag_none }, + { "list_networks", wpa_cli_cmd_list_networks, cli_cmd_flag_none }, + { "select_network", wpa_cli_cmd_select_network, cli_cmd_flag_none }, + { "enable_network", wpa_cli_cmd_enable_network, cli_cmd_flag_none }, + { "disable_network", wpa_cli_cmd_disable_network, cli_cmd_flag_none }, + { "add_network", wpa_cli_cmd_add_network, cli_cmd_flag_none }, + { "remove_network", wpa_cli_cmd_remove_network, cli_cmd_flag_none }, + { "set_network", wpa_cli_cmd_set_network, cli_cmd_flag_none }, + { "get_network", wpa_cli_cmd_get_network, cli_cmd_flag_none }, + { "save_config", wpa_cli_cmd_save_config, cli_cmd_flag_none }, + { "disconnect", wpa_cli_cmd_disconnect, cli_cmd_flag_none }, + { "reconnect", wpa_cli_cmd_reconnect, cli_cmd_flag_none }, + { "scan", wpa_cli_cmd_scan, cli_cmd_flag_none }, + { "scan_results", wpa_cli_cmd_scan_results, cli_cmd_flag_none }, + { "get_capability", wpa_cli_cmd_get_capability, cli_cmd_flag_none }, + { "reconfigure", wpa_cli_cmd_reconfigure, cli_cmd_flag_none }, + { "terminate", wpa_cli_cmd_terminate, cli_cmd_flag_none }, + { "interface_add", wpa_cli_cmd_interface_add, cli_cmd_flag_none }, + { "interface_remove", wpa_cli_cmd_interface_remove, cli_cmd_flag_none }, + { "ap_scan", wpa_cli_cmd_ap_scan, cli_cmd_flag_none }, + { "stkstart", wpa_cli_cmd_stkstart, cli_cmd_flag_none }, + { NULL, NULL, cli_cmd_flag_none } }; =20 +static int _cmd_has_sensitive_data(const char *cmd) +{ + const char *c, *delim; + int n; + size_t len; + + delim =3D strpbrk(cmd, " \t"); + if (delim) + len =3D delim - cmd; + else + len =3D strlen(cmd); + + for (n =3D 0; c =3D wpa_cli_commands[n].cmd; n++) { + if (os_strncasecmp(cmd, c, len) =3D=3D 0 && len =3D=3D strlen(c)) + return (wpa_cli_commands[n].flags & + cli_cmd_flag_sensitive); + } + return 0; +} =20 static int wpa_request(struct wpa_ctrl *ctrl, int argc, char *argv[]) { @@ -1349,24 +1373,20 @@ static void wpa_cli_interactive(void) * passwords. */ HIST_ENTRY *h; history_set_pos(0); - h =3D next_history(); - while (h) { + while (h =3D current_history()) { char *p =3D h->line; while (*p =3D=3D ' ' || *p =3D=3D '\t') p++; - if (os_strncasecmp(p, "pa", 2) =3D=3D 0 || - os_strncasecmp(p, "o", 1) =3D=3D 0 || - os_strncasecmp(p, "n", 1)) { + if (_cmd_has_sensitive_data(p)) { h =3D remove_history(where_history()); if (h) { os_free(h->line); os_free(h->data); os_free(h); - } - h =3D current_history(); - } else { - h =3D next_history(); - } + } else + next_history(); + } else + next_history(); } write_history(hfile); os_free(hfile); --=20 1.6.0.3 --NzB8fVQJ5HfG6fxh-- --i0/AhcQY5QxfSsSZ Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (FreeBSD) iEYEARECAAYFAkkR3t0ACgkQthUKNsbL7YijtgCfcGVUtEI5SQbGLjj0lWkmtQD4 fhkAnRToWxLfVDpF8FDgXQiekkxUpb7U =dpiq -----END PGP SIGNATURE----- --i0/AhcQY5QxfSsSZ--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200811051830.mA5IU9VA074406>