remove src/sys/netinet/ip_ether.c because it is not used
jca@ has already oked this. anyone else want to get on board? Index: ip_ether.c === RCS file: ip_ether.c diff -N ip_ether.c --- ip_ether.c 10 Feb 2018 08:12:01 - 1.99 +++ /dev/null 1 Jan 1970 00:00:00 - @@ -1,28 +0,0 @@ -/* $OpenBSD: ip_ether.c,v 1.99 2018/02/10 08:12:01 dlg Exp $ */ -/* - * The author of this code is Angelos D. Keromytis (ker...@adk.gr) - * - * This code was written by Angelos D. Keromytis for OpenBSD in October 1999. - * - * Copyright (C) 1999-2001 Angelos D. Keromytis. - * - * Permission to use, copy, and modify this software with or without fee - * is hereby granted, provided that this entire notice is included in - * all copies of any software which is or includes a copy or - * modification of this software. - * You may use this code under the GNU public license if you so wish. Please - * contribute changes back to the authors under this freer than GPL license - * so that we may further the use of strong encryption without limitations to - * all. - * - * THIS SOFTWARE IS BEING PROVIDED "AS IS", WITHOUT ANY EXPRESS OR - * IMPLIED WARRANTY. IN PARTICULAR, NONE OF THE AUTHORS MAKES ANY - * REPRESENTATION OR WARRANTY OF ANY KIND CONCERNING THE - * MERCHANTABILITY OF THIS SOFTWARE OR ITS FITNESS FOR ANY PARTICULAR - * PURPOSE. - */ - -/* - * Ethernet-inside-IP processing (RFC3378). - */ -
Re: Attach Hyper-V guest services to VMBus 4.0
On 04.10.2019 15:54, Mike Belopuhov wrote: > Thanks a lot for dmesgs. Do I understand correctly that paravirtualized > disk and network work fine for you regardless of whether OpenBSD-current, > attached one-liner diff or the linked larger diff are used? > > With best regards, > Mike I'm not sure I understand correctly, but if you mean that hvn(4) and hvs(4) work flawlessly, then yes. That has been the case for some releases now. Thanks for your work on these drivers! Let me know if I can provide more information or test patches. Regards, Andre
Re: Attach Hyper-V guest services to VMBus 4.0
Andre Stoebe writes: > On 03.10.2019 02:13, Mike Belopuhov wrote: >> And what about OpenBSD-current or an attached patch as opposed >> to the linked one? >> >> Please don't go half the way if you're willing to help us out, >> we'd like to make OpenBSD 6.6-release work in these setups >> especially since we believe that all it takes is an one-line >> diff. > > Hi Mike, > > you're right, I should know better by now to include all information. > > Here are the three dmesgs (-current, attached patch, linked patch) with > all integration services enabled. > Thanks a lot for dmesgs. Do I understand correctly that paravirtualized disk and network work fine for you regardless of whether OpenBSD-current, attached one-liner diff or the linked larger diff are used? With best regards, Mike > Regards, > Andre > > dmesg (-current): > > OpenBSD 6.6 (GENERIC.MP) #344: Wed Oct 2 11:48:47 MDT 2019 > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP > real mem = 8573091840 (8175MB) > avail mem = 8300560384 (7916MB) > mpath0 at root > scsibus0 at mpath0: 256 targets > mainbus0 at root > bios0 at mainbus0: SMBIOS rev. 2.3 @ 0xf93d0 (338 entries) > bios0: vendor American Megatrends Inc. version "090008" date 12/07/2018 > bios0: Microsoft Corporation Virtual Machine > acpi0 at bios0: ACPI 2.0 > acpi0: sleep states S0 S5 > acpi0: tables DSDT FACP WAET SLIC OEM0 SRAT APIC OEMB > acpi0: wakeup devices > acpitimer0 at acpi0: 3579545 Hz, 32 bits > acpihve0 at acpi0 > acpimadt0 at acpi0 addr 0xfee0: PC-AT compat > ioapic0 at mainbus0: apid 0 pa 0xfec0, version 11, 24 pins, remapped > cpu0 at mainbus0: apid 0 (boot processor) > cpu0: Intel(R) Xeon(R) CPU E3-1245 v5 @ 3.50GHz, 2811.42 MHz, 06-5e-03 > cpu0: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN > cpu0: 256KB 64b/line 8-way L2 cache > tsc_timecounter_init: TSC skew=0 observed drift=0 > cpu0: smt 0, core 0, package 0 > mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges > cpu0: apic clock running at 163MHz > cpu1 at mainbus0: apid 1 (application processor) > TSC skew=-9 > cpu1: Intel(R) Xeon(R) CPU E3-1245 v5 @ 3.50GHz, 2855.87 MHz, 06-5e-03 > cpu1: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN > cpu1: 256KB 64b/line 8-way L2 cache > tsc_timecounter_init: TSC skew=-9 observed drift=0 > cpu1: smt 0, core 1, package 0 > cpu2 at mainbus0: apid 2 (application processor) > TSC skew=8 > cpu2: Intel(R) Xeon(R) CPU E3-1245 v5 @ 3.50GHz, 2855.90 MHz, 06-5e-03 > cpu2: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN > cpu2: 256KB 64b/line 8-way L2 cache > tsc_timecounter_init: TSC skew=8 observed drift=0 > cpu2: smt 0, core 2, package 0 > cpu3 at mainbus0: apid 3 (application processor) > TSC skew=-20 > cpu3: Intel(R) Xeon(R) CPU E3-1245 v5 @ 3.50GHz, 2855.89 MHz, 06-5e-03 > cpu3: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN > cpu3: 256KB 64b/line 8-way L2 cache > tsc_timecounter_init: TSC skew=-20 observed drift=0 > cpu3: smt 0, core 3, package 0 > acpiprt0 at acpi0: bus 0 (PCI0) > acpicpu0 at acpi0: C1(@1 halt!) > acpicpu1 at acpi0: C1(@1 halt!) > acpicpu2 at acpi0: C1(@1 halt!) > acpicpu3 at acpi0: C1(@1 halt!) > acpipci0 at acpi0 PCI0: _OSC failed > acpicmos0 at acpi0 > "VMBus" at acpi0 not configured > "Hyper_V_Gen_Counter_V1" at acpi0 not configured > cpu0: using VERW MDS workaround (except on vmm entry) > pvbus0 at mainbus0: Hyper-V 10.0 > hyperv0 at pvbus0: protocol 3.0, features 0x2e7f > hyperv0: heartbeat, kvp, shutdown, timesync > hvs0 at hyperv0 channel 2: ide, protocol 6.2 > scsibus1 at hvs0: 2 targets > sd0 at scsibus1 targ 0 lun 0: > naa.600224802050cb43ae008c0f97857048
Re: Minor vi MAN helper/precisions
On Fri, Oct 04, 2019 at 12:35:40PM +0100, Stuart Henderson wrote: > On 2019/10/04 07:22, sven falempin wrote: > > On Fri, Oct 4, 2019 at 4:29 AM Stuart Henderson > > wrote: > > > > > On 2019/10/03 11:56, sven falempin wrote: > > > > BTW, on modern system RAM is plentiful while SSD are not always '/tmp' > > > > friendly. > > > > > > Some of the slower media (CF/SD cards etc) might have issues eventually > > > but > > > I would expect anything worthy of the name 'SSD' should be fine. > > > > > > > Using tmpfs for /tmp, vi unless you create a proto with > > > vi.recover > > > > subdir or create it on rc.local. > > > > maybe a mkdir /tmp/vi.recover.uid would make sense, or just a mktemp . > > > > > > eh? > > > > > > - vi.recover is created by /usr/libexec/vi.recover which is run from > > > /etc/rc already > > > > > > - tmpfs is disabled in the kernel config for a reason > > > > > > > I will investigate why I lose the directory so often . Meant MFS , no > > custom kernel. > > Last time it???s just a no reboot situation * > > > > Are you ok with the man changes ? > > > > Index: vi.1 > > === > > RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v > > retrieving revision 1.76 > > diff -u -r1.76 vi.1 > > --- vi.121 May 2019 09:24:58 - 1.76 > > +++ vi.13 Oct 2019 17:01:14 - > > @@ -2089,7 +2089,9 @@ > > .Op option? ... > > .Op Ar all > > .Xc > > -Display or set editor options. > > +Display or > > +.Sx SET OPTIONS > > +of the editor. > > .Pp > > .It Cm sh Ns Op Cm ell > > Run a shell program. > > Not really a fan of this, it doesn't read very nicely to me and it's > totally obvious already.. > hmm. i have to admit, i agree with stuart. i overlooked the ugliness because it seemed neat to have a handy link, but i do think it reads worse. > > @@ -2273,9 +2275,11 @@ > > not in > > .Nm ex . > > .Sh SET OPTIONS > > -There are a large number of options that may be set > > -.Pq or unset > > -to change the editor's behavior. > > +There are a large number of options that can > > +change the editor's behavior, > > +using the > > +.Cm set > > +command. > > I don't mind this, I suppose it might help people who read "SET OPTIONS" > without reading 'FAST STARTUP' or the ex commands list. > so i will probably commit just this chunk. jmc > > This section describes the options, their abbreviations and their > > default values. > > .Pp > > >
[patch] www.openbsd.org/events.html
Hi All, Does it make sense to add my talk on vmm/vmd at EuroBSDCon to the events.html page? If it does, below is the diff. (Thanx Paul! :)) Mischa Index: events.html === RCS file: /home/OpenBSD/cvs/www/events.html,v retrieving revision 1.1182 diff -u -p -r1.1182 events.html --- events.html 22 Sep 2019 19:36:35 - 1.1182 +++ events.html 4 Oct 2019 07:47:37 - @@ -61,6 +61,8 @@ September 19-22, 2019, Lillehammer, Norw (slides) Marc Espie - Advanced ports toolkit: near-perfect packing-list generation (slides) +Mischa Peters - The OpenBSD hypervisor in the wild, a short story. +(https://2019.eurobsdcon.org/slides/The%20OpenBSD%20hypervisor%20in%20the%20wild,%20a%20short%20story%20-%20Mischa%20Peters.pdf;>slides)
Re: Minor vi MAN helper/precisions
On 2019/10/04 07:22, sven falempin wrote: > On Fri, Oct 4, 2019 at 4:29 AM Stuart Henderson wrote: > > > On 2019/10/03 11:56, sven falempin wrote: > > > BTW, on modern system RAM is plentiful while SSD are not always '/tmp' > > > friendly. > > > > Some of the slower media (CF/SD cards etc) might have issues eventually but > > I would expect anything worthy of the name 'SSD' should be fine. > > > > > Using tmpfs for /tmp, vi unless you create a proto with > > vi.recover > > > subdir or create it on rc.local. > > > maybe a mkdir /tmp/vi.recover.uid would make sense, or just a mktemp . > > > > eh? > > > > - vi.recover is created by /usr/libexec/vi.recover which is run from > > /etc/rc already > > > > - tmpfs is disabled in the kernel config for a reason > > > > I will investigate why I lose the directory so often . Meant MFS , no > custom kernel. > Last time it’s just a no reboot situation * > > Are you ok with the man changes ? > Index: vi.1 > === > RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v > retrieving revision 1.76 > diff -u -r1.76 vi.1 > --- vi.1 21 May 2019 09:24:58 - 1.76 > +++ vi.1 3 Oct 2019 17:01:14 - > @@ -2089,7 +2089,9 @@ > .Op option? ... > .Op Ar all > .Xc > -Display or set editor options. > +Display or > +.Sx SET OPTIONS > +of the editor. > .Pp > .It Cm sh Ns Op Cm ell > Run a shell program. Not really a fan of this, it doesn't read very nicely to me and it's totally obvious already.. > @@ -2273,9 +2275,11 @@ > not in > .Nm ex . > .Sh SET OPTIONS > -There are a large number of options that may be set > -.Pq or unset > -to change the editor's behavior. > +There are a large number of options that can > +change the editor's behavior, > +using the > +.Cm set > +command. I don't mind this, I suppose it might help people who read "SET OPTIONS" without reading 'FAST STARTUP' or the ex commands list. > This section describes the options, their abbreviations and their > default values. > .Pp >
Re: Minor vi MAN helper/precisions
On Fri, Oct 4, 2019 at 4:29 AM Stuart Henderson wrote: > On 2019/10/03 11:56, sven falempin wrote: > > BTW, on modern system RAM is plentiful while SSD are not always '/tmp' > > friendly. > > Some of the slower media (CF/SD cards etc) might have issues eventually but > I would expect anything worthy of the name 'SSD' should be fine. > > > Using tmpfs for /tmp, vi unless you create a proto with > vi.recover > > subdir or create it on rc.local. > > maybe a mkdir /tmp/vi.recover.uid would make sense, or just a mktemp . > > eh? > > - vi.recover is created by /usr/libexec/vi.recover which is run from > /etc/rc already > > - tmpfs is disabled in the kernel config for a reason > I will investigate why I lose the directory so often . Meant MFS , no custom kernel. Last time it’s just a no reboot situation * Are you ok with the man changes ? -- -- - Knowing is not enough; we must apply. Willing is not enough; we must do
bgpd fix for pftable panic
Because of delaying the commits in the RDE it is now possible that table
additions and deletions mix. This triggers a fatal("attempt to mix pf table
additions/deletions") in the parent. Instead of the fatal() it is actually
safe to just commit the pending work and start with a fresh worklist
afterwards.
--
:wq Claudio
Index: pftable.c
===
RCS file: /cvs/src/usr.sbin/bgpd/pftable.c,v
retrieving revision 1.14
diff -u -p -r1.14 pftable.c
--- pftable.c 8 Aug 2019 20:06:29 - 1.14
+++ pftable.c 4 Oct 2019 10:22:15 -
@@ -191,10 +191,13 @@ pftable_add_work(const char *table, stru
return (-1);
}
- /* Only one type of work on the list at a time */
+ /*
+* Only one type of work on the list at a time,
+* commit pending work first before adding new work
+*/
what = del ? DIOCRDELADDRS : DIOCRADDADDRS;
if (pft->naddrs != 0 && pft->what != what)
- fatal("attempt to mix pf table additions/deletions");
+ pftable_commit();
if (pft->nalloc <= pft->naddrs)
pft->nalloc = pft->nalloc == 0 ? 1 : pft->nalloc * 2;
Re: Minor vi MAN helper/precisions
On 2019/10/03 11:56, sven falempin wrote: > BTW, on modern system RAM is plentiful while SSD are not always '/tmp' > friendly. Some of the slower media (CF/SD cards etc) might have issues eventually but I would expect anything worthy of the name 'SSD' should be fine. > Using tmpfs for /tmp, vi unless you create a proto with vi.recover > subdir or create it on rc.local. > maybe a mkdir /tmp/vi.recover.uid would make sense, or just a mktemp . eh? - vi.recover is created by /usr/libexec/vi.recover which is run from /etc/rc already - tmpfs is disabled in the kernel config for a reason
Re: smtpd handling of \r in DATA part
On Fri, Oct 04, 2019 at 10:09:06AM +0200, Martijn van Duren wrote:
> On 10/4/19 10:03 AM, gil...@poolp.org wrote:
> > October 4, 2019 9:55 AM, "Martijn van Duren"
> > wrote:
> >>
> >> This is similar to the diff I send a few months ago, but still doesn't
> >> fix the case when someone sends a standalone '\n' as line-ending.
> >>
> >
> > Unsure I understand that, can you elaborate ?
>
> I send the diff below to you and Eric back in April, but it was dropped
> in favour of forbidding the totally (the thing being discussed
> now).
>
Yes, I recall.
> >> I'd prefer if we go with reverting (=my previous diff) until we have
> >> something that definitively fixes things. If people send broken mails
> >> they get broken signatures until that time.
> >
> > Fine by me
> >
>
ok lets revert and find the proper fix post release.
> Index: bounce.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/bounce.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 bounce.c
> --- bounce.c 8 Dec 2018 08:01:15 - 1.80
> +++ bounce.c 24 Apr 2019 09:33:35 -
> @@ -718,7 +718,7 @@ bounce_io(struct io *io, int evt, void *
> switch (evt) {
> case IO_DATAIN:
> nextline:
> - line = io_getline(s->io, );
> + line = io_getline_rn(s->io, );
> if (line == NULL && io_datalen(s->io) >= LINE_MAX) {
> bounce_status(s, "Input too long");
> bounce_free(s);
> Index: iobuf.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/iobuf.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 iobuf.c
> --- iobuf.c 17 Mar 2017 20:56:04 - 1.10
> +++ iobuf.c 24 Apr 2019 09:33:35 -
> @@ -156,7 +156,35 @@ iobuf_drop(struct iobuf *io, size_t n)
> }
>
> char *
> -iobuf_getline(struct iobuf *iobuf, size_t *rlen)
> +iobuf_getline_n(struct iobuf *iobuf, size_t *rlen)
> +{
> + char*buf;
> + size_t len, i;
> +
> + buf = iobuf_data(iobuf);
> + len = iobuf_len(iobuf);
> +
> + for (i = 0; i + 1 <= len; i++)
> + if (buf[i] == '\n') {
> + /* Note: the returned address points into the iobuf
> + * buffer. We NUL-end it for convenience, and discard
> + * the data from the iobuf, so that the caller doesn't
> + * have to do it. The data remains "valid" as long
> + * as the iobuf does not overwrite it, that is until
> + * the next call to iobuf_normalize() or iobuf_extend().
> + */
> + iobuf_drop(iobuf, i + 1);
> + buf[i] = '\0';
> + if (rlen)
> + *rlen = i;
> + return (buf);
> + }
> +
> + return (NULL);
> +}
> +
> +char *
> +iobuf_getline_rn(struct iobuf *iobuf, size_t *rlen)
> {
> char*buf;
> size_t len, i;
> Index: iobuf.h
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/iobuf.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 iobuf.h
> --- iobuf.h 20 Jan 2015 17:37:54 - 1.4
> +++ iobuf.h 24 Apr 2019 09:33:35 -
> @@ -51,7 +51,8 @@ size_t iobuf_space(struct iobuf *);
> size_t iobuf_len(struct iobuf *);
> size_t iobuf_left(struct iobuf *);
> char *iobuf_data(struct iobuf *);
> -char *iobuf_getline(struct iobuf *, size_t *);
> +char *iobuf_getline_n(struct iobuf *, size_t *);
> +char *iobuf_getline_rn(struct iobuf *, size_t *);
> ssize_t iobuf_read(struct iobuf *, int);
> ssize_t iobuf_read_ssl(struct iobuf *, void *);
>
> Index: ioev.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/ioev.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 ioev.c
> --- ioev.c17 May 2017 14:00:06 - 1.41
> +++ ioev.c24 Apr 2019 09:33:35 -
> @@ -501,9 +501,15 @@ io_datalen(struct io *io)
> }
>
> char *
> -io_getline(struct io *io, size_t *sz)
> +io_getline_n(struct io *io, size_t *sz)
> {
> - return iobuf_getline(>iobuf, sz);
> + return iobuf_getline_n(>iobuf, sz);
> +}
> +
> +char *
> +io_getline_rn(struct io *io, size_t *sz)
> +{
> + return iobuf_getline_rn(>iobuf, sz);
> }
>
> void
> Index: ioev.h
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/ioev.h,v
> retrieving revision 1.16
> diff -u -p -r1.16 ioev.h
> --- ioev.h30 Nov 2016 17:43:32 - 1.16
> +++ ioev.h24 Apr 2019 09:33:35 -
> @@ -65,5 +65,6 @@ size_t io_queued(struct io *);
> /* Buffered input functions */
> void* io_data(struct io *);
> size_t io_datalen(struct io *);
> -char* io_getline(struct io *, size_t *);
> +char* io_getline_n(struct io *, size_t *);
> +char*
Re: smtpd handling of \r in DATA part
On 10/4/19 10:03 AM, gil...@poolp.org wrote:
> October 4, 2019 9:55 AM, "Martijn van Duren"
> wrote:
>>
>> This is similar to the diff I send a few months ago, but still doesn't
>> fix the case when someone sends a standalone '\n' as line-ending.
>>
>
> Unsure I understand that, can you elaborate ?
I send the diff below to you and Eric back in April, but it was dropped
in favour of forbidding the totally (the thing being discussed
now).
>
>
>> I'd prefer if we go with reverting (=my previous diff) until we have
>> something that definitively fixes things. If people send broken mails
>> they get broken signatures until that time.
>
> Fine by me
>
Index: bounce.c
===
RCS file: /cvs/src/usr.sbin/smtpd/bounce.c,v
retrieving revision 1.80
diff -u -p -r1.80 bounce.c
--- bounce.c8 Dec 2018 08:01:15 - 1.80
+++ bounce.c24 Apr 2019 09:33:35 -
@@ -718,7 +718,7 @@ bounce_io(struct io *io, int evt, void *
switch (evt) {
case IO_DATAIN:
nextline:
- line = io_getline(s->io, );
+ line = io_getline_rn(s->io, );
if (line == NULL && io_datalen(s->io) >= LINE_MAX) {
bounce_status(s, "Input too long");
bounce_free(s);
Index: iobuf.c
===
RCS file: /cvs/src/usr.sbin/smtpd/iobuf.c,v
retrieving revision 1.10
diff -u -p -r1.10 iobuf.c
--- iobuf.c 17 Mar 2017 20:56:04 - 1.10
+++ iobuf.c 24 Apr 2019 09:33:35 -
@@ -156,7 +156,35 @@ iobuf_drop(struct iobuf *io, size_t n)
}
char *
-iobuf_getline(struct iobuf *iobuf, size_t *rlen)
+iobuf_getline_n(struct iobuf *iobuf, size_t *rlen)
+{
+ char*buf;
+ size_t len, i;
+
+ buf = iobuf_data(iobuf);
+ len = iobuf_len(iobuf);
+
+ for (i = 0; i + 1 <= len; i++)
+ if (buf[i] == '\n') {
+ /* Note: the returned address points into the iobuf
+* buffer. We NUL-end it for convenience, and discard
+* the data from the iobuf, so that the caller doesn't
+* have to do it. The data remains "valid" as long
+* as the iobuf does not overwrite it, that is until
+* the next call to iobuf_normalize() or iobuf_extend().
+*/
+ iobuf_drop(iobuf, i + 1);
+ buf[i] = '\0';
+ if (rlen)
+ *rlen = i;
+ return (buf);
+ }
+
+ return (NULL);
+}
+
+char *
+iobuf_getline_rn(struct iobuf *iobuf, size_t *rlen)
{
char*buf;
size_t len, i;
Index: iobuf.h
===
RCS file: /cvs/src/usr.sbin/smtpd/iobuf.h,v
retrieving revision 1.4
diff -u -p -r1.4 iobuf.h
--- iobuf.h 20 Jan 2015 17:37:54 - 1.4
+++ iobuf.h 24 Apr 2019 09:33:35 -
@@ -51,7 +51,8 @@ size_tiobuf_space(struct iobuf *);
size_t iobuf_len(struct iobuf *);
size_t iobuf_left(struct iobuf *);
char *iobuf_data(struct iobuf *);
-char *iobuf_getline(struct iobuf *, size_t *);
+char *iobuf_getline_n(struct iobuf *, size_t *);
+char *iobuf_getline_rn(struct iobuf *, size_t *);
ssize_tiobuf_read(struct iobuf *, int);
ssize_tiobuf_read_ssl(struct iobuf *, void *);
Index: ioev.c
===
RCS file: /cvs/src/usr.sbin/smtpd/ioev.c,v
retrieving revision 1.41
diff -u -p -r1.41 ioev.c
--- ioev.c 17 May 2017 14:00:06 - 1.41
+++ ioev.c 24 Apr 2019 09:33:35 -
@@ -501,9 +501,15 @@ io_datalen(struct io *io)
}
char *
-io_getline(struct io *io, size_t *sz)
+io_getline_n(struct io *io, size_t *sz)
{
- return iobuf_getline(>iobuf, sz);
+ return iobuf_getline_n(>iobuf, sz);
+}
+
+char *
+io_getline_rn(struct io *io, size_t *sz)
+{
+ return iobuf_getline_rn(>iobuf, sz);
}
void
Index: ioev.h
===
RCS file: /cvs/src/usr.sbin/smtpd/ioev.h,v
retrieving revision 1.16
diff -u -p -r1.16 ioev.h
--- ioev.h 30 Nov 2016 17:43:32 - 1.16
+++ ioev.h 24 Apr 2019 09:33:35 -
@@ -65,5 +65,6 @@ size_t io_queued(struct io *);
/* Buffered input functions */
void* io_data(struct io *);
size_t io_datalen(struct io *);
-char* io_getline(struct io *, size_t *);
+char* io_getline_n(struct io *, size_t *);
+char* io_getline_rn(struct io *, size_t *);
void io_drop(struct io *, size_t);
Index: lka_filter.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
retrieving revision 1.35
diff -u -p -r1.35 lka_filter.c
--- lka_filter.c8 Apr 2019 07:44:45 - 1.35
+++
Re: smtpd handling of \r in DATA part
October 4, 2019 9:55 AM, "Martijn van Duren" wrote: > > This is similar to the diff I send a few months ago, but still doesn't > fix the case when someone sends a standalone '\n' as line-ending. > Unsure I understand that, can you elaborate ? > I'd prefer if we go with reverting (=my previous diff) until we have > something that definitively fixes things. If people send broken mails > they get broken signatures until that time. Fine by me
Re: src/sys/netinet/ip_ether.c is empty, and also not used by gif
On Fri, Oct 04 2019, David Gwynne wrote: > so we can remove it, starting with taking it out of sys/conf/files. > > ok? I once had a similar diff. ok for this and the ip_ether.c removal. > Index: files > === > RCS file: /cvs/src/sys/conf/files,v > retrieving revision 1.674 > diff -u -p -r1.674 files > --- files 29 Sep 2019 13:04:03 - 1.674 > +++ files 4 Oct 2019 05:15:49 - > @@ -866,7 +870,6 @@ file netinet/ip_gre.c > file netinet/ip_ipsp.c ipsec | tcp_signature > file netinet/ip_spd.cipsec | tcp_signature > file netinet/ip_ipip.c > -file netinet/ip_ether.c gif > file netinet/ipsec_input.c ipsec > file netinet/ipsec_output.c ipsec > file netinet/ip_esp.cipsec > -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: smtpd handling of \r in DATA part
On 10/4/19 9:36 AM, Gilles Chehade wrote:
> On Fri, Oct 04, 2019 at 08:43:28AM +0200, Martijn van Duren wrote:
annoying bumps will just be moved around. If we *really* want to fix
this we need to make it fit within the specifications:
[...]
This means stop opportunistic scanning for '\r' in iobuf!
>>>
>>> Sure but fixing iobuf is not a two liner and it affects virtually all of
>>> the daemon and at this point we're looking for stability in the code, so
>>> unless eric@ or you can come up with a diff that's trivial and that will
>>> not affect any code paths beyond smtp client and filter getlines(), I'll
>>> prefer a degraded margin case than taking the risk of a regression.
>>>
>>> I don't know iobuf enough to dive into this in a rush but if you managed
>>> to pull a iobuf_getline_lf() that's fully isolated from rest of iobuf so
>>> we could use that one in smtp_session.c and lka_proc.c, I'd be fine too,
>>> and it would solve your concern.
>>>
>>>
>> This whole ordeal was originally triggered by filter-dkimsign returning
>> invalid signatures because additional \r were stripped. This is an
>> uncommon usecase.
>> The "fix" at the time was disallowing random '\r', which seemed to work
>> for some time, but now it turns out that it breaks semarie's printer.
>>
>> If we don't want to rush anything and keep as many systems running as
>> possible in a similar way we did before I propose we just revert the
>> '\r' forbidden diff and accept that signatures are broken for clients
>> that don't send valid mails until we fix this in a proper way.
>>
>> Doing it this way will keep things best effort running with minimal
>> code that could cause confusion once we start fixing this.
>>
>> Index: smtp_session.c
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
>> retrieving revision 1.414
>> diff -u -p -r1.414 smtp_session.c
>> --- smtp_session.c 3 Oct 2019 05:08:21 - 1.414
>> +++ smtp_session.c 4 Oct 2019 06:41:58 -
>> @@ -1109,15 +1109,6 @@ smtp_io(struct io *io, int evt, void *ar
>> if (line == NULL)
>> return;
>>
>> -if (strchr(line, '\r')) {
>> -s->flags |= SF_BADINPUT;
>> -smtp_reply(s, "500 %s is only allowed before ",
>> -esc_code(ESC_STATUS_PERMFAIL, ESC_OTHER_STATUS));
>> -smtp_enter_state(s, STATE_QUIT);
>> -io_set_write(io);
>> -return;
>> -}
>> -
>> /* Message body */
>> eom = 0;
>> if (s->state == STATE_BODY) {
>
>
> I'm fine with that.
>
> Alternatively, I took a look at what it would take to write an
> iobuf_getline_nostrip() function which would be called _only_ in the
> filtering code path.
>
> The diff is not too invasive:
>
> - introduce a iobuf_getline_nostrip() function so we don't touch the
> iobuf_getline() function (it's used in the MTA layer too and we're
> not getting anywhere close that), we can factor post release.
>
> - the client input is read with iobuf_getline() which strips \r, but
> the indirection through filters use iobuf_getline_nostrip(), which
> will let the '\r', effectively stopping opportunistic strip as you
> suggest.
>
> Setup that don't have filters will go _exactly_ through old behavior
> whereas setups with filters will have all \r preserved, keeping DKIM
> ok.
>
> I'm running with that diff right now.
>
> I'm ok with either one of committing a degraded DKIM for trailing-\r
> or going towards that diff.
>
> comments ?
>
This is similar to the diff I send a few months ago, but still doesn't
fix the case when someone sends a standalone '\n' as line-ending.
I'd prefer if we go with reverting (=my previous diff) until we have
something that definitively fixes things. If people send broken mails
they get broken signatures until that time.
Re: smtpd handling of \r in DATA part
On Fri, Oct 04, 2019 at 08:43:28AM +0200, Martijn van Duren wrote:
> >> annoying bumps will just be moved around. If we *really* want to fix
> >> this we need to make it fit within the specifications:
> >>
> >> [...]
> >>
> >> This means stop opportunistic scanning for '\r' in iobuf!
> >>
> >
> > Sure but fixing iobuf is not a two liner and it affects virtually all of
> > the daemon and at this point we're looking for stability in the code, so
> > unless eric@ or you can come up with a diff that's trivial and that will
> > not affect any code paths beyond smtp client and filter getlines(), I'll
> > prefer a degraded margin case than taking the risk of a regression.
> >
> > I don't know iobuf enough to dive into this in a rush but if you managed
> > to pull a iobuf_getline_lf() that's fully isolated from rest of iobuf so
> > we could use that one in smtp_session.c and lka_proc.c, I'd be fine too,
> > and it would solve your concern.
> >
> >
> This whole ordeal was originally triggered by filter-dkimsign returning
> invalid signatures because additional \r were stripped. This is an
> uncommon usecase.
> The "fix" at the time was disallowing random '\r', which seemed to work
> for some time, but now it turns out that it breaks semarie's printer.
>
> If we don't want to rush anything and keep as many systems running as
> possible in a similar way we did before I propose we just revert the
> '\r' forbidden diff and accept that signatures are broken for clients
> that don't send valid mails until we fix this in a proper way.
>
> Doing it this way will keep things best effort running with minimal
> code that could cause confusion once we start fixing this.
>
> Index: smtp_session.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> retrieving revision 1.414
> diff -u -p -r1.414 smtp_session.c
> --- smtp_session.c3 Oct 2019 05:08:21 - 1.414
> +++ smtp_session.c4 Oct 2019 06:41:58 -
> @@ -1109,15 +1109,6 @@ smtp_io(struct io *io, int evt, void *ar
> if (line == NULL)
> return;
>
> - if (strchr(line, '\r')) {
> - s->flags |= SF_BADINPUT;
> - smtp_reply(s, "500 %s is only allowed before ",
> - esc_code(ESC_STATUS_PERMFAIL, ESC_OTHER_STATUS));
> - smtp_enter_state(s, STATE_QUIT);
> - io_set_write(io);
> - return;
> - }
> -
> /* Message body */
> eom = 0;
> if (s->state == STATE_BODY) {
I'm fine with that.
Alternatively, I took a look at what it would take to write an
iobuf_getline_nostrip() function which would be called _only_ in the
filtering code path.
The diff is not too invasive:
- introduce a iobuf_getline_nostrip() function so we don't touch the
iobuf_getline() function (it's used in the MTA layer too and we're
not getting anywhere close that), we can factor post release.
- the client input is read with iobuf_getline() which strips \r, but
the indirection through filters use iobuf_getline_nostrip(), which
will let the '\r', effectively stopping opportunistic strip as you
suggest.
Setup that don't have filters will go _exactly_ through old behavior
whereas setups with filters will have all \r preserved, keeping DKIM
ok.
I'm running with that diff right now.
I'm ok with either one of committing a degraded DKIM for trailing-\r
or going towards that diff.
comments ?
diff --git a/smtpd/iobuf.c b/smtpd/iobuf.c
index f5d8b20a..78a83cc6 100644
--- a/smtpd/iobuf.c
+++ b/smtpd/iobuf.c
@@ -184,6 +184,34 @@ iobuf_getline(struct iobuf *iobuf, size_t *rlen)
return (NULL);
}
+char *
+iobuf_getline_nostrip(struct iobuf *iobuf, size_t *rlen)
+{
+ char*buf;
+ size_t len, i;
+
+ buf = iobuf_data(iobuf);
+ len = iobuf_len(iobuf);
+
+ for (i = 0; i + 1 <= len; i++)
+ if (buf[i] == '\n') {
+ /* Note: the returned address points into the iobuf
+* buffer. We NUL-end it for convenience, and discard
+* the data from the iobuf, so that the caller doesn't
+* have to do it. The data remains "valid" as long
+* as the iobuf does not overwrite it, that is until
+* the next call to iobuf_normalize() or iobuf_extend().
+*/
+ iobuf_drop(iobuf, i + 1);
+ buf[i] = '\0';
+ if (rlen)
+ *rlen = i;
+ return (buf);
+ }
+
+ return (NULL);
+}
+
void
iobuf_normalize(struct iobuf *io)
{
diff --git a/smtpd/iobuf.h b/smtpd/iobuf.h
index c454d0a1..e6e8023c 100644
--- a/smtpd/iobuf.h
+++ b/smtpd/iobuf.h
@@ -52,6 +52,7 @@ size_t
Re: smtpd handling of \r in DATA part
On 10/4/19 8:23 AM, Gilles Chehade wrote:
> On Fri, Oct 04, 2019 at 07:35:39AM +0200, Martijn van Duren wrote:
>> On 10/3/19 9:05 PM, Eric Faurot wrote:
>>> On Thu, Sep 19, 2019 at 05:48:17PM +, gil...@poolp.org wrote:
>>>
> To me, the only real problem with '\r' is at the end of lines. It's
> confusing
> since you never really know whether it's part of the content or the
> protocol.
>
> So I suggest that we strip all '\r' found at the end of a line,
> and retain the others.
>
I'm not sure the only problem is at the end of lines, but I don't think
any solution that's graceful to bad clients will be correct so I'm okay
with your suggestion.
>>>
>>> Here is a diff for that.
>>> Note that it strips the '\r' on all input, not just DATA.
>>>
>>> Eric.
>>>
>>> Index: smtp_session.c
>>> ===
>>> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
>>> retrieving revision 1.414
>>> diff -u -p -r1.414 smtp_session.c
>>> --- smtp_session.c 3 Oct 2019 05:08:21 - 1.414
>>> +++ smtp_session.c 3 Oct 2019 18:55:52 -
>>> @@ -1109,14 +1109,9 @@ smtp_io(struct io *io, int evt, void *ar
>>> if (line == NULL)
>>> return;
>>>
>>> - if (strchr(line, '\r')) {
>>> - s->flags |= SF_BADINPUT;
>>> - smtp_reply(s, "500 %s is only allowed before ",
>>> - esc_code(ESC_STATUS_PERMFAIL, ESC_OTHER_STATUS));
>>> - smtp_enter_state(s, STATE_QUIT);
>>> - io_set_write(io);
>>> - return;
>>> - }
>>> + /* Strip trailing '\r' */
>>> + while (len && line[len - 1] == '\r')
>>> + line[--len] = '\0';
>>>
>>> /* Message body */
>>> eom = 0;
>>>
>> See my previous mail.
>> This will break existing dkim signatures on mails which have trailing
>> '\r'. Even though clients must not send those, they apparently do.
>>
>
> I agree with you for the most part but let's also put perspective here.
> This week, on my machines, out of 15k incoming and 14k outgoing e-mails
> only 2 mails had '\r' in the incoming path, 0 on the outgoing path, the
> 2 mails were spam.
>
> Not saying the '\r' issue doesn't need to be adressed but just pointing
> that it is a margin case and that the "quick fix" we want at this point
> in the release cycle must not take risks of regressions for everyone to
> cope with that margin case.
>
>
>> Do we
>> want to invalidate them as well? If that's the case, just remove this
>> check, because if the old signatures don't matter than the new signature
>> from filter-dkimsign (and others) won't matter as well.
>>
>
> I agree, both solutions break DKIM for trailing '\r' e-mails, but if we
> go that path I prefer that we handle this upfront rather than on filter
> returns.
>
>
>> Seriously, these solutions are like trying to fit a too large carpet,
>> annoying bumps will just be moved around. If we *really* want to fix
>> this we need to make it fit within the specifications:
>>
>> [...]
>>
>> This means stop opportunistic scanning for '\r' in iobuf!
>>
>
> Sure but fixing iobuf is not a two liner and it affects virtually all of
> the daemon and at this point we're looking for stability in the code, so
> unless eric@ or you can come up with a diff that's trivial and that will
> not affect any code paths beyond smtp client and filter getlines(), I'll
> prefer a degraded margin case than taking the risk of a regression.
>
> I don't know iobuf enough to dive into this in a rush but if you managed
> to pull a iobuf_getline_lf() that's fully isolated from rest of iobuf so
> we could use that one in smtp_session.c and lka_proc.c, I'd be fine too,
> and it would solve your concern.
>
>
This whole ordeal was originally triggered by filter-dkimsign returning
invalid signatures because additional \r were stripped. This is an
uncommon usecase.
The "fix" at the time was disallowing random '\r', which seemed to work
for some time, but now it turns out that it breaks semarie's printer.
If we don't want to rush anything and keep as many systems running as
possible in a similar way we did before I propose we just revert the
'\r' forbidden diff and accept that signatures are broken for clients
that don't send valid mails until we fix this in a proper way.
Doing it this way will keep things best effort running with minimal
code that could cause confusion once we start fixing this.
Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.414
diff -u -p -r1.414 smtp_session.c
--- smtp_session.c 3 Oct 2019 05:08:21 - 1.414
+++ smtp_session.c 4 Oct 2019 06:41:58 -
@@ -1109,15 +1109,6 @@ smtp_io(struct io *io, int evt, void *ar
if (line == NULL)
Re: smtpd handling of \r in DATA part
On Fri, Oct 04, 2019 at 07:35:39AM +0200, Martijn van Duren wrote:
> On 10/3/19 9:05 PM, Eric Faurot wrote:
> > On Thu, Sep 19, 2019 at 05:48:17PM +, gil...@poolp.org wrote:
> >
> >>> To me, the only real problem with '\r' is at the end of lines. It's
> >>> confusing
> >>> since you never really know whether it's part of the content or the
> >>> protocol.
> >>>
> >>> So I suggest that we strip all '\r' found at the end of a line,
> >>> and retain the others.
> >>>
> >>
> >> I'm not sure the only problem is at the end of lines, but I don't think
> >> any solution that's graceful to bad clients will be correct so I'm okay
> >> with your suggestion.
> >>
> >
> > Here is a diff for that.
> > Note that it strips the '\r' on all input, not just DATA.
> >
> > Eric.
> >
> > Index: smtp_session.c
> > ===
> > RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> > retrieving revision 1.414
> > diff -u -p -r1.414 smtp_session.c
> > --- smtp_session.c 3 Oct 2019 05:08:21 - 1.414
> > +++ smtp_session.c 3 Oct 2019 18:55:52 -
> > @@ -1109,14 +1109,9 @@ smtp_io(struct io *io, int evt, void *ar
> > if (line == NULL)
> > return;
> >
> > - if (strchr(line, '\r')) {
> > - s->flags |= SF_BADINPUT;
> > - smtp_reply(s, "500 %s is only allowed before ",
> > - esc_code(ESC_STATUS_PERMFAIL, ESC_OTHER_STATUS));
> > - smtp_enter_state(s, STATE_QUIT);
> > - io_set_write(io);
> > - return;
> > - }
> > + /* Strip trailing '\r' */
> > + while (len && line[len - 1] == '\r')
> > + line[--len] = '\0';
> >
> > /* Message body */
> > eom = 0;
> >
> See my previous mail.
> This will break existing dkim signatures on mails which have trailing
> '\r'. Even though clients must not send those, they apparently do.
>
I agree with you for the most part but let's also put perspective here.
This week, on my machines, out of 15k incoming and 14k outgoing e-mails
only 2 mails had '\r' in the incoming path, 0 on the outgoing path, the
2 mails were spam.
Not saying the '\r' issue doesn't need to be adressed but just pointing
that it is a margin case and that the "quick fix" we want at this point
in the release cycle must not take risks of regressions for everyone to
cope with that margin case.
> Do we
> want to invalidate them as well? If that's the case, just remove this
> check, because if the old signatures don't matter than the new signature
> from filter-dkimsign (and others) won't matter as well.
>
I agree, both solutions break DKIM for trailing '\r' e-mails, but if we
go that path I prefer that we handle this upfront rather than on filter
returns.
> Seriously, these solutions are like trying to fit a too large carpet,
> annoying bumps will just be moved around. If we *really* want to fix
> this we need to make it fit within the specifications:
>
> [...]
>
> This means stop opportunistic scanning for '\r' in iobuf!
>
Sure but fixing iobuf is not a two liner and it affects virtually all of
the daemon and at this point we're looking for stability in the code, so
unless eric@ or you can come up with a diff that's trivial and that will
not affect any code paths beyond smtp client and filter getlines(), I'll
prefer a degraded margin case than taking the risk of a regression.
I don't know iobuf enough to dive into this in a rush but if you managed
to pull a iobuf_getline_lf() that's fully isolated from rest of iobuf so
we could use that one in smtp_session.c and lka_proc.c, I'd be fine too,
and it would solve your concern.
--
Gilles Chehade @poolpOrg
https://www.poolp.orgpatreon: https://www.patreon.com/gilles
