ifconfig inet6 input validation

2019-11-07 Thread Alexander Bluhm
Hi,

Like in v4 I want to check incoming inet6 socket adresses early
with the in6_sa2sin6() function.

in6_update_ifa() needs more input validation in a later diff.

ok?

bluhm

Index: netinet6/in6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.231
diff -u -p -r1.231 in6.c
--- netinet6/in6.c  22 Oct 2019 21:40:12 -  1.231
+++ netinet6/in6.c  7 Nov 2019 22:14:16 -
@@ -182,6 +182,18 @@ in6_nam2sin6(const struct mbuf *nam, str
 }

 int
+in6_sa2sin6(struct sockaddr *sa, struct sockaddr_in6 **sin6)
+{
+   if (sa->sa_family != AF_INET6)
+   return EAFNOSUPPORT;
+   if (sa->sa_len != sizeof(struct sockaddr_in6))
+   return EINVAL;
+   *sin6 = satosin6(sa);
+
+   return 0;
+}
+
+int
 in6_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp)
 {
int privileged;
@@ -243,10 +255,10 @@ in6_ioctl(u_long cmd, caddr_t data, stru
 int
 in6_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
 {
-   struct  in6_ifreq *ifr = (struct in6_ifreq *)data;
struct  in6_ifaddr *ia6 = NULL;
struct  in6_aliasreq *ifra = (struct in6_aliasreq *)data;
-   struct sockaddr_in6 *sa6;
+   struct  sockaddr *sa;
+   struct  sockaddr_in6 *sa6 = NULL;
int error = 0, newifaddr = 0, plen;

/*
@@ -260,21 +272,29 @@ in6_ioctl_change_ifaddr(u_long cmd, cadd
 * on a single interface, we almost always look and check the
 * presence of ifra_addr, and reject invalid ones here.
 * It also decreases duplicated code among SIOC*_IN6 operations.
+*
+* We always require users to specify a valid IPv6 address for
+* the corresponding operation.
 */
switch (cmd) {
case SIOCAIFADDR_IN6:
-   sa6 = >ifra_addr;
+   sa = sin6tosa(>ifra_addr);
break;
case SIOCDIFADDR_IN6:
-   sa6 = >ifr_addr;
+   sa = sin6tosa(&((struct in6_ifreq *)data)->ifr_addr);
break;
default:
-   panic("unknown ioctl %lu", cmd);
+   panic("%s: invalid ioctl %lu", __func__, cmd);
+   }
+   if (sa->sa_family == AF_INET6) {
+   error = in6_sa2sin6(sa, );
+   if (error)
+   return (error);
}

NET_LOCK();

-   if (sa6 && sa6->sin6_family == AF_INET6) {
+   if (sa6 != NULL) {
error = in6_check_embed_scope(sa6, ifp->if_index);
if (error)
goto err;
@@ -297,30 +317,11 @@ in6_ioctl_change_ifaddr(u_long cmd, cadd
error = EADDRNOTAVAIL;
break;
}
-   /*
-* We always require users to specify a valid IPv6 address for
-* the corresponding operation.
-*/
-   if (ifra->ifra_addr.sin6_family != AF_INET6 ||
-   ifra->ifra_addr.sin6_len != sizeof(struct sockaddr_in6)) {
-   error = EAFNOSUPPORT;
-   break;
-   }
in6_purgeaddr(>ia_ifa);
dohooks(ifp->if_addrhooks, 0);
break;

case SIOCAIFADDR_IN6:
-   /*
-* We always require users to specify a valid IPv6 address for
-* the corresponding operation.
-*/
-   if (ifra->ifra_addr.sin6_family != AF_INET6 ||
-   ifra->ifra_addr.sin6_len != sizeof(struct sockaddr_in6)) {
-   error = EAFNOSUPPORT;
-   break;
-   }
-
/* reject read-only flags */
if ((ifra->ifra_flags & IN6_IFF_DUPLICATED) != 0 ||
(ifra->ifra_flags & IN6_IFF_DETACHED) != 0 ||
@@ -346,13 +347,15 @@ in6_ioctl_change_ifaddr(u_long cmd, cadd
 * is no link-local yet.
 */
error = in6_ifattach(ifp);
-   if (error != 0)
+   if (error)
break;
error = in6_update_ifa(ifp, ifra, ia6);
-   if (error != 0)
+   if (error)
break;

-   ia6 = in6ifa_ifpwithaddr(ifp, >ifra_addr.sin6_addr);
+   ia6 = NULL;
+   if (sa6 != NULL)
+   ia6 = in6ifa_ifpwithaddr(ifp, >sin6_addr);
if (ia6 == NULL) {
/*
 * this can happen when the user specify the 0 valid
@@ -397,14 +400,20 @@ in6_ioctl_get(u_long cmd, caddr_t data,
 {
struct  in6_ifreq *ifr = (struct in6_ifreq *)data;
struct  in6_ifaddr *ia6 = NULL;
-   struct sockaddr_in6 *sa6;
+   struct  sockaddr *sa;
+   struct  sockaddr_in6 *sa6 = NULL;
int error = 0;

-   

Re: right/ tzdata files (was ports@ Re: (Mozilla) Thunderbird time zone issue)

2019-11-07 Thread Todd C . Miller
On Thu, 07 Nov 2019 15:38:04 +, Stuart Henderson wrote:

> Now I've gone through mkr, here's the diff.
>
> base66 goes from 8100->6870 inodes as it avoids the duplicated copy
> of the standard files, as well as the leap-seconds files.

Fine with me.

 - todd



Re: right/ tzdata files (was ports@ Re: (Mozilla) Thunderbird time zone issue)

2019-11-07 Thread Theo de Raadt
Fine with me.

Stuart Henderson  wrote:

> On 2019/10/26 15:32, Mark Kettenis wrote:
> > 
> > The fundamental problem with the "right" files is that the time_t
> > values end up being different from their POSIX values for the same UTC
> > time.  So whenever these are stored and compared between systems (or
> > environments that set the TZ environment variable) things get weird.
> > 
> > > I think so.  Unless there are programs that use these files directly
> > > I don't see a real use for them.
> > 
> > Agreed.  Software that really cares probably has its own leap-second
> > table and will actually rely on the POSIX definition of time_t to
> > convert times into human readable form.  That's at least what the
> > software I've seen and written does ;).
> 
> Now I've gone through mkr, here's the diff.
> 
> base66 goes from 8100->6870 inodes as it avoids the duplicated copy
> of the standard files, as well as the leap-seconds files.
> 
> OK?
> 
> Index: share/zoneinfo/Makefile
> ===
> RCS file: /cvs/src/share/zoneinfo/Makefile,v
> retrieving revision 1.13
> diff -u -p -r1.13 Makefile
> --- share/zoneinfo/Makefile   22 Jan 2019 05:44:40 -  1.13
> +++ share/zoneinfo/Makefile   7 Nov 2019 15:34:07 -
> @@ -36,7 +36,7 @@ TZDIR=  ${DESTDIR}/usr/share/zoneinfo
>  #REDO=   right_posix
>  # below.
>  
> -REDO=posix_right
> +REDO=posix_only
>  
>  # Since "." may not be in PATH...
>  YEARISTYPE=  ${.CURDIR}/datfiles/yearistype.sh
> Index: distrib/sets/lists/base/mi
> ===
> RCS file: /cvs/src/distrib/sets/lists/base/mi,v
> retrieving revision 1.966
> diff -u -p -r1.966 mi
> --- distrib/sets/lists/base/mi2 Nov 2019 13:54:04 -   1.966
> +++ distrib/sets/lists/base/mi7 Nov 2019 15:34:07 -
> @@ -6235,1237 +6235,7 @@
>  ./usr/share/zoneinfo/W-SU
>  ./usr/share/zoneinfo/WET
>  ./usr/share/zoneinfo/Zulu
> -./usr/share/zoneinfo/posix
> -./usr/share/zoneinfo/posix/Africa
> -./usr/share/zoneinfo/posix/Africa/Abidjan
> -./usr/share/zoneinfo/posix/Africa/Accra
> -./usr/share/zoneinfo/posix/Africa/Addis_Ababa
> -./usr/share/zoneinfo/posix/Africa/Algiers
> -./usr/share/zoneinfo/posix/Africa/Asmara
> -./usr/share/zoneinfo/posix/Africa/Asmera
> -./usr/share/zoneinfo/posix/Africa/Bamako
> -./usr/share/zoneinfo/posix/Africa/Bangui
> -./usr/share/zoneinfo/posix/Africa/Banjul
> -./usr/share/zoneinfo/posix/Africa/Bissau
> -./usr/share/zoneinfo/posix/Africa/Blantyre
> -./usr/share/zoneinfo/posix/Africa/Brazzaville
> -./usr/share/zoneinfo/posix/Africa/Bujumbura
> -./usr/share/zoneinfo/posix/Africa/Cairo
> -./usr/share/zoneinfo/posix/Africa/Casablanca
> -./usr/share/zoneinfo/posix/Africa/Ceuta
> -./usr/share/zoneinfo/posix/Africa/Conakry
> -./usr/share/zoneinfo/posix/Africa/Dakar
> -./usr/share/zoneinfo/posix/Africa/Dar_es_Salaam
> -./usr/share/zoneinfo/posix/Africa/Djibouti
> -./usr/share/zoneinfo/posix/Africa/Douala
> -./usr/share/zoneinfo/posix/Africa/El_Aaiun
> -./usr/share/zoneinfo/posix/Africa/Freetown
> -./usr/share/zoneinfo/posix/Africa/Gaborone
> -./usr/share/zoneinfo/posix/Africa/Harare
> -./usr/share/zoneinfo/posix/Africa/Johannesburg
> -./usr/share/zoneinfo/posix/Africa/Juba
> -./usr/share/zoneinfo/posix/Africa/Kampala
> -./usr/share/zoneinfo/posix/Africa/Khartoum
> -./usr/share/zoneinfo/posix/Africa/Kigali
> -./usr/share/zoneinfo/posix/Africa/Kinshasa
> -./usr/share/zoneinfo/posix/Africa/Lagos
> -./usr/share/zoneinfo/posix/Africa/Libreville
> -./usr/share/zoneinfo/posix/Africa/Lome
> -./usr/share/zoneinfo/posix/Africa/Luanda
> -./usr/share/zoneinfo/posix/Africa/Lubumbashi
> -./usr/share/zoneinfo/posix/Africa/Lusaka
> -./usr/share/zoneinfo/posix/Africa/Malabo
> -./usr/share/zoneinfo/posix/Africa/Maputo
> -./usr/share/zoneinfo/posix/Africa/Maseru
> -./usr/share/zoneinfo/posix/Africa/Mbabane
> -./usr/share/zoneinfo/posix/Africa/Mogadishu
> -./usr/share/zoneinfo/posix/Africa/Monrovia
> -./usr/share/zoneinfo/posix/Africa/Nairobi
> -./usr/share/zoneinfo/posix/Africa/Ndjamena
> -./usr/share/zoneinfo/posix/Africa/Niamey
> -./usr/share/zoneinfo/posix/Africa/Nouakchott
> -./usr/share/zoneinfo/posix/Africa/Ouagadougou
> -./usr/share/zoneinfo/posix/Africa/Porto-Novo
> -./usr/share/zoneinfo/posix/Africa/Sao_Tome
> -./usr/share/zoneinfo/posix/Africa/Timbuktu
> -./usr/share/zoneinfo/posix/Africa/Tripoli
> -./usr/share/zoneinfo/posix/Africa/Tunis
> -./usr/share/zoneinfo/posix/Africa/Windhoek
> -./usr/share/zoneinfo/posix/America
> -./usr/share/zoneinfo/posix/America/Adak
> -./usr/share/zoneinfo/posix/America/Anchorage
> -./usr/share/zoneinfo/posix/America/Anguilla
> -./usr/share/zoneinfo/posix/America/Antigua
> -./usr/share/zoneinfo/posix/America/Araguaina
> -./usr/share/zoneinfo/posix/America/Argentina
> -./usr/share/zoneinfo/posix/America/Argentina/Buenos_Aires
> -./usr/share/zoneinfo/posix/America/Argentina/Catamarca
> 

httpd(8): fix duplicate location detection

2019-11-07 Thread Matthias

During httpd setup I realized that duplicate location names are not
being detected at all, even though I remembered having seen a
corresponding piece of code in 'usr.sbin/httpd/parse.y' the other day.
As far as I understand, the comparison 's->srv_conf.id == srv_conf->id'
will never be true as the ID of any newly created location surely won't
be found in any location created before.

To check whether or not I was right, I recompiled httpd with DEBUG
options and tried to start the server with the following httpd.conf:


server "testserver" {
listen on 127.0.0.1 port www
location "/foo" { block }
location "/foo" { block }
}


# httpd -vvd
startup
adding location "/foo" for "testserver[2]"
adding location "/foo" for "testserver[3]"
adding server "testserver[1]"

(httpd running)


I guess the intention was to only compare the new name with all other
location names available under the same parent server. I accomplished
this by applying the patch at the bottom of this message. After
recompiling, httpd startup terminates as expected.

# httpd -vvd
startup
adding location "/foo" for "testserver[2]"
/etc/httpd.conf:4: location "/foo" defined twice
.
logger exiting, pid 98967
server exiting, pid 27723
server exiting, pid 78507
server exiting, pid 25743


Let me know if I got something wrong.

---


Index: usr.sbin/httpd/parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.113
diff -u -p -r1.113 parse.y
--- usr.sbin/httpd/parse.y  28 Jun 2019 13:32:47 -  1.113
+++ usr.sbin/httpd/parse.y  7 Nov 2019 11:36:14 -
@@ -564,7 +564,8 @@ serveroptsl : LISTEN ON STRING opttls po

TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
if ((s->srv_conf.flags &
SRVFLAG_LOCATION) &&
-   s->srv_conf.id == srv_conf->id &&
+   s->srv_conf.parent_id ==
+   srv_conf->parent_id &&
strcmp(s->srv_conf.location,
srv_conf->location) == 0)
break;



Re: sysupgrade: Allow to use another directory for the sets

2019-11-07 Thread Craig Skinner
On Wed, 6 Nov 2019 13:41:07 +0100 Renaud Allard wrote:
> Given the amount of people which encrypt /home directory on their 
> servers, it might be useful to be able to define another directory for 
> the sets in sysupgrade as /home_sysupgrade will not be available in that 
> case.

How about /var/cache/sysupgrade/ as the default?

i.e: sysupgrade caches files which are variable over time.


Cheers,
-- 
Craig Skinner | http://linkd.in/yGqkv7



use tasks and a task_list to manage if_addrhooks

2019-11-07 Thread David Gwynne
this applies the use of tasks and a task_list to interface address
hooks. it's like the detach and linkstate hooks, except it seems other
things run the hooks more than things register hooks, and i can't tell
if the places that run the hooks have the NET_LOCK or not. not by casual
reading anyway.

to cope with if_addrhooks_run maybe not being called with NET_LOCK being
held, i made it safe to call the hook runner multiple times
concurrently.

one of the users of address hooks is pf, and the pfi_kif struct. it's
part of the ABI, pfctl and snmpd use it, so i kept it using a void * and
had it allocate the task separately. it should be as robust as it was
before.

everything else was pretty straightforward.

tests? ok?

Index: kern/kern_task.c
===
RCS file: /cvs/src/sys/kern/kern_task.c,v
retrieving revision 1.26
diff -u -p -r1.26 kern_task.c
--- kern/kern_task.c23 Jun 2019 12:56:10 -  1.26
+++ kern/kern_task.c7 Nov 2019 11:21:00 -
@@ -258,6 +258,8 @@ taskq_barrier_task(void *p)
 void
 task_set(struct task *t, void (*fn)(void *), void *arg)
 {
+   KASSERT(fn != NULL);
+
t->t_func = fn;
t->t_arg = arg;
t->t_flags = 0;
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.591
diff -u -p -r1.591 if.c
--- net/if.c7 Nov 2019 08:03:18 -   1.591
+++ net/if.c7 Nov 2019 11:21:00 -
@@ -630,9 +630,7 @@ if_attach_common(struct ifnet *ifp)
ifp->if_iqs = ifp->if_rcv.ifiq_ifiqs;
ifp->if_niqs = 1;
 
-   ifp->if_addrhooks = malloc(sizeof(*ifp->if_addrhooks),
-   M_TEMP, M_WAITOK);
-   TAILQ_INIT(ifp->if_addrhooks);
+   TAILQ_INIT(>if_addrhooks);
TAILQ_INIT(>if_linkstatehooks);
TAILQ_INIT(>if_detachhooks);
 
@@ -1046,19 +1044,18 @@ if_netisr(void *unused)
 void
 if_hooks_run(struct task_list *hooks)
 {
-   struct task *t, *nt, cursor;
+   struct task *t, *nt;
+   struct task cursor = { .t_func = NULL };
void (*func)(void *);
void *arg;
 
-   /*
-* holding the NET_LOCK guarantees that concurrent if_hooks_run
-* calls can't happen, and they therefore can't try and call
-* each others cursors as actual hooks.
-*/
-   NET_ASSERT_LOCKED();
-
mtx_enter(_hooks_mtx);
for (t = TAILQ_FIRST(hooks); t != NULL; t = nt) {
+   while (t->t_func == NULL) { /* skip cursors */
+   t = TAILQ_NEXT(t, t_entry);
+   if (t == NULL)
+   break;
+   }
func = t->t_func;
arg = t->t_arg;
 
@@ -1177,7 +1174,7 @@ if_detach(struct ifnet *ifp)
}
}
 
-   free(ifp->if_addrhooks, M_TEMP, sizeof(*ifp->if_addrhooks));
+   KASSERT(TAILQ_EMPTY(>if_addrhooks));
KASSERT(TAILQ_EMPTY(>if_linkstatehooks));
KASSERT(TAILQ_EMPTY(>if_detachhooks));
 
@@ -3100,7 +3097,7 @@ ifnewlladdr(struct ifnet *ifp)
ifa = _ifpforlinklocal(ifp, 0)->ia_ifa;
if (ifa) {
in6_purgeaddr(ifa);
-   dohooks(ifp->if_addrhooks, 0);
+   if_hooks_run(>if_addrhooks);
in6_ifattach(ifp);
}
}
@@ -3112,6 +3109,28 @@ ifnewlladdr(struct ifnet *ifp)
(*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t));
}
splx(s);
+}
+
+void
+if_addrhook_add(struct ifnet *ifp, struct task *t)
+{
+   mtx_enter(_hooks_mtx);
+   TAILQ_INSERT_TAIL(>if_addrhooks, t, t_entry);
+   mtx_leave(_hooks_mtx);
+}
+
+void
+if_addrhook_del(struct ifnet *ifp, struct task *t)
+{
+   mtx_enter(_hooks_mtx);
+   TAILQ_REMOVE(>if_addrhooks, t, t_entry);
+   mtx_leave(_hooks_mtx);
+}
+
+void
+if_addrhooks_run(struct ifnet *ifp)
+{
+   if_hooks_run(>if_addrhooks);
 }
 
 int net_ticks;
Index: net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.68
diff -u -p -r1.68 if_pppx.c
--- net/if_pppx.c   24 Jun 2019 13:43:19 -  1.68
+++ net/if_pppx.c   7 Nov 2019 11:21:00 -
@@ -919,7 +919,7 @@ pppx_add_session(struct pppx_dev *pxd, s
printf("pppx: unable to set addresses for %s, error=%d\n",
ifp->if_xname, error);
} else {
-   dohooks(ifp->if_addrhooks, 0);
+   if_addrhooks_run(ifp);
}
rw_enter_write(_ifs_lk);
pxi->pxi_ready = 1;
Index: net/if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.179
diff -u -p -r1.179 if_spppsubr.c
--- net/if_spppsubr.c   24 Jun 2019 21:36:53 -  1.179
+++ net/if_spppsubr.c   7 Nov 2019 11:21:00 -
@@ -4230,7 +4230,7 @@ 

Re: make 'ifconfig scan' trigger a background scan

2019-11-07 Thread Stefan Sperling
On Thu, Nov 07, 2019 at 09:22:40AM +, Stuart Henderson wrote:
> On 2019/11/07 11:10, Lauri Tirkkonen wrote:
> > I think it might be a little confusing to make this operation implicit
> > in another operation (ie. list APs), and for root only. How does a user
> > find out about this feature?
> 
> The manpage part of the diff is yet to be written ;)
> 
> >  Wouldn't it be better to have a separate
> > command to trigger the scan, eg. 'rescan'? That could be documented and
> > restricted to root.
> 
> But the command is not "ifconfig list_aps", it's "ifconfig scan" and
> it's really expected that this would trigger a scan. To me the current
> behaviour (where "ifconfig iwm0 scan" might give you a list from 300km
> while you were travelling connected to your phone hotspot) with no
> way to update it short of forcing a disconnection is more confusing.

Note also that this diff only fixes the issue for drivers which support
background scanning in the first place, namely iwn(4) and iwm(4).
Other drivers will still require down/up because they need to be moved
out of RUN (associated) state in order to scan.



Re: make 'ifconfig scan' trigger a background scan

2019-11-07 Thread Lauri Tirkkonen
On Thu, Nov 07 2019 09:22:40 +, Stuart Henderson wrote:
> >  Wouldn't it be better to have a separate
> > command to trigger the scan, eg. 'rescan'? That could be documented and
> > restricted to root.
> 
> But the command is not "ifconfig list_aps", it's "ifconfig scan" and
> it's really expected that this would trigger a scan. To me the current
> behaviour (where "ifconfig iwm0 scan" might give you a list from 300km
> while you were travelling connected to your phone hotspot) with no
> way to update it short of forcing a disconnection is more confusing.
> 
> It's not ideal that "ifconfig scan" prints an old list at all, but that
> is the current status and this diff doesn't change that. Still it's an
> improvement. Could it be made better still? Yes, but I don't see that as
> a reason to hold off on making *this* change.

Yeah, I totally agree.

-- 
Lauri Tirkkonen | lotheac @ IRCnet



Re: make 'ifconfig scan' trigger a background scan

2019-11-07 Thread Stuart Henderson
On 2019/11/07 11:10, Lauri Tirkkonen wrote:
> I think it might be a little confusing to make this operation implicit
> in another operation (ie. list APs), and for root only. How does a user
> find out about this feature?

The manpage part of the diff is yet to be written ;)

>  Wouldn't it be better to have a separate
> command to trigger the scan, eg. 'rescan'? That could be documented and
> restricted to root.

But the command is not "ifconfig list_aps", it's "ifconfig scan" and
it's really expected that this would trigger a scan. To me the current
behaviour (where "ifconfig iwm0 scan" might give you a list from 300km
while you were travelling connected to your phone hotspot) with no
way to update it short of forcing a disconnection is more confusing.

It's not ideal that "ifconfig scan" prints an old list at all, but that
is the current status and this diff doesn't change that. Still it's an
improvement. Could it be made better still? Yes, but I don't see that as
a reason to hold off on making *this* change.



Re: make 'ifconfig scan' trigger a background scan

2019-11-07 Thread Lauri Tirkkonen
Hi, a small, hopefully not too bikesheddy note from a user perspective:

On Thu, Nov 07 2019 08:59:54 +, Stuart Henderson wrote:
> On 2019/11/06 19:17, Stefan Sperling wrote:
> > This diff allows the root user to trigger a background scan with:
> > 
> > ifconfig iwm0 scan
> > 
> > It supports two use cases which are currently not supported:
> > 
> > 1) It will force an attempt at finding a better AP, even if the
> > current AP is above the signal level threshold which will usually
> > trigger a background scan.
> > 
> > 2) It will update the list of cached APs. The updated list will be
> > shown by subsequent invocations of 'ifconfig scan'. This allows the
> > root user to view an up-to-date list of available networks without
> > disassociating from the current AP. Currently, the list is updated only
> > if a background scan is triggered via the signal strength threshold.
> > 
> > Suggested by sthen@
> > 
> > ok?
> 
> I'm finding this very helpful, if I'm on a low signal but working AP and
> suspect that another one may be better this allows me to easily rescan
> and (fairly seamlessly) move to a better one. Latency is a little higher
> for a few seconds while the scan completes but packets continue flowing.
> 
> Currently the only way to trigger a scan on a bgscan interface (unless
> moving to a low signal area) is ifconfig down+up which interrupts
> connectivity.
> 
> The main thing I'm unsure about is whether to restrict this to root (as
> this diff does) or not, there are arguments both ways - some will think
> that it's also useful for an unprivileged user to be able to trigger a
> search for a better AP, some will think that it's more useful to prevent
> an unprivileged [possibly remote] user from being able to potentially
> trigger a move to a different AP. Anyway this is an improvement and we
> could remove the restriction later if wanted, so I'm OK with this as-is.

