scp(1) changes in snaps

2021-08-05 Thread Damien Miller
Hi,

Just a head-up: snaps currently contain a set of changes[1] to
make scp(1) use the SFTP protocol by default. This has a number of
advantages, mostly relating to the improved security that comes from
avoiding the use of a protocol that shambled out of the 1980s (SCP/RCP).

A certain amount in incompatibility is to be expected: the SCP/RCP
protocol implicitly uses the remote shell for glob expansion, and this
can make quoting/escpaing problematic as it gets expanded twice, by both
the local and remote shells. SFTP-based scp(1) avoids this and functions
a lot more like what you'd typically expect. Given this, I consider this
an improvement and so I'm leaning towards not trying to make the new
code bug-compatible with SCP/RCP quoting.

Note that the code supporting scp -3 over SFTP is very new and not very
well-tested. So if you are a user of this feature then please give it a
try.

Please report any incompatibilities or bugs that you encounter here
(tech@), to bugs@ or to openssh@.

Thanks,
Damien

[1] https://github.com/djmdjm/openssh-wip/pull/7



TCP missing window update stalls connection

2021-08-05 Thread Alexander Bluhm
Hi,

I have seen some stalling TCP connections while doing unidirectional
throughput tests.  The sending machine is doing zero window probes,
but is not sending more data although the other machine announced
that it has space again.

I guess this commit in tcp_input.c triggered it:

revision 1.362
date: 2019/11/11 21:17:21;  author: bluhm;  state: Exp;  lines: +9 -3;  
commitid: wXndkYTLO9jvCPdW;
Prevent underflows in tp->snd_wnd if the remote side ACKs more than
tp->snd_wnd.  This can happen, for example, when the remote side
responds to a window probe by ACKing the one byte it contains.
from FreeBSD; via markus@; OK sashan@ tobhe@


There we fixed an integer underflow.  So we no longer have uint32
max in the send window, but 0.  I see this in my test case.  Now
we need the machanism that writes the correct send window size.

There is a commit in FreeBSD from 2002:
https://github.com/freebsd/freebsd-src/commit/1645d0903ef7162f5c4516373f0c9df0501ac893

The header prediction code did not update snd_wl2.  If there is a
sequence number wrap, the send window update is not reached.

Although I did not obverve it, there seems to be the same problem
for snd_wl1 and rcv_up.  For rcv_up I copied the comparison with
rcv_nxt from urgent processing to keep future urgent data.

ok?

bluhm

Index: netinet/tcp_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.368
diff -u -p -r1.368 tcp_input.c
--- netinet/tcp_input.c 16 Apr 2021 12:08:25 -  1.368
+++ netinet/tcp_input.c 5 Aug 2021 21:51:50 -
@@ -966,6 +966,8 @@ findpcb:
tp->t_pmtud_mss_acked = acked;
 
tp->snd_una = th->th_ack;
+   /* Pull snd_wl2 up to prevent seq wrap. */
+   tp->snd_wl2 = th->th_ack;
/*
 * We want snd_last to track snd_una so
 * as to avoid sequence wraparound problems
@@ -1015,6 +1017,10 @@ findpcb:
tcp_clean_sackreport(tp);
tcpstat_inc(tcps_preddat);
tp->rcv_nxt += tlen;
+   /* Pull snd_wl1 and rcv_up up to prevent seq wrap. */
+   tp->snd_wl1 = th->th_seq;
+   if (SEQ_GT(tp->rcv_nxt, tp->rcv_up))
+   tp->rcv_up = tp->rcv_nxt;
tcpstat_pkt(tcps_rcvpack, tcps_rcvbyte, tlen);
ND6_HINT(tp);
 



vmx(4): remove useless code

2021-08-05 Thread Jan Klemkow
Hi,

The following diff removes useless code from the driver.  As discussed
here [1] and committed there [2], the hypervisor doesn't do anything
with the data structures.  We even just set NULL to the pointer since
the initial commit of vmx(4).  So, I guess it better to remove all of
these.  The variables are bzero'd in vmxnet3_dma_allocmem() anyway.

OK?

bye,
Jan

[1]: https://www.lkml.org/lkml/2021/1/19/1225
[2]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/vmxnet3/vmxnet3_drv.c?id=de1da8bcf40564a2adada2d5d5426e05355f66e8

