unveil spamlogd

2018-10-24 Thread Ricardo Mestre
Hi,

The only file that spamlogd needs to access after calling pledge is
PATH_SPAMD_DB, so unveil it with O_RDWR permissions.

OK?

Index: spamlogd.c
===
RCS file: /cvs/src/libexec/spamlogd/spamlogd.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 spamlogd.c
--- spamlogd.c  16 Mar 2016 14:47:04 -  1.27
+++ spamlogd.c  24 Oct 2018 07:00:09 -
@@ -375,6 +375,8 @@ main(int argc, char **argv)
openlog_r("spamlogd", LOG_PID | LOG_NDELAY, LOG_DAEMON, &sdata);
}
 
+   if (unveil(PATH_SPAMD_DB, "rw") == -1)
+   err(1, "unveil");
if (syncsend) {
if (pledge("stdio rpath wpath inet flock", NULL) == -1)
err(1, "pledge");



Re: Add acpipci(4) on amd64

2018-10-24 Thread Paul de Weerd
Hi Mark,

On Mon, Oct 22, 2018 at 09:45:06PM +0200, Mark Kettenis wrote:
| I'd like to see this tested on a wide range of amd64 hardware, but
| especially on laptops.  Please reply with a diff of your dmesg before
| and after.  Make sure you run make config before building a new kernel.

Thanks for your work!  Not much difference on my GPD Win (pocket)
computer.  Diff and full dmesg below.

Note that there's a kernel page fault when running bwfm attach hooks;
this happens frequently, I avoid it by booting a kernel with bwfm
disabled.

Cheers,

Paul

--- /home/weerd/taco.20181019.dmesg Wed Oct 24 08:52:07 2018
+++ /home/weerd/taco.20181024.dmesg Wed Oct 24 08:52:07 2018
@@ -1,7 +1,7 @@
-OpenBSD 6.4-current (GENERIC.MP) #370: Fri Oct 19 13:09:31 MDT 2018
-dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
+OpenBSD 6.4-current (GENERIC.MP) #7: Wed Oct 24 07:42:29 CEST 2018
+we...@pom.alm.weirdnet.nl:/usr/src/sys/arch/amd64/compile/GENERIC.MP
 real mem = 4182315008 (3988MB)
-avail mem = 4046282752 (3858MB)
+avail mem = 4046286848 (3858MB)
 mpath0 at root
 scsibus0 at mpath0: 256 targets
 mainbus0 at root
@@ -15,7 +15,7 @@
 acpitimer0 at acpi0: 3579545 Hz, 24 bits
 acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
 cpu0 at mainbus0: apid 0 (boot processor)
-cpu0: Intel(R) Atom(TM) x7-Z8700 CPU @ 1.60GHz, 1600.26 MHz, 06-4c-03
+cpu0: Intel(R) Atom(TM) x7-Z8700 CPU @ 1.60GHz, 1600.25 MHz, 06-4c-03
 cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,RDRAND,NXE,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,SMEP,ERMS,SENSOR,ARAT,MELTDOWN
 cpu0: 1MB 64b/line 16-way L2 cache
 cpu0: smt 0, core 0, package 0
@@ -28,12 +28,12 @@
 cpu1: 1MB 64b/line 16-way L2 cache
 cpu1: smt 0, core 1, package 0
 cpu2 at mainbus0: apid 4 (application processor)
-cpu2: Intel(R) Atom(TM) x7-Z8700 CPU @ 1.60GHz, 1599.95 MHz, 06-4c-03
+cpu2: Intel(R) Atom(TM) x7-Z8700 CPU @ 1.60GHz, 1599.96 MHz, 06-4c-03
 cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,RDRAND,NXE,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,SMEP,ERMS,SENSOR,ARAT,MELTDOWN
 cpu2: 1MB 64b/line 16-way L2 cache
 cpu2: smt 0, core 2, package 0
 cpu3 at mainbus0: apid 6 (application processor)
-cpu3: Intel(R) Atom(TM) x7-Z8700 CPU @ 1.60GHz, 1599.95 MHz, 06-4c-03
+cpu3: Intel(R) Atom(TM) x7-Z8700 CPU @ 1.60GHz, 1599.96 MHz, 06-4c-03
 cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,RDRAND,NXE,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,SMEP,ERMS,SENSOR,ARAT,MELTDOWN
 cpu3: 1MB 64b/line 16-way L2 cache
 cpu3: smt 0, core 3, package 0
@@ -96,6 +96,7 @@
 "INT33FE" at iic0 addr 0x54 not configured
 chvgpio2 at acpi0: GPO0 uid 1 addr 0xfed8/0x8000 irq 49, 56 pins
 chvgpio3 at acpi0: GPO2 uid 3 addr 0xfed9/0x8000 irq 50, 24 pins
+acpipci0 at acpi0 PCI0: 0x 0x0011 0x0001
 sdhc0 at acpi0 SDHA addr 0xa1a3a000/0x1000 irq 45
 sdhc0: SDHC 3.0, 200 MHz base clock
 sdmmc0 at sdhc0: 8-bit, sd high-speed, mmc high-speed, dma
@@ -197,15 +198,15 @@
 vmm0 at mainbus0: VMX/EPT (using slow L1TF mitigation)
 efifb at mainbus0 not configured
 sdmmc1: can't enable card
-scsibus1 at sdmmc0: 2 targets, initiator 0
-sd0 at scsibus1 targ 1 lun 0:  SCSI2 0/direct removable
-sd0: 59640MB, 512 bytes/sector, 122142720 sectors
+scsibus1 at sdmmc2: 2 targets, initiator 0
+sd0 at scsibus1 targ 1 lun 0:  SCSI2 0/direct removable
+sd0: 61071MB, 512 bytes/sector, 125073408 sectors
+scsibus2 at sdmmc0: 2 targets, initiator 0
+sd1 at scsibus2 targ 1 lun 0:  SCSI2 0/direct removable
+sd1: 59640MB, 512 bytes/sector, 122142720 sectors
 uhidev0 at uhub0 port 2 configuration 1 interface 0 "HK-XBOXKB-US-A0-10-00 USB 
KEYBOARD" rev 1.10/1.00 addr 2
 uhidev0: iclass 3/1
 ukbd0 at uhidev0: 8 variable keys, 6 key codes
-scsibus2 at sdmmc2: 2 targets, initiator 0
-sd1 at scsibus2 targ 1 lun 0:  SCSI2 0/direct removable
-sd1: 61071MB, 512 bytes/sector, 125073408 sectors
 wskbd0 at ukbd0: console keyboard, using wsdisplay0
 uhidev1 at uhub0 port 2 configuration 1 interface 1 "HK-XBOXKB-US-A0-10-00 USB 
KEYBOARD" rev 1.10/1.00 addr 2
 uhidev1: iclass 3/0, 9 report ids
@@ -224,14 +225,17 @@
 ukbd1 at uhidev3: 8 variable keys, 6 key codes
 wskbd1 at ukbd1 mux 1
 wskbd1: connecting to wsdisplay0
-ure0 at uhub0 port 9 configuration 1 interface 0 "Realtek USB 10/100/1000 LAN" 
rev 3.00/30.00 addr 4
-ure0: ver 5c20, address 00:e0:97:00:65:53
-rgephy0 at ure0 phy 0: RTL8251 PHY, rev. 0
 vscsi0 at root
 scsibus3 at vscsi0: 256

Re: Qcow2: Clean up logging/error handling

2018-10-24 Thread Michael Mikonos
On Tue, Oct 23, 2018 at 09:44:24PM -0700, Ori Bernstein wrote:
> This patch turns most warnings into errors, and uses the
> appropriate fatal/fatalx so that we don't print bogus error
> strings. It also adds checks for unsupported refcount sizes
> and writes that clobber the header.
> 
> Ok?

Hello, two comments below.

> 
> diff --git usr.sbin/vmd/vioqcow2.c usr.sbin/vmd/vioqcow2.c
> index 3a215599d49..e550f3b84b5 100644
> --- usr.sbin/vmd/vioqcow2.c
> +++ usr.sbin/vmd/vioqcow2.c
> @@ -137,6 +137,12 @@ virtio_init_qcow2(struct virtio_backing *file, off_t 
> *szp, int *fd, size_t nfd)
>   return 0;
>  }
>  
> +/*
> + * Return the path to the base image given a disk image.
> + *
> + * This is used when resolving base images from vmd, so it should avoid
> + * fatalx'ing, or we will bring down multiple vms on a corrupt disk.
> + */
>  ssize_t
>  virtio_qcow2_get_base(int fd, char *path, size_t npath, const char *dpath)
>  {
> @@ -151,7 +157,7 @@ virtio_qcow2_get_base(int fd, char *path, size_t npath, 
> const char *dpath)
>   return -1;
>   }
>   if (strncmp(header.magic, VM_MAGIC_QCOW, strlen(VM_MAGIC_QCOW)) != 0) {
> - log_warn("%s: invalid magic numbers", __func__);
> + log_warnx("%s: invalid magic numbers", __func__);
>   return -1;
>   }
>   backingoff = be64toh(header.backingoff);
> @@ -160,7 +166,7 @@ virtio_qcow2_get_base(int fd, char *path, size_t npath, 
> const char *dpath)
>   return 0;
>  
>   if (backingsz >= npath - 1) {
> - log_warn("%s: snapshot path too long", __func__);
> + log_warnx("%s: snapshot path too long", __func__);
>   return -1;
>   }
>   if (pread(fd, path, backingsz, backingoff) != backingsz) {
> @@ -178,20 +184,19 @@ virtio_qcow2_get_base(int fd, char *path, size_t npath, 
> const char *dpath)
>   if (path[0] == '/') {
>   if (realpath(path, expanded) == NULL ||
>   strlcpy(path, expanded, npath) >= npath) {
> - log_warn("unable to resolve %s", path);
> + log_warnx("unable to resolve %s", path);
>   return -1;
>   }
>   } else {
>   s = dirname(dpath);
>   if (snprintf(expanded, sizeof(expanded),
>   "%s/%s", s, path) >= (int)sizeof(expanded)) {
> - log_warn("path too long: %s/%s",
> - s, path);
> + log_warnx("path too long: %s/%s", s, path);
>   return -1;
>   }
>   if (npath < PATH_MAX ||
>   realpath(expanded, path) == NULL) {
> - log_warn("unable to resolve %s", path);
> + log_warnx("unable to resolve %s", path);
>   return -1;
>   }
>   }
> @@ -216,15 +221,10 @@ qc2_open(struct qcdisk *disk, int *fds, size_t nfd)
>   disk->base = NULL;
>   disk->l1 = NULL;
>  
> - if (pread(fd, &header, sizeof(header), 0) != sizeof(header)) {
> - log_warn("%s: short read on header", __func__);
> - goto error;
> - }
> - if (strncmp(header.magic,
> - VM_MAGIC_QCOW, strlen(VM_MAGIC_QCOW)) != 0) {
> - log_warn("%s: invalid magic numbers", __func__);
> - goto error;
> - }
> + if (pread(fd, &header, sizeof(header), 0) != sizeof(header))
> + fatalx("%s: short read on header", __func__);
> + if (strncmp(header.magic, VM_MAGIC_QCOW, strlen(VM_MAGIC_QCOW)) != 0)
> + fatalx("%s: invalid magic numbers", __func__);
>  
>   disk->clustersz = (1ull << be32toh(header.clustershift));
>   disk->disksz= be64toh(header.disksz);
> @@ -249,79 +249,59 @@ qc2_open(struct qcdisk *disk, int *fds, size_t nfd)
>   /*
>* We only know about the dirty or corrupt bits here.
>*/
> - if (disk->incompatfeatures & ~(QCOW2_DIRTY|QCOW2_CORRUPT)) {
> - log_warnx("%s: unsupported features %llx", __func__,
> + if (disk->incompatfeatures & ~(QCOW2_DIRTY|QCOW2_CORRUPT))
> + fatalx("%s: unsupported features %llx", __func__,
>   disk->incompatfeatures & ~(QCOW2_DIRTY|QCOW2_CORRUPT));
> - goto error;
> - }
> + if (be32toh(header.reforder) != 4)
> + fatalx("%s: unsupported refcount size\n", __func__);
>  
>   disk->l1 = calloc(disk->l1sz, sizeof(*disk->l1));
>   if (!disk->l1)
> - goto error;
> + fatalx("%s: could not allocate l1 table", __func__);
>   if (pread(disk->fd, disk->l1, 8 * disk->l1sz, disk->l1off)
> - != 8 * disk->l1sz) {
> - log_warn("%s: unable to read qcow2 L1 table", __func__);
> - goto error;
> - }
> + != 8 * disk->l1sz)
> + fatalx("%s: unable to read qcow2 L1 table", __func__);
>   for (i = 0; i < disk->l1sz; i++)
> 

ldap(1) add SAFE-INIT-CHAR

2018-10-24 Thread Martijn van Duren
In my previous ldap mail I proclaimed that we should encode whitespace. 
Reading rfc2849 a bit further, encoding a string with leading space is  
mandatory by SAFE-INIT-CHAR. This is needed because of the definition
of value-spec, which allows additional space, colon, and less-than
after the colon separating the AttributeDescription.

The code below adds these definitions. I also changed the outlen
calculation because it at least fails b64_ntop on inlen == 1.

OK?

martijn@

Index: ldapclient.c
===
RCS file: /cvs/src/usr.bin/ldap/ldapclient.c,v
retrieving revision 1.5
diff -u -p -r1.5 ldapclient.c
--- ldapclient.c23 Oct 2018 08:28:34 -  1.5
+++ ldapclient.c24 Oct 2018 08:21:27 -
@@ -404,8 +404,13 @@ ldapc_printattr(struct ldapc *ldap, cons
 * in SAFE-STRINGs. String value that do not match the
 * criteria must be encoded as Base64.
 */
-   for (cp = (const unsigned char *)value;
-   encode == 0 &&*cp != '\0'; cp++) {
+   cp = (const unsigned char *)value;
+   /* !SAFE-INIT-CHAR: SAFE-CHAR minus %x20 %x3A %x3C */
+   if (*cp == ' ' ||
+   *cp == ':' ||
+   *cp == '<')
+   encode = 1;
+   for (; encode == 0 &&*cp != '\0'; cp++) {
/* !SAFE-CHAR %x01-09 / %x0B-0C / %x0E-7F */
if (*cp > 127 ||
*cp == '\0' ||
@@ -421,7 +426,7 @@ ldapc_printattr(struct ldapc *ldap, cons
}
} else {
inlen = strlen(value);
-   outlen = inlen * 2 + 1;
+   outlen = (((inlen + 2) / 3) * 4) + 1;
 
if ((out = calloc(1, outlen)) == NULL ||
b64_ntop(value, inlen, out, outlen) == -1) {



Re: bgplg: fix show ip bgp out/in

2018-10-24 Thread Sebastian Benoit
ok benno@

Denis Fondras(de...@openbsd.org) on 2018.10.24 08:26:59 +0200:
> This may have been broken for quite some time...
> 
> Fix usage message for "show ip bgp in/out" and add missing "neighbor" 
> argument.
> 
> Index: bgplg.h
> ===
> RCS file: /cvs/src/usr.bin/bgplg/bgplg.h,v
> retrieving revision 1.14
> diff -u -p -r1.14 bgplg.h
> --- bgplg.h   2 Feb 2018 13:46:17 -   1.14
> +++ bgplg.h   24 Oct 2018 06:20:37 -
> @@ -59,10 +59,10 @@ struct cmd {
>   { BGPCTL, "show","ip", "bgp", "detail", NULL } },   \
>   { "show ip bgp detail as", 1, 1, "",   \
>   { BGPCTL, "show","ip", "bgp", "detail", "as", NULL } }, \
> - { "show ip bgp in", 1, 1, "", \
> - { BGPCTL, "show","ip", "bgp", "in", NULL } },   \
> - { "show ip bgp out", 1, 1, "",\
> - { BGPCTL, "show","ip", "bgp", "out", NULL } },  \
> + { "show ip bgp in", 1, 1, "",   \
> + { BGPCTL, "show","ip", "bgp", "in", "neighbor", NULL } },   \
> + { "show ip bgp out", 1, 1, "",  \
> + { BGPCTL, "show","ip", "bgp", "out", "neighbor", NULL } },  \
>   { "show ip bgp memory", 0, 0, NULL, \
>   { BGPCTL, "show", "ip", "bgp", "memory", NULL } },  \
>   { "show neighbor", 0, 1, NULL,  \
> 



unveil passwd

2018-10-24 Thread Ricardo Mestre
Hi,

The diff below unveils passwd with exactly the same ones used on vipw, the only
difference is that in this case _PATH_BSHELL is used to spawn an external
passwordcheck program (if defined in /etc/login.conf) instead of an EDITOR.

Tested by changing my users' passwords back and forth several times, but if you
want to test this please backup your /etc/master.passwd first otherwise it may
eat your kittens!

OK?

Index: local_passwd.c
===
RCS file: /cvs/src/usr.bin/passwd/local_passwd.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 local_passwd.c
--- local_passwd.c  30 Dec 2016 23:32:14 -  1.53
+++ local_passwd.c  24 Oct 2018 09:18:44 -
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -71,6 +72,14 @@ local_passwd(char *uname, int authentica
return(1);
}
 
+   if (unveil(_PATH_MASTERPASSWD_LOCK, "wc") == -1)
+   err(1, "unveil");
+   if (unveil(_PATH_MASTERPASSWD, "r") == -1)
+   err(1, "unveil");
+   if (unveil(_PATH_BSHELL, "x") == -1)
+   err(1, "unveil");
+   if (unveil(_PATH_PWD_MKDB, "x") == -1)
+   err(1, "unveil");
if (pledge("stdio rpath wpath cpath getpw tty id proc exec", NULL) == 
-1)
err(1, "pledge");
 



join(1) add UTF-8 support

2018-10-24 Thread Martijn van Duren
This adds UTF-8 support for join(1). Since we don't support collation we
can skip that part of POSIX. This patch does add support for splitting  
columns on UTF-8 characters.

Using schwarze@'s favorite UTF-8 character:
$ cat /tmp/z1 
aßbßc
$ cat /tmp/z2 
aßdße
$ ./join -tß /tmp/z1 /tmp/z2
aßbßcßdße

All regression tests pass, and lightly tested.

OK?

martijn@

Index: join.c
===
RCS file: /cvs/src/usr.bin/join/join.c,v
retrieving revision 1.30
diff -u -p -r1.30 join.c
--- join.c  23 Oct 2018 08:41:45 -  1.30
+++ join.c  24 Oct 2018 09:32:54 -
@@ -34,10 +34,14 @@
  */
 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
 
@@ -81,11 +85,12 @@ int joinout = 1;/* show lines with mat
 int needsep;   /* need separator character */
 int spans = 1; /* span multiple delimiters (-t) */
 char *empty;   /* empty field replacement string (-e) */
-char *tabchar = " \t"; /* delimiter characters (-t) */
+wchar_t tabchar[] = L" \t";/* delimiter characters (-t) */
 
 int  cmp(LINE *, u_long, LINE *, u_long);
 void fieldarg(char *);
 void joinlines(INPUT *, INPUT *);
+char *mbssep(char **, const wchar_t *);
 void obsolete(char **);
 void outfield(LINE *, u_long, int);
 void outoneline(INPUT *, LINE *);
@@ -101,6 +106,8 @@ main(int argc, char *argv[])
int aflag, ch, cval, vflag;
char *end;
 
+   setlocale(LC_CTYPE, "");
+
if (pledge("stdio rpath", NULL) == -1)
err(1, "pledge");
 
@@ -161,8 +168,10 @@ main(int argc, char *argv[])
break;
case 't':
spans = 0;
-   if (strlen(tabchar = optarg) != 1)
+   if (mbtowc(tabchar, optarg, MB_CUR_MAX) !=
+   strlen(optarg))
errx(1, "illegal tab character specification");
+   tabchar[1] = L'\0';
break;
case 'v':
vflag = 1;
@@ -333,7 +342,7 @@ slurp(INPUT *F)
/* Split the line into fields, allocate space as necessary. */
lp->fieldcnt = 0;
bp = lp->line;
-   while ((fieldp = strsep(&bp, tabchar)) != NULL) {
+   while ((fieldp = mbssep(&bp, tabchar)) != NULL) {
if (spans && *fieldp == '\0')
continue;
if (lp->fieldcnt == lp->fieldalloc) {
@@ -358,6 +367,36 @@ slurp(INPUT *F)
free(line);
 }
 
+char *
+mbssep(char **stringp, const wchar_t *wcdelim)
+{
+   char *s, *p;
+   size_t ndelim;
+   int i;
+   /* tabchar is never more than 2 */
+   char mbdelim[2][MB_LEN_MAX + 1];
+   size_t mblen[2];
+
+   if ((s = *stringp) == NULL)
+   return NULL;
+   ndelim = wcslen(wcdelim);
+   for (i = 0; i < ndelim; i++) {
+   if ((mblen[i] = wctomb(mbdelim[i], wcdelim[i])) == -1)
+   errc(1, EILSEQ, "wctomb");
+   }
+   for (p = s; *p != '\0'; p++) {
+   for (i = 0; i < ndelim; i++) {
+   if (strncmp(p, mbdelim[i], mblen[i]) == 0) {
+   *p = '\0';
+   *stringp = p + mblen[i];
+   return s;
+   }
+   }
+   }
+   *stringp = NULL;
+   return s;
+}
+
 int
 cmp(LINE *lp1, u_long fieldno1, LINE *lp2, u_long fieldno2)
 {
@@ -463,7 +502,7 @@ void
 outfield(LINE *lp, u_long fieldno, int out_empty)
 {
if (needsep++)
-   putchar((int)*tabchar);
+   putwchar(*tabchar);
if (!ferror(stdout)) {
if (lp->fieldcnt <= fieldno || out_empty) {
if (empty != NULL)



Re: unveil xserver's priv proc

2018-10-24 Thread Ricardo Mestre
Hello,

semarie@ already gave positive feedback for unveiling xserver, did
anyone tested it yet and comment on it or OK?

Index: privsep.c
===
RCS file: /cvs/xenocara/xserver/os/privsep.c,v
retrieving revision 1.29
diff -u -p -u -r1.29 privsep.c
--- privsep.c   6 Aug 2018 20:11:34 -   1.29
+++ privsep.c   24 Oct 2018 09:35:01 -
@@ -274,6 +274,10 @@ priv_init(uid_t uid, gid_t gid)
setproctitle("[priv]");
close(socks[1]);
 
+   for (dev = allowed_devices; dev->name != NULL; dev++) {
+   if (unveil(dev->name, "rw") == -1)
+   err(1, "unveil");
+   }
if (pledge("stdio rpath wpath sendfd proc", NULL) == -1)
err(1, "pledge");
 



unveil bdftopcf

2018-10-24 Thread Ricardo Mestre
Hi,

If input_name is provided we can unveil it with read permissions, if
output_name is provided we need to unveil this one with rwc. Additionally
depending on the different combinations of if these files are passed via args
or from stdin/to stdout we can also pledge accordingly to the code path. This
has been tested succefully with bdf fonts we have bundled in xenocara.

Since I have several other X apps unveiled and/or pledged could you please
comment not only with the unveil/pledge part, but also err vs fprintf/exit, the
placement of the #includes and also tabs vs spaces?

Index: bdftopcf.c
===
RCS file: /cvs/xenocara/app/bdftopcf/bdftopcf.c,v
retrieving revision 1.5
diff -u -p -u -r1.5 bdftopcf.c
--- bdftopcf.c  29 Mar 2018 20:34:30 -  1.5
+++ bdftopcf.c  24 Oct 2018 10:15:41 -
@@ -38,7 +38,9 @@ from The Open Group.
 #include "fntfil.h"
 #include "bdfint.h"
 #include "pcf.h"
+#include 
 #include 
+#include 
 #include 
 
 int
@@ -158,6 +160,26 @@ main(int argc, char *argv[])
 }
 argv++;
 }
+
+   if (input_name) {
+   if (unveil(input_name, "r") == -1)
+   err(1, "unveil");
+   }
+   if (output_name) {
+   if (unveil(output_name, "rwc") == -1)
+   err(1, "unveil");
+   if (pledge("stdio rpath wpath cpath", NULL) == -1)
+   err(1, "pledge");
+   }
+   if (input_name && !output_name) {
+   if (pledge("stdio rpath", NULL) == -1)
+   err(1, "pledge");
+   }
+   if (!input_name && !output_name) {
+   if (pledge("stdio", NULL) == -1)
+   err(1, "pledge");
+   }
+
 if (input_name) {
 input = FontFileOpen(input_name);
 if (!input) {



bgplg: add missing commands

2018-10-24 Thread Denis Fondras
This diff adds 2 missing commands :
- show ip bgp ovs
- show ip bgp ext-community

Index: bgplg.h
===
RCS file: /cvs/src/usr.bin/bgplg/bgplg.h,v
retrieving revision 1.15
diff -u -p -r1.15 bgplg.h
--- bgplg.h 24 Oct 2018 09:02:48 -  1.15
+++ bgplg.h 24 Oct 2018 10:44:56 -
@@ -51,6 +51,10 @@ struct cmd {
{ BGPCTL, "show","ip", "bgp", "community", NULL } },\
{ "show ip bgp detail community", 1, 1, "",\
{ BGPCTL, "show","ip", "bgp", "detail", "community", NULL } },\
+   { "show ip bgp ext-community", 2, 2, "",   \
+   { BGPCTL, "show","ip", "bgp", "ext-community", NULL } },\
+   { "show ip bgp detail ext-community", 2, 2, "",\
+   { BGPCTL, "show","ip", "bgp", "detail", "ext-community", NULL } },\
{ "show ip bgp large-community", 1, 1, "",\
{ BGPCTL, "show","ip", "bgp", "large-community", NULL } },  \
{ "show ip bgp detail large-community", 1, 1, 
"",\
@@ -63,6 +67,8 @@ struct cmd {
{ BGPCTL, "show","ip", "bgp", "in", "neighbor", NULL } },   \
{ "show ip bgp out", 1, 1, "",  \
{ BGPCTL, "show","ip", "bgp", "out", "neighbor", NULL } },  \
+   { "show ip bgp ovs", 1, 1, "", \
+   { BGPCTL, "show","ip", "bgp", "ovs", NULL } },  \
{ "show ip bgp memory", 0, 0, NULL, \
{ BGPCTL, "show", "ip", "bgp", "memory", NULL } },  \
{ "show neighbor", 0, 1, NULL,  \



Re: Add acpipci(4) on amd64

2018-10-24 Thread Jeremie Courreges-Anglas
On Mon, Oct 22 2018, Mark Kettenis  wrote:
> Diff below adds an acpipci(4) driver on amd64.  For now the main
> purpose of this driver is to make the PCI-specific _OSC calls to
> advertise the functionality we support.  Most notably this advertises
> support for PCIE native hotplug as we have some indications that this
> will help Thunderbolt 3 support on some machines.
>
> I'd like to see this tested on a wide range of amd64 hardware, but
> especially on laptops.  Please reply with a diff of your dmesg before
> and after.  Make sure you run make config before building a new kernel.

KVM 2.8 vm, fwiw.
tl;dr "+acpipci0 at acpi0 PCI0: _OSC failed"

--- old Wed Oct 24 13:34:31 2018
+++ new Wed Oct 24 13:34:44 2018
@@ -1,4 +1,4 @@
-OpenBSD 6.4-current (GENERIC.MP) #0: Wed Oct 24 09:45:19 CEST 2018
+OpenBSD 6.4-current (GENERIC.MP) #1: Wed Oct 24 13:31:30 CEST 2018
 j...@russell.wxcvbn.org:/sys/arch/amd64/compile/GENERIC.MP
 real mem = 8573018112 (8175MB)
 avail mem = 8303935488 (7919MB)
@@ -15,16 +15,16 @@
 acpitimer0 at acpi0: 3579545 Hz, 24 bits
 acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
 cpu0 at mainbus0: apid 0 (boot processor)
-cpu0: Intel Core Processor (Broadwell), 3408.50 MHz, 06-3d-02
+cpu0: Intel Core Processor (Broadwell), 3408.56 MHz, 06-3d-02
 cpu0: 
FPU,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,PCLMUL,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,HV,NXE,RDTSCP,LONG,LAHF,3DNOWP,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,RDSEED,ADX,SMAP,XSAVEOPT,MELTDOWN
 cpu0: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 64b/line 
16-way L2 cache
 cpu0: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
 cpu0: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
 cpu0: smt 0, core 0, package 0
 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
-cpu0: apic clock running at 1000MHz
+cpu0: apic clock running at 999MHz
 cpu1 at mainbus0: apid 1 (application processor)
-cpu1: Intel Core Processor (Broadwell), 3408.05 MHz, 06-3d-02
+cpu1: Intel Core Processor (Broadwell), 3408.04 MHz, 06-3d-02
 cpu1: 
FPU,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,PCLMUL,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,HV,NXE,RDTSCP,LONG,LAHF,3DNOWP,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,RDSEED,ADX,SMAP,XSAVEOPT,MELTDOWN
 cpu1: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 64b/line 
16-way L2 cache
 cpu1: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
@@ -35,6 +35,7 @@
 acpicpu0 at acpi0: C1(@1 halt!)
 acpicpu1 at acpi0: C1(@1 halt!)
 "ACPI0006" at acpi0 not configured
+acpipci0 at acpi0 PCI0: _OSC failed
 acpicmos0 at acpi0
 "PNP0A06" at acpi0 not configured
 "PNP0A06" at acpi0 not configured
@@ -85,5 +86,3 @@
 scsibus4 at softraid0: 256 targets
 root on sd0a (871b81ad3a5d5c9e.a) swap on sd0b dump on sd0b
 fd0 at fdc0 drive 1: density unknown
-syncing disks... done
-rebooting...


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: unveil spamlogd

2018-10-24 Thread Todd C. Miller
On Wed, 24 Oct 2018 08:05:11 +0100, Ricardo Mestre wrote:

> The only file that spamlogd needs to access after calling pledge is
> PATH_SPAMD_DB, so unveil it with O_RDWR permissions.

Looks good.  OK millert@

 - todd



Re: unveil bdftopcf

2018-10-24 Thread Theo de Raadt
bdftopcf is intended to be portable code.  I don't think it is
right to start using  functions in here.  They are within
an unveil-block which we'll carry as a diff, but still.. it doesn't
feel right.  I think you should use fprintf to stderr and exit as
the existing code does.

> If input_name is provided we can unveil it with read permissions, if
> output_name is provided we need to unveil this one with rwc. Additionally
> depending on the different combinations of if these files are passed via args
> or from stdin/to stdout we can also pledge accordingly to the code path. This
> has been tested succefully with bdf fonts we have bundled in xenocara.
> 
> Since I have several other X apps unveiled and/or pledged could you please
> comment not only with the unveil/pledge part, but also err vs fprintf/exit, 
> the
> placement of the #includes and also tabs vs spaces?
> 
> Index: bdftopcf.c
> ===
> RCS file: /cvs/xenocara/app/bdftopcf/bdftopcf.c,v
> retrieving revision 1.5
> diff -u -p -u -r1.5 bdftopcf.c
> --- bdftopcf.c29 Mar 2018 20:34:30 -  1.5
> +++ bdftopcf.c24 Oct 2018 10:15:41 -
> @@ -38,7 +38,9 @@ from The Open Group.
>  #include "fntfil.h"
>  #include "bdfint.h"
>  #include "pcf.h"
> +#include 
>  #include 
> +#include 
>  #include 
>  
>  int
> @@ -158,6 +160,26 @@ main(int argc, char *argv[])
>  }
>  argv++;
>  }
> +
> + if (input_name) {
> + if (unveil(input_name, "r") == -1)
> + err(1, "unveil");
> + }
> + if (output_name) {
> + if (unveil(output_name, "rwc") == -1)
> + err(1, "unveil");
> + if (pledge("stdio rpath wpath cpath", NULL) == -1)
> + err(1, "pledge");
> + }
> + if (input_name && !output_name) {
> + if (pledge("stdio rpath", NULL) == -1)
> + err(1, "pledge");
> + }
> + if (!input_name && !output_name) {
> + if (pledge("stdio", NULL) == -1)
> + err(1, "pledge");
> + }
> +
>  if (input_name) {
>  input = FontFileOpen(input_name);
>  if (!input) {



Re: unveil xserver's priv proc

2018-10-24 Thread Matthieu Herrb
On Wed, Oct 24, 2018 at 10:36:58AM +0100, Ricardo Mestre wrote:
> Hello,
> 
> semarie@ already gave positive feedback for unveiling xserver, did
> anyone tested it yet and comment on it or OK?

Sorry I almost forgot I was running with this patch for some days
now.

ok matthieu@

> 
> Index: privsep.c
> ===
> RCS file: /cvs/xenocara/xserver/os/privsep.c,v
> retrieving revision 1.29
> diff -u -p -u -r1.29 privsep.c
> --- privsep.c 6 Aug 2018 20:11:34 -   1.29
> +++ privsep.c 24 Oct 2018 09:35:01 -
> @@ -274,6 +274,10 @@ priv_init(uid_t uid, gid_t gid)
>   setproctitle("[priv]");
>   close(socks[1]);
>  
> + for (dev = allowed_devices; dev->name != NULL; dev++) {
> + if (unveil(dev->name, "rw") == -1)
> + err(1, "unveil");
> + }
>   if (pledge("stdio rpath wpath sendfd proc", NULL) == -1)
>   err(1, "pledge");
>  

-- 
Matthieu Herrb



Re: Reuse VM ids.

2018-10-24 Thread Carlos Cardenas
On Tue, Oct 23, 2018 at 10:21:08PM -0700, Ori Bernstein wrote:
> On Mon, 8 Oct 2018 07:59:15 -0700, Bob Beck  wrote:
> 
> > works here and I like it.  but probably for after unlock
> > 
> 
> It's after unlock -- pinging for OKs.
> 
> -- 
> Ori Bernstein
> 

ok ccardenas@

+--+
Carlos



Re: unveil bdftopcf

2018-10-24 Thread Matthieu Herrb
On Wed, Oct 24, 2018 at 11:24:59AM +0100, Ricardo Mestre wrote:
> Hi,
> 
> If input_name is provided we can unveil it with read permissions, if
> output_name is provided we need to unveil this one with rwc. Additionally
> depending on the different combinations of if these files are passed via args
> or from stdin/to stdout we can also pledge accordingly to the code path. This
> has been tested succefully with bdf fonts we have bundled in
> xenocara.

This one looks ok, but it lacks autoconf support to allow the
application to build on other systems.
> 
> Since I have several other X apps unveiled and/or pledged could you please
> comment not only with the unveil/pledge part, but also err vs fprintf/exit, 
> the
> placement of the #includes and also tabs vs spaces?


Generally, I'm not too found of pledging/unveiling random X client
programs. There are a lot of "hidden" features in X libraries that
will probably break with too strict pledges and/or unveils.

Also since this is OpenBSD-specific, it will be difficult to get it
upstreams, especially if you don't provide the autoconf goo to make
the code still build/work on Linux. And when not upstreaming it
creates more burden to merge new versions of the applications.


> 
> Index: bdftopcf.c
> ===
> RCS file: /cvs/xenocara/app/bdftopcf/bdftopcf.c,v
> retrieving revision 1.5
> diff -u -p -u -r1.5 bdftopcf.c
> --- bdftopcf.c29 Mar 2018 20:34:30 -  1.5
> +++ bdftopcf.c24 Oct 2018 10:15:41 -
> @@ -38,7 +38,9 @@ from The Open Group.
>  #include "fntfil.h"
>  #include "bdfint.h"
>  #include "pcf.h"
> +#include 
>  #include 
> +#include 
>  #include 
>  
>  int
> @@ -158,6 +160,26 @@ main(int argc, char *argv[])
>  }
>  argv++;
>  }
> +
> + if (input_name) {
> + if (unveil(input_name, "r") == -1)
> + err(1, "unveil");
> + }
> + if (output_name) {
> + if (unveil(output_name, "rwc") == -1)
> + err(1, "unveil");
> + if (pledge("stdio rpath wpath cpath", NULL) == -1)
> + err(1, "pledge");
> + }
> + if (input_name && !output_name) {
> + if (pledge("stdio rpath", NULL) == -1)
> + err(1, "pledge");
> + }
> + if (!input_name && !output_name) {
> + if (pledge("stdio", NULL) == -1)
> + err(1, "pledge");
> + }
> +
>  if (input_name) {
>  input = FontFileOpen(input_name);
>  if (!input) {

-- 
Matthieu Herrb


signature.asc
Description: PGP signature


Re: igmp_slowtimo: drop NETLOCK

2018-10-24 Thread Martin Pieuchot
On 19/10/18(Fri) 12:15, Scott Cheloha wrote:
> Hi,
> 
> If we introduce a mutex for the igmp module we can drop the NETLOCK
> from igmp_slowtimo().  The router_info list, rti_head, and its entries
> are only ever accessed within the igmp module.  The mutex also needs
> to protect igmp_timers_are_running.

Does the mutex only protect the next/previous pointer of "struct
router_info" or does it also serialize access to the other rti_* fields?

> This is largely a monkey-see-monkey-do imitation of what visa@ did for
> frag6.c.  Obviously any errors are mine.
> 
> There are spots in igmp_input_if() where we could push the mutex into
> smaller sections of the switch statement.  I'm under the impression that
> IGMP is a cooler path, though, so that additional complexity is probably
> pointless, but I figure it's worth mentioning.  Maybe on a later pass.
> 
> I must admit I only have a small test setup for multipath routing.  We
> also don't have any regress tests for IGMP (no idea how tricky those
> would be to write).  The introduction of the mutex was pretty
> straightforward, though, given the size of igmp.c and the fact that the
> mutex is module-local.
> 
> Thoughts?  ok?

Some comments inline :)

> P.S. It doesn't look like we need KERNEL_LOCK in igmp_input_if() until
> rip_input().  Is that fair game,  or is there a larger plan to coordinate
> moving KERNEL_LOCK into various *_input_if() routines later?

Why don't we need it?  Can you move it down?

> Index: sys/netinet/igmp.c
> ===
> RCS file: /cvs/src/sys/netinet/igmp.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 igmp.c
> --- sys/netinet/igmp.c18 Oct 2018 15:46:28 -  1.74
> +++ sys/netinet/igmp.c19 Oct 2018 17:07:31 -
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: igmp.c,v 1.74 2018/10/18 15:46:28 cheloha Exp $   */
> +/*   $OpenBSD: igmp.c,v 1.73 2018/10/18 15:23:04 cheloha Exp $   */
>  /*   $NetBSD: igmp.c,v 1.15 1996/02/13 23:41:25 christos Exp $   */
>  
>  /*
> @@ -77,6 +77,8 @@
>  
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -98,7 +100,13 @@
>  
>  int *igmpctl_vars[IGMPCTL_MAXID] = IGMPCTL_VARS;
>  
> -int  igmp_timers_are_running;
> +/*
> + * Protects igmp_timers_are_running and rti_head.  Note that rti_head needs
> + * to be protected when one of its entries is accessed via in_multi->rti.
> + */
> +struct mutex igmp_mutex = MUTEX_INITIALIZER(IPL_SOFTNET);
> +
> +int igmp_timers_are_running;
>  static LIST_HEAD(, router_info) rti_head;
>  static struct mbuf *router_alert;
>  struct cpumem *igmpcounters;
> @@ -150,6 +158,8 @@ rti_fill(struct in_multi *inm)
>  {
>   struct router_info *rti;
>  
> + MUTEX_ASSERT_LOCKED(&igmp_mutex);
> +
>   LIST_FOREACH(rti, &rti_head, rti_list) {
>   if (rti->rti_ifidx == inm->inm_ifidx) {
>   inm->inm_rti = rti;
> @@ -176,6 +186,8 @@ rti_find(struct ifnet *ifp)
>   struct router_info *rti;
>  
>   KERNEL_ASSERT_LOCKED();
> + MUTEX_ASSERT_LOCKED(&igmp_mutex);

It the KERNEL_LOCK() still needed?  When adding a new assertion, make
sure the older ones are still valid.

> +
>   LIST_FOREACH(rti, &rti_head, rti_list) {
>   if (rti->rti_ifidx == ifp->if_index)
>   return (rti);
> @@ -195,13 +207,18 @@ rti_delete(struct ifnet *ifp)
>  {
>   struct router_info *rti, *trti;
>  
> + mtx_enter(&igmp_mutex);
> +
>   LIST_FOREACH_SAFE(rti, &rti_head, rti_list, trti) {
>   if (rti->rti_ifidx == ifp->if_index) {
>   LIST_REMOVE(rti, rti_list);
> + mtx_leave(&igmp_mutex);
>   free(rti, M_MRTABLE, sizeof(*rti));
> - break;
> + return;
>   }
>   }
> +
> + mtx_leave(&igmp_mutex);
>  }
>  
>  int
> @@ -271,6 +288,8 @@ igmp_input_if(struct ifnet *ifp, struct 
>   m->m_len += iphlen;
>   ip = mtod(m, struct ip *);
>  
> + mtx_enter(&igmp_mutex);
> +
>   switch (igmp->igmp_type) {
>  
>   case IGMP_HOST_MEMBERSHIP_QUERY:
> @@ -282,6 +301,7 @@ igmp_input_if(struct ifnet *ifp, struct 
>   if (igmp->igmp_code == 0) {
>   rti = rti_find(ifp);
>   if (rti == NULL) {
> + mtx_leave(&igmp_mutex);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -289,6 +309,7 @@ igmp_input_if(struct ifnet *ifp, struct 
>   rti->rti_age = 0;
>  
>   if (ip->ip_dst.s_addr != INADDR_ALLHOSTS_GROUP) {
> + mtx_leave(&igmp_mutex);
>   igmpstat_inc(igps_rcv_badqueries);
>   m_freem(m);
>   return IPPROTO_DONE;
> @@ -314,6 +335,7 @@ igmp_input_if(struct ifne

Re: diff: split ctloutput into getopt/setopt

2018-10-24 Thread Martin Pieuchot
On 18/10/18(Thu) 11:34, David Hill wrote:
> Hello -
> 
> This diff splits the ctloutput functions into getopt/setopt, which could
> offer more fine-grained locking.  It also removes some indentation and
> imo is easier to read.
> 
> Thoughts?

Splitting the read and write path makes sense, it will allow us to use
read & write locks :)

> Index: net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.279
> diff -u -p -r1.279 rtsock.c
> --- net/rtsock.c  10 Jul 2018 20:28:34 -  1.279
> +++ net/rtsock.c  18 Oct 2018 15:26:28 -
> @@ -110,6 +110,8 @@ void  rcb_unref(void *, void *);
>  int  route_output(struct mbuf *, struct socket *, struct sockaddr *,
>   struct mbuf *);
>  int  route_ctloutput(int, struct socket *, int, int, struct mbuf *);
> +int  route_getopt(struct socket *, int, int, struct mbuf *);
> +int  route_setopt(struct socket *, int, int, struct mbuf *);
>  int  route_usrreq(struct socket *, int, struct mbuf *, struct mbuf *,
>   struct mbuf *, struct proc *);
>  void route_input(struct mbuf *m0, struct socket *, sa_family_t);
> @@ -358,69 +360,98 @@ int
>  route_ctloutput(int op, struct socket *so, int level, int optname,
>  struct mbuf *m)
>  {
> + int error;
> +
> + switch (op) {
> + case PRCO_SETOPT:
> + error = route_setopt(so, level, optname, m);
> + break;
> + case PRCO_GETOPT:
> + error = route_getopt(so, level, optname, m);
> + break;
> + default:
> + error = EINVAL;
> + break;
> + }
> +
> + return error;
> +}
> +
> +int
> +route_setopt(struct socket *so, int level, int optname, struct mbuf *m)
> +{
>   struct rtpcb *rop = sotortpcb(so);
>   int error = 0;
>   unsigned int tid, prio;
>  
>   if (level != AF_ROUTE)
> - return (EINVAL);
> + return EINVAL;
>  
> - switch (op) {
> - case PRCO_SETOPT:
> - switch (optname) {
> - case ROUTE_MSGFILTER:
> - if (m == NULL || m->m_len != sizeof(unsigned int))
> - error = EINVAL;
> - else
> - rop->rop_msgfilter = *mtod(m, unsigned int *);
> - break;
> - case ROUTE_TABLEFILTER:
> - if (m == NULL || m->m_len != sizeof(unsigned int)) {
> - error = EINVAL;
> - break;
> - }
> - tid = *mtod(m, unsigned int *);
> - if (tid != RTABLE_ANY && !rtable_exists(tid))
> - error = ENOENT;
> - else
> - rop->rop_rtableid = tid;
> - break;
> - case ROUTE_PRIOFILTER:
> - if (m == NULL || m->m_len != sizeof(unsigned int)) {
> - error = EINVAL;
> - break;
> - }
> - prio = *mtod(m, unsigned int *);
> - if (prio > RTP_MAX)
> - error = EINVAL;
> - else
> - rop->rop_priority = prio;
> - break;
> - default:
> - error = ENOPROTOOPT;
> + switch (optname) {
> + case ROUTE_MSGFILTER:
> + if (m == NULL || m->m_len != sizeof(unsigned int))
> + error = EINVAL;
> + else
> + rop->rop_msgfilter = *mtod(m, unsigned int *);
> + break;
> + case ROUTE_TABLEFILTER:
> + if (m == NULL || m->m_len != sizeof(unsigned int)) {
> + error = EINVAL;
>   break;
>   }
> + tid = *mtod(m, unsigned int *);
> + if (tid != RTABLE_ANY && !rtable_exists(tid))
> + error = ENOENT;
> + else
> + rop->rop_rtableid = tid;
>   break;
> - case PRCO_GETOPT:
> - switch (optname) {
> - case ROUTE_MSGFILTER:
> - m->m_len = sizeof(unsigned int);
> - *mtod(m, unsigned int *) = rop->rop_msgfilter;
> - break;
> - case ROUTE_TABLEFILTER:
> - m->m_len = sizeof(unsigned int);
> - *mtod(m, unsigned int *) = rop->rop_rtableid;
> - break;
> - case ROUTE_PRIOFILTER:
> - m->m_len = sizeof(unsigned int);
> - *mtod(m, unsigned int *) = rop->rop_priority;
> - break;
> - default:
> - error = ENOPROTOOPT;
> + case ROUTE_PRIOFILTER:
> + if (m == NULL || m->m_len != sizeof(unsigned int)) {
> +   

Re: readelf: fix out-of-bounds error

2018-10-24 Thread Mark Kettenis
> Date: Tue, 23 Oct 2018 23:45:55 +0200
> From: Christian Weisgerber 
> 
> I ran across this:
> 
>   $ readelf -h /usr/local/bin/w3m
>   ...
>   readelf(71968) in free(): bogus pointer (double free?) 0x1
>   Abort trap (core dumped)
> 
> In readelf.c there's a static arrary:
> 
>   static bfd_vma dynamic_info[DT_JMPREL + 1];
> 
> Later this array is written to like this:
> 
>   switch (entry->d_tag)
> {
>   ...
>   case DT_TEXTREL :
>   case DT_JMPREL  :
>   case DT_RUNPATH :
> dynamic_info[entry->d_tag] = entry->d_un.d_val;
> ...
> 
> Alas, DT_RUNPATH > DT_JMPREL, so we have an out-of-bounds write
> that splats somewhere into memory.  In my case it mangled an unrelated
> pointer.
> 
> I checked that DT_RUNPATH is the numerically highest value used to
> index dynamic_info[], so let's size the array appropriately.
> 
> OK?

ok kettenis@

> Index: gnu/usr.bin/binutils/binutils/readelf.c
> ===
> RCS file: /cvs/src/gnu/usr.bin/binutils/binutils/readelf.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 readelf.c
> --- gnu/usr.bin/binutils/binutils/readelf.c   31 Aug 2014 13:40:02 -  
> 1.11
> +++ gnu/usr.bin/binutils/binutils/readelf.c   23 Oct 2018 21:30:49 -
> @@ -129,7 +129,7 @@ Elf_Internal_Syminfo *dynamic_syminfo;
>  unsigned long dynamic_syminfo_offset;
>  unsigned int dynamic_syminfo_nent;
>  char program_interpreter[64];
> -bfd_vma dynamic_info[DT_JMPREL + 1];
> +bfd_vma dynamic_info[DT_RUNPATH + 1];
>  bfd_vma version_info[16];
>  Elf_Internal_Ehdr elf_header;
>  Elf_Internal_Shdr *section_headers;
> Index: gnu/usr.bin/binutils-2.17/binutils/readelf.c
> ===
> RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/binutils/readelf.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 readelf.c
> --- gnu/usr.bin/binutils-2.17/binutils/readelf.c  23 Oct 2017 05:26:58 
> -  1.15
> +++ gnu/usr.bin/binutils-2.17/binutils/readelf.c  23 Oct 2018 21:25:53 
> -
> @@ -136,7 +136,7 @@ static Elf_Internal_Syminfo *dynamic_sym
>  static unsigned long dynamic_syminfo_offset;
>  static unsigned int dynamic_syminfo_nent;
>  static char program_interpreter[64];
> -static bfd_vma dynamic_info[DT_JMPREL + 1];
> +static bfd_vma dynamic_info[DT_RUNPATH + 1];
>  static bfd_vma version_info[16];
>  static Elf_Internal_Ehdr elf_header;
>  static Elf_Internal_Shdr *section_headers;
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 
> 



Re: unveil spamlogd

2018-10-24 Thread Bob Beck
ok beck@ as well

On Wed, Oct 24, 2018 at 06:13 Todd C. Miller  wrote:

> On Wed, 24 Oct 2018 08:05:11 +0100, Ricardo Mestre wrote:
>
> > The only file that spamlogd needs to access after calling pledge is
> > PATH_SPAMD_DB, so unveil it with O_RDWR permissions.
>
> Looks good.  OK millert@
>
>  - todd
>


bgplg: allow neighbors with space in name

2018-10-24 Thread Denis Fondras
I have peers with description containing spaces but bgplg won't accept that by
default.

I'd like some comments on that diff.
It is OK for bgplgsh (show ip bgp in "Peer 1" feels OK) but not for bgplg as I
have to quote the peer description in the input box (feels rather unnatural).

Index: bgplg.c
===
RCS file: /cvs/src/usr.bin/bgplg/bgplg.c,v
retrieving revision 1.19
diff -u -p -r1.19 bgplg.c
--- bgplg.c 5 Mar 2018 10:53:37 -   1.19
+++ bgplg.c 24 Oct 2018 14:32:22 -
@@ -112,7 +112,7 @@ lg_getenv(const char *name, int *lenp)
*lenp = len;
 
 #define allowed_in_string(_x)   \
-   (isalnum((unsigned char)_x) || strchr("-_.:/= ", _x))
+   (isalnum((unsigned char)_x) || strchr("\"-_.:/= ", _x))
 
for (i = 0; i < len; i++) {
if (ptr[i] == '&')
@@ -155,13 +155,18 @@ lg_arg2argv(char *arg, int *argc)
 {
char **argv, *ptr = arg;
size_t len;
-   u_int i, c = 1;
+   u_int i, c = 1, instr = 0;
 
len = strlen(arg);
 
/* Count elements */
for (i = 0; i < len; i++) {
-   if (isspace((unsigned char)arg[i])) {
+   if (arg[i] == '\"') {
+   instr++;
+   memmove(&arg[i], &arg[i+1], (len-i)*sizeof(char));
+   len--;
+   }
+   if (isspace((unsigned char)arg[i]) && !(instr%2)) {
/* filter out additional options */
if (arg[i + 1] == '-') {
printf("invalid input\n");
Index: bgplgsh.c
===
RCS file: /cvs/src/usr.bin/bgplg/bgplgsh.c,v
retrieving revision 1.8
diff -u -p -r1.8 bgplgsh.c
--- bgplgsh.c   9 Dec 2015 17:52:24 -   1.8
+++ bgplgsh.c   24 Oct 2018 14:32:22 -
@@ -79,7 +79,7 @@ lg_arg2argv(char *arg, int *argc)
 {
char **argv, *ptr = arg;
size_t len;
-   u_int i, c = 1;
+   u_int i, c = 1, instr = 0;
 
if (lg_checkarg(arg) != 0)
return (NULL);
@@ -88,7 +88,12 @@ lg_arg2argv(char *arg, int *argc)
 
/* Count elements */
for (i = 0; i < len; i++) {
-   if (isspace((unsigned char)arg[i])) {
+   if (arg[i] == '\"') {
+   instr++;
+   memmove(&arg[i], &arg[i+1], (len-i)*sizeof(char));
+   len--;
+   }
+   if (isspace((unsigned char)arg[i]) && !(instr%2)) {
/* filter out additional options */
if (arg[i + 1] == '-') {
printf("invalid input\n");



Re: Regression in "Add support to create and convert disk images from existing images"

2018-10-24 Thread Anton Lindqvist
On Mon, Oct 22, 2018 at 11:05:13AM -0700, Greg Steuck wrote:
> Hi Reyk & Anton,
> 
> I upgraded the syzkaller machine from Oct 11 to Oct 21 snapshot and started
> seeing:
> Oct 22 10:00:21 ci-openbsd vmd[15707]: qc2_open: missing base image
> /syzkaller/managers/main/current/image
> Oct 22 10:00:21 ci-openbsd vmd[15707]:
> /syzkaller/managers/main/current/image: could not open qc2_open: No such
> file or directory
> Oct 22 10:00:21 ci-openbsd vmd[13268]: qc2_open: missing base image
> /syzkaller/managers/main/current/image
> Oct 22 10:00:21 ci-openbsd vmd[13268]:
> /syzkaller/managers/main/current/image: could not open qc2_open: No such
> file or directory
> Oct 22 10:00:21 ci-openbsd vmd[51186]: qc2_open: missing base image
> /syzkaller/managers/main/current/image
> Oct 22 10:00:21 ci-openbsd vmd[51186]:
> /syzkaller/managers/main/current/image: could not open qc2_open: No such
> file or directory
> 
> Reverting the change in subject and rebuilding vmd&vmctl stops the problem.
> 
> The file in question exists:
> ci-openbsd$ file /syzkaller/managers/main/current/image
> /syzkaller/managers/main/current/image: QEMU QCOW Image (v3), 1073741824
> bytes
> ci-openbsd$ ls -l /syzkaller/managers/main/current/image
> -rw-r-  2 syzkaller  syzkaller  545783808 Oct 22 10:34
> /syzkaller/managers/main/current/image
> 
> Under the hood this command line is executed:
> vmctl start ci-openbsd-main-0 -b /syzkaller/managers/main/current/kernel -d
> /syzkaller/managers/main/workdir/instance-0/disk.qcow2 -m 512M -L -c -t
> syzkaller
> 
> Anton, what does your code is syzkaller do to create this file?
> -rw---  1 syzkaller  syzkaller  262144 Oct 22 10:59
> /syzkaller/managers/main/workdir/instance-0/disk.qcow2
> 
> 
> The file does reference the base image:
> ci-openbsd$ strings /syzkaller/managers/main/workdir/instance-0/disk.qcow2
> 
> h/syzkaller/managers/main/current/image
> 
> Thanks
> Greg
> -- 
> nest.cx is Gmail hosted, use PGP for anything private. Key:
> http://goo.gl/6dMsr
> Fingerprint: 5E2B 2D0E 1E03 2046 BEC3  4D50 0B15 42BD 8DF5 A1B0

I'm also not able to start a VM using a derived disk after this commit.
It looks to me like the code is trying to resolve the base disk for the
derived disk over and over instead of walking down the chain of derived
disks. This causes file descriptors to be opened only for the derived
disk whereas the base disk is expected. With the diff below, I'm able to
boot using a derived disk again.

Index: config.c
===
RCS file: /cvs/src/usr.sbin/vmd/config.c,v
retrieving revision 1.53
diff -u -p -r1.53 config.c
--- config.c19 Oct 2018 10:12:39 -  1.53
+++ config.c24 Oct 2018 15:48:00 -
@@ -364,6 +364,10 @@ config_setvm(struct privsep *ps, struct 
base, vcp->vcp_disks[i]);
goto fail;
}
+   if (strlcpy(path, base, sizeof(path)) >= sizeof(path)) {
+   log_warnx("%s, base path too long", __func__);
+   goto fail;
+   }
}
}
 



Re: unveil bdftopcf

2018-10-24 Thread Theo de Raadt
Matthieu Herrb  wrote:

> Generally, I'm not too found of pledging/unveiling random X client
> programs. There are a lot of "hidden" features in X libraries that
> will probably break with too strict pledges and/or unveils.

Well eventually we want to see if something can be done about xterm.
Especially if the lessons learned (I suspect some hoisting will occur)
can be pushed back upstream, and maybe allow others to apply their
own system call limiter mechanism.  Perhaps not possible...

> Also since this is OpenBSD-specific, it will be difficult to get it
> upstreams, especially if you don't provide the autoconf goo to make
> the code still build/work on Linux. And when not upstreaming it
> creates more burden to merge new versions of the applications.

Well, I doubt it will create too much burden, generally these unveil
or pledge chunks are a small set of + lines, without changing other
logic.

Anyways, bdftopcf is not running near a security boundary.



Re: Regression in "Add support to create and convert disk images from existing images"

2018-10-24 Thread Greg Steuck
Thanks Anton. I confirm that applying this on top of the most recent tree
fixed the problem for me.

I wonder if a regression test of some sort is in order. The peanut gallery
also observes that config_getvm is could use some light refactoring into
smaller units.

On Wed, Oct 24, 2018 at 8:51 AM Anton Lindqvist  wrote:

> On Mon, Oct 22, 2018 at 11:05:13AM -0700, Greg Steuck wrote:
> > Hi Reyk & Anton,
> >
> > I upgraded the syzkaller machine from Oct 11 to Oct 21 snapshot and
> started
> > seeing:
> > Oct 22 10:00:21 ci-openbsd vmd[15707]: qc2_open: missing base image
> > /syzkaller/managers/main/current/image
> > Oct 22 10:00:21 ci-openbsd vmd[15707]:
> > /syzkaller/managers/main/current/image: could not open qc2_open: No such
> > file or directory
> > Oct 22 10:00:21 ci-openbsd vmd[13268]: qc2_open: missing base image
> > /syzkaller/managers/main/current/image
> > Oct 22 10:00:21 ci-openbsd vmd[13268]:
> > /syzkaller/managers/main/current/image: could not open qc2_open: No such
> > file or directory
> > Oct 22 10:00:21 ci-openbsd vmd[51186]: qc2_open: missing base image
> > /syzkaller/managers/main/current/image
> > Oct 22 10:00:21 ci-openbsd vmd[51186]:
> > /syzkaller/managers/main/current/image: could not open qc2_open: No such
> > file or directory
> >
> > Reverting the change in subject and rebuilding vmd&vmctl stops the
> problem.
> >
> > The file in question exists:
> > ci-openbsd$ file /syzkaller/managers/main/current/image
> > /syzkaller/managers/main/current/image: QEMU QCOW Image (v3), 1073741824
> > bytes
> > ci-openbsd$ ls -l /syzkaller/managers/main/current/image
> > -rw-r-  2 syzkaller  syzkaller  545783808 Oct 22 10:34
> > /syzkaller/managers/main/current/image
> >
> > Under the hood this command line is executed:
> > vmctl start ci-openbsd-main-0 -b /syzkaller/managers/main/current/kernel
> -d
> > /syzkaller/managers/main/workdir/instance-0/disk.qcow2 -m 512M -L -c -t
> > syzkaller
> >
> > Anton, what does your code is syzkaller do to create this file?
> > -rw---  1 syzkaller  syzkaller  262144 Oct 22 10:59
> > /syzkaller/managers/main/workdir/instance-0/disk.qcow2
> >
> >
> > The file does reference the base image:
> > ci-openbsd$ strings
> /syzkaller/managers/main/workdir/instance-0/disk.qcow2
> >
> > h/syzkaller/managers/main/current/image
> >
> > Thanks
> > Greg
> > --
> > nest.cx is Gmail hosted, use PGP for anything private. Key:
> > http://goo.gl/6dMsr
> > Fingerprint: 5E2B 2D0E 1E03 2046 BEC3  4D50 0B15 42BD 8DF5 A1B0
>
> I'm also not able to start a VM using a derived disk after this commit.
> It looks to me like the code is trying to resolve the base disk for the
> derived disk over and over instead of walking down the chain of derived
> disks. This causes file descriptors to be opened only for the derived
> disk whereas the base disk is expected. With the diff below, I'm able to
> boot using a derived disk again.
>
> Index: config.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/config.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 config.c
> --- config.c19 Oct 2018 10:12:39 -  1.53
> +++ config.c24 Oct 2018 15:48:00 -
> @@ -364,6 +364,10 @@ config_setvm(struct privsep *ps, struct
> base, vcp->vcp_disks[i]);
> goto fail;
> }
> +   if (strlcpy(path, base, sizeof(path)) >=
> sizeof(path)) {
> +   log_warnx("%s, base path too long",
> __func__);
> +   goto fail;
> +   }
> }
> }
>
>

-- 
nest.cx is Gmail hosted, use PGP for anything private. Key:
http://goo.gl/6dMsr
Fingerprint: 5E2B 2D0E 1E03 2046 BEC3  4D50 0B15 42BD 8DF5 A1B0


bridge(4) ioctl & MP

2018-10-24 Thread Martin Pieuchot
I'd like to take brigde_input() & bridge_output() outside of the
KERNEL_LOCK().  My previous approach relying on the NET_LOCK() didn't
work because wireless drivers might call bridge_output() from their
interrupt handler.

So we'll need another mechanism.  The first step would be to use a
mutex.  One of the fields that will be protect by this lock is
`if_bridgeport'.  So here's a small cleanup diff to make it clear
that the field is read in ioctl path.

ok?

Index: net/bridgectl.c
===
RCS file: /cvs/src/sys/net/bridgectl.c,v
retrieving revision 1.10
diff -u -p -r1.10 bridgectl.c
--- net/bridgectl.c 22 Oct 2018 13:18:23 -  1.10
+++ net/bridgectl.c 24 Oct 2018 19:33:56 -
@@ -55,7 +55,7 @@ int   bridge_rtfind(struct bridge_softc *,
 intbridge_rtdaddr(struct bridge_softc *, struct ether_addr *);
 u_int32_t bridge_hash(struct bridge_softc *, struct ether_addr *);
 
-intbridge_brlconf(struct bridge_softc *, struct ifbrlconf *);
+intbridge_brlconf(struct bridge_iflist *, struct ifbrlconf *);
 intbridge_addrule(struct bridge_iflist *, struct ifbrlreq *, int out);
 
 int
@@ -64,6 +64,7 @@ bridgectl_ioctl(struct ifnet *ifp, u_lon
struct bridge_softc *sc = (struct bridge_softc *)ifp->if_softc;
struct ifbreq *req = (struct ifbreq *)data;
struct ifbrlreq *brlreq = (struct ifbrlreq *)data;
+   struct ifbrlconf *bc = (struct ifbrlconf *)data;
struct ifbareq *bareq = (struct ifbareq *)data;
struct ifbrparam *bparam = (struct ifbrparam *)data;
struct bridge_iflist *bif;
@@ -160,7 +161,17 @@ bridgectl_ioctl(struct ifnet *ifp, u_lon
bridge_flushrule(bif);
break;
case SIOCBRDGGRL:
-   error = bridge_brlconf(sc, (struct ifbrlconf *)data);
+   ifs = ifunit(bc->ifbrl_ifsname);
+   if (ifs == NULL) {
+   error = ENOENT;
+   break;
+   }
+   bif = (struct bridge_iflist *)ifs->if_bridgeport;
+   if (bif == NULL || bif->bridge_sc != sc) {
+   error = ESRCH;
+   break;
+   }
+   error = bridge_brlconf(bif, bc);
break;
default:
break;
@@ -535,21 +546,13 @@ bridge_update(struct ifnet *ifp, struct 
  * bridge filter/matching rules
  */
 int
-bridge_brlconf(struct bridge_softc *sc, struct ifbrlconf *bc)
+bridge_brlconf(struct bridge_iflist *bif, struct ifbrlconf *bc)
 {
-   struct ifnet *ifp;
-   struct bridge_iflist *bif;
+   struct bridge_softc *sc = bif->bridge_sc;
struct brl_node *n;
struct ifbrlreq req;
int error = 0;
u_int32_t i = 0, total = 0;
-
-   ifp = ifunit(bc->ifbrl_ifsname);
-   if (ifp == NULL)
-   return (ENOENT);
-   bif = (struct bridge_iflist *)ifp->if_bridgeport;
-   if (bif == NULL || bif->bridge_sc != sc)
-   return (ESRCH);
 
SIMPLEQ_FOREACH(n, &bif->bif_brlin, brl_next) {
total++;



Re: bridge(4) ioctl & MP

2018-10-24 Thread Alexander Bluhm
On Wed, Oct 24, 2018 at 04:34:43PM -0300, Martin Pieuchot wrote:
> I'd like to take brigde_input() & bridge_output() outside of the
> KERNEL_LOCK().  My previous approach relying on the NET_LOCK() didn't
> work because wireless drivers might call bridge_output() from their
> interrupt handler.
> 
> So we'll need another mechanism.  The first step would be to use a
> mutex.  One of the fields that will be protect by this lock is
> `if_bridgeport'.  So here's a small cleanup diff to make it clear
> that the field is read in ioctl path.
> 
> ok?

OK bluhm@

> Index: net/bridgectl.c
> ===
> RCS file: /cvs/src/sys/net/bridgectl.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 bridgectl.c
> --- net/bridgectl.c   22 Oct 2018 13:18:23 -  1.10
> +++ net/bridgectl.c   24 Oct 2018 19:33:56 -
> @@ -55,7 +55,7 @@ int bridge_rtfind(struct bridge_softc *,
>  int  bridge_rtdaddr(struct bridge_softc *, struct ether_addr *);
>  u_int32_t bridge_hash(struct bridge_softc *, struct ether_addr *);
>  
> -int  bridge_brlconf(struct bridge_softc *, struct ifbrlconf *);
> +int  bridge_brlconf(struct bridge_iflist *, struct ifbrlconf *);
>  int  bridge_addrule(struct bridge_iflist *, struct ifbrlreq *, int out);
>  
>  int
> @@ -64,6 +64,7 @@ bridgectl_ioctl(struct ifnet *ifp, u_lon
>   struct bridge_softc *sc = (struct bridge_softc *)ifp->if_softc;
>   struct ifbreq *req = (struct ifbreq *)data;
>   struct ifbrlreq *brlreq = (struct ifbrlreq *)data;
> + struct ifbrlconf *bc = (struct ifbrlconf *)data;
>   struct ifbareq *bareq = (struct ifbareq *)data;
>   struct ifbrparam *bparam = (struct ifbrparam *)data;
>   struct bridge_iflist *bif;
> @@ -160,7 +161,17 @@ bridgectl_ioctl(struct ifnet *ifp, u_lon
>   bridge_flushrule(bif);
>   break;
>   case SIOCBRDGGRL:
> - error = bridge_brlconf(sc, (struct ifbrlconf *)data);
> + ifs = ifunit(bc->ifbrl_ifsname);
> + if (ifs == NULL) {
> + error = ENOENT;
> + break;
> + }
> + bif = (struct bridge_iflist *)ifs->if_bridgeport;
> + if (bif == NULL || bif->bridge_sc != sc) {
> + error = ESRCH;
> + break;
> + }
> + error = bridge_brlconf(bif, bc);
>   break;
>   default:
>   break;
> @@ -535,21 +546,13 @@ bridge_update(struct ifnet *ifp, struct 
>   * bridge filter/matching rules
>   */
>  int
> -bridge_brlconf(struct bridge_softc *sc, struct ifbrlconf *bc)
> +bridge_brlconf(struct bridge_iflist *bif, struct ifbrlconf *bc)
>  {
> - struct ifnet *ifp;
> - struct bridge_iflist *bif;
> + struct bridge_softc *sc = bif->bridge_sc;
>   struct brl_node *n;
>   struct ifbrlreq req;
>   int error = 0;
>   u_int32_t i = 0, total = 0;
> -
> - ifp = ifunit(bc->ifbrl_ifsname);
> - if (ifp == NULL)
> - return (ENOENT);
> - bif = (struct bridge_iflist *)ifp->if_bridgeport;
> - if (bif == NULL || bif->bridge_sc != sc)
> - return (ESRCH);
>  
>   SIMPLEQ_FOREACH(n, &bif->bif_brlin, brl_next) {
>   total++;



Re: Fix descriptions of smtps vs smtp+tls in smtpd.conf.5

2018-10-24 Thread Gilles Chehade
On Mon, Oct 22, 2018 at 08:37:25PM -0400, trondd wrote:
> Unless I'm confused, it seems the description of the smarthosts smtps and
> smtp+tls are revered in the smtpd.conf man page.
>

You are confused ;-)


> My log seemed to back this up.  When using smtp+tls, which the man page said
> uses STARTTLS but seems to actually use TLS which my ISP does not:
> 
> Oct 21 21:42:58 ember smtpd[41596]: ca9dba5e7f80e6ca mta connecting 
> address=smtp+tls://68.87.20.6:465 host=omta-ch2.sys.comcast.net
> Oct 21 21:42:58 ember smtpd[41596]: ca9dba5e7f80e6ca mta connected
> Oct 21 21:43:59 ember smtpd[41596]: ca9dba5e7f80e6ca mta error 
> reason=Connection closed unexpectedly
> 

You are mistaking smtps and smtp+tls:

In an smtps session, the TLS negotation takes place during the connection so
client and server are already in a secure channel when the SMTP session gets
started.

In a smtp+tls session, the TLS negotiation takes place after the session has
started in plaintext through the use of the STARTTLS SMTP extension.

In your example here, you are using smtp+tls on a host that expects smtps so
the TLS negotation can't play out and you're kicked out.


> And with smtps, which the man page said uses TLS, logs show STARTTLS:
> 
> Oct 21 22:02:06 ember smtpd[66745]: a9193b70dbc40df0 mta connecting 
> address=smtps://68.87.20.6:465 host=omta-ch2.sys.comcast.net
> Oct 21 22:02:06 ember smtpd[66745]: a9193b70dbc40df0 mta connected
> Oct 21 22:02:06 ember smtpd[66745]: a9193b70dbc40df0 mta starttls 
> ciphers=version=TLSv1.2, cipher=ECDHE-RSA-AES256-GCM-SHA384, bits=256
> Oct 21 22:02:06 ember smtpd[66745]: smtp-out: Server certificate verification 
> succeeded on session a9193b70dbc40df0
> 

TLS and STARTTLS are essentially the same as far as you're concerned.

smtpd will _always_ display a 'starttls' log line when the TLS channel starts,
disregarding if TLS was started at connect time (smtps) or within the protocol
(smtp+tls, or even smtp since it does opportunistic tls).

The only issue here is that you attempted to connect in plaintext then upgrade
a session on a host that didn't speak plaintext and expected sessions to speak
TLS from the start.

-- 
Gilles Chehade

https://www.poolp.org  @poolpOrg



Re: Fix descriptions of smtps vs smtp+tls in smtpd.conf.5

2018-10-24 Thread Raf Czlonka
On Thu, Oct 25, 2018 at 07:11:47AM BST, Gilles Chehade wrote:
> 
> smtpd will _always_ display a 'starttls' log line when the TLS channel starts,
> disregarding if TLS was started at connect time (smtps) or within the protocol
> (smtp+tls, or even smtp since it does opportunistic tls).
> 

I guess this is the confusing bit - seeing 'starttls' in the log
file and thinking 'STARTTLS', i.e. the "TLS upgrade".

R.



Re: Fix descriptions of smtps vs smtp+tls in smtpd.conf.5

2018-10-24 Thread Gilles Chehade
On Thu, Oct 25, 2018 at 07:24:33AM +0100, Raf Czlonka wrote:
> On Thu, Oct 25, 2018 at 07:11:47AM BST, Gilles Chehade wrote:
> > 
> > smtpd will _always_ display a 'starttls' log line when the TLS channel 
> > starts,
> > disregarding if TLS was started at connect time (smtps) or within the 
> > protocol
> > (smtp+tls, or even smtp since it does opportunistic tls).
> > 
> 
> I guess this is the confusing bit - seeing 'starttls' in the log
> file and thinking 'STARTTLS', i.e. the "TLS upgrade".
> 

yes, maybe it should just display 'tls' instead of 'starttls'


-- 
Gilles Chehade

https://www.poolp.org  @poolpOrg



bgpd: replace some more walkers with rib_dump

2018-10-24 Thread Claudio Jeker
Next step on my quest to make the RIB code better.
This changes the following things:
- network_flush is now using rib_dump_new to walk the Adj-RIB-In and
  remove all dynamically added announcements
- peer_flush got generalized and is now used also in peer_down.
  It also uses a rib_dump_new call to walk the Adj-RIB-In and remove
  all prefixes from a peer but this is done synchronous for now.
- peer_flush is now working correctly for AID_UNSPEC.
- Change the rib_valid check to continue instead of break out of the loop.
  The rib table can have holes so the loop needs to continue.

This removes the last three places that use the peer -> path -> prefix
tailqs so the next step is to remove these and make rde_aspath just an
object that is referenced by prefixes. After that adding a proper
Adj-RIB-Out should be possible.

Running with this on production :)
-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.440
diff -u -p -r1.440 rde.c
--- rde.c   24 Oct 2018 08:26:37 -  1.440
+++ rde.c   25 Oct 2018 06:34:32 -
@@ -97,7 +97,7 @@ struct rde_peer   *peer_add(u_int32_t, str
 struct rde_peer*peer_get(u_int32_t);
 voidpeer_up(u_int32_t, struct session_up *);
 voidpeer_down(u_int32_t);
-voidpeer_flush(struct rde_peer *, u_int8_t);
+voidpeer_flush(struct rde_peer *, u_int8_t, time_t);
 voidpeer_stale(u_int32_t, u_int8_t);
 voidpeer_dump(u_int32_t, u_int8_t);
 static void peer_recv_eor(struct rde_peer *, u_int8_t);
@@ -105,7 +105,8 @@ static void  peer_send_eor(struct rde_pe
 
 voidnetwork_add(struct network_config *, int);
 voidnetwork_delete(struct network_config *, int);
-voidnetwork_dump_upcall(struct rib_entry *, void *);
+static void network_dump_upcall(struct rib_entry *, void *);
+static void network_flush_upcall(struct rib_entry *, void *);
 
 voidrde_shutdown(void);
 int sa_cmp(struct bgpd_addr *, struct sockaddr *);
@@ -418,7 +419,7 @@ rde_dispatch_imsg_session(struct imsgbuf
imsg.hdr.peerid);
break;
}
-   peer_flush(peer, aid);
+   peer_flush(peer, aid, peer->staletime[aid]);
break;
case IMSG_SESSION_RESTARTED:
if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(aid)) {
@@ -434,7 +435,7 @@ rde_dispatch_imsg_session(struct imsgbuf
break;
}
if (peer->staletime[aid])
-   peer_flush(peer, aid);
+   peer_flush(peer, aid, peer->staletime[aid]);
break;
case IMSG_REFRESH:
if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(aid)) {
@@ -556,7 +557,10 @@ badnetdel:
log_warnx("rde_dispatch: wrong imsg len");
break;
}
-   prefix_network_clean(peerself);
+   if (rib_dump_new(RIB_ADJ_IN, AID_UNSPEC,
+   RDE_RUNNER_ROUNDS, peerself, network_flush_upcall,
+   NULL, NULL) == -1)
+   log_warn("rde_dispatch: IMSG_NETWORK_FLUSH");
break;
case IMSG_FILTER_SET:
if (imsg.hdr.len - IMSG_HEADER_SIZE !=
@@ -770,7 +774,7 @@ rde_dispatch_imsg_parent(struct imsgbuf 
memcpy(nconf, imsg.data, sizeof(struct bgpd_config));
for (rid = 0; rid < rib_size; rid++) {
if (!rib_valid(rid))
-   break;
+   continue;
ribs[rid].state = RECONF_DELETE;
}
SIMPLEQ_INIT(&nconf->rde_prefixsets);
@@ -1411,7 +1415,7 @@ rde_update_update(struct rde_peer *peer,
 
for (i = RIB_LOC_START; i < rib_size; i++) {
if (!rib_valid(i))
-   break;
+   continue;
rde_filterstate_prep(&state, &in->aspath, in->nexthop,
in->nhflags);
/* input filter */
@@ -1443,7 +1447,7 @@ rde_update_withdraw(struct rde_peer *pee
 
for (i = RIB_LOC_START; i < rib_size; i++) {
if (!rib_valid(i))
-   break;
+   continue;
if (prefix_remove(&ribs[i].rib, peer, prefix, prefixlen))
rde_update_log("withdraw", i, peer, NULL, prefix,
prefixlen);
@@ -2957,11 +2961,10 @@ rde_reload_done(void)
 static void
 rde_so