+1 on max Len , keep \0, use length - 1 for sum.
IMHO.
Happy holidays all.
On Fri, Dec 27, 2019 at 7:39 AM Claudio Jeker <cje...@diehard.n-r-g.com>
wrote:
> On Fri, Dec 27, 2019 at 01:12:42PM +0100, Stefan Sperling wrote:
> > If an SSID uses the maximum allowed length, ifconfig overwrites
> > the last byte with \0 when hashing the WPA key.
> > This leads to a wrong WPA key being installed in the kernel:
> >
> > iwm0: SCAN -> AUTH
> > iwm0: sending auth to xx:xx:xx:xx:xx:xx on channel 1 mode 11g
> > iwm0: AUTH -> ASSOC
> > iwm0: sending assoc_req to xx:xx:xx:xx:xx:xx on channel 1 mode 11g
> > iwm0: ASSOC -> RUN
> > iwm0: associated with xx:xx:xx:xx:xx:xx ssid "FRITZ!Box7430
> Internetmanufaktur" channel 1 start MCS 0 short preamble short slot time
> protection enabled HT enabled
> > iwm0: missed beacon threshold set to 30 beacons, beacon interval is 100
> TU
> > iwm0: received msg 1/4 of the 4-way handshake from xx:xx:xx:xx:xx:xx
> > iwm0: sending msg 2/4 of the 4-way handshake to xx:xx:xx:xx:xx:xx
> > iwm0: received msg 1/4 of the 4-way handshake from xx:xx:xx:xx:xx:xx
> > iwm0: sending msg 2/4 of the 4-way handshake to xx:xx:xx:xx:xx:xx
> > iwm0: received msg 1/4 of the 4-way handshake from xx:xx:xx:xx:xx:xx
> > iwm0: sending msg 2/4 of the 4-way handshake to xx:xx:xx:xx:xx:xx
> > iwm0: received msg 1/4 of the 4-way handshake from xx:xx:xx:xx:xx:xx
> > iwm0: sending msg 2/4 of the 4-way handshake to xx:xx:xx:xx:xx:xx
> >
> >
> > The diff below fixes this. Using strlcpy is incorrect for SSIDs.
> > We need to manually track the length instead and treat the SSID
> > as an opaque byte string which might not be NUL-terminated.
> >
> > iwm0: SCAN -> AUTH
> > iwm0: sending auth to xx:xx:xx:xx:xx:xx on channel 1 mode 11g
> > iwm0: AUTH -> ASSOC
> > iwm0: sending assoc_req to xx:xx:xx:xx:xx:xx on channel 1 mode 11g
> > iwm0: ASSOC -> RUN
> > iwm0: associated with xx:xx:xx:xx:xx:xx ssid "FRITZ!Box7430
> Internetmanufaktur" channel 1 start MCS 0 short preamble short slot time
> protection enabled HT enabled
> > iwm0: missed beacon threshold set to 30 beacons, beacon interval is 100
> TU
> > iwm0: received msg 1/4 of the 4-way handshake from xx:xx:xx:xx:xx:xx
> > iwm0: sending msg 2/4 of the 4-way handshake to xx:xx:xx:xx:xx:xx
> > iwm0: received msg 3/4 of the 4-way handshake from xx:xx:xx:xx:xx:xx
> > iwm0: sending msg 4/4 of the 4-way handshake to xx:xx:xx:xx:xx:xx
> >
> > ok?
>
> OK claudio@
>
> > diff 5181eb992cbbf64c135f177197957b0e0b427e21 /usr/src
> > blob - 0ee441181c9f85d12056accc352833cf31c41895
> > file + sbin/ifconfig/ifconfig.c
> > --- sbin/ifconfig/ifconfig.c
> > +++ sbin/ifconfig/ifconfig.c
> > @@ -714,7 +714,9 @@ const struct afswtch {
> > const struct afswtch *afp; /*the address family being set or asked
> about*/
> >
> > char joinname[IEEE80211_NWID_LEN];
> > +size_t joinlen;
> > char nwidname[IEEE80211_NWID_LEN];
> > +size_t nwidlen;
> >
> > int ifaliases = 0;
> > int aflag = 0;
> > @@ -1735,11 +1737,11 @@ setifnwid(const char *val, int d)
> > struct ieee80211_nwid nwid;
> > int len;
> >
> > - if (strlen(joinname) != 0) {
> > + if (joinlen != 0) {
> > errx(1, "nwid and join may not be used at the same time");
> > }
> >
> > - if (strlen(nwidname) != 0) {
> > + if (nwidlen != 0) {
> > errx(1, "nwid may not be specified twice");
> > }
> >
> > @@ -1752,9 +1754,9 @@ setifnwid(const char *val, int d)
> > if (get_string(val, NULL, nwid.i_nwid, &len) == NULL)
> > return;
> > }
> > - nwid.i_len = len;
> > + nwidlen = nwid.i_len = len;
> > (void)strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
> > - (void)strlcpy(nwidname, nwid.i_nwid, sizeof(nwidname));
> > + memcpy(nwidname, nwid.i_nwid, len);
> > ifr.ifr_data = (caddr_t)&nwid;
> > if (ioctl(sock, SIOCS80211NWID, (caddr_t)&ifr) == -1)
> > warn("SIOCS80211NWID");
> > @@ -1777,11 +1779,11 @@ setifjoin(const char *val, int d)
> > {
> > int len;
> >
> > - if (strlen(nwidname) != 0) {
> > + if (nwidlen != 0) {
> > errx(1, "nwid and join may not be used at the same time");
> > }
> >
> > - if (strlen(joinname) != 0) {
> > + if (joinlen != 0) {
> > errx(1, "join may not be specified twice");
> > }
> >
> > @@ -1796,9 +1798,9 @@ setifjoin(const char *val, int d)
> > if (len == 0)
> > join.i_flags |= IEEE80211_JOIN_ANY;
> > }
> > - join.i_len = len;
> > + joinlen = join.i_len = len;
> > (void)strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
> > - (void)strlcpy(joinname, join.i_nwid, sizeof(joinname));
> > + memcpy(joinname, join.i_nwid, len);
> >
> > actions |= A_JOIN;
> > }
> > @@ -2181,12 +2183,12 @@ setifwpakey(const char *val, int d)
> > strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
> >
> > /* Use the value specified in 'join' or 'nwid' */
> > - if (strlen(joinname) != 0) {
> > - strlcpy(nwid.i_nwid, joinname,
> sizeof(nwid.i_nwid));
> > - nwid.i_len = strlen(joinname);
> > - } else if (strlen(nwidname) != 0) {
> > - strlcpy(nwid.i_nwid, nwidname,
> sizeof(nwid.i_nwid));
> > - nwid.i_len = strlen(nwidname);
> > + if (joinlen != 0) {
> > + memcpy(nwid.i_nwid, joinname, joinlen);
> > + nwid.i_len = joinlen;
> > + } else if (nwidlen != 0) {
> > + memcpy(nwid.i_nwid, nwidname, nwidlen);
> > + nwid.i_len = nwidlen;
> > } else {
> > warnx("no nwid or join command, guessing nwid to
> use");
> >
> >
>
> --
> :wq Claudio
>
> --
--
---------------------------------------------------------------------------------------------------------------------
Knowing is not enough; we must apply. Willing is not enough; we must do