Index: dev/pci/if_vmx.c
===
RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
retrieving revision 1.66
diff -u -p -r1.66 if_vmx.c
--- dev/pci/if_vmx.c23 Jul 2021 00:29:14 -  1.66
+++ dev/pci/if_vmx.c5 Aug 2021 11:12:26 -
@@ -157,7 +157,6 @@ struct vmxnet3_softc {
 #define WRITE_BAR1(sc, reg, val) \
bus_space_write_4((sc)->sc_iot1, (sc)->sc_ioh1, reg, val)
 #define WRITE_CMD(sc, cmd) WRITE_BAR1(sc, VMXNET3_BAR1_CMD, cmd)
-#define vtophys(va) 0  /* XXX ok? */
 
 int vmxnet3_match(struct device *, void *, void *);
 void vmxnet3_attach(struct device *, struct device *, void *);
@@ -468,8 +467,6 @@ vmxnet3_dma_init(struct vmxnet3_softc *s
ds->vmxnet3_revision = 1;
ds->upt_version = 1;
ds->upt_features = UPT1_F_CSUM | UPT1_F_VLAN;
-   ds->driver_data = vtophys(sc);
-   ds->driver_data_len = sizeof(struct vmxnet3_softc);
ds->queue_shared = qs_pa;
ds->queue_shared_len = qs_len;
ds->mtu = VMXNET3_MAX_MTU;
@@ -546,8 +543,6 @@ vmxnet3_alloc_txring(struct vmxnet3_soft
ts->cmd_ring_len = NTXDESC;
ts->comp_ring = comp_pa;
ts->comp_ring_len = NTXCOMPDESC;
-   ts->driver_data = vtophys(tq);
-   ts->driver_data_len = sizeof *tq;
ts->intr_idx = intr;
ts->stopped = 1;
ts->error = 0;
@@ -598,8 +593,6 @@ vmxnet3_alloc_rxring(struct vmxnet3_soft
rs->cmd_ring_len[1] = NRXDESC;
rs->comp_ring = comp_pa;
rs->comp_ring_len = NRXCOMPDESC;
-   rs->driver_data = vtophys(rq);
-   rs->driver_data_len = sizeof *rq;
rs->intr_idx = intr;
rs->stopped = 1;
rs->error = 0;



Re: time.3: miscellaneous cleanup and rewrites

2021-08-05 Thread Ingo Schwarze
Hi Scott,

since i see this wasn't committed yet, it is OK schwarze@, too.

Consider leaving the .Nd alone since both jmc@ and millert@ recommended
that, and use "That version counted the time" in the HISTORY section
as recommended by jmc@.

I think you should indeed remove the sentence about gettimeofday(2)
from the DESCRIPTION because millert@ has a point that it's an
implementation detail, and even if considered interesting, it's
already mentioned in the HISTORY section.

Yours,
  Ingo


Scott Cheloha wrote on Fri, Jul 30, 2021 at 01:01:21PM -0500:

> Index: time.3
> ===
> RCS file: /cvs/src/lib/libc/gen/time.3,v
> retrieving revision 1.16
> diff -u -p -r1.16 time.3
> --- time.329 Jan 2015 01:46:30 -  1.16
> +++ time.330 Jul 2021 17:59:37 -
> @@ -36,52 +36,54 @@
>  .Os
>  .Sh NAME
>  .Nm time
> -.Nd get time of day
> +.Nd get the time of day
>  .Sh SYNOPSIS
>  .In time.h
>  .Ft time_t
> -.Fn time "time_t *tloc"
> +.Fn time "time_t *now"
>  .Sh DESCRIPTION
>  The
>  .Fn time
> -function returns the value of time in seconds since 0 hours, 0 minutes,
> -0 seconds, January 1, 1970, Coordinated Universal Time (UTC).
> +function returns the the number of seconds elapsed since
> +Jan 1 1970 00:00:00 UTC.
> +This value is also written to
> +.Fa now
> +unless
> +.Fa now
> +is
> +.Dv NULL .
>  .Pp
> -A copy of the time value may be saved to the area indicated by the
> -pointer
> -.Fa tloc .
> -If
> -.Fa tloc
> -is a null pointer, no value is stored.
> +This version of
> +.Fn time
> +is implemented with
> +.Xr gettimeofday 2 .
>  .Sh RETURN VALUES
> -Upon successful completion,
> +The
>  .Fn time
> -returns the value of time.
> -Otherwise a value of
> -.Po Fa time_t Pc Ns -1
> -is returned and the global variable
> -.Va errno
> -is set to indicate the error.
> -.Sh ERRORS
> -The following error codes may be set in
> -.Va errno :
> -.Bl -tag -width Er
> -.It Bq Er EFAULT
> -An argument address referenced invalid memory.
> -.El
> +function is always successful,
> +and no return value is reserved to indicate an error.
>  .Sh SEE ALSO
>  .Xr clock_gettime 2 ,
>  .Xr gettimeofday 2 ,
>  .Xr ctime 3
> +.Sh STANDARDS
> +The
> +.Fn time
> +function conforms to
> +.St -p1003.1-2008 .
>  .Sh HISTORY
>  A
>  .Fn time
>  system call first appeared in
> -.At v1
> -and used to return time in sixtieths of a second in 32 bits,
> -which was to guarantee a crisis every 2.26 years.
> -Since
> -.At v6 ,
> -.Fn time
> -scale was changed to seconds, extending the pre-crisis stagnation
> -period up to a total of 68 years.
> +.At v1 .
> +This version counted time in sixtieths of a second with a 32-bit return 
> value,
> +ensuring an integer overflow crisis every 2.26 years.
> +In
> +.At v6
> +the granularity of the return value was reduced to whole seconds,
> +delaying the aforementioned crisis until 2038.
> +In
> +.Bx 4.1c
> +the function was moved out of the kernel into the C standard library and
> +reimplemented with
> +.Xr gettimeofday 2 .



Re: Add versioned lib to system perl's @INC for non-packaged modules

2021-08-05 Thread Ingo Schwarze
Hi,

thanks to sthen@ for providing more background!

so the bottom line seems to be that, once the code changes are
tested, committed, and prove stable and once people come round to
it, moving the site manual page directories out of /usr/local/man/
and making them versioned in exactly the same way as the sitelib
directories may be a reasonable option.

Even if the main effect of versioning them might be to make it more
obvious for people what went wrong in case things get mixed up.

Yours,
  Ingo



Re: less(1): refreshing file of size 0 results in file being treated as a pipe

2021-08-05 Thread user
Oops, forgot that OpenBSD doesn't have ! capability in less. Instead of !echo a 
> % and !echo b > %, run 
$ echo a > /tmp/test
Press h and q in less to reload the file
$ echo b > /tmp/test
Press h and q in less to reload the file

On Thu, Aug 05, 2021 at 12:37:00AM -0500, user wrote:
> Bug Reproduction:
> $ touch /tmp/test
> $ less /tmp/test
> Press r
> Run !echo a > %
> Run !echo b > %
> Output:
> a
> b
> 
> On Fri, Jul 23, 2021 at 11:15:59AM -0500, user wrote:
> > Less contains a hack to force files of size 0 to become non-seekable in 
> > order to workaround a linux kernel bug. 
> > 
> > When the file becomes non-seekable any further reads from the file are 
> > appended rather than overwriting the original contents of the file.
> > 
> > diff --git ch.c ch.c
> > index 1a679767a42..d7c0aa34e90 100644
> > --- ch.c
> > +++ ch.c
> > @@ -643,19 +643,6 @@ ch_flush(void)
> > ch_block = 0; /* ch_fpos / LBUFSIZE; */
> > ch_offset = 0; /* ch_fpos % LBUFSIZE; */
> > 
> > -#if 1
> > -   /*
> > -* This is a kludge to workaround a Linux kernel bug: files in
> > -* /proc have a size of 0 according to fstat() but have readable
> > -* data.  They are sometimes, but not always, seekable.
> > -* Force them to be non-seekable here.
> > -*/
> > -   if (ch_fsize == 0) {
> > -   ch_fsize = -1;
> > -   ch_flags &= ~CH_CANSEEK;
> > -   }
> > -#endif
> > -
> > if (lseek(ch_file, (off_t)0, SEEK_SET) == (off_t)-1) {
> > /*
> >  * Warning only; even if the seek fails for some reason
> > 
> 



Re: less(1): refreshing file of size 0 results in file being treated as a pipe

2021-08-05 Thread user
Bug Reproduction:
$ touch /tmp/test
$ less /tmp/test
Press r
Run !echo a > %
Run !echo b > %
Output:
a
b

On Fri, Jul 23, 2021 at 11:15:59AM -0500, user wrote:
> Less contains a hack to force files of size 0 to become non-seekable in order 
> to workaround a linux kernel bug. 
> 
> When the file becomes non-seekable any further reads from the file are 
> appended rather than overwriting the original contents of the file.
> 
> diff --git ch.c ch.c
> index 1a679767a42..d7c0aa34e90 100644
> --- ch.c
> +++ ch.c
> @@ -643,19 +643,6 @@ ch_flush(void)
> ch_block = 0; /* ch_fpos / LBUFSIZE; */
> ch_offset = 0; /* ch_fpos % LBUFSIZE; */
> 
> -#if 1
> -   /*
> -* This is a kludge to workaround a Linux kernel bug: files in
> -* /proc have a size of 0 according to fstat() but have readable
> -* data.  They are sometimes, but not always, seekable.
> -* Force them to be non-seekable here.
> -*/
> -   if (ch_fsize == 0) {
> -   ch_fsize = -1;
> -   ch_flags &= ~CH_CANSEEK;
> -   }
> -#endif
> -
> if (lseek(ch_file, (off_t)0, SEEK_SET) == (off_t)-1) {
> /*
>  * Warning only; even if the seek fails for some reason
> 



Re: Fix unsafe snmpd defaults

2021-08-05 Thread Stuart Henderson
On 2021/08/03 23:46, Martijn van Duren wrote:
> On Tue, 2021-08-03 at 21:58 +0100, Stuart Henderson wrote:
> > On 2021/08/03 22:07, Martijn van Duren wrote:
> > > On Tue, 2021-08-03 at 18:24 +0100, Stuart Henderson wrote:
> > > > On 2021/06/15 17:39, Stuart Henderson wrote:
> > > > > > Then again, I don't get the feeling many people use snmpd at this 
> > > > > > time
> > > > > > and maybe it's a good moment to bite the bullet and go for safest
> > > > > > defaults possible at this time. But if that's the case I would like 
> > > > > > to
> > > > > > follow up with a diff to changes the default auth to hmac-sha512,
> > > > > > because snmp drops trailing bytes of the result and enc to aes 
> > > > > > instead
> > > > > > of des.
> > > > > 
> > > > > This is the change that feels most likely to affect existing SNMPv3 
> > > > > users.
> > > > > Support in management software beyond aes/sha1 is a bit lacking and 
> > > > > prone
> > > > > to incompatibility (I had issues with net-snmp and snmpd using 
> > > > > hmac-sha256
> > > > > though it seems it will work with hmac-sha512..)
> > > > 
> > > > BTW, having updated a few machines now, I am finding the change to
> > > > sha2-256 by default to be a complete pain, especially considering that
> > > > /etc/examples/snmpd.conf uses "enc aes" but has no setting for auth
> > > > so relies on defaults for that..
> > > > 
> > > I can't do a lot with "a complete pain".
> > > 
> > > Does something like the diff below make things more intuitive? If not,
> > > could you be a little more concrete?
> > > 
> > > martijn@
> > > 
> > > Index: snmpd.conf
> > > ===
> > > RCS file: /cvs/src/etc/examples/snmpd.conf,v
> > > retrieving revision 1.1
> > > diff -u -p -r1.1 snmpd.conf
> > > --- snmpd.conf  11 Jul 2014 21:20:10 -  1.1
> > > +++ snmpd.conf  3 Aug 2021 20:05:53 -
> > > @@ -18,7 +18,9 @@ system services 74
> > >  oid 1.3.6.1.4.1.30155.42.3.1 name testStringValue read-only string "Test"
> > >  oid 1.3.6.1.4.1.30155.42.3.4 name testIntValue read-write integer 1
> > >  
> > > -# Enable SNMPv3 USM with authentication, encryption and two defined users
> > > -#seclevel enc
> > > -#user "user1" authkey "password123" enc aes enckey "321drowssap"
> > > -#user "user2" authkey "password456" enckey "654drowssap"
> > > +# Create two SNMPv3 USM users:
> > > +# User with default crypto values
> > > +#user "defaultuser" authkey "password123" enckey "321drowssap"
> > > +# User with backwards compatible crypto:
> > > +# Only enable and use when client absolutely can't deal with modern 
> > > defaults.
> > > +#user "compatuser" authkey "password456" auth hmac-md5 enckey 
> > > "654drowssap" enc des
> > > 
> > > 
> > 
> > Given the lack of support for SHA2-256 in much management software until
> > recently AES+SHA is a pretty common configuration. And given the old 
> > snmpd.conf
> > example I think that is often done by copying/editing so just "enc aes" is 
> > there
> > with no auth setting. Wondering if that part might not have been such a good
> > change and what anyone else thinks..
> > 
> I think that these management software applications should join 2016 and start
> implementing it and until then its just two or four minor keywords per user.
> But I'm not a heavy user of 3rd party mangement software.

"Given the lack of support for SHA2-256 in much management software
_until recently_". What they do now is only relevant to new
configurations, it's what they did in the past when they were configured
that determines what people are using in existing config and whether
they'll have to reconfigure things to cope with the default changing.

Since even hmac-md5 is still AFAIK not considered unsafe (hmac is a
very different thing to using the algorithm for file integrity checks)
and there are clearly still issues with sha2-256 in SNMP I think we're
probably better off reverting that part of the defaults change and
go back to hmac-sha1.

Diff for that below - it also fixes some text missed in the previous
change des->aes, and adds explicit auth setting to examples/snmpd.conf.
Another option might be to remove the default algorithms altogether,
requiring that they are specified explicitly instead. That would mean
users would still have to change config but then we don't run into
the situation again (it doesn't really seem sensible to rely on
a default for a protocol like this which is not 'agile' with respect
to algorithms).

> Also note that the first time I suggested changing the defaults[0] I offered
> to help with getting perl's snmp into shape. That offer still stands with the
> same caveats. Similar for other open source software that I'm not aware of.
> 
> [0] https://marc.info/?l=openbsd-tech=157226549212943=2

I think the bits which you were having trouble with for the Perl
implementation are the same bits that most of us would have trouble
with! Also Net-SNMP typically has quite slow update cycles so even