Re: tcpdump fork+exec?

2017-09-06 Thread Bryan Steele 
On Thu, Sep 07, 2017 at 06:58:28AM +0200, Otto Moerbeek wrote:
> On Thu, Sep 07, 2017 at 12:27:18AM -0400, Bryan Steele wrote:
> 
> > Hi,
> > 
> > This turned out easier then pflogd thanks to the existing privsep design
> > work done by deraadt@ and canacar@ many years ago. While tcpdump isn't a
> 
> Small correction for the record:
> 
> done by otto@ and cancacar@ while being prodded almost gently by deraadt@
> 
>   -Otto

Doh! Sorry otto@! :-)

Re: tcpdump fork+exec?

2017-09-06 Thread Theo de Raadt 
> done by otto@ and cancacar@ while being prodded almost gently by deraadt@


So untrue.  I don't do gently...

Re: tcpdump fork+exec?

2017-09-06 Thread Otto Moerbeek 
On Thu, Sep 07, 2017 at 12:27:18AM -0400, Bryan Steele wrote:

> Hi,
> 
> This turned out easier then pflogd thanks to the existing privsep design
> work done by deraadt@ and canacar@ many years ago. While tcpdump isn't a

Small correction for the record:

done by otto@ and cancacar@ while being prodded almost gently by deraadt@

-Otto


