Re: ttymalloc(int baud)

2020-05-26 Thread Theo de Raadt
The decision to allocate more storage is decided by an unexpected
usage pattern we encountered after the API was designed, which isn't
immediately obvious from the device name...

Some of these devices are designed for high-speed PPP.  Some of them (in
particular on USB) actually move bytes faster than their advertised baud
rate.  They were not working well.  So the buffers were grown.  That
also adjusted the watermarks, and then things worked better.  tty.c 1.128

But was the same good for all ttys?  Do consider the calculated watermarks.

At the time -- cautiously -- we didn't want to grow the buffers for all
the ttys.

Maybe that decision can change?  This one leads me to think we can:

tty_pty.c:  pti->pt_tty = ttymalloc(100);

992 maximum ptys, that's almost 8MB of memory.

But we come back to the watermark calculation... which is why the
expected baud rate (maximum baud rate?) is the argument..

Wow. I wrote this code earlier than 1993 when replacing the broken Jolitz
rings and creating a proper clist-compat layer...


 wrote:

> Another trivial code readability comment:
> 
> I see 24 references to this in the code, 20 of which pass 0, and 4 of
> which pass 1,000,000 as the parameter.
> 
> Passing 0 defaults to a baud of 115200.  The baud determines the qlen,
> which is 4096 for 115200 and 8192 for anything larger, so all it is
> doing now is selecting between two buffer sizes via magic number
> parameters.
> 
> It would probably be appropriate today to just always use 8192 and not
> even take a parameter; it doesn't look like the selection between the
> two is done in a rigorous way.
> 
> There is also a comment in umodem.c that has an outdated assumption:
> /*
>  * These are the maximum number of bytes transferred per frame.
>  * Buffers are this large to deal with high speed wireless devices.
>  * Capped at 1024 as ttymalloc() is limited to this amount.
>  */
> #define UMODEMIBUFSIZE 1024
> #define UMODEMOBUFSIZE 1024

Yes, that is weird and wrong.



Re: libpcap: allow breaking out of loop when using savefile

2020-05-26 Thread David Gwynne
I just committed this, thank you :)

dlg