I think it might be a little confusing to make this operation implicit
in another operation (ie. list APs), and for root only. How does a user
find out about this feature? Wouldn't it be better to have a separate
command to trigger the scan, eg. 'rescan'? That could be documented and
restricted to root.

-- 
Lauri Tirkkonen | lotheac @ IRCnet



Re: make 'ifconfig scan' trigger a background scan

2019-11-07 Thread Stuart Henderson
On 2019/11/06 19:17, Stefan Sperling wrote:
> This diff allows the root user to trigger a background scan with:
> 
>   ifconfig iwm0 scan
> 
> It supports two use cases which are currently not supported:
> 
> 1) It will force an attempt at finding a better AP, even if the
> current AP is above the signal level threshold which will usually
> trigger a background scan.
> 
> 2) It will update the list of cached APs. The updated list will be
> shown by subsequent invocations of 'ifconfig scan'. This allows the
> root user to view an up-to-date list of available networks without
> disassociating from the current AP. Currently, the list is updated only
> if a background scan is triggered via the signal strength threshold.
> 
> Suggested by sthen@
> 
> ok?

I'm finding this very helpful, if I'm on a low signal but working AP and
suspect that another one may be better this allows me to easily rescan
and (fairly seamlessly) move to a better one. Latency is a little higher
for a few seconds while the scan completes but packets continue flowing.

