I don't think anything has been committed regarding this issue, right?

On Sat, Nov 18 2017, Stuart Henderson <s...@spacehopper.org> wrote:
> On 2017/11/17 18:28, Ted Unangst wrote:
>> Stefan Sperling wrote:
>> > Or is modifying ifconfig sufficient?
>> > We are more concerned about textual display rather than the
>> > kernel/userland ioctl boundary, correct?
>> > 
>> > The option list for ifconfig is [-AaC]. Plenty of letters available.
>> > We could add:
>> > 
>> >    -P  Show authentication details such as passwords (not displayed by 
>> > default))
>> 
>> I think putting this logic in ifconfig is much better than the kernel. That
>> didn't make much sense to me, I'm afraid. 
>> 
>
> Reviewing the others:
>
> - sppp / pppoe: blocked in kernel (as of if_spppsubr.c,v 1.73 2009/02/16)
>
>                 /* do not copy the secret, and only let root know the name */
>                 if (auth->name != NULL && suser(curproc, 0) == 0)
>                         strlcpy(spa->name, auth->name, sizeof(spa->name));
>
>
> - carp: passed in kernel to root (SIOCGVH), not displayed in ifconfig.
> ports/shells/nsh does read it though.
>
>                 if (suser(p, 0) == 0)
>                         bcopy(sc->sc_key, carpr.carpr_key,
>                             sizeof(carpr.carpr_key));
>
> I think that's all besides wifi?
>
> Consistency would be nice. I would be reasonably happy with any of these:
>
> - kernel never passes these keys.
> - kernel only passes these keys if securelevel < 2 (I think this would
> be my first choice).
> - kernel always passes these keys to root.

I'm not terribly concerned with putting carp and ppp on the same line as
wifi, since you point out that they are "safe" in the current situation.
I'd rather let people who care handle this.

> For either of the last two, I like the suggested -P flag in ifconfig
> to control whether it's displayed.

Looks like the sanest solution to me.  I'm not even sure we need to
provide the knob but the diff is short anyway...

> I'm not convinced that having the kernel pass wifi keys to root if
> IFF_DEBUG is set fixes the original problem - users having problems
> with wifi may well use "ifconfig XX debug" before they fetch ifconfig
> output to send in a list post.

I did not check, maybe it would be worthwhile to check that ifconfig
<wireless> debug doesn't actually leak auth info.  Anyway, I agree that
hiding this behind "debug" doesn't look like the best approach.

So here's a simple diff to just hide wifi keys unless -P is passed;
documented as a generic feature, but specifying the actual behavior so
that people don't hope that -P would enable printing carp passphrases /
ppp auth details.

Feedback / oks?


Index: ifconfig.8
===================================================================
RCS file: /d/cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.289
diff -u -p -r1.289 ifconfig.8
--- ifconfig.8  17 Nov 2017 18:04:51 -0000      1.289
+++ ifconfig.8  26 Nov 2017 17:05:53 -0000
@@ -88,6 +88,9 @@ This is the default, if no parameters ar
 Print the names of all network pseudo-devices that
 can be created dynamically at runtime using
 .Nm Cm create .
+.It Fl P
+Display passwords, keys and other authentication material used by
+interfaces (only affects wireless devices).
 .It Ar interface
 The
 .Ar interface
Index: ifconfig.c
===================================================================
RCS file: /d/cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.351
diff -u -p -r1.351 ifconfig.c
--- ifconfig.c  17 Nov 2017 18:04:51 -0000      1.351
+++ ifconfig.c  26 Nov 2017 16:57:07 -0000
@@ -623,6 +623,7 @@ const struct afswtch *afp;  /*the address
 
 int ifaliases = 0;
 int aflag = 0;
+int displayauth = 0;
 
 int
 main(int argc, char *argv[])
@@ -661,6 +662,9 @@ main(int argc, char *argv[])
                                Cflag = 1;
                                nomore = 1;
                                break;
+                       case 'P':
+                               displayauth = 1;
+                               break;
                        default:
                                usage();
                                break;
@@ -2131,7 +2135,8 @@ ieee80211_status(void)
                        nwkey.i_key[i].i_keydat = keybuf[i];
                        nwkey.i_key[i].i_keylen = sizeof(keybuf[i]);
                }
-               if (ioctl(s, SIOCG80211NWKEY, (caddr_t)&nwkey) == -1) {
+               if (ioctl(s, SIOCG80211NWKEY, (caddr_t)&nwkey) == -1 ||
+                   !displayauth) {
                        fputs("<not displayed>", stdout);
                } else {
                        nwkey_verbose = 0;
@@ -2183,7 +2188,7 @@ ieee80211_status(void)
 
        if (ipsk == 0 && psk.i_enabled) {
                fputs(" wpakey ", stdout);
-               if (psk.i_enabled == 2)
+               if (psk.i_enabled == 2 || !displayauth)
                        fputs("<not displayed>", stdout);
                else
                        print_string(psk.i_psk, sizeof(psk.i_psk));
@@ -5510,7 +5515,7 @@ __dead void
 usage(void)
 {
        fprintf(stderr,
-           "usage: ifconfig [-AaC] [interface] [address_family] "
+           "usage: ifconfig [-AaCP] [interface] [address_family] "
            "[address [dest_address]]\n"
            "\t\t[parameters]\n");
        exit(1);


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to