> daemon in the traditional sense, it isn't so uncommon for people to have
> long running sessions. At least on OpenBSD, this is even safe thanks to
> privsep and very strict pledge(2) promises.
> 
> This does shuffle things around a bit internally, but I don't think I've
> totally broken anything.
> 
> Seems to work with some light testing, live captures, -w/-r offline pcap
> files.
> 
> Comments?
> 
> -Bryan.
> 
> Index: privsep.c
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v
> retrieving revision 1.45
> diff -u -p -u -r1.45 privsep.c
> --- usr.sbin/tcpdump/privsep.c14 Jun 2017 20:48:54 -  1.45
> +++ usr.sbin/tcpdump/privsep.c7 Sep 2017 03:19:26 -
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -132,13 +133,10 @@ static void logmsg(int, const char *, ..
>  int
>  priv_init(int argc, char **argv)
>  {
> - int bpfd = -1;
> - int i, socks[2], cmd, nflag = 0;
> + int i, nargc, socks[2];
>   struct passwd *pw;
> - char *cmdbuf, *infile = NULL;
> - char *RFileName = NULL;
> - char *WFileName = NULL;
>   sigset_t allsigs, oset;
> + char childnum[11], **privargv;
>  
>   closefrom(STDERR_FILENO + 1);
>   for (i = 1; i < _NSIG; i++)
> @@ -155,7 +153,7 @@ priv_init(int argc, char **argv)
>   if (child_pid < 0)
>   err(1, "fork() failed");
>  
> - if (child_pid) {
> + if (!child_pid) {
>   close(socks[0]);
>   priv_fd = socks[1];
>  
> @@ -188,14 +186,48 @@ priv_init(int argc, char **argv)
>  
>   return (0);
>   }
> + close(socks[1]);
> +
> + if (dup2(socks[0], 3) == -1)
> + err(1, "dup2 priv sock failed");
> + closefrom(4);
> +
> + snprintf(childnum, sizeof(childnum), "%d", child_pid);
> + if ((privargv = reallocarray(NULL, argc + 3, sizeof(char *))) == NULL)
> + err(1, "alloc priv argv failed");
> + nargc = 0;
> + privargv[nargc++] = argv[0];
> + privargv[nargc++] = "-P";
> + privargv[nargc++] = childnum;
> + for (i = 1; i < argc; i++)
> + privargv[nargc++] = argv[i];
> + privargv[nargc] = NULL;
> + execvp(privargv[0], privargv);
> + err(1, "exec priv '%s' failed", privargv[0]);
> +}
> +
> +__dead void
> +priv_exec(int argc, char *argv[])
> +{
> + int bpfd = -1;
> + int i, sock, cmd, nflag = 0;
> + char *cmdbuf, *infile = NULL;
> + char *RFileName = NULL;
> + char *WFileName = NULL;
> + const char *errstr;
> +
> + sock = 3;
> +
> + closefrom(4);
> + for (i = 1; i < _NSIG; i++)
> + signal(i, SIG_DFL);
>  
> - sigprocmask(SIG_SETMASK, , NULL);
>   signal(SIGINT, SIG_IGN);
>  
>   /* parse the arguments for required options */
>   opterr = 0;
>   while ((i = getopt(argc, argv,
> - "ac:D:deE:fF:i:lLnNOopqr:s:StT:vw:xXy:Y")) != -1) {
> + "ac:D:deE:fF:i:lLnNOopP:qr:s:StT:vw:xXy:Y")) != -1) {
>   switch (i) {
>   case 'n':
>   nflag++;
> @@ -213,12 +245,21 @@ priv_init(int argc, char **argv)
>   infile = optarg;
>   break;
>  
> + case 'P':
> + child_pid = strtonum(optarg, 2, INT_MAX, );
> + if (errstr)
> + errx(1, "priv child %s: %s", errstr, optarg);
> + break;
> +
>   default:
>   /* nothing */
>   break;
>   }
>   }
>  
> + if (child_pid < 2)
> + errx(1, "exec without priv");
> +
>   if (RFileName != NULL) {
>   if (strcmp(RFileName, "-") != 0)
>   allowed_ext[STATE_INIT] |= ALLOW(PRIV_OPEN_DUMP);
> @@ -245,31 +286,30 @@ priv_init(int argc, char **argv)
>   cmdbuf = copy_argv([optind]);
>  
>   setproctitle("[priv]");
> - close(socks[1]);
>  
>   for (;;) {
> - if (may_read(socks[0], , sizeof(int)))
> + if (may_read(sock, , sizeof(int)))
>   break;
>   switch (cmd) {
>   case PRIV_OPEN_BPF:
>   test_state(cmd, STATE_BPF);
> - impl_open_bpf(socks[0], );
> + impl_open_bpf(sock, );
>   break;
>   case PRIV_OPEN_DUMP:
>   test_state(cmd, STATE_BPF);
> - impl_open_dump(socks[0], RFileName);
> +

tcpdump fork+exec?

2017-09-06 Thread Bryan Steele 
Hi,

This turned out easier then pflogd thanks to the existing privsep design
work done by deraadt@ and canacar@ many years ago. While tcpdump isn't a
daemon in the traditional sense, it isn't so uncommon for people to have
long running sessions. At least on OpenBSD, this is even safe thanks to
privsep and very strict pledge(2) promises.

This does shuffle things around a bit internally, but I don't think I've
totally broken anything.

Seems to work with some light testing, live captures, -w/-r offline pcap
files.

Comments?

-Bryan.

Index: privsep.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v
retrieving revision 1.45
diff -u -p -u -r1.45 privsep.c
--- usr.sbin/tcpdump/privsep.c  14 Jun 2017 20:48:54 -  1.45
+++ usr.sbin/tcpdump/privsep.c  7 Sep 2017 03:19:26 -
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -132,13 +133,10 @@ static void   logmsg(int, const char *, ..
 int
 priv_init(int argc, char **argv)
 {
-   int bpfd = -1;
-   int i, socks[2], cmd, nflag = 0;
+   int i, nargc, socks[2];
struct passwd *pw;
-   char *cmdbuf, *infile = NULL;
-   char *RFileName = NULL;
-   char *WFileName = NULL;
sigset_t allsigs, oset;
+   char childnum[11], **privargv;
 
closefrom(STDERR_FILENO + 1);
for (i = 1; i < _NSIG; i++)
@@ -155,7 +153,7 @@ priv_init(int argc, char **argv)
if (child_pid < 0)
err(1, "fork() failed");
 
-   if (child_pid) {
+   if (!child_pid) {
close(socks[0]);
priv_fd = socks[1];
 
@@ -188,14 +186,48 @@ priv_init(int argc, char **argv)
 
return (0);
}
+   close(socks[1]);
+
+   if (dup2(socks[0], 3) == -1)
+   err(1, "dup2 priv sock failed");
+   closefrom(4);
+
+   snprintf(childnum, sizeof(childnum), "%d", child_pid);
+   if ((privargv = reallocarray(NULL, argc + 3, sizeof(char *))) == NULL)
+   err(1, "alloc priv argv failed");
+   nargc = 0;
+   privargv[nargc++] = argv[0];
+   privargv[nargc++] = "-P";
+   privargv[nargc++] = childnum;
+   for (i = 1; i < argc; i++)
+   privargv[nargc++] = argv[i];
+   privargv[nargc] = NULL;
+   execvp(privargv[0], privargv);
+   err(1, "exec priv '%s' failed", privargv[0]);
+}
+
+__dead void
+priv_exec(int argc, char *argv[])
+{
+   int bpfd = -1;
+   int i, sock, cmd, nflag = 0;
+   char *cmdbuf, *infile = NULL;
+   char *RFileName = NULL;
+   char *WFileName = NULL;
+   const char *errstr;
+
+   sock = 3;
+
+   closefrom(4);
+   for (i = 1; i < _NSIG; i++)
+   signal(i, SIG_DFL);
 
-   sigprocmask(SIG_SETMASK, , NULL);
signal(SIGINT, SIG_IGN);
 
/* parse the arguments for required options */
opterr = 0;
while ((i = getopt(argc, argv,
-   "ac:D:deE:fF:i:lLnNOopqr:s:StT:vw:xXy:Y")) != -1) {
+   "ac:D:deE:fF:i:lLnNOopP:qr:s:StT:vw:xXy:Y")) != -1) {
switch (i) {
case 'n':
nflag++;
@@ -213,12 +245,21 @@ priv_init(int argc, char **argv)
infile = optarg;
break;
 
+   case 'P':
+   child_pid = strtonum(optarg, 2, INT_MAX, );
+   if (errstr)
+   errx(1, "priv child %s: %s", errstr, optarg);
+   break;
+
default:
/* nothing */
break;
}
}
 
+   if (child_pid < 2)
+   errx(1, "exec without priv");
+
if (RFileName != NULL) {
if (strcmp(RFileName, "-") != 0)
allowed_ext[STATE_INIT] |= ALLOW(PRIV_OPEN_DUMP);
@@ -245,31 +286,30 @@ priv_init(int argc, char **argv)
cmdbuf = copy_argv([optind]);
 
setproctitle("[priv]");
-   close(socks[1]);
 
for (;;) {
-   if (may_read(socks[0], , sizeof(int)))
+   if (may_read(sock, , sizeof(int)))
break;
switch (cmd) {
case PRIV_OPEN_BPF:
test_state(cmd, STATE_BPF);
-   impl_open_bpf(socks[0], );
+   impl_open_bpf(sock, );
break;
case PRIV_OPEN_DUMP:
test_state(cmd, STATE_BPF);
-   impl_open_dump(socks[0], RFileName);
+   impl_open_dump(sock, RFileName);
break;
case PRIV_OPEN_OUTPUT:
test_state(cmd, STATE_RUN);
-   impl_open_output(socks[0], WFileName);
+   impl_open_output(sock, WFileName);
break;

Re: SSE2 instructions emitted in libcompiler_rt

2017-09-06 Thread Mike Larkin 
On Wed, Sep 06, 2017 at 11:54:38PM +0200, Patrick Wildt wrote:
> On Wed, Sep 06, 2017 at 11:43:15PM +0200, Christian Weisgerber wrote:
> > Somebody noticed this on FreeBSD:
> > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221733
> > 
> > On i386, libcompiler_rt includes assembly implementations for
> > floating point conversion functions that unconditionally use SSE2
> > instructions, which are not supported by older CPUs.  Specifically,
> > these files:
> > 
> > floatdidf.S
> > floatdisf.S
> > floatdixf.S
> > floatundidf.S
> > floatundisf.S
> > floatundixf.S
> > 
> > We need to use the generic C implementation instead.  The way the
> > Makefile is currently set up, I guess we would just delete the
> > affected files under lib/libcompiler_rt/i386/?
> > 
> > -- 
> > Christian "naddy" Weisgerber  na...@mips.inka.de
> > 
> 
> Maybe this would already help?  Would at least not throw stones into the
> way of the next person doing an upgrade of compiler-rt...
> 
> Patrick
> 
> diff --git a/lib/libcompiler_rt/Makefile b/lib/libcompiler_rt/Makefile
> index f82c8156c2f..070b6e7cbde 100644
> --- a/lib/libcompiler_rt/Makefile
> +++ b/lib/libcompiler_rt/Makefile
> @@ -89,17 +89,11 @@ GEN_SRCS= absvdi2 \
>   fixunsxfti \
>   fixxfdi \
>   fixxfti \
> - floatdidf \
> - floatdisf \
> - floatdixf \
>   floatsidf \
>   floatsisf \
>   floattidf \
>   floattisf \
>   floattixf \
> - floatundidf \
> - floatundisf \
> - floatundixf \
>   floatunsidf \
>   floatunsisf \
>   floatuntidf \
> @@ -165,6 +159,22 @@ GEN_SRCS=absvdi2 \
>   umodsi3 \
>   umodti3
>  
> +.if ${RTARCH} == "i386"
> +SRCS+=   floatdidf.c \
> + floatdisf.c \
> + floatdixf.c.c \
> + floatundidf.c \
> + floatundisf.c \
> + floatundixf.c
> +.else
> +GEN_SRCS+=   floatdidf \
> + floatdisf \
> + floatdixf \
> + floatundidf \
> + floatundisf \
> + floatundixf
> +.endif
> +
>  .for file in ${GEN_SRCS}
>  .if exists(${.CURDIR}/${RTARCH}/${file}.S)
>  SRCS+=   ${file}.S
> 

this looks ok to me. I can run at least a build test later, but I don't
have any hardware that lacks these instructions.

-ml

Re: SSE2 instructions emitted in libcompiler_rt

2017-09-06 Thread Mike Larkin 
On Wed, Sep 06, 2017 at 11:43:15PM +0200, Christian Weisgerber wrote:
> Somebody noticed this on FreeBSD:
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221733
> 
> On i386, libcompiler_rt includes assembly implementations for
> floating point conversion functions that unconditionally use SSE2
> instructions, which are not supported by older CPUs.  Specifically,
> these files:
> 
> floatdidf.S
> floatdisf.S
> floatdixf.S
> floatundidf.S
> floatundisf.S
> floatundixf.S
> 
> We need to use the generic C implementation instead.  The way the
> Makefile is currently set up, I guess we would just delete the
> affected files under lib/libcompiler_rt/i386/?
> 
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 

Removing sse2 instructions on i386 seems to be the right thing to do.
Whether or not it's a simple makefile change or removing files from
libcompiler_rt/i386, I'm not sure.

Did you try to remove these files and did it work?

-ml

Re: SSE2 instructions emitted in libcompiler_rt

2017-09-06 Thread Patrick Wildt 
On Wed, Sep 06, 2017 at 11:43:15PM +0200, Christian Weisgerber wrote:
> Somebody noticed this on FreeBSD:
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221733
> 
> On i386, libcompiler_rt includes assembly implementations for
> floating point conversion functions that unconditionally use SSE2
> instructions, which are not supported by older CPUs.  Specifically,
> these files:
> 
> floatdidf.S
> floatdisf.S
> floatdixf.S
> floatundidf.S
> floatundisf.S
> floatundixf.S
> 
> We need to use the generic C implementation instead.  The way the
> Makefile is currently set up, I guess we would just delete the
> affected files under lib/libcompiler_rt/i386/?
> 
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 

Maybe this would already help?  Would at least not throw stones into the
way of the next person doing an upgrade of compiler-rt...

Patrick

diff --git a/lib/libcompiler_rt/Makefile b/lib/libcompiler_rt/Makefile
index f82c8156c2f..070b6e7cbde 100644
--- a/lib/libcompiler_rt/Makefile
+++ b/lib/libcompiler_rt/Makefile
@@ -89,17 +89,11 @@ GEN_SRCS=   absvdi2 \
fixunsxfti \
fixxfdi \
fixxfti \
-   floatdidf \
-   floatdisf \
-   floatdixf \
floatsidf \
floatsisf \
floattidf \
floattisf \
floattixf \
-   floatundidf \
-   floatundisf \
-   floatundixf \
floatunsidf \
floatunsisf \
floatuntidf \
@@ -165,6 +159,22 @@ GEN_SRCS=  absvdi2 \
umodsi3 \
umodti3
 
+.if ${RTARCH} == "i386"
+SRCS+= floatdidf.c \
+   floatdisf.c \
+   floatdixf.c.c \
+   floatundidf.c \
+   floatundisf.c \
+   floatundixf.c
+.else
+GEN_SRCS+= floatdidf \
+   floatdisf \
+   floatdixf \
+   floatundidf \
+   floatundisf \
+   floatundixf
+.endif
+
 .for file in ${GEN_SRCS}
 .  if exists(${.CURDIR}/${RTARCH}/${file}.S)
 SRCS+= ${file}.S

SSE2 instructions emitted in libcompiler_rt

2017-09-06 Thread Christian Weisgerber 
Somebody noticed this on FreeBSD:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221733

On i386, libcompiler_rt includes assembly implementations for
floating point conversion functions that unconditionally use SSE2
instructions, which are not supported by older CPUs.  Specifically,
these files:

floatdidf.S
floatdisf.S
floatdixf.S
floatundidf.S
floatundisf.S
floatundixf.S

We need to use the generic C implementation instead.  The way the
Makefile is currently set up, I guess we would just delete the
affected files under lib/libcompiler_rt/i386/?

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: [PATCH v2 2/2] VMD: Prevent vmd crashing when stopping a stopped vm

2017-09-06 Thread Carlos Cardenas 
On 2017-09-05 23:55, Mike Larkin wrote:
> On Mon, Sep 04, 2017 at 12:03:31AM -0700, Carlos Cardenas wrote:
>> * Fix logic handling stopping a VM.  Prevents VMD from crashing.
>> * Add additional error code to notify the user that a vm cannot be
>>   stopped when not running.
>> * Add additional log_debug statements.
>>
> 
> See below. Also this diff has tabs vs spaces problems like the
> previous one.
> 
> If we fix the style issues and you can shed some light on the part
> I noted below, I think we can get both these diffs in.
> 
> -ml
> 
>> diff --git usr.sbin/vmctl/vmctl.c usr.sbin/vmctl/vmctl.c
>> index 64d82ca847d..5fb7fbfd74c 100644
>> --- usr.sbin/vmctl/vmctl.c
>> +++ usr.sbin/vmctl/vmctl.c
>> @@ -439,12 +439,19 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
>>  vmr = (struct vmop_result *)imsg->data;
>>  res = vmr->vmr_result;
>>  if (res) {
>> -errno = res;
>> -if (res == ENOENT)
>> -warnx("vm not found");
>> -else
>> -warn("terminate vm command failed");
>> -*ret = EIO;
>> +   switch (res) {
>> +   case VMD_VM_STOP_INVALID:
>> +   warnx("cannot stop vm that is not running");
>> +   *ret = EINVAL;
>> +   break;
>> +   case ENOENT:
>> +   warnx("vm not found");
>> +   *ret = EIO;
>> +   break;
>> +   default:
>> +   warn("terminate vm command failed");
>> +   *ret = EIO;
>> +   }
>>  } else {
>>  warnx("sent request to terminate vm %d", vmr->vmr_id);
>>  *ret = 0;
>> @@ -453,6 +460,7 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
>>  warnx("unexpected response received from vmd");
>>  *ret = EINVAL;
>>  }
>> +errno = *ret;
>>  
>>  return (1);
>>  }
>> diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
>> index 22da6d58a7b..1240339db52 100644
>> --- usr.sbin/vmd/vmd.h
>> +++ usr.sbin/vmd/vmd.h
>> @@ -54,6 +54,7 @@
>>  #define VMD_BIOS_MISSING1001
>>  #define VMD_DISK_MISSING1002
>>  #define VMD_DISK_INVALID1003
>> +#define VMD_VM_STOP_INVALID 1004
>>  
>>  /* 100.64.0.0/10 from rfc6598 (IPv4 Prefix for Shared Address Space) */
>>  #define VMD_DHCP_PREFIX "100.64.0.0/10"
>> diff --git usr.sbin/vmd/vmm.c usr.sbin/vmd/vmm.c
>> index d3233147cff..66083a5f959 100644
>> --- usr.sbin/vmd/vmm.c
>> +++ usr.sbin/vmd/vmm.c
>> @@ -169,10 +169,9 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p, 
>> struct imsg *imsg)
>>  else
>>  res = 0;
>>  } else {
>> -/* Terminate VMs that are unknown or shutting down */
>> -vtp.vtp_vm_id = vm_vmid2id(vm->vm_vmid, vm);
>> -res = terminate_vm();
>> -vm_remove(vm);
>> +log_debug("%s: cannot stop vm that is not running",
>> +__func__);
>> +res = VMD_VM_STOP_INVALID;

> 
> Is this really what we want? (Was this the part that was crashing vmd?)
> This will break (I think) stopping a vm, then while it is shutting down
> during vmmci shutdown processing, stopping it again to force kill it.
> 
> Is the problem that we are doing vm_remove unconditionally, regardless of
> the result of the previous calls? (Eg, what if vm_vmid2id or terminate_vm
> failed?)
> 
Using the previous patch in the series, this is what is observed when
attempting to stop a running vm (normal use case):
control_dispatch_imsg vid: 1, name: , uid: 0
proc_compose_imsg: about to compose_event to PROC 0
proc_compose_imsg: about to compose_event to PROC 2
vmm_dispatch_parent: recv'ed TERMINATE_VM for 1
vmm_dispatch_parent: sending shutdown request to vm 1
proc_compose_imsg: about to compose_event to PROC 0
vmd_dispatch_vmm: forwarding TERMINATE VM for 1
proc_compose_imsg: about to compose_event to PROC 1
control_close
vmm_sighdlr: handling signal
vmm_sighdlr: attempting to terminate vm 1
terminate_vm: terminating vmid 1
proc_compose_imsg: about to compose_event to PROC 0
vmm_sighdlr: calling vm_remove
vm_remove: removing vm 1 from running config
vm_remove: calling vm_stop
vm_stop: stopping vm 1
vmd_dispatch_vmm: handling TERMINATE_EVENT for 1
vmd_dispatch_vmm: about to stop vm 1
vm_stop: stopping vm 1

For the non-normal cases:
* Stopping a non-running vm:
** if vm is NULL (means vmd and control has the vm in it's list but vmm
doesn't) -> vmd crash
** if vm is not NULL but is missing a VM managed by vmm (like attempting
to stop a non-running vm that has been configured by vm.conf),

If devs need any old Dell spares

2017-09-06 Thread Tom Smyth 
Hello
if devs need  some spares for older
dell systems / hp systems let me know
and Ill see if I have the parts.

DRAC Cards,
PERC Controlers
Power Supplies

Let me know and Ill ship them to you
Thanks
Tom Smyth

Re: want some working PCI-e 4Port Nics Atheros Chipset

2017-09-06 Thread Tom Smyth 
sorry forgot to mention these
are copper RJ45 cards (not sfp)

Thanks

On 6 September 2017 at 19:29, Tom Smyth  wrote:
>
> Hello lads & Ladies
>
> I have a few  Port Gb/s PCI-E (X4)  Cards
> from a few systems Im retiring
>
> Product name Mikrotik  RB44Ge
> Chipset  Atheros AR8131/M
> PCIe 4X
> Formfactor Full  / Half Height half Lenghth
>
> i have fullheight brackets on them and Ill
> try to root out the half height brackets if I can if required
>
> Im willing to ship them to Developers / porters
> if they think they would help them
>
>
> --
> Kindest regards,
> Tom Smyth
>

Need Hotswap Seagate 300GB 10K U320 SCSI Drives ?

2017-09-06 Thread Tom Smyth 
Hello Lads & Ladies

8x working  Ultra U320 10K 300G Seagate
Cheetah  ST337LC

I have a few 15K 73GB Ultra 320 Drives also

not super awesome but may help for spares
for aging systems that you want to test on

Im willing to ship them to Developers / porters
if they think they would help them


-- 
Kindest regards,
Tom Smyth

want some working PCI-e 4Port Nics Atheros Chipset

2017-09-06 Thread Tom Smyth 
Hello lads & Ladies

I have a few  Port Gb/s PCI-E (X4)  Cards
from a few systems Im retiring

Product name Mikrotik  RB44Ge
Chipset  Atheros AR8131/M
PCIe 4X
Formfactor Full  / Half Height half Lenghth

i have fullheight brackets on them and Ill
try to root out the half height brackets if I can if required

Im willing to ship them to Developers / porters
if they think they would help them


-- 
Kindest regards,
Tom Smyth

Re: softraid: force assemble issues

2017-09-06 Thread Patrick Wildt 
On Wed, Sep 06, 2017 at 07:10:15PM +0200, Patrick Wildt wrote:
> On Wed, Sep 06, 2017 at 05:44:21PM +0200, Patrick Wildt wrote:
> > Hi,
> > 
> > I'm testing some SoftRAID in a Mirroring setup and stumbled upon
> > something.  When I set a disk offline, zero out the disk and the
> > metadata blocks, reboot, and then try to assemble it, it will
> > fail.  Rightfully so, since the metadata information is completely
> > gone from one disk.  That's not a reassemble, that's a rebuild.
> > 
> > Now in the case that I want to actually force a whole new RAID
> > it looks like I have to pass -C force, to clear the metadata and
> > force creation of a new RAID.  Unfortunately this does not work,
> > because sr_meta_native_attach(), which is called by
> > sr_meta_native_attach(), wants to make sure that all devices are
> > either non-SoftRAID or SoftRAID.  Now in my case the disks are
> > kinda both.  One zeroed, one not.  In that case sr && not_sr is
> > true and it will exit with "not all chunks are of the native
> > metadata format".
> > 
> > Is there a reason that sr_meta_attach() has to be called before
> > clearing the metadata on force?
> > 
> > Untested diff below.
> > 
> > Patrick
> 
> Ah, well, sr_meta_clear() memsets sd->sd_meta, which is only later
> allocated during sr_meta_native_attach(), and sr_meta_read() of
> course also makes use of it.  Maybe sr_meta_native_attach() can
> be more lax.

Simply ignoring the "incorrect" metadata format when we are forcing
assembly (and thus clearing the metadata) makes it work.

Opinions?

Patrick

diff --git a/sys/dev/softraid.c b/sys/dev/softraid.c
index 8b75f238072..b46cc9e27a4 100644
--- a/sys/dev/softraid.c
+++ b/sys/dev/softraid.c
@@ -1658,7 +1658,7 @@ sr_meta_native_attach(struct sr_discipline *sd, int force)
not_sr++;
}
 
-   if (sr && not_sr) {
+   if (sr && not_sr && !force) {
sr_error(sc, "not all chunks are of the native metadata "
"format");
goto bad;

Re: lock(1): wipe hash before exit in one-time password case

2017-09-06 Thread Theo Buehler 
On Wed, Aug 30, 2017 at 08:49:20PM -0500, Scott Cheloha wrote:
> Hi,
> 
> In the one-time password case we want to wipe the hash itself
> before exit, right?

Yes, clearing s1 there makes no sense anymore, it was already zeroed out
further up.

ok tb

> 
> This must have slipped through when tedu@ patiently rewrote
> and committed my botched patch a little while back.
> 
> --
> Scott Cheloha
> 
> P.S. I didn't botch it this time, right?
> 
> Index: usr.bin/lock/lock.c
> ===
> RCS file: /cvs/src/usr.bin/lock/lock.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 lock.c
> --- usr.bin/lock/lock.c   8 Jul 2017 22:27:17 -   1.40
> +++ usr.bin/lock/lock.c   31 Aug 2017 01:41:09 -
> @@ -211,7 +211,7 @@ main(int argc, char *argv[])
>   }
>   } else if (crypt_checkpass(s, hash) == 0) {
>   explicit_bzero(s, sizeof(s));
> - explicit_bzero(s1, sizeof(s1));
> + explicit_bzero(hash, sizeof(hash));
>   break;
>   }
>   putc('\a', stderr);
>

Re: mg: extract exit status from pclose return value

2017-09-06 Thread Scott Cheloha 
~2 week bump.

Any thoughts or feedback?

--
Scott Cheloha

> On Aug 25, 2017, at 11:27 PM, Scott Cheloha  wrote:
> 
> Hi,
> 
> compile_mode() currently just reports the value returned by
> pclose(3).  This is incorrect because pclose gives you
> whatever wait4(2) returned, which needs to be examined
> with the various W* macros in  to derive a proper
> exit status.
> 
> This patch checks how the popen'd process exited and chooses
> an exit status as the shell would.
> 
> --
> Scott Cheloha
> 
> Index: usr.bin/mg/grep.c
> ===
> RCS file: /cvs/src/usr.bin/mg/grep.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 grep.c
> --- usr.bin/mg/grep.c 19 Mar 2015 21:48:05 -  1.44
> +++ usr.bin/mg/grep.c 26 Aug 2017 04:06:35 -
> @@ -2,8 +2,10 @@
> 
> /* This file is in the public domain */
> 
> -#include 
> #include 
> +#include 
> +#include 
> +
> #include 
> #include 
> #include 
> @@ -179,7 +181,7 @@ compile_mode(const char *name, const cha
>   FILE*fpipe;
>   char*buf;
>   size_t   len;
> - int  ret, n;
> + int  n, ret, status;
>   char cwd[NFILEN], qcmd[NFILEN];
>   char timestr[NTIME];
>   time_t   t;
> @@ -220,12 +222,13 @@ compile_mode(const char *name, const cha
>   addline(bp, buf);
>   }
>   ret = pclose(fpipe);
> + status = WIFEXITED(ret) ? WEXITSTATUS(ret) : 128 + WTERMSIG(ret);
>   t = time(NULL);
>   strftime(timestr, sizeof(timestr), "%a %b %e %T %Y", localtime());
>   addline(bp, "");
> - if (ret != 0)
> - addlinef(bp, "Command exited abnormally with code %d"
> - " at %s", ret, timestr);
> + if (status != 0)
> + addlinef(bp, "Command exited abnormally with status %d"
> + " at %s", status, timestr);
>   else
>   addlinef(bp, "Command finished at %s", timestr);
>

Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960

2017-09-06 Thread Andreas Bartelt 

On 09/06/17 16:24, Bob Beck wrote:

effectivelyu providing a limitless OCSP staple is kind of stupid - you may
as well simply *not staple*



I guess a stapled response without the next_update field set would be 
treated as valid until the client considers this_update to be too old 
(for ocspcheck, this seems to be set to 14 days via MAXAGE_SEC). In the 
case of stapling, I agree that it typically would be much better to use 
a short period for next_update than not to provide it at all.


In my case, I didn't want to use ocspcheck specifically for storing OCSP 
responses for stapling but in order to check if my local OCSP responder 
is actually working (i.e., the out-of-band way). In the out-of-band 
case, clients could also check for freshness by using nonces.


During these kinds of tests, I've also noticed that ocspcheck currently 
only connects to HTTP and HTTPS over their well-known ports which seems 
to be fine for all public CAs but not necessarily for all local CAs with 
a corresponding OCSP daemon.


In case this lack of flexibility is intended in order to keep the tool 
simple, I'm also fine with it.

softraid: force assemble issues

2017-09-06 Thread Patrick Wildt 
Hi,

I'm testing some SoftRAID in a Mirroring setup and stumbled upon
something.  When I set a disk offline, zero out the disk and the
metadata blocks, reboot, and then try to assemble it, it will
fail.  Rightfully so, since the metadata information is completely
gone from one disk.  That's not a reassemble, that's a rebuild.

Now in the case that I want to actually force a whole new RAID
it looks like I have to pass -C force, to clear the metadata and
force creation of a new RAID.  Unfortunately this does not work,
because sr_meta_native_attach(), which is called by
sr_meta_native_attach(), wants to make sure that all devices are
either non-SoftRAID or SoftRAID.  Now in my case the disks are
kinda both.  One zeroed, one not.  In that case sr && not_sr is
true and it will exit with "not all chunks are of the native
metadata format".

Is there a reason that sr_meta_attach() has to be called before
clearing the metadata on force?

Untested diff below.

Patrick

diff --git a/sys/dev/softraid.c b/sys/dev/softraid.c
index 8b75f238072..24d64fb8226 100644
--- a/sys/dev/softraid.c
+++ b/sys/dev/softraid.c
@@ -3368,9 +3368,6 @@ sr_ioctl_createraid(struct sr_softc *sc, struct 
bioc_createraid *bc,
goto unwind;
}
 
-   if (sr_meta_attach(sd, no_chunk, bc->bc_flags & BIOC_SCFORCE))
-   goto unwind;
-
/* force the raid volume by clearing metadata region */
if (bc->bc_flags & BIOC_SCFORCE) {
/* make sure disk isn't up and running */
@@ -3390,6 +3387,9 @@ sr_ioctl_createraid(struct sr_softc *sc, struct 
bioc_createraid *bc,
}
}
 
+   if (sr_meta_attach(sd, no_chunk, bc->bc_flags & BIOC_SCFORCE))
+   goto unwind;
+
no_meta = sr_meta_read(sd);
if (no_meta == -1) {

Re: warnings in pax

2017-09-06 Thread Alexander Bluhm 
On Wed, Sep 06, 2017 at 04:25:55PM +0200, Otto Moerbeek wrote:
> clang complains about a quite some signed compares in pax. This fixes
> a few of them, all of the form
> 
>   if (var < sizeof(...))
> 
> where var is an int.
> 
> Here a conversion of the int value to unsigned takes place, due to
> the conversion rules. This causes negative values of var not to be caught.
> By casting the sizeof value to int that wil happen. No more warning
> and better invalid value checking
> 
> OK?

OK bluhm@

> Index: cpio.c
> ===
> RCS file: /cvs/src/bin/pax/cpio.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 cpio.c
> --- cpio.c26 Aug 2016 04:11:16 -  1.30
> +++ cpio.c6 Sep 2017 14:18:58 -
> @@ -251,7 +251,7 @@ rd_ln_nm(ARCHD *arcn)
>  int
>  cpio_id(char *blk, int size)
>  {
> - if ((size < sizeof(HD_CPIO)) ||
> + if ((size < (int)sizeof(HD_CPIO)) ||
>   (strncmp(blk, AMAGIC, sizeof(AMAGIC) - 1) != 0))
>   return(-1);
>   return(0);
> @@ -488,7 +488,7 @@ cpio_wr(ARCHD *arcn)
>  int
>  vcpio_id(char *blk, int size)
>  {
> - if ((size < sizeof(HD_VCPIO)) ||
> + if ((size < (int)sizeof(HD_VCPIO)) ||
>   (strncmp(blk, AVMAGIC, sizeof(AVMAGIC) - 1) != 0))
>   return(-1);
>   return(0);
> @@ -505,7 +505,7 @@ vcpio_id(char *blk, int size)
>  int
>  crc_id(char *blk, int size)
>  {
> - if ((size < sizeof(HD_VCPIO)) ||
> + if ((size < (int)sizeof(HD_VCPIO)) ||
>   (strncmp(blk, AVCMAGIC, sizeof(AVCMAGIC) - 1) != 0))
>   return(-1);
>   return(0);
> @@ -799,7 +799,7 @@ vcpio_wr(ARCHD *arcn)
>  int
>  bcpio_id(char *blk, int size)
>  {
> - if (size < sizeof(HD_BCPIO))
> + if (size < (int)sizeof(HD_BCPIO))
>   return(-1);
>  
>   /*
>

fix -Wuninitialized in kernel

2017-09-06 Thread Alexander Bluhm 
Hi,

I have compiled the kernel with clang -Wuninitialized and would
like to fix these findings:
- toshiba_hotkey() is a bug
- in rasops_bitops.h is useless code
- in elf_load_file() it is nicer to call free(NULL, type, 0) instead
  of free(NULL, type, undefined).  Not a real bug as free(9) checks
  the pointer before the size, but the compiler cannot know that.
- nfs_connect() returns EINVAL at the beginning if nm_sotype is
  invalid.  But the compiler cannot know whether it has changed in
  the meantime, so in the else case a bunch of variables would not
  be initialized.  A panic() there changes the compiler assumptions
  and should not be reached anyway.

I did not fix all -Wuninitialized warnings:
- The warnings in dev/pci/drm/ are false positives and I don't want
  to touch the linux code.
- In net/art.c is a false positive and the compiler could know
  better.

ok?

bluhm

Index: dev/acpi/acpitoshiba.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/acpi/acpitoshiba.c,v
retrieving revision 1.8
diff -u -p -r1.8 acpitoshiba.c
--- dev/acpi/acpitoshiba.c  28 Feb 2017 10:39:07 -  1.8
+++ dev/acpi/acpitoshiba.c  6 Sep 2017 13:58:02 -
@@ -381,7 +381,7 @@ int
 toshiba_hotkey(struct aml_node *node, int notify, void *arg)
 {
struct acpitoshiba_softc *sc = arg;
-   int event, ret;
+   int event, ret = HCI_FAILURE;
 
event = toshiba_read_events(sc);
if (!event)
Index: dev/rasops/rasops_bitops.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/rasops/rasops_bitops.h,v
retrieving revision 1.6
diff -u -p -r1.6 rasops_bitops.h
--- dev/rasops/rasops_bitops.h  28 Aug 2010 12:48:14 -  1.6
+++ dev/rasops/rasops_bitops.h  6 Sep 2017 13:34:21 -
@@ -237,10 +237,8 @@ NAME(copycols)(void *cookie, int row, in
rnum = 32 - lnum;
db = dst & 31;
 
-   if ((src -= db) < 0) {
-   sp--;
+   if ((src -= db) < 0)
src += 32;
-   }
 
while (height--) {
sp = srp;
Index: kern/exec_elf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.140
diff -u -p -r1.140 exec_elf.c
--- kern/exec_elf.c 20 Mar 2017 00:05:21 -  1.140
+++ kern/exec_elf.c 6 Sep 2017 13:23:49 -
@@ -318,7 +318,7 @@ elf_load_file(struct proc *p, char *path
struct nameidata nd;
Elf_Ehdr eh;
Elf_Phdr *ph = NULL;
-   u_long phsize;
+   u_long phsize = 0;
Elf_Addr addr;
struct vnode *vp;
Elf_Phdr *base_ph = NULL;
Index: nfs/nfs_socket.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/nfs/nfs_socket.c,v
retrieving revision 1.127
diff -u -p -r1.127 nfs_socket.c
--- nfs/nfs_socket.c5 Sep 2017 08:02:48 -   1.127
+++ nfs/nfs_socket.c6 Sep 2017 13:31:24 -
@@ -357,6 +357,8 @@ nfs_connect(struct nfsmount *nmp, struct
sizeof (u_int32_t)) * 2;
rcvreserve = (nmp->nm_rsize + NFS_MAXPKTHDR +
sizeof (u_int32_t)) * 2;
+   } else {
+   panic("%s: nm_sotype %d", __func__, nmp->nm_sotype);
}
error = soreserve(so, sndreserve, rcvreserve);
if (error)

warnings in pax

2017-09-06 Thread Otto Moerbeek 
Hi,

clang complains about a quite some signed compares in pax. This fixes
a few of them, all of the form

if (var < sizeof(...))

where var is an int.

Here a conversion of the int value to unsigned takes place, due to
the conversion rules. This causes negative values of var not to be caught.
By casting the sizeof value to int that wil happen. No more warning
and better invalid value checking

OK?

-Otto

Index: cpio.c
===
RCS file: /cvs/src/bin/pax/cpio.c,v
retrieving revision 1.30
diff -u -p -r1.30 cpio.c
--- cpio.c  26 Aug 2016 04:11:16 -  1.30
+++ cpio.c  6 Sep 2017 14:18:58 -
@@ -251,7 +251,7 @@ rd_ln_nm(ARCHD *arcn)
 int
 cpio_id(char *blk, int size)
 {
-   if ((size < sizeof(HD_CPIO)) ||
+   if ((size < (int)sizeof(HD_CPIO)) ||
(strncmp(blk, AMAGIC, sizeof(AMAGIC) - 1) != 0))
return(-1);
return(0);
@@ -488,7 +488,7 @@ cpio_wr(ARCHD *arcn)
 int
 vcpio_id(char *blk, int size)
 {
-   if ((size < sizeof(HD_VCPIO)) ||
+   if ((size < (int)sizeof(HD_VCPIO)) ||
(strncmp(blk, AVMAGIC, sizeof(AVMAGIC) - 1) != 0))
return(-1);
return(0);
@@ -505,7 +505,7 @@ vcpio_id(char *blk, int size)
 int
 crc_id(char *blk, int size)
 {
-   if ((size < sizeof(HD_VCPIO)) ||
+   if ((size < (int)sizeof(HD_VCPIO)) ||
(strncmp(blk, AVCMAGIC, sizeof(AVCMAGIC) - 1) != 0))
return(-1);
return(0);
@@ -799,7 +799,7 @@ vcpio_wr(ARCHD *arcn)
 int
 bcpio_id(char *blk, int size)
 {
-   if (size < sizeof(HD_BCPIO))
+   if (size < (int)sizeof(HD_BCPIO))
return(-1);
 
/*

Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960

2017-09-06 Thread Bob Beck 
effectivelyu providing a limitless OCSP staple is kind of stupid - you may
as well simply *not staple*

On Wed, Sep 6, 2017 at 8:23 AM, Bob Beck  wrote:

> I'm not super inclined to make this "flexible" unless we see this used int
> the wild, which I have not. We are more restrictive than
> OpenSSL in many areas.
>
> On Wed, Sep 6, 2017 at 1:31 AM, Andreas Bartelt  wrote:
>
>> On 09/06/17 04:40, Bob Beck wrote:
>>
>>> Andreas where are you seeing this as being a real issue - who is shipping
>>> out OCSP responses without a next update field?
>>>
>>>
>> I've noticed this while playing with a local CA and a corresponding OCSP
>> responder on my LAN. For openssl ocsp, the -nmin or -ndays argument is
>> optional. If these arguments are not explicitly provided, the next update
>> field will not be set.
>>
>>
>>
>>>
>>> On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt 
>>> wrote:
>>>
>>> ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
 always provides a warning and no staplefile is written out. According to
 RFC 6960, the nextUpdate field is optional. The following patch should
 handle this case more gracefully and include a suitable debug message
 only
 in case -vv is specified.

 OK?

 Index: src/usr.sbin/ocspcheck/ocspcheck.c
 ===
 RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
 retrieving revision 1.21
 diff -u -p -u -r1.21 ocspcheck.c
 --- src/usr.sbin/ocspcheck/ocspcheck.c  8 May 2017 20:15:34 -
   1.21
 +++ src/usr.sbin/ocspcheck/ocspcheck.c  2 Sep 2017 17:09:00 -
 @@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
   {
  ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL,
 *nextupd =
 NULL;
  const unsigned char **p = (const unsigned char **)
 -   int status, cert_status=0, crl_reason=0;
 +   int status, cert_status=0, crl_reason=0, next_update=0;
  time_t now, rev_t = -1, this_t, next_t;
  OCSP_RESPONSE *resp;
  OCSP_BASICRESP *bresp;
 @@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
  return 0;
  }
  if ((next_t = parse_ocsp_time(nextupd)) == -1) {
 -   warnx("unable to parse next update time in OCSP reply");
 -   return 0;
 +   if (verbose >= 2)
 +   fprintf(stderr, "Optional timestamp for next
 update not included in OCSP reply\n");
  }
 +   else
 +   next_update = 1;

  /* Don't allow this update to precede next update */
 -   if (this_t >= next_t) {
 +   if (next_update == 1 && this_t >= next_t) {
  warnx("Invalid OCSP reply: this update >= next
 update");
  return 0;
  }
 @@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
  /*
   * Check that next update is still valid
   */
 -   if (next_t < now - JITTER_SEC) {
 +   if (next_update == 1 && next_t < now - JITTER_SEC) {
  warnx("Invalid OCSP reply: reply has expired (%s)",
  ctime(_t));
  return 0;
 @@ -489,7 +491,8 @@ validate_response(char *buf, size_t size

  vspew("OCSP response validated from %s\n", host);
  vspew("This Update: %s", ctime(_t));
 -   vspew("Next Update: %s", ctime(_t));
 +   if (next_update == 1)
 +   vspew("Next Update: %s", ctime(_t));
  return 1;
   }



>>>
>>
>

Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960

2017-09-06 Thread Bob Beck 
I'm not super inclined to make this "flexible" unless we see this used int
the wild, which I have not. We are more restrictive than
OpenSSL in many areas.

On Wed, Sep 6, 2017 at 1:31 AM, Andreas Bartelt  wrote:

> On 09/06/17 04:40, Bob Beck wrote:
>
>> Andreas where are you seeing this as being a real issue - who is shipping
>> out OCSP responses without a next update field?
>>
>>
> I've noticed this while playing with a local CA and a corresponding OCSP
> responder on my LAN. For openssl ocsp, the -nmin or -ndays argument is
> optional. If these arguments are not explicitly provided, the next update
> field will not be set.
>
>
>
>>
>> On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt  wrote:
>>
>> ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
>>> always provides a warning and no staplefile is written out. According to
>>> RFC 6960, the nextUpdate field is optional. The following patch should
>>> handle this case more gracefully and include a suitable debug message
>>> only
>>> in case -vv is specified.
>>>
>>> OK?
>>>
>>> Index: src/usr.sbin/ocspcheck/ocspcheck.c
>>> ===
>>> RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
>>> retrieving revision 1.21
>>> diff -u -p -u -r1.21 ocspcheck.c
>>> --- src/usr.sbin/ocspcheck/ocspcheck.c  8 May 2017 20:15:34 -
>>>   1.21
>>> +++ src/usr.sbin/ocspcheck/ocspcheck.c  2 Sep 2017 17:09:00 -
>>> @@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
>>>   {
>>>  ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL, *nextupd
>>> =
>>> NULL;
>>>  const unsigned char **p = (const unsigned char **)
>>> -   int status, cert_status=0, crl_reason=0;
>>> +   int status, cert_status=0, crl_reason=0, next_update=0;
>>>  time_t now, rev_t = -1, this_t, next_t;
>>>  OCSP_RESPONSE *resp;
>>>  OCSP_BASICRESP *bresp;
>>> @@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
>>>  return 0;
>>>  }
>>>  if ((next_t = parse_ocsp_time(nextupd)) == -1) {
>>> -   warnx("unable to parse next update time in OCSP reply");
>>> -   return 0;
>>> +   if (verbose >= 2)
>>> +   fprintf(stderr, "Optional timestamp for next
>>> update not included in OCSP reply\n");
>>>  }
>>> +   else
>>> +   next_update = 1;
>>>
>>>  /* Don't allow this update to precede next update */
>>> -   if (this_t >= next_t) {
>>> +   if (next_update == 1 && this_t >= next_t) {
>>>  warnx("Invalid OCSP reply: this update >= next update");
>>>  return 0;
>>>  }
>>> @@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
>>>  /*
>>>   * Check that next update is still valid
>>>   */
>>> -   if (next_t < now - JITTER_SEC) {
>>> +   if (next_update == 1 && next_t < now - JITTER_SEC) {
>>>  warnx("Invalid OCSP reply: reply has expired (%s)",
>>>  ctime(_t));
>>>  return 0;
>>> @@ -489,7 +491,8 @@ validate_response(char *buf, size_t size
>>>
>>>  vspew("OCSP response validated from %s\n", host);
>>>  vspew("This Update: %s", ctime(_t));
>>> -   vspew("Next Update: %s", ctime(_t));
>>> +   if (next_update == 1)
>>> +   vspew("Next Update: %s", ctime(_t));
>>>  return 1;
>>>   }
>>>
>>>
>>>
>>
>

Re: cwm: Don't ignore case when sorting exec menu

2017-09-06 Thread Okan Demirmen 
On Sun, Sep 3, 2017 at 3:43 PM,  wrote:

> Hi tech@
> ​,
>

​Hi,
​

> ​
>
> I wasn't able to execute "zzz" (lower case) from the exec menu in cwm.
> It
> skipped over it when ordering the search results because "ZZZ" (upper
> case) was already there
> ​.
>

​Indeed, I agree that case matters here.
​

> ​
>
> This is my first patch, so please let me know if anything else is
> needed.
> ​
>

​Thanks!
​

> ​
>
> Thanks,
>
> Ben
>
>
> Index: app/cwm/search.c
> ===
> RCS file: /cvs/xenocara/app/cwm/search.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 search.c
> --- app/cwm/search.c25 Apr 2017 13:40:33 -  1.62
> +++ app/cwm/search.c3 Sep 2017 19:21:44 -
> @@ -201,7 +201,7 @@ search_match_exec(struct menu_q *menuq,
> fnmatch(search, mi->text, 0) == FNM_NOMATCH)
> continue;
> TAILQ_FOREACH(mj, resultq, resultentry) {
> -   r = strcasecmp(mi->text, mj->text);
> +   r = strcmp(mi->text, mj->text);
> if (r < 0)
> TAILQ_INSERT_BEFORE(mj, mi,
> resultentry);
> if (r <= 0)
>
>

Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960

2017-09-06 Thread Andreas Bartelt 

On 09/06/17 04:40, Bob Beck wrote:

Andreas where are you seeing this as being a real issue - who is shipping
out OCSP responses without a next update field?



I've noticed this while playing with a local CA and a corresponding OCSP 
responder on my LAN. For openssl ocsp, the -nmin or -ndays argument is 
optional. If these arguments are not explicitly provided, the next 
update field will not be set.





On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt  wrote:


ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
always provides a warning and no staplefile is written out. According to
RFC 6960, the nextUpdate field is optional. The following patch should
handle this case more gracefully and include a suitable debug message only
in case -vv is specified.

OK?

Index: src/usr.sbin/ocspcheck/ocspcheck.c
===
RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 ocspcheck.c
--- src/usr.sbin/ocspcheck/ocspcheck.c  8 May 2017 20:15:34 -
  1.21
+++ src/usr.sbin/ocspcheck/ocspcheck.c  2 Sep 2017 17:09:00 -
@@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
  {
 ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL, *nextupd =
NULL;
 const unsigned char **p = (const unsigned char **)
-   int status, cert_status=0, crl_reason=0;
+   int status, cert_status=0, crl_reason=0, next_update=0;
 time_t now, rev_t = -1, this_t, next_t;
 OCSP_RESPONSE *resp;
 OCSP_BASICRESP *bresp;
@@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
 return 0;
 }
 if ((next_t = parse_ocsp_time(nextupd)) == -1) {
-   warnx("unable to parse next update time in OCSP reply");
-   return 0;
+   if (verbose >= 2)
+   fprintf(stderr, "Optional timestamp for next
update not included in OCSP reply\n");
 }
+   else
+   next_update = 1;

 /* Don't allow this update to precede next update */
-   if (this_t >= next_t) {
+   if (next_update == 1 && this_t >= next_t) {
 warnx("Invalid OCSP reply: this update >= next update");
 return 0;
 }
@@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
 /*
  * Check that next update is still valid
  */
-   if (next_t < now - JITTER_SEC) {
+   if (next_update == 1 && next_t < now - JITTER_SEC) {
 warnx("Invalid OCSP reply: reply has expired (%s)",
 ctime(_t));
 return 0;
@@ -489,7 +491,8 @@ validate_response(char *buf, size_t size

 vspew("OCSP response validated from %s\n", host);
 vspew("This Update: %s", ctime(_t));
-   vspew("Next Update: %s", ctime(_t));
+   if (next_update == 1)
+   vspew("Next Update: %s", ctime(_t));
 return 1;
  }

Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960

2017-09-06 Thread Andreas Bartelt 

On 09/06/17 04:40, Bob Beck wrote:

Andreas where are you seeing this as being a real issue - who is shipping
out OCSP responses without a next update field?



I've noticed this while playing with a local CA and a corresponding OCSP 
responder on my LAN. For openssl ocsp, the -nmin or -ndays argument is 
optional. If these arguments are not explicitly provided, the next 
update field will not be set.





On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt  wrote:


ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
always provides a warning and no staplefile is written out. According to
RFC 6960, the nextUpdate field is optional. The following patch should
handle this case more gracefully and include a suitable debug message only
in case -vv is specified.

OK?

Index: src/usr.sbin/ocspcheck/ocspcheck.c
===
RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 ocspcheck.c
--- src/usr.sbin/ocspcheck/ocspcheck.c  8 May 2017 20:15:34 -
  1.21
+++ src/usr.sbin/ocspcheck/ocspcheck.c  2 Sep 2017 17:09:00 -
@@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
  {
 ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL, *nextupd =
NULL;
 const unsigned char **p = (const unsigned char **)
-   int status, cert_status=0, crl_reason=0;
+   int status, cert_status=0, crl_reason=0, next_update=0;
 time_t now, rev_t = -1, this_t, next_t;
 OCSP_RESPONSE *resp;
 OCSP_BASICRESP *bresp;
@@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
 return 0;
 }
 if ((next_t = parse_ocsp_time(nextupd)) == -1) {
-   warnx("unable to parse next update time in OCSP reply");
-   return 0;
+   if (verbose >= 2)
+   fprintf(stderr, "Optional timestamp for next
update not included in OCSP reply\n");
 }
+   else
+   next_update = 1;

 /* Don't allow this update to precede next update */
-   if (this_t >= next_t) {
+   if (next_update == 1 && this_t >= next_t) {
 warnx("Invalid OCSP reply: this update >= next update");
 return 0;
 }
@@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
 /*
  * Check that next update is still valid
  */
-   if (next_t < now - JITTER_SEC) {
+   if (next_update == 1 && next_t < now - JITTER_SEC) {
 warnx("Invalid OCSP reply: reply has expired (%s)",
 ctime(_t));
 return 0;
@@ -489,7 +491,8 @@ validate_response(char *buf, size_t size

 vspew("OCSP response validated from %s\n", host);
 vspew("This Update: %s", ctime(_t));
-   vspew("Next Update: %s", ctime(_t));
+   if (next_update == 1)
+   vspew("Next Update: %s", ctime(_t));
 return 1;
  }

Re: [PATCH v2 2/2] VMD: Prevent vmd crashing when stopping a stopped vm

2017-09-06 Thread Mike Larkin 
On Mon, Sep 04, 2017 at 12:03:31AM -0700, Carlos Cardenas wrote:
> * Fix logic handling stopping a VM.  Prevents VMD from crashing.
> * Add additional error code to notify the user that a vm cannot be
>   stopped when not running.
> * Add additional log_debug statements.
> 

See below. Also this diff has tabs vs spaces problems like the
previous one.

If we fix the style issues and you can shed some light on the part
I noted below, I think we can get both these diffs in.

-ml

> diff --git usr.sbin/vmctl/vmctl.c usr.sbin/vmctl/vmctl.c
> index 64d82ca847d..5fb7fbfd74c 100644
> --- usr.sbin/vmctl/vmctl.c
> +++ usr.sbin/vmctl/vmctl.c
> @@ -439,12 +439,19 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
>   vmr = (struct vmop_result *)imsg->data;
>   res = vmr->vmr_result;
>   if (res) {
> - errno = res;
> - if (res == ENOENT)
> - warnx("vm not found");
> - else
> - warn("terminate vm command failed");
> - *ret = EIO;
> +   switch (res) {
> +   case VMD_VM_STOP_INVALID:
> +   warnx("cannot stop vm that is not running");
> +   *ret = EINVAL;
> +   break;
> +   case ENOENT:
> +   warnx("vm not found");
> +   *ret = EIO;
> +   break;
> +   default:
> +   warn("terminate vm command failed");
> +   *ret = EIO;
> +   }
>   } else {
>   warnx("sent request to terminate vm %d", vmr->vmr_id);
>   *ret = 0;
> @@ -453,6 +460,7 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
>   warnx("unexpected response received from vmd");
>   *ret = EINVAL;
>   }
> + errno = *ret;
>  
>   return (1);
>  }
> diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
> index 22da6d58a7b..1240339db52 100644
> --- usr.sbin/vmd/vmd.h
> +++ usr.sbin/vmd/vmd.h
> @@ -54,6 +54,7 @@
>  #define VMD_BIOS_MISSING 1001
>  #define VMD_DISK_MISSING 1002
>  #define VMD_DISK_INVALID 1003
> +#define VMD_VM_STOP_INVALID  1004
>  
>  /* 100.64.0.0/10 from rfc6598 (IPv4 Prefix for Shared Address Space) */
>  #define VMD_DHCP_PREFIX  "100.64.0.0/10"
> diff --git usr.sbin/vmd/vmm.c usr.sbin/vmd/vmm.c
> index d3233147cff..66083a5f959 100644
> --- usr.sbin/vmd/vmm.c
> +++ usr.sbin/vmd/vmm.c
> @@ -169,10 +169,9 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p, 
> struct imsg *imsg)
>   else
>   res = 0;
>   } else {
> - /* Terminate VMs that are unknown or shutting down */
> - vtp.vtp_vm_id = vm_vmid2id(vm->vm_vmid, vm);
> - res = terminate_vm();
> - vm_remove(vm);
> +log_debug("%s: cannot stop vm that is not running",
> +__func__);
> +res = VMD_VM_STOP_INVALID;

Is this really what we want? (Was this the part that was crashing vmd?)
This will break (I think) stopping a vm, then while it is shutting down
during vmmci shutdown processing, stopping it again to force kill it.

Is the problem that we are doing vm_remove unconditionally, regardless of
the result of the previous calls? (Eg, what if vm_vmid2id or terminate_vm
failed?)

>   }
>   cmd = IMSG_VMDOP_TERMINATE_VM_RESPONSE;
>   break;
> @@ -346,6 +345,8 @@ vmm_sighdlr(int sig, short event, void *arg)
>  
>   vmid = vm->vm_params.vmc_params.vcp_id;
>   vtp.vtp_vm_id = vmid;
> +log_debug("%s: attempting to terminate vm 
> %d",
> +__func__, vm->vm_vmid);
>   if (terminate_vm() == 0) {
>   memset(, 0, sizeof(vmr));
>   vmr.vmr_result = ret;
> @@ -505,7 +506,7 @@ vmm_dispatch_vm(int fd, short event, void *arg)
>   * supplied vm_terminate_params structure (vtp->vtp_vm_id)
>   *
>   * Parameters
> - *  vtp: vm_create_params struct containing the ID of the VM to terminate
> + *  vtp: vm_terminate_params struct containing the ID of the VM to terminate
>   *
>   * Return values:
>   *  0: success
> @@ -515,6 +516,7 @@ vmm_dispatch_vm(int fd, short event, void *arg)
>  int
>  terminate_vm(struct vm_terminate_params *vtp)
>  {
> +log_debug("%s: terminating vmid %d", __func__, vtp->vtp_vm_id);
>   if (ioctl(env->vmd_fd, VMM_IOC_TERM, vtp) < 0)
>   return (errno);
>  
> -- 
> 2.14.1
>

Re: [PATCH v2 1/2] VMD: Place log_debug statements in key places

2017-09-06 Thread Mike Larkin 
On Mon, Sep 04, 2017 at 12:03:30AM -0700, Carlos Cardenas wrote:
> Add log_debug statements in key places to assist with
> troubleshooting.
> 

These are fine, if you fix the spaces vs tab problem on most of
these added/changed lines.

-ml

> diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
> index 9ea87eb86e8..623a9b9d4f3 100644
> --- usr.sbin/vmd/config.c
> +++ usr.sbin/vmd/config.c
> @@ -81,14 +81,18 @@ config_purge(struct vmd *env, unsigned int reset)
>   struct vmd_switch   *vsw;
>   unsigned int what;
>  
> +log_debug("%s: purging vms and switches from running config",
> +__func__);
>   /* Reset global configuration (prefix was verified before) */
>   (void)host(VMD_DHCP_PREFIX, >vmd_cfg.cfg_localprefix);
>  
>   /* Reset other configuration */
>   what = ps->ps_what[privsep_process] & reset;
>   if (what & CONFIG_VMS && env->vmd_vms != NULL) {
> - while ((vm = TAILQ_FIRST(env->vmd_vms)) != NULL)
> + while ((vm = TAILQ_FIRST(env->vmd_vms)) != NULL) {
> +log_debug("%s: calling vm_remove", __func__);
>   vm_remove(vm);
> +}
>   env->vmd_nvm = 0;
>   }
>   if (what & CONFIG_SWITCHES && env->vmd_switches != NULL) {
> @@ -104,6 +108,7 @@ config_setconfig(struct vmd *env)
>   struct privsep  *ps = >vmd_ps;
>   unsigned int id;
>  
> +log_debug("%s: setting config", __func__);
>   for (id = 0; id < PROC_MAX; id++) {
>   if (id == privsep_process)
>   continue;
> @@ -117,6 +122,7 @@ config_setconfig(struct vmd *env)
>  int
>  config_getconfig(struct vmd *env, struct imsg *imsg)
>  {
> +log_debug("%s: retrieving config", __func__);
>   IMSG_SIZE_CHECK(imsg, >vmd_cfg);
>   memcpy(>vmd_cfg, imsg->data, sizeof(env->vmd_cfg));
>  
> @@ -129,6 +135,7 @@ config_setreset(struct vmd *env, unsigned int reset)
>   struct privsep  *ps = >vmd_ps;
>   unsigned int id;
>  
> +log_debug("%s: resetting state", __func__);
>   for (id = 0; id < PROC_MAX; id++) {
>   if ((reset & ps->ps_what[id]) == 0 ||
>   id == privsep_process)
> @@ -147,6 +154,7 @@ config_getreset(struct vmd *env, struct imsg *imsg)
>   IMSG_SIZE_CHECK(imsg, );
>   memcpy(, imsg->data, sizeof(mode));
>  
> +log_debug("%s: resetting state", __func__);
>   config_purge(env, mode);
>  
>   return (0);
> @@ -374,6 +382,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, 
> uint32_t peerid, uid_t uid)
>   free(tapfds);
>   }
>  
> +log_debug("%s: calling vm_remove", __func__);
>   vm_remove(vm);
>   errno = saved_errno;
>   if (errno == 0)
> @@ -406,6 +415,7 @@ config_getvm(struct privsep *ps, struct imsg *imsg)
>   imsg->fd = -1;
>   }
>  
> +log_debug("%s: calling vm_remove", __func__);
>   vm_remove(vm);
>   if (errno == 0)
>   errno = EINVAL;
> diff --git usr.sbin/vmd/control.c usr.sbin/vmd/control.c
> index 1af8a0d5e14..9263d0a3ae1 100644
> --- usr.sbin/vmd/control.c
> +++ usr.sbin/vmd/control.c
> @@ -276,6 +276,7 @@ control_close(int fd, struct control_sock *cs)
>  {
>   struct ctl_conn *c;
>  
> +log_debug("%s", __func__);
>   if ((c = control_connbyfd(fd)) == NULL) {
>   log_warn("%s: fd %d: not found", __func__, fd);
>   return;
> @@ -401,9 +402,14 @@ control_dispatch_imsg(int fd, short event, void *arg)
>   goto fail;
>   memcpy(, imsg.data, sizeof(vid));
>   vid.vid_uid = c->peercred.uid;
> +log_debug("%s vid: %d, name: %s, uid: %d",
> +__func__, vid.vid_id, vid.vid_name,
> +vid.vid_uid);
>  
>   if (proc_compose_imsg(ps, PROC_PARENT, -1,
>   imsg.hdr.type, fd, -1, , sizeof(vid)) == -1) {
> +log_debug("%s: proc_compose_imsg failed...",
> +__func__);
>   control_close(fd, cs);
>   return;
>   }
> diff --git usr.sbin/vmd/proc.c usr.sbin/vmd/proc.c
> index 183db93a678..2bf0a98c8ee 100644
> --- usr.sbin/vmd/proc.c
> +++ usr.sbin/vmd/proc.c
> @@ -756,6 +756,8 @@ proc_compose_imsg(struct privsep *ps, enum privsep_procid 
> id, int n,
>  
>   proc_range(ps, id, , );
>   for (; n < m; n++) {
> +log_debug("%s: about to compose_event to PROC %d", __func__,
> +id);
>   if (imsg_compose_event(>ps_ievs[id][n],
>   type, peerid, ps->ps_instance + 1, fd, data, datalen) == -1)
>   return (-1);
> diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
> index c7b9247d326..f34917e1a3b 100644
> ---