Currently the only way to trigger a scan on a bgscan interface (unless
moving to a low signal area) is ifconfig down+up which interrupts
connectivity.

The main thing I'm unsure about is whether to restrict this to root (as
this diff does) or not, there are arguments both ways - some will think
that it's also useful for an unprivileged user to be able to trigger a
search for a better AP, some will think that it's more useful to prevent
an unprivileged [possibly remote] user from being able to potentially
trigger a move to a different AP. Anyway this is an improvement and we
could remove the restriction later if wanted, so I'm OK with this as-is.


> diff 1bc5f745876be1d7923afca9b40fb6641254e697 /usr/src (staged changes)
> blob - b7b2ea6d631958b607ffdee06d272cd56320bf2e
> blob + d4c2a0ba6b1893af9652837998c759a6c0857048
> --- sys/net80211/ieee80211_ioctl.c
> +++ sys/net80211/ieee80211_ioctl.c
> @@ -906,6 +906,8 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t
>   na->na_nodes++;
>   ni = RBT_NEXT(ieee80211_tree, ni);
>   }
> + if (suser(curproc) == 0)
> + ieee80211_begin_bgscan(ifp);
>   break;
>   case SIOCG80211FLAGS:
>   flags = ic->ic_userflags;
> blob - b493ff0a5362fa11d7824fd8bd0ae0d57462d6c4
> blob + 501cbca0b9b8422fc9384ffbb9fb2c36b16cd2c0
> --- sys/net80211/ieee80211_var.h
> +++ sys/net80211/ieee80211_var.h
> @@ -460,6 +460,7 @@ void  ieee80211_del_ess(struct ieee80211com *, char 
> *, 
>  void ieee80211_set_ess(struct ieee80211com *, struct ieee80211_ess *,
>   struct ieee80211_node *);
>  struct ieee80211_ess *ieee80211_get_ess(struct ieee80211com *, const char *, 
> int);
> +void ieee80211_begin_bgscan(struct ifnet *);
>  
>  extern   int ieee80211_cache_size;
>  
> 



Re: tcpdump(8) mention USB interfaces in -i

2019-11-07 Thread Stuart Henderson
On 2019/11/07 19:05, Damien Miller wrote:
> goddamn it, I could have used this last week :/

Oh, you may be annoyed to find out that wireshark usb capture works too
then - your method looked more fun though!

> (ok djm)

Thanks.



Re: tcpdump(8) mention USB interfaces in -i

2019-11-07 Thread Damien Miller
goddamn it, I could have used this last week :/

(ok djm)

On Wed, 6 Nov 2019, Stuart Henderson wrote:

> Found this diff when updating an old tree, ok?
> 
> 
> Index: usr.sbin/tcpdump/tcpdump.8
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v
> retrieving revision 1.108
> diff -u -p -r1.108 tcpdump.8
> --- usr.sbin/tcpdump/tcpdump.831 Oct 2019 18:10:22 -  1.108
> +++ usr.sbin/tcpdump/tcpdump.86 Nov 2019 12:12:54 -
> @@ -146,6 +146,9 @@ searches the system interface list for t
>  interface
>  .Pq excluding loopback .
>  Ties are broken by choosing the earliest match.
> +.Ar interface
> +may be either a network interface or a USB interface, for example
> +.Ar usb0 .
>  .It Fl L
>  List the supported data link types for the interface and exit.
>  .It Fl l
> 
>