> On 16 May 2020, at 05:14, Caspar Schutijser  wrote:
> 
> Hi,
> 
> Below is a patch that makes breaking out of the loop work when using
> a savefile.
> 
> The pcap_breakloop() function was backported from tcpdump.org libpcap
> to OpenBSD libpcap by djm@ on Nov 18, 2005. The bits to make
> pcap_breakloop() work were backported to pcap-bpf.c [1] but not to
> savefile.c even though tcpdump.org implemented support there too [2].
> 
> The diff below backports this piece of code to savefile.c after all.
> 
> Thanks,
> Caspar Schutijser
> 
> [1] 
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libpcap/pcap-bpf.c.diff?r1=1.16=1.17
> [2] 
> https://github.com/the-tcpdump-group/libpcap/commit/991d444f7116bef16893826b46f3950f62281507#diff-95d4d29d0f11145ff40b850860667a97R745
> 
> 
> Index: savefile.c
> ===
> RCS file: /cvs/src/lib/libpcap/savefile.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 savefile.c
> --- savefile.c22 Dec 2015 19:51:04 -  1.16
> +++ savefile.c15 May 2020 19:03:44 -
> @@ -307,6 +307,23 @@ pcap_offline_read(pcap_t *p, int cnt, pc
>   while (status == 0) {
>   struct pcap_pkthdr h;
> 
> + /*
> +  * Has "pcap_breakloop()" been called?
> +  * If so, return immediately - if we haven't read any
> +  * packets, clear the flag and return -2 to indicate
> +  * that we were told to break out of the loop, otherwise
> +  * leave the flag set, so that the *next* call will break
> +  * out of the loop without having read any packets, and
> +  * return the number of packets we've processed so far.
> +  */
> + if (p->break_loop) {
> + if (n == 0) {
> + p->break_loop = 0;
> + return (PCAP_ERROR_BREAK);
> + } else
> + return (n);
> + }
> +
>   status = sf_next_packet(p, , p->buffer, p->bufsize);
>   if (status) {
>   if (status == 1)
> 



Re: [PATCH] pipex(4): rework PPP input

2020-05-26 Thread Vitaliy Makkoveev



> On 27 May 2020, at 01:29, Sergey Ryazanov  wrote:
> 
> On Tue, May 26, 2020 at 12:07 PM Vitaliy Makkoveev
>  wrote:
>>> On 25 May 2020, at 22:04, Sergey Ryazanov  wrote:
>>> On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev
>>>  wrote:
 For example, each pipex session should have unique pair of `protocol’ and
 `session_id’. These values are passed from userland. While the only
 instance of npppd(8) uses pipex(4) this is not the problem. But you
 introduce the case while pipex(4) will be used by multiple independent
 userland programs. At least, I have interest how you handle this.
>>> 
>>> This should not be a problem here. npppd(8) support server mode only.
>>> While my work is to implement acceleration for client side of L2TP
>>> connection.
>> 
>> I guess they can coexist. Also you can have multiple connections to
>> ppp servers simultaneously.
> 
> With 16 bits long session id field, according to birthday problem to
> reach 0.9 collision probability I need 549 simultaneous sessions.
> Should I still be worried or I have a time to complete integration
> work and then update UDP  filter for love of the game?

Why do you ask me? Do what you want.

> 
> -- 
> Sergey
> 



Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Jason A. Donenfeld
On Tue, May 26, 2020 at 4:52 PM Jason A. Donenfeld  wrote:
> With regards to your crash, though, that's a bit more puzzling, and
> I'd be interested to learn more details. Because these structs are
> already naturally aligned, the __packed attribute, even with the odd
> nesting Matt had prior, should have produced all entirely aligned
> accesses. That makes me think your kaboom was coming from someplace
> else. One possibility is that you were running the git tree on the two
> days that I was playing with uint128_t, only to find out that some of
> openbsd's patches to clang miscalculate stack sizes when they're in
> use, so that work was shelved for another day and the commits removed;
> perhaps you were just unlucky? Or you hit some other bug that's
> lurking. Either way, output from ddb's `bt` would at least be useful.

Do you know off hand if we're able to assume any type of alignment
with mbuf->m_data? mtod just casts without any address fixup, which
means if mbuf->m_data isn't aligned by some other mechanism, we're in
trouble. But I would assume there _is_ some alignment imposed, since
the rest of the stack appears to parse tcp headers and such directly
without byte-by-byte copies being made.



Re: [PATCH] pipex(4): rework PPP input

2020-05-26 Thread Sergey Ryazanov
On Wed, May 27, 2020 at 2:12 AM Vitaliy Makkoveev
 wrote:
> > On 27 May 2020, at 01:29, Sergey Ryazanov  wrote:
> > On Tue, May 26, 2020 at 12:07 PM Vitaliy Makkoveev
> >  wrote:
> >>> On 25 May 2020, at 22:04, Sergey Ryazanov  wrote:
> >>> On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev
> >>>  wrote:
>  For example, each pipex session should have unique pair of `protocol’ and
>  `session_id’. These values are passed from userland. While the only
>  instance of npppd(8) uses pipex(4) this is not the problem. But you
>  introduce the case while pipex(4) will be used by multiple independent
>  userland programs. At least, I have interest how you handle this.
> >>>
> >>> This should not be a problem here. npppd(8) support server mode only.
> >>> While my work is to implement acceleration for client side of L2TP
> >>> connection.
> >>
> >> I guess they can coexist. Also you can have multiple connections to
> >> ppp servers simultaneously.
> >
> > With 16 bits long session id field, according to birthday problem to
> > reach 0.9 collision probability I need 549 simultaneous sessions.
> > Should I still be worried or I have a time to complete integration
> > work and then update UDP  filter for love of the game?
>
> Why do you ask me? Do what you want.

I want to prioritize my todo list, and I in need of advice, is
possible collision is non-critical at the moment, or I missed
something and should solve it first?

-- 
Serge



Re: [PATCH] tcpdump: add ppp address/protocol compression support

2020-05-26 Thread Sergey Ryazanov
Hello!

On Mon, May 4, 2020 at 9:37 PM Sergey Ryazanov  wrote:
> Add support for parsing ppp frames with compressed address and(or)
> protocol fields. Since we have no apriory information than try to
> guess such frames by inability to parse a frame in a regular way.
>
> ok?

Does someone have any objections on this diff?

-- 
Sergey



Re: [RFC] pppd: add pipex(4) L2TP control support

2020-05-26 Thread Sergey Ryazanov
On Tue, May 26, 2020 at 11:31 AM Claudio Jeker  wrote:
> On Tue, May 26, 2020 at 09:22:28AM +0200, Martin Pieuchot wrote:
> > On 25/05/20(Mon) 21:42, Sergey Ryazanov wrote:
> > > Add dedicated option to activate kernel L2TP acceleration via
> > > the pipex(4). The options should be passed by a L2TP tunnel
> > > management daemon (e.g. xl2tpd).
> >
> > What is the difference between npppd(8) and pppd(8)?   Aren't those two
> > redundant?  Why did you choose to modify pppd(8) and not npppd(8)?
>
> npppd(8) is server only it can not establish a connection. pppd(8) on the
> other hand is more client side (but I think it can do both).

True.

> I think the split in userland makes sense (using npppd as LNS/LAC and pppd
> to connect to a ISP via PPP (even though we also use sppp/pppoe(4) for
> that). Now for the kernel side it would be great if the interface used
> would be the same for all ppp users. In this regard this is a step in the
> right direction.

Comparing addition of client support to npppd and reworking pppd to
support pppx(4), I will choose the npppd reworking. The pppd code is
the mess that is too tightly connected with TTY.

Moreover, pppd is designed to establish the PPP connection over any
type of serial line, including RS232 connected Dial-Up modem. So
introducing the pppx(4) support will require preserving of all
existing code *and* creation of a big module that is responsible
pppx(4) handling in case of L2TP connection. And such feature will
require a hard xl2tpd daemon reworking too. It is easier to rework
npppd or ever create a new L2TP client daemon from the scratch.

So I found a 3rd solution - add a pipex(4) support for the good
old-fashioned ppp(4) and a tiny option for pppd. And this my plan if
someone do not have a better solution.

> > > diff --git usr.sbin/pppd/ipcp.c usr.sbin/pppd/ipcp.c
> > > index 1296d897b14..e7a6dbd6ee7 100644
> > > --- usr.sbin/pppd/ipcp.c
> > > +++ usr.sbin/pppd/ipcp.c
> > > @@ -1251,6 +1251,10 @@ ipcp_up(f)
> > > return;
> > > }
> > >
> > > +   /* enable pipex(4), keep working if failed */
> > > +   if (pipex_conf.pr_protocol && !apipex(go->ouraddr, ho->hisaddr, mask))
> > > +   IPCPDEBUG((LOG_WARNING, "apipex failed"));
> > > +
> > >  #if (defined(SVR4) && (defined(SNI) || defined(__USLC__)))
> > > if (!sifaddr(f->unit, go->ouraddr, ho->hisaddr, mask)) {
> > > IPCPDEBUG((LOG_WARNING, "sifaddr failed"));
> > > @@ -1304,6 +1308,7 @@ ipcp_down(f)
> > >  if (demand) {
> > > sifnpmode(f->unit, PPP_IP, NPMODE_QUEUE);
> > >  } else {
> > > +   dpipex();
> > > sifdown(f->unit);
> > > ipcp_clear_addrs(f->unit);
> > >  }
> > > diff --git usr.sbin/pppd/options.c usr.sbin/pppd/options.c
> > > index 828f7cdce65..fe0e2e6b54b 100644
> > > --- usr.sbin/pppd/options.c
> > > +++ usr.sbin/pppd/options.c
> > > @@ -144,6 +144,9 @@ struct  bpf_program pass_filter;/* Filter program for 
> > > packets to pass */
> > >  struct bpf_program active_filter; /* Filter program for link-active 
> > > pkts */
> > >  pcap_t  pc;/* Fake struct pcap so we can compile 
> > > expr */
> > >  #endif
> > > +struct pipex_session_req pipex_conf = {/* pipex(4) session 
> > > configuration */
> > > +  .pr_protocol = 0,
> > > +};
> > >
> > >  /*
> > >   * Prototypes
> > > @@ -253,6 +256,8 @@ static int setactivefilter(char **);
> > >  static int setmslanman(char **);
> > >  #endif
> > >
> > > +static int setpipexl2tp(char **);
> > > +
> > >  static int number_option(char *, u_int32_t *, int);
> > >  static int int_option(char *, int *);
> > >  static int readable(int fd);
> > > @@ -391,6 +396,8 @@ static struct cmd {
> > >  {"ms-lanman", 0, setmslanman}, /* Use LanMan psswd when using 
> > > MS-CHAP */
> > >  #endif
> > >
> > > +{"pipex-l2tp", 6, setpipexl2tp},   /* set pipex(4) L2TP 
> > > parameters */
> > > +
> > >  {NULL, 0, NULL}
> > >  };
> > >
> > > @@ -2283,3 +2290,78 @@ setmslanman(argv)
> > >  return (1);
> > >  }
> > >  #endif
> > > +
> > > +static int
> > > +inet_atoss(cp, ss)
> > > +char *cp;
> > > +struct sockaddr_storage *ss;
> > > +{
> > > +struct sockaddr_in *sin = (struct sockaddr_in *)ss;
> > > +char *c, *p;
> > > +unsigned port;
> > > +
> > > +if ((c = strchr(cp, ':')) == NULL)
> > > +   return 0;
> > > +*c = '\0';
> > > +if (!inet_aton(cp, >sin_addr))
> > > +   return 0;
> > > +if (!(port = strtoul(c + 1, , 10)) || *p != '\0')
> > > +   return 0;
> > > +sin->sin_port = htons(port);
> > > +sin->sin_family = AF_INET;
> > > +ss->ss_len = sizeof(*sin);
> > > +
> > > +return 1;
> > > +}
> > > +
> > > +static int
> > > +strtou16(str, valp)
> > > +char *str;
> > > +uint16_t *valp;
> > > +{
> > > +char *ptr;
> > > +
> > > +*valp = strtoul(str, , 0);
> > > +
> > > +return *ptr == '\0' ? 1 : 0;
> > > +}
> > > +
> > > +static int
> > > +setpipexl2tp(argv)
> > > +char **argv;
> > > +{
> > > +BZERO((char *)_conf, 

Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Jason A. Donenfeld
Hey Klemens, Theo,

On Tue, May 26, 2020 at 2:38 PM Klemens Nanni  wrote:
>
> On Tue, May 26, 2020 at 02:23:06PM -0600, Jason A. Donenfeld wrote:
> > That's good news that it's working for you now, but I didn't change
> > anything within the last 24 hours (you mentioned "yesterday") that
> > would seem related to this. So perhaps don't get too excited just yet.
> I was surprised, too.  Perhaps something went wrong applying all those
> single patches from the wireguard-openbsd resository, but if so, I'd
> doubt that it would build in the first place...
>
> So far the interface is stable, but I keep testing.  When I find the
> time, I'll also take a look at the pieces of code which blew up earlier.

My audit is complete, and every single usage of __packed has been
removed. Those never should have been there in the first place.
WireGuard was specifically designed for kernels, and I distinctly
remember making sure things would be high performance on little mips
devices, which necessitated natural struct alignment. And indeed, all
of the structs that wireguard uses on the wire have fields that are
always aligned to each field's size, so no __packed is ever necessary.
This all has been fixed up in the git repo now.

With regards to your crash, though, that's a bit more puzzling, and
I'd be interested to learn more details. Because these structs are
already naturally aligned, the __packed attribute, even with the odd
nesting Matt had prior, should have produced all entirely aligned
accesses. That makes me think your kaboom was coming from someplace
else. One possibility is that you were running the git tree on the two
days that I was playing with uint128_t, only to find out that some of
openbsd's patches to clang miscalculate stack sizes when they're in
use, so that work was shelved for another day and the commits removed;
perhaps you were just unlucky? Or you hit some other bug that's
lurking. Either way, output from ddb's `bt` would at least be useful.

Jason



Re: packets to bridged interfaces bypass input filter

2020-05-26 Thread Stephan Mending
On Tue, May 26, 2020 at 09:26:07PM +0200, Sven M. Hallberg wrote:
> hi all,
> 
> i sent the following question to misc@ on march 29th but received no
> response. i hope you don't mind me retrying on tech@.
> 
> while playing around with pf, i noticed that some connections that i
> thought should be blocked, were in fact not. here is my fairly standard
> bridge setup between a wlan interface and a wired ethernet.
> 
># cat /etc/hostname.em0
>group lan
>up
> 
># cat /etc/hostname.athn0
>mediaopt hostap
>nwid xxx wpakey abcd1234
>group lan
>up
> 
># cat /etc/hostname.vether0
>group lan
>inet 192.168.22.145/28
> 
># cat /etc/hostname.bridge0
>add em0 add athn0 add vether0
>blocknonip athn0
>up
> 
> i then set up pf rules that should let the (w)lan interfaces talk to
> the world.
> 
> # cat /etc/pf.conf
> set skip on lo
> block return
> pass out# ingress filter
> pass in on lan from lan:network to any
> 
> now this all works as intended. but, when i remove athn0 from the lan
> group, i would expect the above ruleset to block any incoming
> connections from the wlan.
> 
> # ifconfig athn0 -group lan
Did you flush states between your tests (pfctl -F states) ? -> After you 
removed athn0 from lan group ? 
> 
> and indeed, i could not ping a wired host from the wlan. however, i
> could still access the router (192.168.22.145) itself from the wireless.
> as far as i can tell, this is due to the following code in if_bridge.c,
> bridge_process() -- comments by me:
> 
> if (bridge_ourether(bif0->ifp, eh->ether_dhost)) {
> /* addressed to the iface it came in on */
> bif = bif0;
> } else {
> SMR_SLIST_FOREACH_LOCKED(bif, >sc_iflist, bif_next) {
> if (bif->ifp == ifp)
> continue;
> if (bridge_ourether(bif->ifp, eh->ether_dhost))
> /* addressed to some other bridge member */
> break;
> }
> }
> if (bif != NULL) {
> /* ... */
> if (bridge_filterrule(>bif_brlin, eh, m) ==
> BRL_ACTION_BLOCK) {
> goto bad;
> }
> /* ... */
> }
> 
> a packet that comes in on a member of a bridge and is addressed to _any_
> bridge member interface, is processed as input to (only) the destination
> interface. if that destination interface differs from the one that the
> packet actually came in on, pf rules for the actual input interface are
> not evaluated.
> 
> this seems to contradict the following statement in the NOTES section of
> bridge(4) -- in any case, it certainly contradicted my intuition:
> 
>  Bridged packets pass through pf(4) filters once as input on the receiving
>  interface and once as output on all interfaces on which they are
>  forwarded.  In order to pass through the bridge packets must pass any in
>  rules on the input and any out rules on the output interface.  Packets
>  may be blocked either entering or leaving the bridge.
> 
> now, to be clear, i don't mind that the packet passes through 'in' rules for
> vether0 in my example, but i wonder if it should not also pass through
> the 'in' rules for 'athn0'.
> 
> i would appreciate enlightenment on this. note that (contrary to misc) i
> am subscribed to tech@, so no cc needed.
> 
> 
> thanks,
> pesco
> 
> PS: at the time, i was using OpenBSD 6.6, but the code in question
> appears unchanged in current.
> 



Re: [PATCH] pipex(4): rework PPP input

2020-05-26 Thread Sergey Ryazanov
On Tue, May 26, 2020 at 12:07 PM Vitaliy Makkoveev
 wrote:
>> On 25 May 2020, at 22:04, Sergey Ryazanov  wrote:
>> On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev
>>  wrote:
>>> For example, each pipex session should have unique pair of `protocol’ and
>>> `session_id’. These values are passed from userland. While the only
>>> instance of npppd(8) uses pipex(4) this is not the problem. But you
>>> introduce the case while pipex(4) will be used by multiple independent
>>> userland programs. At least, I have interest how you handle this.
>>
>> This should not be a problem here. npppd(8) support server mode only.
>> While my work is to implement acceleration for client side of L2TP
>> connection.
>
> I guess they can coexist. Also you can have multiple connections to
> ppp servers simultaneously.

With 16 bits long session id field, according to birthday problem to
reach 0.9 collision probability I need 549 simultaneous sessions.
Should I still be worried or I have a time to complete integration
work and then update UDP  filter for love of the game?

-- 
Sergey



[patch] correct return type in pcap_open_live.3

2020-05-26 Thread Edgar Pettijohn
Please see attached diff.
Index: pcap_open_live.3
===
RCS file: /cvs/src/lib/libpcap/pcap_open_live.3,v
retrieving revision 1.3
diff -u -p -u -r1.3 pcap_open_live.3
--- pcap_open_live.325 Sep 2019 17:02:00 -  1.3
+++ pcap_open_live.326 May 2020 22:09:24 -
@@ -95,7 +95,7 @@
 .Fn pcap_dump_fopen "pcap_t *p" "FILE *f"
 .Ft "char *"
 .Fn pcap_lookupdev "char *errbuf"
-.Ft uint
+.Ft int
 .Fn pcap_lookupnet "const char *device" "bpf_u_int32 *netp" "bpf_u_int32 
*maskp" "char *errbuf"
 .Ft int
 .Fn pcap_dispatch "pcap_t *p" "int cnt" "pcap_handler callback" "u_char *user"


ttymalloc(int baud)

2020-05-26 Thread johnc
Another trivial code readability comment:

I see 24 references to this in the code, 20 of which pass 0, and 4 of
which pass 1,000,000 as the parameter.

Passing 0 defaults to a baud of 115200.  The baud determines the qlen,
which is 4096 for 115200 and 8192 for anything larger, so all it is
doing now is selecting between two buffer sizes via magic number
parameters.

It would probably be appropriate today to just always use 8192 and not
even take a parameter; it doesn't look like the selection between the
two is done in a rigorous way.

There is also a comment in umodem.c that has an outdated assumption:
/*
 * These are the maximum number of bytes transferred per frame.
 * Buffers are this large to deal with high speed wireless devices.
 * Capped at 1024 as ttymalloc() is limited to this amount.
 */
#define UMODEMIBUFSIZE 1024
#define UMODEMOBUFSIZE 1024





Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Jason A. Donenfeld
On Tue, May 26, 2020 at 2:33 PM Theo de Raadt  wrote:
>
> Jason A. Donenfeld  wrote:
>
> > Hey Klemens,
> >
> > On Tue, May 26, 2020 at 9:13 AM Klemens Nanni  wrote:
> > > I worked with the patches from the wireguard-openbsd repository after
> > > version one of this diff on tech@ became a bit old.
> > >
> > > That was until yesterday;  the kernel would panic due to memory
> > > alignment issues in various spots, `amd64# ping6 tunnel-ip.sparc64'
> > > would receive a single ICMP6 echo reply from the sparc64 peer before
> > > panicing its kernel very early in noise init code.
> > >
> > > I've had fixed a few bugs and ran into different panics during later
> > > code paths.
> > >
> > > Fortunately, using "rev. 2" of your diff on top of a clean checkout just
> > > works so far on sparc64 without any additional patches required - this
> > > is a very pleasnt suprise and I can start looking at it under production
> > > use cases more thoroughly now.
> >
> > That's good news that it's working for you now, but I didn't change
> > anything within the last 24 hours (you mentioned "yesterday") that
> > would seem related to this. So perhaps don't get too excited just yet.
> > I've got a decent MIPS setup here and a hacked up qemu for having a
> > bit more detail on alignment traps. I'll see if I can reproduce any
> > issues.
>
> I suspect problems around forcing __packed.
>
> __packed is very dangerous.
>
> If you know the sizes of the objects are correctly sized and placed to
> not have gap-pads, then not using __packed might be better.

Right, most times, uses of packed are wrong and can be avoided. I'll
work through that all and see what can be minimized. In a cursory look
I did see code to the effect of `struct { u32 a; u64 b; } __packed`,
which could trap.

Jason



Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Theo de Raadt
Jason A. Donenfeld  wrote:

> Hey Klemens,
> 
> On Tue, May 26, 2020 at 9:13 AM Klemens Nanni  wrote:
> > I worked with the patches from the wireguard-openbsd repository after
> > version one of this diff on tech@ became a bit old.
> >
> > That was until yesterday;  the kernel would panic due to memory
> > alignment issues in various spots, `amd64# ping6 tunnel-ip.sparc64'
> > would receive a single ICMP6 echo reply from the sparc64 peer before
> > panicing its kernel very early in noise init code.
> >
> > I've had fixed a few bugs and ran into different panics during later
> > code paths.
> >
> > Fortunately, using "rev. 2" of your diff on top of a clean checkout just
> > works so far on sparc64 without any additional patches required - this
> > is a very pleasnt suprise and I can start looking at it under production
> > use cases more thoroughly now.
> 
> That's good news that it's working for you now, but I didn't change
> anything within the last 24 hours (you mentioned "yesterday") that
> would seem related to this. So perhaps don't get too excited just yet.
> I've got a decent MIPS setup here and a hacked up qemu for having a
> bit more detail on alignment traps. I'll see if I can reproduce any
> issues.

I suspect problems around forcing __packed.

__packed is very dangerous.

If you know the sizes of the objects are correctly sized and placed to
not have gap-pads, then not using __packed might be better.

__packed won't sae you from leaking gap bytes, either.

We only run on ILP32 and I32LP64 ABIs, and all of them pack in the classic
way...



packets to bridged interfaces bypass input filter

2020-05-26 Thread Sven M. Hallberg
hi all,

i sent the following question to misc@ on march 29th but received no
response. i hope you don't mind me retrying on tech@.

while playing around with pf, i noticed that some connections that i
thought should be blocked, were in fact not. here is my fairly standard
bridge setup between a wlan interface and a wired ethernet.

   # cat /etc/hostname.em0
   group lan
   up

   # cat /etc/hostname.athn0
   mediaopt hostap
   nwid xxx wpakey abcd1234
   group lan
   up

   # cat /etc/hostname.vether0
   group lan
   inet 192.168.22.145/28

   # cat /etc/hostname.bridge0
   add em0 add athn0 add vether0
   blocknonip athn0
   up

i then set up pf rules that should let the (w)lan interfaces talk to
the world.

# cat /etc/pf.conf
set skip on lo
block return
pass out# ingress filter
pass in on lan from lan:network to any

now this all works as intended. but, when i remove athn0 from the lan
group, i would expect the above ruleset to block any incoming
connections from the wlan.

# ifconfig athn0 -group lan

and indeed, i could not ping a wired host from the wlan. however, i
could still access the router (192.168.22.145) itself from the wireless.
as far as i can tell, this is due to the following code in if_bridge.c,
bridge_process() -- comments by me:

if (bridge_ourether(bif0->ifp, eh->ether_dhost)) {
/* addressed to the iface it came in on */
bif = bif0;
} else {
SMR_SLIST_FOREACH_LOCKED(bif, >sc_iflist, bif_next) {
if (bif->ifp == ifp)
continue;
if (bridge_ourether(bif->ifp, eh->ether_dhost))
/* addressed to some other bridge member */
break;
}
}
if (bif != NULL) {
/* ... */
if (bridge_filterrule(>bif_brlin, eh, m) ==
BRL_ACTION_BLOCK) {
goto bad;
}
/* ... */
}

a packet that comes in on a member of a bridge and is addressed to _any_
bridge member interface, is processed as input to (only) the destination
interface. if that destination interface differs from the one that the
packet actually came in on, pf rules for the actual input interface are
not evaluated.

this seems to contradict the following statement in the NOTES section of
bridge(4) -- in any case, it certainly contradicted my intuition:

 Bridged packets pass through pf(4) filters once as input on the receiving
 interface and once as output on all interfaces on which they are
 forwarded.  In order to pass through the bridge packets must pass any in
 rules on the input and any out rules on the output interface.  Packets
 may be blocked either entering or leaving the bridge.

now, to be clear, i don't mind that the packet passes through 'in' rules for
vether0 in my example, but i wonder if it should not also pass through
the 'in' rules for 'athn0'.

i would appreciate enlightenment on this. note that (contrary to misc) i
am subscribed to tech@, so no cc needed.


thanks,
pesco

PS: at the time, i was using OpenBSD 6.6, but the code in question
appears unchanged in current.



Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Jason A. Donenfeld
Hey Klemens,

On Tue, May 26, 2020 at 9:13 AM Klemens Nanni  wrote:
> I worked with the patches from the wireguard-openbsd repository after
> version one of this diff on tech@ became a bit old.
>
> That was until yesterday;  the kernel would panic due to memory
> alignment issues in various spots, `amd64# ping6 tunnel-ip.sparc64'
> would receive a single ICMP6 echo reply from the sparc64 peer before
> panicing its kernel very early in noise init code.
>
> I've had fixed a few bugs and ran into different panics during later
> code paths.
>
> Fortunately, using "rev. 2" of your diff on top of a clean checkout just
> works so far on sparc64 without any additional patches required - this
> is a very pleasnt suprise and I can start looking at it under production
> use cases more thoroughly now.

That's good news that it's working for you now, but I didn't change
anything within the last 24 hours (you mentioned "yesterday") that
would seem related to this. So perhaps don't get too excited just yet.
I've got a decent MIPS setup here and a hacked up qemu for having a
bit more detail on alignment traps. I'll see if I can reproduce any
issues.

Jason



Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Jason A. Donenfeld
Hey Tobias,

On Tue, May 26, 2020 at 5:28 AM Tobias Heider  wrote:
> > + if (((SIZE_MAX - size) / sizeof(struct wg_aip_io)) < sc->sc_aip_num)
> > + goto error;
>
> I still think those two should return an error.  'goto error' is misleading as
> it doesn't actually set ret != 0.  'error' should probably be renamed
> to 'cleanup' to avoid confusion and ret should be set to ERANGE .

I'll change the label name to be more clear; I agree "error" is
misleading. Returning ERANGE wouldn't be correct though; we still want
the struct to be copied back to the user so that they can use the
information in it to optionally try again (optionally, because maybe
they don't care about info) with a different buffer size.



pppx(4): prevent access to `pxi' being destroyed

2020-05-26 Thread Vitaliy Makkoveev
`pppx_if' has `pxi_ready' field used to prevent access to incomplete
`pxi'. But we don't prevent access to `pxi' which we destroy.
pppx_if_destroy() can sleep so we can grab `pxi' which we already
destroying by concurrent thread and cause use-after-free issue. I guess
to use `pxi_ready' to prevent access to `pxi' at destroy stage too.

Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.84
diff -u -p -r1.84 if_pppx.c
--- sys/net/if_pppx.c   18 Apr 2020 04:03:56 -  1.84
+++ sys/net/if_pppx.c   26 May 2020 12:59:56 -
@@ -594,8 +594,10 @@ pppxclose(dev_t dev, int flags, int mode
 
/* XXX */
NET_LOCK();
-   while ((pxi = LIST_FIRST(>pxd_pxis)))
+   while ((pxi = LIST_FIRST(>pxd_pxis))) {
+   pxi->pxi_ready = 0;
pppx_if_destroy(pxd, pxi);
+   }
NET_UNLOCK();
 
LIST_REMOVE(pxd, pxd_entry);
@@ -951,6 +953,7 @@ pppx_del_session(struct pppx_dev *pxd, s
pxi = pppx_if_find(pxd, req->pcr_session_id, req->pcr_protocol);
if (pxi == NULL)
return (EINVAL);
+   pxi->pxi_ready = 0;
 
req->pcr_stat = pxi->pxi_session.stat;
 



Error reporting in ikev2_ike_sa_alive (was: Improve error reporting in pfkey_sa_last_used)

2020-05-26 Thread matthew j weaver
During childsa last use checks, iked debug logs results, per SA, after a
successful pfkey_sa_last_used call.

This patch makes logging behavior more closely match that, on error.

I chose log_warn instead of log_debug since iked will complain about the
nonzero errno after pfkey_reply:
pfkey_sa_last_used: message: No such process

With this patch an operator can at least troubleshoot which SAs are
causing the trouble.

Comments? Make sense?

thank you, all
matthew weaver

---

Index: ikev2.c
===
RCS file: /cvs/src/sbin/iked/ikev2.c,v
retrieving revision 1.223
diff -u -p -u -r1.223 ikev2.c
--- ikev2.c 2 May 2020 13:01:37 -   1.223
+++ ikev2.c 26 May 2020 15:53:42 -
@@ -4347,8 +4347,15 @@ ikev2_ike_sa_alive(struct iked *env, voi
TAILQ_FOREACH(csa, >sa_childsas, csa_entry) {
if (!csa->csa_loaded)
continue;
-   if (pfkey_sa_last_used(env->sc_pfkey, csa, _used) != 0)
+   if (pfkey_sa_last_used(env->sc_pfkey, csa, _used) != 0) {
+   log_warn(
+   "%s: %s CHILD SA spi %s failed to determine "
+   "last use", __func__,
+   csa->csa_dir == IPSP_DIRECTION_IN ?
+   "incoming" : "outgoing",
+   print_spi(csa->csa_spi.spi, csa->csa_spi.spi_size));
continue;
+   }
diff = (uint32_t)(gettime() - last_used);
log_debug("%s: %s CHILD SA spi %s last used %llu second(s) ago",
__func__,



Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Klemens Nanni
On Tue, May 26, 2020 at 08:09:48AM -0600, Theo de Raadt wrote:
> I'll let you know who has sparc64 machines to help out:
> 
> kn was the developer who saw the problem.  jca is also adept
> enough to look at this with you.
I worked with the patches from the wireguard-openbsd repository after
version one of this diff on tech@ became a bit old.

That was until yesterday;  the kernel would panic due to memory
alignment issues in various spots, `amd64# ping6 tunnel-ip.sparc64'
would receive a single ICMP6 echo reply from the sparc64 peer before
panicing its kernel very early in noise init code.

I've had fixed a few bugs and ran into different panics during later
code paths.

Fortunately, using "rev. 2" of your diff on top of a clean checkout just
works so far on sparc64 without any additional patches required - this
is a very pleasnt suprise and I can start looking at it under production
use cases more thoroughly now.

playground$ arch -s ; doas ifconfig wg0
sparc64
wg0: flags=8043 mtu 1420
index 5 priority 0 llprio 3
wgport 2345
wgpubkey mrtNB07tzEJKyJDvhaov7QYt487BXLK3hnnZB+pDIhM=
wgpeer 9xuyga6Z/W7l/vnLIl67PiRcf57VujfoqpIH5VArpR4=
wgendpoint 2001:xxx 2345
tx: 701892, rx: 13283708
last handshake: 29 seconds ago
wgaip ::/0
groups: wg
inet6 2001:db8::1 prefixlen 126



Re: filesystem code integer and many inodes

2020-05-26 Thread Otto Moerbeek
On Tue, May 26, 2020 at 03:54:15PM +0200, Otto Moerbeek wrote:

> On Tue, May 26, 2020 at 07:51:28AM -0600, Todd C. Miller wrote:
> 
> > On Tue, 26 May 2020 12:07:21 +0200, Otto Moerbeek wrote:
> > 
> > > Apart from the noting the strange Subject: I also like to mention one
> > > change in the way cylinder groups are scanned. The current code scans
> > > forward and backward, which causes an uneven distribution of full cgs
> > > (the upper end of the cgs will get full first). Fix that by always
> > > scanning forward, wrapping to cg 0 if needed.
> > 
> > Should that be a separate commit?  I can't find any problems
> > with the diff but I haven't tried running with it yet.
> > 
> >  - todd
> 
> Yeah, I can do that. Note that it must be comitted first, since the
> loop condition is always true if I change the loop var to unsigned.
> 
>   -Otto
> 

And a new diff. I accidentally capitalized a letter just before sending.
Thanks to naddy for spotting that.

-Otto

Index: sbin/clri/clri.c
===
RCS file: /cvs/src/sbin/clri/clri.c,v
retrieving revision 1.20
diff -u -p -r1.20 clri.c
--- sbin/clri/clri.c28 Jun 2019 13:32:43 -  1.20
+++ sbin/clri/clri.c26 May 2020 09:41:18 -
@@ -68,7 +68,8 @@ main(int argc, char *argv[])
char *fs, sblock[SBLOCKSIZE];
size_t bsize;
off_t offset;
-   int i, fd, imax, inonum;
+   int i, fd;
+   ino_t imax, inonum;
 
if (argc < 3)
usage();
Index: sbin/dumpfs/dumpfs.c
===
RCS file: /cvs/src/sbin/dumpfs/dumpfs.c,v
retrieving revision 1.35
diff -u -p -r1.35 dumpfs.c
--- sbin/dumpfs/dumpfs.c17 Feb 2020 16:11:25 -  1.35
+++ sbin/dumpfs/dumpfs.c26 May 2020 09:41:18 -
@@ -69,7 +69,7 @@ union {
 #define acgcgun.cg
 
 intdumpfs(int, const char *);
-intdumpcg(const char *, int, int);
+intdumpcg(const char *, int, u_int);
 intmarshal(const char *);
 intopen_disk(const char *);
 void   pbits(void *, int);
@@ -163,6 +163,7 @@ dumpfs(int fd, const char *name)
size_t size;
off_t off;
int i, j;
+   u_int cg;
 
switch (afs.fs_magic) {
case FS_UFS2_MAGIC:
@@ -172,7 +173,7 @@ dumpfs(int fd, const char *name)
afs.fs_magic, ctime());
printf("superblock location\t%jd\tid\t[ %x %x ]\n",
(intmax_t)afs.fs_sblockloc, afs.fs_id[0], afs.fs_id[1]);
-   printf("ncg\t%d\tsize\t%jd\tblocks\t%jd\n",
+   printf("ncg\t%u\tsize\t%jd\tblocks\t%jd\n",
afs.fs_ncg, (intmax_t)fssize, (intmax_t)afs.fs_dsize);
break;
case FS_UFS1_MAGIC:
@@ -198,7 +199,7 @@ dumpfs(int fd, const char *name)
printf("cylgrp\t%s\tinodes\t%s\tfslevel %d\n",
i < 1 ? "static" : "dynamic",
i < 2 ? "4.2/4.3BSD" : "4.4BSD", i);
-   printf("ncg\t%d\tncyl\t%d\tsize\t%d\tblocks\t%d\n",
+   printf("ncg\t%u\tncyl\t%d\tsize\t%d\tblocks\t%d\n",
afs.fs_ncg, afs.fs_ncyl, afs.fs_ffs1_size, 
afs.fs_ffs1_dsize);
break;
default:
@@ -223,9 +224,9 @@ dumpfs(int fd, const char *name)
(intmax_t)afs.fs_cstotal.cs_ndir,
(intmax_t)afs.fs_cstotal.cs_nifree, 
(intmax_t)afs.fs_cstotal.cs_nffree);
-   printf("bpg\t%d\tfpg\t%d\tipg\t%d\n",
+   printf("bpg\t%d\tfpg\t%d\tipg\t%u\n",
afs.fs_fpg / afs.fs_frag, afs.fs_fpg, afs.fs_ipg);
-   printf("nindir\t%d\tinopb\t%d\tmaxfilesize\t%ju\n",
+   printf("nindir\t%d\tinopb\t%u\tmaxfilesize\t%ju\n",
afs.fs_nindir, afs.fs_inopb, 
(uintmax_t)afs.fs_maxfilesize);
printf("sbsize\t%d\tcgsize\t%d\tcsaddr\t%jd\tcssize\t%d\n",
@@ -238,10 +239,10 @@ dumpfs(int fd, const char *name)
printf("nbfree\t%d\tndir\t%d\tnifree\t%d\tnffree\t%d\n",
afs.fs_ffs1_cstotal.cs_nbfree, afs.fs_ffs1_cstotal.cs_ndir,
afs.fs_ffs1_cstotal.cs_nifree, 
afs.fs_ffs1_cstotal.cs_nffree);
-   printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%d\n",
+   printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%u\n",
afs.fs_cpg, afs.fs_fpg / afs.fs_frag, afs.fs_fpg,
afs.fs_ipg);
-   printf("nindir\t%d\tinopb\t%d\tnspf\t%d\tmaxfilesize\t%ju\n",
+   printf("nindir\t%d\tinopb\t%u\tnspf\t%d\tmaxfilesize\t%ju\n",
afs.fs_nindir, afs.fs_inopb, afs.fs_nspf,
(uintmax_t)afs.fs_maxfilesize);
printf("sbsize\t%d\tcgsize\t%d\tcgoffset %d\tcgmask\t0x%08x\n",
@@ -261,7 +262,7 @@ dumpfs(int fd, const char *name)
afs.fs_sblkno, afs.fs_cblkno, afs.fs_iblkno, afs.fs_dblkno);

Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Theo de Raadt
I'll let you know who has sparc64 machines to help out:

kn was the developer who saw the problem.  jca is also adept
enough to look at this with you.



Re: filesystem code integer and many inodes

2020-05-26 Thread Otto Moerbeek
On Tue, May 26, 2020 at 07:51:28AM -0600, Todd C. Miller wrote:

> On Tue, 26 May 2020 12:07:21 +0200, Otto Moerbeek wrote:
> 
> > Apart from the noting the strange Subject: I also like to mention one
> > change in the way cylinder groups are scanned. The current code scans
> > forward and backward, which causes an uneven distribution of full cgs
> > (the upper end of the cgs will get full first). Fix that by always
> > scanning forward, wrapping to cg 0 if needed.
> 
> Should that be a separate commit?  I can't find any problems
> with the diff but I haven't tried running with it yet.
> 
>  - todd

Yeah, I can do that. Note that it must be comitted first, since the
loop condition is always true if I change the loop var to unsigned.

-Otto



Re: filesystem code integer and many inodes

2020-05-26 Thread Todd C . Miller
On Tue, 26 May 2020 12:07:21 +0200, Otto Moerbeek wrote:

> Apart from the noting the strange Subject: I also like to mention one
> change in the way cylinder groups are scanned. The current code scans
> forward and backward, which causes an uneven distribution of full cgs
> (the upper end of the cgs will get full first). Fix that by always
> scanning forward, wrapping to cg 0 if needed.

Should that be a separate commit?  I can't find any problems
with the diff but I haven't tried running with it yet.

 - todd



Re: [PATCH] pipex(4): rework PPP input

2020-05-26 Thread Vitaliy Makkoveev


> On 25 May 2020, at 22:04, Sergey Ryazanov  wrote:
> 
> Hello Vitaliy,
> 
> On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev
>  wrote:
>>> On 23 May 2020, at 13:11, Sergey Ryazanov  wrote:
>>> On Wed, May 20, 2020 at 10:13 PM Vitaliy Makkoveev
>>>  wrote:
 On Wed, May 20, 2020 at 04:08:01AM +0300, Sergey Ryazanov wrote:
> On Tue, May 19, 2020 at 12:11 PM Vitaliy Makkoveev
>  wrote:
>> On Mon, May 04, 2020 at 10:03:40PM +0300, Sergey Ryazanov wrote:
>>> Split checks from frame accepting with header removing in the common
>>> PPP input function. This should fix packet capture on a PPP interfaces.
>> 
>> Can you describe the problem you fix? As mpi@ pointed to me, reviewers
>> are stupid and have no telepathy skills :)
> 
> When I tried to capture packets on a ppp (4) interface (with pipex
> activated), I noticed that all the PPP CCP frames were ok, but all the
> ingress PPP IP frames were mangled, and they did not contain the PPP
> header at all.
 
 This time only pppx(4) and pppac(4) have pipex(4) support.
>>> 
>>> Yes, and as I wrote in the first mail, now I am working on ppp(4) &
>>> pipex(4) integration to speed up client side of L2TP.
>> 
>> May be you can share you work? Not for commit, but for feedback.
>> 
> 
> I send a couple of diffs in separate mails. First change is for ppp(4)
> to support pipex(4) acceleration in DL path. Second diff adds a new
> option for pppd to control pipex activation.
> 
> We will need also to update xl2tpd package to teach it how to
> configure pppd for pipex usage. This is quite simple change, but it
> still require some cleanup so I do not publish it yet.
> 
>> For example, each pipex session should have unique pair of `protocol’ and
>> `session_id’. These values are passed from userland. While the only
>> instance of npppd(8) uses pipex(4) this is not the problem. But you
>> introduce the case while pipex(4) will be used by multiple independent
>> userland programs. At least, I have interest how you handle this.
> 
> This should not be a problem here. npppd(8) support server mode only.
> While my work is to implement acceleration for client side of L2TP
> connection.

I guess they can coexist. Also you can have multiple connections to
ppp servers simultaneously.



Re: iked(8): AES_GCM ciphers for IKE

2020-05-26 Thread Tobias Heider
On Thu, May 14, 2020 at 10:07:30PM +0200, Tobias Heider wrote:
> Hi,
> 
> currently iked(8) supports AES-GCM only for ESP.
> The diff below adds the ENCR_AES_GCM_16 and ENCR_AES_GCM_12 variants for IKE.
> (for more information see [1] and [2]).
> Both variants support the 128, 196, and 256 bit key lengths.
> 
> The new new ciphers can be configured with:
> - aes-128-gcm, aes-196-gcm and aes-256-gcm for ENCR_AES_GCM_16
> - aes-128-gcm-12, aes-196-gcm-12 and aes-256-gcm-12 for ENCR_AES_GCM_12
> 
> It would be nice if we could get some interop testing with different IKEv2
> implementations.  I have so far successfully tested strongswan <-> iked and
> of course iked <-> iked.
> 
> Feedback welcome ;)
> 
> [1] https://tools.ietf.org/html/rfc5282
> [2] 
> https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xhtml#ikev2-parameters-5

Here is the latest revision of the diff with a few more comments and without 
aes-192-gcm.
We have so far successfully tested against iked, strongswan and juniper srx.

ok?

Index: crypto.c
===
RCS file: /cvs/src/sbin/iked/crypto.c,v
retrieving revision 1.27
diff -u -p -r1.27 crypto.c
--- crypto.c14 May 2020 15:08:30 -  1.27
+++ crypto.c26 May 2020 12:58:28 -
@@ -92,7 +92,7 @@ hash_new(uint8_t type, uint16_t id)
struct iked_hash*hash;
const EVP_MD*md = NULL;
HMAC_CTX*ctx = NULL;
-   int  length = 0, fixedkey = 0, trunc = 0;
+   int  length = 0, fixedkey = 0, trunc = 0, isaead = 
0;
 
switch (type) {
case IKEV2_XFORMTYPE_PRF:
@@ -156,6 +156,14 @@ hash_new(uint8_t type, uint16_t id)
length = SHA512_DIGEST_LENGTH;
trunc = 32;
break;
+   case IKEV2_XFORMAUTH_AES_GCM_12:
+   length = 12;
+   isaead = 1;
+   break;
+   case IKEV2_XFORMAUTH_AES_GCM_16:
+   length = 16;
+   isaead = 1;
+   break;
case IKEV2_XFORMAUTH_NONE:
case IKEV2_XFORMAUTH_DES_MAC:
case IKEV2_XFORMAUTH_KPDK_MD5:
@@ -177,7 +185,7 @@ hash_new(uint8_t type, uint16_t id)
print_map(id, ikev2_xformtype_map));
break;
}
-   if (md == NULL)
+   if (!isaead && md == NULL)
return (NULL);
 
if ((hash = calloc(1, sizeof(*hash))) == NULL) {
@@ -192,6 +200,10 @@ hash_new(uint8_t type, uint16_t id)
hash->hash_trunc = trunc;
hash->hash_length = length;
hash->hash_fixedkey = fixedkey;
+   hash->hash_isaead = isaead;
+
+   if (isaead)
+   return (hash);
 
if ((ctx = calloc(1, sizeof(*ctx))) == NULL) {
log_debug("%s: alloc hash ctx", __func__);
@@ -276,6 +288,7 @@ cipher_new(uint8_t type, uint16_t id, ui
const EVP_CIPHER*cipher = NULL;
EVP_CIPHER_CTX  *ctx = NULL;
int  length = 0, fixedkey = 0, ivlength = 0;
+   int  saltlength = 0, authid = 0;
 
switch (type) {
case IKEV2_XFORMTYPE_ENCR:
@@ -309,6 +322,36 @@ cipher_new(uint8_t type, uint16_t id, ui
ivlength = EVP_CIPHER_iv_length(cipher);
fixedkey = EVP_CIPHER_key_length(cipher);
break;
+   case IKEV2_XFORMENCR_AES_GCM_16:
+   case IKEV2_XFORMENCR_AES_GCM_12:
+   switch (id_length) {
+   case 128:
+   cipher = EVP_aes_128_gcm();
+   break;
+   case 256:
+   cipher = EVP_aes_256_gcm();
+   break;
+   default:
+   log_debug("%s: invalid key length %d"
+   " for cipher %s", __func__, id_length,
+   print_map(id, ikev2_xformencr_map));
+   break;
+   }
+   if (cipher == NULL)
+   break;
+   switch(id) {
+   case IKEV2_XFORMENCR_AES_GCM_16:
+   authid = IKEV2_XFORMAUTH_AES_GCM_16;
+   break;
+   case IKEV2_XFORMENCR_AES_GCM_12:
+   authid = IKEV2_XFORMAUTH_AES_GCM_12;
+   break;
+   }
+   length = EVP_CIPHER_block_size(cipher);
+   ivlength = 8;
+   saltlength = 4;
+   fixedkey = EVP_CIPHER_key_length(cipher) + saltlength;
+ 

Re: [RFC] pppd: add pipex(4) L2TP control support

2020-05-26 Thread Vitaliy Makkoveev


> On 26 May 2020, at 11:31, Claudio Jeker  wrote:
> 
> [skip]
> 
> Is pppd(8) still using K function declarations? Can we please add new
> functions with ANSI declarations instead and convert the rest as well. 
> Also it looks like something strange is going on with indentation (just
> look at the last function above, it seems lines start with 4 spaces
> instead of 1 tab). Again this could be bad pppd code.

pppd(8) is not KNF compliant.



iwx: fix tx queue index in iwx_tx()

2020-05-26 Thread Stefan Sperling
When enabling Tx queues in iwx_enable_data_tx_queues() the driver computes
the Tx queue index as:

int qid = ac + IWX_DQA_AUX_QUEUE + 1;

with:
#define IWX_DQA_AUX_QUEUE   1

In iwx_tx(), we use a different way of computing the Tx queue index,
which is a leftover from iwm(4):

ring = >txq[IWX_DQA_MIN_MGMT_QUEUE + ac];

with:
#define IWX_DQA_MIN_MGMT_QUEUE  5

This kind of bug would usually have severe consequences.
But in this case I got very lucky:

'ac' ranges from 0 to 3, and iwx_tx() always ends up picking Tx queue 5
from the range 5-8 because 'ac' is always passed in as 0 by the caller.

iwx_enable_data_tx_queues() enables Tx queues 2-5 inclusive.

So from the firmware's point of view, Tx queue 5 is enabled and is used
to send frames, and it all just works in spite of the bug.

With this fix, iwx_tx() uses Tx queue 2 and would work correctly for values
of 'ac' other than 0, as intended.

ok?
 
diff 6fed087d6e00a5c30641d2658d09ced7cb1e3090 
1e58df09264b5ea64bbe8f02e26da8b4e738c867
blob - 2f41c5a7c8a943e38bbd69a81d46cf432d329607
blob + 831f4d766af8fbf4e00f3fc0ad1624838869d92e
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -1674,13 +1674,13 @@ iwx_alloc_tx_ring(struct iwx_softc *sc, struct iwx_tx_
 *
 * In DQA mode we use 1 command queue + 4 DQA mgmt/data queues.
 * The command is queue 0 (sc->txq[0]), and 4 mgmt/data frame queues
-* are sc->tqx[IWX_DQA_MIN_MGMT_QUEUE + ac], i.e. sc->txq[5:8],
+* are sc->tqx[ac + IWX_DQA_AUX_QUEUE + 1], i.e. sc->txq[2:5],
 * in order to provide one queue per EDCA category.
 *
 * Tx aggregation will require additional queues (one queue per TID
 * for which aggregation is enabled) but we do not implement this yet.
 */
-   if (qid > IWX_DQA_MAX_MGMT_QUEUE)
+   if (qid > IWX_DQA_MIN_MGMT_QUEUE)
return 0;
 
err = iwx_dma_contig_alloc(sc->sc_dmat, >bc_tbl,
@@ -4246,7 +4246,7 @@ iwx_tx(struct iwx_softc *sc, struct mbuf *m, struct ie
 * Tx aggregation will require additional queues (one queue per TID
 * for which aggregation is enabled) but we do not implement this yet.
 */
-   ring = >txq[IWX_DQA_MIN_MGMT_QUEUE + ac];
+   ring = >txq[ac + IWX_DQA_AUX_QUEUE + 1];
desc = >desc[ring->cur];
memset(desc, 0, sizeof(*desc));
data = >data[ring->cur];



Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Tobias Heider
On Tue, May 26, 2020 at 07:39:01PM +1000, Matt Dunwoodie wrote:
> Hi tech,
> 
> After some feedback and comments, we've addressed the concerns, and
> fixed a few things from our side too. Overall the structure is familiar
> with no major changes, so any prior readings mostly carry over.
> 
> This is a quick summary of the changes. Detailed changes can be found
> on the out-of-tree repo or we can elaborate if more detail is needed.
> 
> * Allow non-root to read WireGuard network configuration of the
>   interface.
>   - This allows users to help debug issues without
> escalating to root.
> 
> * Removal of wgconf option from ifconfig.
>   - This option was not mature enough for inclusion into base. Upon
> reflection, this behaviour probably needs further consideration
> about how to integrate into ifconfig.
> 
> * Fix ioctl interface.
>   - The linked list setup was fragile, complicated, and broken, so
> we've gone with something much simpler, and user easy to program
> for, using a boring ioctl pattern that's been tried successfully
> elsewhere already.
> 
> * Limit the padding of the packet to the MTU.
>   - Fits the protocol spec and prevents unneccessary fragmentation.
> 
> * Align handshake/identity locks and timer semantics with spec and other
>   implementations.
>   - This avoids any subtle edge cases that may occur.
> 
> Here are the changes that shouldn't need any explanation:
> 
> * Updated documentation based on feedback from tech and others.
> * Maintain order of peers when adding/deleting  .
> * Rename noise_alloc to noise_upcall.
> * Remove wg_timers_fn indirection.
> 
> Minor formatting changes may also be present.
> 
> Once someone picks this up, we're more than happy to help integrate
> this and maintain it in-tree.
> 
> Signed
> Matt Dunwoodie

Hi Matt,

just repeating what I commented yesterday for the new diff to make sure
it isn't overlooked.

> +int
> +wg_ioctl_get(struct wg_softc *sc, struct wg_data_io *data)
> +{
> + struct wg_interface_io  *iface_p, iface_o;
> + struct wg_peer_io   *peer_p, peer_o;
> + struct wg_aip_io*aip_p;
> +
> + struct wg_peer  *peer;
> + struct wg_aip   *aip;
> +
> + size_t   size, peer_count, aip_count;
> + int  ret = 0, is_suser = suser(curproc) == 0;
> +
> + size = sizeof(struct wg_interface_io);
> + if (data->wgd_size < size && !is_suser)
> + goto ret_size;
> +
> + iface_p = data->wgd_interface;
> + bzero(_o, sizeof(iface_o));
> +
> + rw_enter_read(>sc_lock);
> +
> + if (sc->sc_udp_port != 0) {
> + iface_o.i_port = ntohs(sc->sc_udp_port);
> + iface_o.i_flags |= WG_INTERFACE_HAS_PORT;
> + }
> +
> + if (sc->sc_udp_rtable != 0) {
> + iface_o.i_rtable = sc->sc_udp_rtable;
> + iface_o.i_flags |= WG_INTERFACE_HAS_RTABLE;
> + }
> +
> + if (!is_suser)
> + goto out;
> +
> + if (noise_local_keys(>sc_local, iface_o.i_public,
> + iface_o.i_private) == 0) {
> + iface_o.i_flags |= WG_INTERFACE_HAS_PUBLIC;
> + iface_o.i_flags |= WG_INTERFACE_HAS_PRIVATE;
> + }
> +
> + if (((SIZE_MAX - size) / sizeof(struct wg_peer_io)) < sc->sc_peer_num)
> + goto error;
> + size += sizeof(struct wg_peer_io) * sc->sc_peer_num;
> + if (((SIZE_MAX - size) / sizeof(struct wg_aip_io)) < sc->sc_aip_num)
> + goto error;

I still think those two should return an error.  'goto error' is misleading as
it doesn't actually set ret != 0.  'error' should probably be renamed
to 'cleanup' to avoid confusion and ret should be set to ERANGE .

> + size += sizeof(struct wg_aip_io) * sc->sc_aip_num;
> + if (data->wgd_size < size)
> + goto error;
> +
> + peer_count = 0;
> + peer_p = _p->i_peers[0];
> + WG_PEERS_FOREACH(peer, sc) {
> + bzero(_o, sizeof(peer_o));
> + peer_o.p_flags = WG_PEER_HAS_PUBLIC;
> + peer_o.p_protocol_version = 1;
> +
> + if (noise_remote_keys(>p_remote, peer_o.p_public,
> + peer_o.p_psk) == 0)
> + peer_o.p_flags |= WG_PEER_HAS_PSK;
> +
> + if (wg_timers_get_persistent_keepalive(>p_timers,
> + _o.p_pka) == 0)
> + peer_o.p_flags |= WG_PEER_HAS_PKA;
> +
> + if (wg_peer_get_sockaddr(peer, _o.p_sa) == 0)
> + peer_o.p_flags |= WG_PEER_HAS_ENDPOINT;
> +
> + mtx_enter(>p_counters_mtx);
> + peer_o.p_txbytes = peer->p_counters_tx;
> + peer_o.p_rxbytes = peer->p_counters_rx;
> + mtx_leave(>p_counters_mtx);
> +
> + wg_timers_get_last_handshake(>p_timers,
> + _o.p_last_handshake);
> +
> + aip_count = 0;
> + aip_p = _p->p_aips[0];
> + LIST_FOREACH(aip, >p_aip, a_entry) {
> + if 

Re: filesystem code integer and many inodes

2020-05-26 Thread Otto Moerbeek
On Tue, May 26, 2020 at 11:58:39AM +0200, Otto Moerbeek wrote:

> Hi,
> 
> In theory ffs code support a maximum of UINT_MAX inodes, but in
> practice, due to integer overflows in the current code, the limit is
> INT_MAX inodes.
> 
> This fixes that, and allows me to create and use filesystems with more
> than INT_MAX inodes. This is partly from FreeBSD code.
> 
> Main change is in fs.h, modifying a few fields to unsigned, most
> notably fs_ipg (inodes per cylinder group) and fs_ncg (number of
> cylinder groups). In various places fs_ipg * fs_ncg is computed, so
> both should be unsigned. I also made sure cg indexes are unsigned and
> ino_t is used for inode numbers.
> 
> Tested on a 30TB partition with various parameters (notably -f x -b y
> and -i z combinations).
> 
> Please test and/or review,

Apart from the noting the strange Subject: I also like to mention one
change in the way cylinder groups are scanned. The current code scans
forward and backward, which causes an uneven distribution of full cgs
(the upper end of the cgs will get full first). Fix that by always
scanning forward, wrapping to cg 0 if needed.

-Otto



filesystem code integer and many inodes

2020-05-26 Thread Otto Moerbeek
Hi,

In theory ffs code support a maximum of UINT_MAX inodes, but in
practice, due to integer overflows in the current code, the limit is
INT_MAX inodes.

This fixes that, and allows me to create and use filesystems with more
than INT_MAX inodes. This is partly from FreeBSD code.

Main change is in fs.h, modifying a few fields to unsigned, most
notably fs_ipg (inodes per cylinder group) and fs_ncg (number of
cylinder groups). In various places fs_ipg * fs_ncg is computed, so
both should be unsigned. I also made sure cg indexes are unsigned and
ino_t is used for inode numbers.

Tested on a 30TB partition with various parameters (notably -f x -b y
and -i z combinations).

Please test and/or review,

-Otto

Index: sbin/clri/clri.c
===
RCS file: /cvs/src/sbin/clri/clri.c,v
retrieving revision 1.20
diff -u -p -r1.20 clri.c
--- sbin/clri/clri.c28 Jun 2019 13:32:43 -  1.20
+++ sbin/clri/clri.c26 May 2020 09:41:18 -
@@ -68,7 +68,8 @@ main(int argc, char *argv[])
char *fs, sblock[SBLOCKSIZE];
size_t bsize;
off_t offset;
-   int i, fd, imax, inonum;
+   int i, fd;
+   ino_t imax, inonum;
 
if (argc < 3)
usage();
Index: sbin/dumpfs/dumpfs.c
===
RCS file: /cvs/src/sbin/dumpfs/dumpfs.c,v
retrieving revision 1.35
diff -u -p -r1.35 dumpfs.c
--- sbin/dumpfs/dumpfs.c17 Feb 2020 16:11:25 -  1.35
+++ sbin/dumpfs/dumpfs.c26 May 2020 09:41:18 -
@@ -69,7 +69,7 @@ union {
 #define acgcgun.cg
 
 intdumpfs(int, const char *);
-intdumpcg(const char *, int, int);
+intdumpcg(const char *, int, u_int);
 intmarshal(const char *);
 intopen_disk(const char *);
 void   pbits(void *, int);
@@ -163,6 +163,7 @@ dumpfs(int fd, const char *name)
size_t size;
off_t off;
int i, j;
+   u_int cg;
 
switch (afs.fs_magic) {
case FS_UFS2_MAGIC:
@@ -172,7 +173,7 @@ dumpfs(int fd, const char *name)
afs.fs_magic, ctime());
printf("superblock location\t%jd\tid\t[ %x %x ]\n",
(intmax_t)afs.fs_sblockloc, afs.fs_id[0], afs.fs_id[1]);
-   printf("ncg\t%d\tsize\t%jd\tblocks\t%jd\n",
+   printf("ncg\t%u\tsize\t%jd\tblocks\t%jd\n",
afs.fs_ncg, (intmax_t)fssize, (intmax_t)afs.fs_dsize);
break;
case FS_UFS1_MAGIC:
@@ -198,7 +199,7 @@ dumpfs(int fd, const char *name)
printf("cylgrp\t%s\tinodes\t%s\tfslevel %d\n",
i < 1 ? "static" : "dynamic",
i < 2 ? "4.2/4.3BSD" : "4.4BSD", i);
-   printf("ncg\t%d\tncyl\t%d\tsize\t%d\tblocks\t%d\n",
+   printf("ncg\t%u\tncyl\t%d\tsize\t%d\tblocks\t%d\n",
afs.fs_ncg, afs.fs_ncyl, afs.fs_ffs1_size, 
afs.fs_ffs1_dsize);
break;
default:
@@ -223,9 +224,9 @@ dumpfs(int fd, const char *name)
(intmax_t)afs.fs_cstotal.cs_ndir,
(intmax_t)afs.fs_cstotal.cs_nifree, 
(intmax_t)afs.fs_cstotal.cs_nffree);
-   printf("bpg\t%d\tfpg\t%d\tipg\t%d\n",
+   printf("bpg\t%d\tfpg\t%d\tipg\t%u\n",
afs.fs_fpg / afs.fs_frag, afs.fs_fpg, afs.fs_ipg);
-   printf("nindir\t%d\tinopb\t%d\tmaxfilesize\t%ju\n",
+   printf("nindir\t%d\tinopb\t%u\tmaxfilesize\t%ju\n",
afs.fs_nindir, afs.fs_inopb, 
(uintmax_t)afs.fs_maxfilesize);
printf("sbsize\t%d\tcgsize\t%d\tcsaddr\t%jd\tcssize\t%d\n",
@@ -238,10 +239,10 @@ dumpfs(int fd, const char *name)
printf("nbfree\t%d\tndir\t%d\tnifree\t%d\tnffree\t%d\n",
afs.fs_ffs1_cstotal.cs_nbfree, afs.fs_ffs1_cstotal.cs_ndir,
afs.fs_ffs1_cstotal.cs_nifree, 
afs.fs_ffs1_cstotal.cs_nffree);
-   printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%d\n",
+   printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%u\n",
afs.fs_cpg, afs.fs_fpg / afs.fs_frag, afs.fs_fpg,
afs.fs_ipg);
-   printf("nindir\t%d\tinopb\t%d\tnspf\t%d\tmaxfilesize\t%ju\n",
+   printf("nindir\t%d\tinopb\t%u\tnspf\t%d\tmaxfilesize\t%ju\n",
afs.fs_nindir, afs.fs_inopb, afs.fs_nspf,
(uintmax_t)afs.fs_maxfilesize);
printf("sbsize\t%d\tcgsize\t%d\tcgoffset %d\tcgmask\t0x%08x\n",
@@ -261,7 +262,7 @@ dumpfs(int fd, const char *name)
afs.fs_sblkno, afs.fs_cblkno, afs.fs_iblkno, afs.fs_dblkno);
printf("cgrotor\t%d\tfmod\t%d\tronly\t%d\tclean\t%d\n",
afs.fs_cgrotor, afs.fs_fmod, afs.fs_ronly, afs.fs_clean);
-   printf("avgfpdir %d\tavgfilesize %d\n",
+   printf("avgfpdir %u\tavgfilesize %u\n",
afs.fs_avgfpdir, 

Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Jason A. Donenfeld
Hey tech@,

A few things I thought I should add to our v2 revision:

First, the improvements we've made in the last few weeks have been
pretty substantial, and we've now got a much more faithful protocol
implementation. I've been running this on a few high traffic servers,
and I'll probably move demo.wireguard.com over to it soonish. The ioctl
stuff has also been cleaned up and finalized now.

It's this last point that I'm quite happy about: the latest
wireguard-tools package in ports now fully supports this patch. So that
means after patching your kernel you can easily run:

# pkg_add -Dsnap wireguard-tools

And it'll interface with this implementation like usual, providing wg(8)
and wg-quick(8). Go programmers will also be happy to learn that Matt
Layher has added support for the ioctl interface to his wgctrl-go
project.

As you might have surmised, I'm very interested in getting expanded
testing of this patch. We're still very early in the 6.8 cycle. I'm
wondering if it would make sense to check this v2 revision into cvs
_now_, which will then make it included in -current, so that folks can
test and use this easily. Then, Matt and I can continue to develop and
refine this in one of two ways:

  - Send (bi-?)monthly patches to tech@ with fixes we've been working
on.
  - Enable both of our commit bits for that part of the tree, and we'll
push patches directly.

Either one works, or maybe you have a third idea for that. I'm fairly
committed to working on this full time to get it perfect for 6.8.

The other thing I've been exploring is re-licensing the wireguard-tools
package as MIT or ISC in case we decide at some later point down the
road (not now) that maybe wg(8) would fit well in the base system.

Anyway, please go forth and test this! And thanks a lot for the feedback
from our v1. Looking forward to the same on v2.

Jason



Initialize v4l2_requestbuffers struct to avoid invalid mmap

2020-05-26 Thread Ingo Feinerer
video(1) supports reading frames from a webcam via mmap(). To inform the
V4L2 device about the number of desired buffers containing the frames to
be memory-mapped, a VIDIOC_REQBUFS ioctl call is used.

At the moment the v4l2_requestbuffers struct used for the VIDIOC_REQBUFS ioctl
is only partially initialized which leads to an invalid mmap for my webcam:

video: mmap: Invalid argument

With the following diff applied my camera works (only on EHCI ports with USB
3.0 disabled in BIOS but that is a different story).

OK?

Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.29
diff -u -p -r1.29 video.c
--- video.c 6 Nov 2019 05:46:51 -   1.29
+++ video.c 26 May 2020 09:14:39 -
@@ -1354,6 +1354,8 @@ mmap_init(struct video *vid)
 
/* request buffers */
rb.count = MMAP_NUM_BUFS;
+   rb.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+   rb.memory = V4L2_MEMORY_MMAP;
r = ioctl(vid->dev.fd, VIDIOC_REQBUFS, );
if (r == -1) {
warn("ioctl VIDIOC_REQBUFS");



Re: sysupgrade change to allow installing from url

2020-05-26 Thread Florian Obser
On Tue, May 26, 2020 at 12:03:50AM +0200, Sebastian Benoit wrote:
> Solene Rapenne(sol...@perso.pw) on 2020.05.25 15:25:40 +0200:
> > Hi,
> > 
> > I don't know if this will be accepted but I propose to add a -u [url]
> > parameter to use older snapshots from an archive server for example.
> > 
> > I wanted to add an optional parameter to -s at first but in case of
> > sysupgrade -s [installurl] the install url would have been mistaken by
> > this.
> > 
> > If you specify a -u url and installurl at the same time, -u url
> > superseed installurl.
> > 
> > This would allow easier bisecting using older snapshots.
> 
> I have fixed the way the directories are laid out, so now you can do this
> 
>  # echo "http://ftp.hostserver.de/archive/2020-05-14-0105; > /etc/installurl
>  # sysupgrade -n
>  # strings /home/_sysupgrade/bsd | grep 6.7-current 
> @(#)OpenBSD 6.7-current (GENERIC) #181: Wed May 13 12:43:30 MDT 2020
> 

Just for the archives, one can provide the URL on the command line,
which I think is more sensible for this particular usecase:

$ doas sysupgrade -n http://ftp.hostserver.de/archive/2020-05-14-0105
[...]
$ what /home/_sysupgrade/bsd
/home/_sysupgrade/bsd:
OpenBSD 6.7-current (GENERIC) #181: Wed May 13 12:43:30 MDT 2020

-- 
I'm not entirely sure you are real.



Re: [RFC] pppd: add pipex(4) L2TP control support

2020-05-26 Thread Claudio Jeker
On Tue, May 26, 2020 at 09:22:28AM +0200, Martin Pieuchot wrote:
> On 25/05/20(Mon) 21:42, Sergey Ryazanov wrote:
> > Add dedicated option to activate kernel L2TP acceleration via
> > the pipex(4). The options should be passed by a L2TP tunnel
> > management daemon (e.g. xl2tpd).
> 
> What is the difference between npppd(8) and pppd(8)?   Aren't those two
> redundant?  Why did you choose to modify pppd(8) and not npppd(8)?

npppd(8) is server only it can not establish a connection. pppd(8) on the
other hand is more client side (but I think it can do both).

I think the split in userland makes sense (using npppd as LNS/LAC and pppd
to connect to a ISP via PPP (even though we also use sppp/pppoe(4) for
that). Now for the kernel side it would be great if the interface used
would be the same for all ppp users. In this regard this is a step in the
right direction.

 
> > This diff is complete, but kernel support in ppp(4) is not ready.
> > ---
> >  usr.sbin/pppd/ipcp.c|  5 +++
> >  usr.sbin/pppd/options.c | 82 +
> >  usr.sbin/pppd/pppd.8| 18 +
> >  usr.sbin/pppd/pppd.h|  7 
> >  usr.sbin/pppd/sys-bsd.c | 56 
> >  5 files changed, 168 insertions(+)
> > 
> > diff --git usr.sbin/pppd/ipcp.c usr.sbin/pppd/ipcp.c
> > index 1296d897b14..e7a6dbd6ee7 100644
> > --- usr.sbin/pppd/ipcp.c
> > +++ usr.sbin/pppd/ipcp.c
> > @@ -1251,6 +1251,10 @@ ipcp_up(f)
> > return;
> > }
> >  
> > +   /* enable pipex(4), keep working if failed */
> > +   if (pipex_conf.pr_protocol && !apipex(go->ouraddr, ho->hisaddr, mask))
> > +   IPCPDEBUG((LOG_WARNING, "apipex failed"));
> > +
> >  #if (defined(SVR4) && (defined(SNI) || defined(__USLC__)))
> > if (!sifaddr(f->unit, go->ouraddr, ho->hisaddr, mask)) {
> > IPCPDEBUG((LOG_WARNING, "sifaddr failed"));
> > @@ -1304,6 +1308,7 @@ ipcp_down(f)
> >  if (demand) {
> > sifnpmode(f->unit, PPP_IP, NPMODE_QUEUE);
> >  } else {
> > +   dpipex();
> > sifdown(f->unit);
> > ipcp_clear_addrs(f->unit);
> >  }
> > diff --git usr.sbin/pppd/options.c usr.sbin/pppd/options.c
> > index 828f7cdce65..fe0e2e6b54b 100644
> > --- usr.sbin/pppd/options.c
> > +++ usr.sbin/pppd/options.c
> > @@ -144,6 +144,9 @@ struct  bpf_program pass_filter;/* Filter program for 
> > packets to pass */
> >  struct bpf_program active_filter; /* Filter program for link-active 
> > pkts */
> >  pcap_t  pc;/* Fake struct pcap so we can compile 
> > expr */
> >  #endif
> > +struct pipex_session_req pipex_conf = {/* pipex(4) session 
> > configuration */
> > +  .pr_protocol = 0,
> > +};
> >  
> >  /*
> >   * Prototypes
> > @@ -253,6 +256,8 @@ static int setactivefilter(char **);
> >  static int setmslanman(char **);
> >  #endif
> >  
> > +static int setpipexl2tp(char **);
> > +
> >  static int number_option(char *, u_int32_t *, int);
> >  static int int_option(char *, int *);
> >  static int readable(int fd);
> > @@ -391,6 +396,8 @@ static struct cmd {
> >  {"ms-lanman", 0, setmslanman}, /* Use LanMan psswd when using MS-CHAP 
> > */
> >  #endif
> >  
> > +{"pipex-l2tp", 6, setpipexl2tp},   /* set pipex(4) L2TP parameters 
> > */
> > +
> >  {NULL, 0, NULL}
> >  };
> >  
> > @@ -2283,3 +2290,78 @@ setmslanman(argv)
> >  return (1);
> >  }
> >  #endif
> > +
> > +static int
> > +inet_atoss(cp, ss)
> > +char *cp;
> > +struct sockaddr_storage *ss;
> > +{
> > +struct sockaddr_in *sin = (struct sockaddr_in *)ss;
> > +char *c, *p;
> > +unsigned port;
> > +
> > +if ((c = strchr(cp, ':')) == NULL)
> > +   return 0;
> > +*c = '\0';
> > +if (!inet_aton(cp, >sin_addr))
> > +   return 0;
> > +if (!(port = strtoul(c + 1, , 10)) || *p != '\0')
> > +   return 0;
> > +sin->sin_port = htons(port);
> > +sin->sin_family = AF_INET;
> > +ss->ss_len = sizeof(*sin);
> > +
> > +return 1;
> > +}
> > +
> > +static int
> > +strtou16(str, valp)
> > +char *str;
> > +uint16_t *valp;
> > +{
> > +char *ptr;
> > +
> > +*valp = strtoul(str, , 0);
> > +
> > +return *ptr == '\0' ? 1 : 0;
> > +}
> > +
> > +static int
> > +setpipexl2tp(argv)
> > +char **argv;
> > +{
> > +BZERO((char *)_conf, sizeof(pipex_conf));
> > +
> > +if (!inet_atoss(argv[0], _conf.pr_local_address)) {
> > +   option_error("pipex-l2tp: invalid local address of tunnel: %s", 
> > argv[0]);
> > +   return 0;
> > +}
> > +if (!inet_atoss(argv[1], _conf.pr_peer_address)) {
> > +   option_error("pipex-l2tp: invalid peer address of tunnel: %s", argv[1]);
> > +   return 0;
> > +}
> > +if (!strtou16(argv[2], _conf.pr_proto.l2tp.tunnel_id)) {
> > +   option_error("pipex-l2tp: invalid local tunnel id: %s", argv[2]);
> > +   return 0;
> > +}
> > +if (!strtou16(argv[3], _conf.pr_proto.l2tp.peer_tunnel_id)) {
> > +   option_error("pipex-l2tp: invalid peer tunnel id: %s", argv[3]);
> > +   return 0;
> > +}
> > +if 

Re: [RFC] pppd: add pipex(4) L2TP control support

2020-05-26 Thread Martin Pieuchot
On 25/05/20(Mon) 21:42, Sergey Ryazanov wrote:
> Add dedicated option to activate kernel L2TP acceleration via
> the pipex(4). The options should be passed by a L2TP tunnel
> management daemon (e.g. xl2tpd).

What is the difference between npppd(8) and pppd(8)?   Aren't those two
redundant?  Why did you choose to modify pppd(8) and not npppd(8)?

> This diff is complete, but kernel support in ppp(4) is not ready.
> ---
>  usr.sbin/pppd/ipcp.c|  5 +++
>  usr.sbin/pppd/options.c | 82 +
>  usr.sbin/pppd/pppd.8| 18 +
>  usr.sbin/pppd/pppd.h|  7 
>  usr.sbin/pppd/sys-bsd.c | 56 
>  5 files changed, 168 insertions(+)
> 
> diff --git usr.sbin/pppd/ipcp.c usr.sbin/pppd/ipcp.c
> index 1296d897b14..e7a6dbd6ee7 100644
> --- usr.sbin/pppd/ipcp.c
> +++ usr.sbin/pppd/ipcp.c
> @@ -1251,6 +1251,10 @@ ipcp_up(f)
>   return;
>   }
>  
> + /* enable pipex(4), keep working if failed */
> + if (pipex_conf.pr_protocol && !apipex(go->ouraddr, ho->hisaddr, mask))
> + IPCPDEBUG((LOG_WARNING, "apipex failed"));
> +
>  #if (defined(SVR4) && (defined(SNI) || defined(__USLC__)))
>   if (!sifaddr(f->unit, go->ouraddr, ho->hisaddr, mask)) {
>   IPCPDEBUG((LOG_WARNING, "sifaddr failed"));
> @@ -1304,6 +1308,7 @@ ipcp_down(f)
>  if (demand) {
>   sifnpmode(f->unit, PPP_IP, NPMODE_QUEUE);
>  } else {
> + dpipex();
>   sifdown(f->unit);
>   ipcp_clear_addrs(f->unit);
>  }
> diff --git usr.sbin/pppd/options.c usr.sbin/pppd/options.c
> index 828f7cdce65..fe0e2e6b54b 100644
> --- usr.sbin/pppd/options.c
> +++ usr.sbin/pppd/options.c
> @@ -144,6 +144,9 @@ structbpf_program pass_filter;/* Filter program for 
> packets to pass */
>  struct   bpf_program active_filter; /* Filter program for link-active 
> pkts */
>  pcap_t  pc;  /* Fake struct pcap so we can compile expr */
>  #endif
> +struct pipex_session_req pipex_conf = {  /* pipex(4) session 
> configuration */
> +  .pr_protocol = 0,
> +};
>  
>  /*
>   * Prototypes
> @@ -253,6 +256,8 @@ static int setactivefilter(char **);
>  static int setmslanman(char **);
>  #endif
>  
> +static int setpipexl2tp(char **);
> +
>  static int number_option(char *, u_int32_t *, int);
>  static int int_option(char *, int *);
>  static int readable(int fd);
> @@ -391,6 +396,8 @@ static struct cmd {
>  {"ms-lanman", 0, setmslanman},   /* Use LanMan psswd when using MS-CHAP 
> */
>  #endif
>  
> +{"pipex-l2tp", 6, setpipexl2tp}, /* set pipex(4) L2TP parameters */
> +
>  {NULL, 0, NULL}
>  };
>  
> @@ -2283,3 +2290,78 @@ setmslanman(argv)
>  return (1);
>  }
>  #endif
> +
> +static int
> +inet_atoss(cp, ss)
> +char *cp;
> +struct sockaddr_storage *ss;
> +{
> +struct sockaddr_in *sin = (struct sockaddr_in *)ss;
> +char *c, *p;
> +unsigned port;
> +
> +if ((c = strchr(cp, ':')) == NULL)
> + return 0;
> +*c = '\0';
> +if (!inet_aton(cp, >sin_addr))
> + return 0;
> +if (!(port = strtoul(c + 1, , 10)) || *p != '\0')
> + return 0;
> +sin->sin_port = htons(port);
> +sin->sin_family = AF_INET;
> +ss->ss_len = sizeof(*sin);
> +
> +return 1;
> +}
> +
> +static int
> +strtou16(str, valp)
> +char *str;
> +uint16_t *valp;
> +{
> +char *ptr;
> +
> +*valp = strtoul(str, , 0);
> +
> +return *ptr == '\0' ? 1 : 0;
> +}
> +
> +static int
> +setpipexl2tp(argv)
> +char **argv;
> +{
> +BZERO((char *)_conf, sizeof(pipex_conf));
> +
> +if (!inet_atoss(argv[0], _conf.pr_local_address)) {
> + option_error("pipex-l2tp: invalid local address of tunnel: %s", 
> argv[0]);
> + return 0;
> +}
> +if (!inet_atoss(argv[1], _conf.pr_peer_address)) {
> + option_error("pipex-l2tp: invalid peer address of tunnel: %s", argv[1]);
> + return 0;
> +}
> +if (!strtou16(argv[2], _conf.pr_proto.l2tp.tunnel_id)) {
> + option_error("pipex-l2tp: invalid local tunnel id: %s", argv[2]);
> + return 0;
> +}
> +if (!strtou16(argv[3], _conf.pr_proto.l2tp.peer_tunnel_id)) {
> + option_error("pipex-l2tp: invalid peer tunnel id: %s", argv[3]);
> + return 0;
> +}
> +if (!strtou16(argv[4], _conf.pr_session_id)) {
> + option_error("pipex-l2tp: invalid local call id: %s", argv[4]);
> + return 0;
> +}
> +if (!strtou16(argv[5], _conf.pr_peer_session_id)) {
> + option_error("pipex-l2tp: invalid peer call id: %s", argv[5]);
> + return 0;
> +}
> +
> +/* Indicate address field presense */
> +pipex_conf.pr_ppp_flags = PIPEX_PPP_HAS_ACF;
> +
> +/* Finally set the protocol type to implicitly indicate config validity 
> */
> +pipex_conf.pr_protocol = PIPEX_PROTO_L2TP;
> +
> +return 1;
> +}
> diff --git usr.sbin/pppd/pppd.8 usr.sbin/pppd/pppd.8
> index 5fba6f1714d..6a7f6e01c09 100644
> --- usr.sbin/pppd/pppd.8
> +++ usr.sbin/pppd/pppd.8
> @@ -829,6 +829,24 @@ option is used.
>  .It 

Re: TP-Link TL-WN822N-EU v5 USB ID

2020-05-26 Thread Jonathan Gray
On Mon, May 25, 2020 at 11:27:27PM +0300, Tero Koskinen wrote:
> Hi,
> 
> I have TP-Link TL-WN822N-EU v5 USB adapter[1] with
> USB VID/PID 2357:0108:
> 
> $ usbdevs
> Controller /dev/usb0:
> addr 01: 1022: AMD, xHCI root hub
> addr 02: 2357:0108 Realtek, 802.11n NIC
> Controller /dev/usb1:
> addr 01: 1022: AMD, EHCI root hub
> addr 02: 0438:7900 Advanced Micro Devices, Hub
> $
> 
> Patch at the end adds it to the usbdevs. I hope I got it right
> as I added it between existing 0x0107 and 0x0109 IDs.
> 
> Relevant dmesg bits with the patch:
> urtwn0 at uhub0 port 3 configuration 1 interface 0 "Realtek 802.11n NIC" rev 
> 2.10/2.00 addr 2
> urtwn0: MAC/BB RTL8192EU, RF 6052 2T2R, address 50:3e:aa:24:32:9e
> uhub2 at uhub1 port 1 configuration 1 interface 0 "Advanced Micro Devices 
> Hub" rev 2.00/0.18 addr 2
> 
> Full dmesg at https://tkoskine.me/dmesg.tplink-TL-WN822N.txt
> 
> Yours,
>  Tero

thanks, committed