Re: remove elansc(4)

2023-01-16 Thread Jason McIntyre
On Tue, Jan 17, 2023 at 04:12:10PM +1100, Jonathan Gray wrote:
> AMD Elan SC520 (found on soekris net45xx) has a 486 class processor
> We require at least a 586/pentium class processor
> 

if you do, there's a little cleanup:

/usr/src/share/man/man4/gpio.4:.Cd "gpio* at elansc?" Pq i386
/usr/src/share/man/man4/pci.4:.It Xr elansc 4
/usr/src/share/man/man9/tc_init.9:.Xr elansc 4 ,

jmc



Re: make: arguments can have multiple variable assignments

2023-01-16 Thread Jason McIntyre
On Mon, Jan 16, 2023 at 10:03:03PM +, Klemens Nanni wrote:
> SYNOPSIS and usage say [NAME=value] while multiple assigments are fine:
> 
>   $ make -p FOO=1 BAR=2 | grep -e^FOO -e^BAR
>   BAR  = 2
>   FOO  = 1
> 
> I'm sure ports(7) wouldn't work if only one was accepted, hence it suprises
> me that none of the BSDs document it with triple dots, like targets.
> 
> Am I missing some detail here or is this just a very old oversight?
> 

seems correct. posix spec shows "..." too.
jmc

> 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/make/main.c,v
> retrieving revision 1.128
> diff -u -p -r1.128 main.c
> --- main.c4 Dec 2022 23:50:48 -   1.128
> +++ main.c16 Jan 2023 21:52:48 -
> @@ -891,7 +891,7 @@ usage()
>   (void)fprintf(stderr,
>  "usage: make [-BeiknpqrSst] [-C directory] [-D variable] [-d flags] [-f 
> mk]\n\
>   [-I directory] [-j max_processes] [-m directory] [-V variable]\n\
> - [NAME=value] [target ...]\n");
> + [NAME=value ...] [target ...]\n");
>   exit(2);
>  }
>  
> Index: make.1
> ===
> RCS file: /cvs/src/usr.bin/make/make.1,v
> retrieving revision 1.138
> diff -u -p -r1.138 make.1
> --- make.128 Dec 2022 13:00:57 -  1.138
> +++ make.116 Jan 2023 21:57:18 -
> @@ -47,7 +47,7 @@
>  .Op Fl j Ar max_processes
>  .Op Fl m Ar directory
>  .Op Fl V Ar variable
> -.Op Ar NAME Ns = Ns Ar value
> +.Op Ar NAME Ns = Ns Ar value ...
>  .Bk -words
>  .Op Ar target ...
>  .Ek
> 



remove elansc(4)

2023-01-16 Thread Jonathan Gray
AMD Elan SC520 (found on soekris net45xx) has a 486 class processor
We require at least a 586/pentium class processor

diff --git share/man/man4/man4.i386/Makefile share/man/man4/man4.i386/Makefile
index 3fae7fbbe42..55d52939e28 100644
--- share/man/man4/man4.i386/Makefile
+++ share/man/man4/man4.i386/Makefile
@@ -2,7 +2,7 @@
 #  from: @(#)Makefile  5.1 (Berkeley) 2/12/91
 #  Id: Makefile,v 1.4 1995/12/14 05:41:38 deraadt Exp $
 
-MAN=   amdpcib.4 amdmsr.4 apm.4 autoconf.4 bios.4 cpu.4 elansc.4 \
+MAN=   amdpcib.4 amdmsr.4 apm.4 autoconf.4 bios.4 cpu.4 \
esm.4 geodesc.4 glxpcib.4 glxsb.4 gscpcib.4 gscpm.4 gus.4 ie.4 \
ichpcib.4 intro.4 ioapic.4 \
joy.4 le.4 lms.4 mem.4 mms.4 mpbios.4 mtrr.4 npx.4 nvram.4 \
diff --git share/man/man4/man4.i386/elansc.4 share/man/man4/man4.i386/elansc.4
deleted file mode 100644
index a34429fc2bd..000
--- share/man/man4/man4.i386/elansc.4
+++ /dev/null
@@ -1,97 +0,0 @@
-.\"$OpenBSD: elansc.4,v 1.22 2013/08/14 06:32:33 jmc Exp $
-.\"$NetBSD: elansc.4,v 1.1 2002/08/12 03:45:25 thorpej Exp $
-.\"
-.\" Copyright (c) 2002 The NetBSD Foundation, Inc.
-.\" All rights reserved.
-.\"
-.\" This code is derived from software contributed to The NetBSD Foundation
-.\" by Jason R. Thorpe.
-.\"
-.\" Redistribution and use in source and binary forms, with or without
-.\" modification, are permitted provided that the following conditions
-.\" are met:
-.\" 1. Redistributions of source code must retain the above copyright
-.\"notice, this list of conditions and the following disclaimer.
-.\" 2. Redistributions in binary form must reproduce the above copyright
-.\"notice, this list of conditions and the following disclaimer in the
-.\"documentation and/or other materials provided with the distribution.
-.\"
-.\" THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
-.\" ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
-.\" TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
-.\" PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
-.\" BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
-.\" CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
-.\" SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
-.\" INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
-.\" CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
-.\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
-.\" POSSIBILITY OF SUCH DAMAGE.
-.\"
-.Dd $Mdocdate: August 14 2013 $
-.Dt ELANSC 4 i386
-.Os
-.Sh NAME
-.Nm elansc
-.Nd AMD Elan SC520 System Controller with watchdog timer and GPIO
-.Sh SYNOPSIS
-.Cd "elansc* at pci?"
-.Cd "gpio* at elansc?"
-.Sh DESCRIPTION
-The
-.Nm
-driver supports the system controller of the AMD Elan SC520 microcontroller.
-The SC520 consists of an AMD Am5x86 processor core, integrated PCI host
-controller, and several standard on-chip devices, such as NS16550-compatible
-UARTs, real-time clock, and timers.
-.Pp
-The Elan SC520 also provides several special on-chip devices.
-The following are supported by the
-.Nm
-driver:
-.Bl -bullet
-.It
-Watchdog timer.
-The watchdog timer may be configured for a 1 second, 2 second, 4 second,
-8 second, 16 second, or 32 second expiration period.
-.It
-Programmable Input/Output.
-The SC520 microcontroller supports 32 programmable I/O signals (PIOs)
-that can be used on the system board to monitor signals or control devices
-that are not handled by the other functions in the SC520 microcontroller.
-These signals can be programmed to be inputs or to be driven out
-.Dq high
-or
-.Dq low
-as outputs.
-Pins can be accessed through the
-.Xr gpio 4
-framework.
-The
-.Xr gpioctl 8
-program allows easy manipulation of pins from userland.
-.El
-.Sh SEE ALSO
-.Xr gpio 4 ,
-.Xr intro 4 ,
-.Xr watchdog 4 ,
-.Xr gpioctl 8 ,
-.Xr sysctl 8
-.Sh HISTORY
-Support for the
-.Nm
-was added in
-.Ox 3.3 .
-PIO function support appeared in
-.Ox 3.6 .
-.Sh AUTHORS
-.An -nosplit
-The
-.Nm
-driver was written by
-.An Jason R. Thorpe Aq Mt thor...@netbsd.org .
-.An Jasper Wallace
-provided the work-around for a hardware bug related to the watchdog timer
-in some steppings of the SC520 CPU.
-.An Alexander Yurchenko Aq Mt gra...@openbsd.org
-added support for the PIO function.
diff --git sys/arch/i386/conf/GENERIC sys/arch/i386/conf/GENERIC
index 85836aa74c5..43d454f211e 100644
--- sys/arch/i386/conf/GENERIC
+++ sys/arch/i386/conf/GENERIC
@@ -98,8 +98,6 @@ amas* at pci? disable # AMD memory configuration
 pchtemp* at pci?   # Intel C610 temperature sensor
 
 # power management and other environmental stuff
-elansc*at pci? # AMD Elan SC520 System Controller
-gpio*  at elansc?
 geodesc* at pci?   # Geode SC1100/SCx200 IAOC
 #gscpm*at pci? # NS Geode 

make: arguments can have multiple variable assignments

2023-01-16 Thread Klemens Nanni
SYNOPSIS and usage say [NAME=value] while multiple assigments are fine:

$ make -p FOO=1 BAR=2 | grep -e^FOO -e^BAR
BAR  = 2
FOO  = 1

I'm sure ports(7) wouldn't work if only one was accepted, hence it suprises
me that none of the BSDs document it with triple dots, like targets.

Am I missing some detail here or is this just a very old oversight?


Index: main.c
===
RCS file: /cvs/src/usr.bin/make/main.c,v
retrieving revision 1.128
diff -u -p -r1.128 main.c
--- main.c  4 Dec 2022 23:50:48 -   1.128
+++ main.c  16 Jan 2023 21:52:48 -
@@ -891,7 +891,7 @@ usage()
(void)fprintf(stderr,
 "usage: make [-BeiknpqrSst] [-C directory] [-D variable] [-d flags] [-f mk]\n\
[-I directory] [-j max_processes] [-m directory] [-V variable]\n\
-   [NAME=value] [target ...]\n");
+   [NAME=value ...] [target ...]\n");
exit(2);
 }
 
Index: make.1
===
RCS file: /cvs/src/usr.bin/make/make.1,v
retrieving revision 1.138
diff -u -p -r1.138 make.1
--- make.1  28 Dec 2022 13:00:57 -  1.138
+++ make.1  16 Jan 2023 21:57:18 -
@@ -47,7 +47,7 @@
 .Op Fl j Ar max_processes
 .Op Fl m Ar directory
 .Op Fl V Ar variable
-.Op Ar NAME Ns = Ns Ar value
+.Op Ar NAME Ns = Ns Ar value ...
 .Bk -words
 .Op Ar target ...
 .Ek



libevent manuals

2023-01-16 Thread Ted Bullock
Hi folks,

I'm writing manual pages for the libevent variant bundled with OpenBSD,
this is a documentation project that I started in something like 2012
but got pulled away from due to life circumstances. I've returned to
it after reading an old Things to Do - *urgent* list I left in a binder
over a decade ago. heh.

The impetus is that the event(3) manual page isn't all that good for
documenting how to use the library and it is missing functions that are
included in the API and available in the shared library.

Upstream of course has moved on to version 2.x, as far as I can tell
there is no more maintenance being done on the 1.4 version that OpenBSD
ships except the work that is done internally in the OS.

Anyway, here is what I came up with for a few functions after reading
the source tree. I also went back through historical releases to see
how the API evolved over time.

I'm planning to finish off documenting the rest of the library, if
folks have feedback, I'm interested.

Take Care,

event_init.3


.\" Copyright (c) 2023 Ted Bullock 
.\"
.\" Permission to use, copy, modify, and distribute this software for any
.\" purpose with or without fee is hereby granted, provided that the above
.\" copyright notice and this permission notice appear in all copies.
.\"
.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
.\"
.Dd $Mdocdate: January 9 2023 $
.Dt EVENT_INIT 3
.Os
.Sh NAME
.Nm event_base_new ,
.Nm event_init
.Nd initializes an
.Vt event_base
instance
.Sh SYNOPSIS
.In event.h
.Ft "struct event_base *"
.Fn event_base_new void
.Ft "struct event_base *"
.Fn event_init void
.Sh DESCRIPTION
The functions
.Fn event_base_new
and
.Fn event_init
initialize the
.Lb libevent
and need to be called prior to scheduling an event or
starting the event-loop. The two functions are similar, although global
variables are set when calling
.Fn event_init
to help simplify the API for programs requiring only one event-loop.
.Pp
The
.Lb libevent
API is generally not thread-safe unless only one
.Vt "event_base"
instance is accessible per thread or care is taken to lock access.
.Pp
These functions allocate memory that should be freed using
.Xr event_base_free 3 .
.Pp
Diagnostic and error messages are retrievable from these functions by
configuring a message log callback with
.Xr event_set_log_callback 3 .
.Pp
After a call to
.Xr fork 2
some notification methods will not persist and need to reinitiliazed with
.Xr event_reinit 3 .
.Sh RETURN VALUES
The
.Fn event_base_new
and
.Fn event_init
functions return a
.Vt "struct event_base"
pointer.
If the functions fail, for example, due to a lack of memory, they do
.Em NOT
return.
.Sh ENVIRONMENT
Programs using the
.Lb libevent
are configurable with environment variables prior to calling
.Fn event_base_new
or
.Fn event_init .
.Bl -tag
.It Ev Sy EVENT_SHOW_METHOD
If the log callback is configured, report which kernel notification method the
library is using.
.It Ev Sy EVENT_NOKQUEUE
Disable library support for
.Xr kqueue 2
.It Ev Sy EVENT_NOPOLL
Disable library support for
.Xr poll 2
.It Ev Sy EVENT_NOSELECT
Disable library support for
.Xr select 2
.El
.Sh EXAMPLES
In this example code a new
.Vt "struct event_base"
is initialized with support for logging.
This example also applies to
.Fn event_base_new ,
the difference being that internal global variables are configured to simplify
other parts of the API.
.Bd -literal -offset indent
#include 
#include 

void cb(int sev, const char* msg)
{
printf("%d: %s\n", sev, msg);
}

int main()
{
event_set_log_callback(cb);

struct event_base *base = event_init();
if (base == NULL) {
printf("event_init failed, will not return\\n");
return 1;
}
/* do something with the event library here */

event_base_free(base);
return 0;
}
.Ed
.Sh SEE ALSO
.Xr event_set_log_callback 3 ,
.Xr event_base_free 3 ,
.Xr event_dispatch 3
.Sh HISTORY
The function
.Fn event_init
first appeared in
.Lb libevent-0.1
although it's protoype has changed various times since that release. In
.Lb libevent-1.4.1beta
.Fn event_base_new
was added and
.Fn event_init
was once again changed to wrap around the new function.
.Pp
Environment variable options first appeared in
.Lb libevent-0.7a .
.Sh AUTHORS
The
.Lb libevent
was written by
.An -nosplit
.An Niels Provos
and
.An Nick Mathewson
.Pp
This manual was written by
.An Ted Bullock Aq Mt tbull...@comlore.com


event_set_log_callback.3


.\" Copyright (c) 

help wanted for a specific clang diff

2023-01-16 Thread Theo de Raadt
For this xonly work, we are having to one-by-one find .S files that
are putting data tables into the .text segment

I am hoping to find someone who can do c++ well enough, and maybe
has some familiarity with the clang code, to add a warning message
for this

if a .long, .quad, .byte are placed into a .text section, issue
a warning, then we'll be able to identify all these in a ports build
and decide which need manual fixing, and move the objects into .rodata
and apply __PIC__ handling as neccessary

Yes, there are cases where people use .long to inject an instruction
they don't believe the assembler has.  We can ignore those by inspect.

Can anyone help?  It doesn't need to be fancy, it just needs to get
us moving faster.

Thanks



Re: sshd relinking

2023-01-16 Thread Job Snijders
On Mon, Jan 16, 2023 at 08:57:25AM -0700, Theo de Raadt wrote:
> I propose to relink sshd on every boot, before it gets started.
> 
> This is like kernel, libc.so, libcrypto, and ld.so relinking.
> 
> The sshd design self-protects itself quite well, but this kind of
> address space secrecy is still a good addition.
> 
> Since the sshd binary becomes unique on every openbsd machine, we
> can also block a logged in user from inspecting it, and then using
> that information as part of a remote attack, so mode 511.
> 
> I am surprised how this turned out.  This could easily be done with
> a few other important daemons or tools.

Indeed seems easy, here is the bits for ntpd (patch on top of yours)

Index: etc/rc
===
RCS file: /cvs/src/etc/rc,v
retrieving revision 1.568
diff -u -p -r1.568 rc
--- etc/rc  28 Dec 2022 09:53:33 -  1.568
+++ etc/rc  16 Jan 2023 16:53:15 -
@@ -237,7 +237,7 @@
) || { _error=true; break; }
done
 
-   for _bin in $_relink/usr/sbin/sshd ; do
+   for _bin in $_relink/usr/sbin/{ntpd,sshd} ; do
_tmpdir=$(mktemp -dq $_relink/_rebuild.) &&
(
set -o errexit
Index: usr.sbin/ntpd/Makefile
===
RCS file: /cvs/src/usr.sbin/ntpd/Makefile,v
retrieving revision 1.16
diff -u -p -r1.16 Makefile
--- usr.sbin/ntpd/Makefile  20 Nov 2015 18:53:42 -  1.16
+++ usr.sbin/ntpd/Makefile  16 Jan 2023 16:53:15 -
@@ -1,6 +1,7 @@
 #  $OpenBSD: Makefile,v 1.16 2015/11/20 18:53:42 tedu Exp $
 
 PROG=  ntpd
+BINMODE=511
 SRCS=  ntpd.c log.c ntp.c ntp_msg.c parse.y config.c \
server.c client.c sensors.c util.c ntp_dns.c \
control.c constraint.c
@@ -17,3 +18,23 @@ LINKS=   ${BINDIR}/ntpd ${BINDIR}/ntpctl
 MAN=   ntpd.8 ntpd.conf.5 ntpctl.8
 
 .include 
+
+# The relink kit, used on OpenBSD by /etc/rc
+
+Makefile.relink: ${.CURDIR}/../Makefile.inc ${.CURDIR}/Makefile
+   # XXX assume a concatenation of these is OK
+   cat ${.CURDIR}/../Makefile.inc ${.CURDIR}/Makefile > Makefile.relink
+
+ntpd.tar: ${OBJS} Makefile.relink
+   tar cf $@ ${OBJS} Makefile.relink
+
+afterinstall: ntpd.tar
+   install -d -o root -g wheel -m 755 \
+   ${DESTDIR}/usr/share/relink/usr/sbin/ntpd
+   install -o ${BINOWN} -g ${BINGRP} -m 640 \
+   ntpd.tar ${DESTDIR}/usr/share/relink/usr/sbin/ntpd/ntpd.tar
+
+relink:
+   cc -o ntpd `echo ${OBJS} | tr ' ' '\n' | sort -R` ${LDADD}
+   ./ntpd -n -f /etc/examples/ntpd.conf 2> /dev/zero && \
+   install -o root -g wheel -m ${BINMODE} ntpd /usr/sbin/ntpd



sshd relinking

2023-01-16 Thread Theo de Raadt
I propose to relink sshd on every boot, before it gets started.

This is like kernel, libc.so, libcrypto, and ld.so relinking.

The sshd design self-protects itself quite well, but this kind of
address space secrecy is still a good addition.

Since the sshd binary becomes unique on every openbsd machine, we
can also block a logged in user from inspecting it, and then using
that information as part of a remote attack, so mode 511.

I am surprised how this turned out.  This could easily be done with
a few other important daemons or tools.

Index: usr.bin/ssh/sshd/Makefile
===
RCS file: /cvs/src/usr.bin/ssh/sshd/Makefile,v
retrieving revision 1.106
diff -u -p -u -r1.106 Makefile
--- usr.bin/ssh/sshd/Makefile   27 May 2022 05:02:46 -  1.106
+++ usr.bin/ssh/sshd/Makefile   16 Jan 2023 15:29:01 -
@@ -14,6 +14,7 @@ SRCS+=${SRCS_BASE} ${SRCS_KEX} ${SRCS_K
${SRCS_SK_CLIENT}
 
 PROG=  sshd
+BINMODE=511
 BINDIR=/usr/sbin
 MAN=   sshd.8 sshd_config.5
 
@@ -46,3 +47,22 @@ DPADD+=  ${LIBUTIL}
 LDADD+=-lz
 DPADD+=${LIBZ}
 .endif
+
+# The relink kit, used on OpenBSD by /etc/rc
+
+Makefile.relink: ${.CURDIR}/../Makefile.inc ${.CURDIR}/Makefile
+   # XXX assume a concatenation of these is OK
+   cat ${.CURDIR}/../Makefile.inc ${.CURDIR}/Makefile > Makefile.relink
+
+sshd.tar: ${OBJS} Makefile.relink
+   tar cf $@ ${OBJS} Makefile.relink
+
+afterinstall: sshd.tar
+   install -d -o root -g wheel -m 755 \
+   ${DESTDIR}/usr/share/relink/usr/sbin/sshd
+   install -o ${BINOWN} -g ${BINGRP} -m 640 \
+   sshd.tar ${DESTDIR}/usr/share/relink/usr/sbin/sshd/sshd.tar
+
+relink:
+   cc -o sshd `echo ${OBJS} | tr ' ' '\n' | sort -R` ${LDADD}
+   ./sshd -t && install -o root -g wheel -m ${BINMODE} sshd /usr/sbin/sshd
Index: etc/rc
===
RCS file: /cvs/src/etc/rc,v
retrieving revision 1.568
diff -u -p -u -r1.568 rc
--- etc/rc  28 Dec 2022 09:53:33 -  1.568
+++ etc/rc  16 Jan 2023 07:59:15 -
@@ -188,7 +188,7 @@ reorder_libs() {
fi
done
 
-   echo 'reordering libraries:'
+   echo 'reordering:'
 
# Remount the (read-only) filesystems in _ro_list as read-write.
for _mp in $_ro_list; do
@@ -237,6 +237,19 @@ reorder_libs() {
) || { _error=true; break; }
done
 
+   for _bin in $_relink/usr/sbin/sshd; do
+   _tmpdir=$(mktemp -dq $_relink/_rebuild.) &&
+   (
+   set -o errexit
+   cd $_tmpdir
+   _binn=${_bin##*/}
+   _bint=${_bin}/${_binn}.tar
+   echo " $_binn"
+   tar xf $_bint
+   make -f Makefile.relink relink >/dev/null
+   ) || { _error=true; break; }
+   done
+   
rm -rf $_relink/_rebuild.*
 
# Restore previous mount state if it was changed.



KRL 4/4: regression test for signing/verification

2023-01-16 Thread Damien Miller
Hi,

The final OpenSSH key revocation list (KRL) diff for now :)

This extends the existing krl.sh regression test to exercise signing and
verification. (This depends on the last two diffs)

ok?

Index: krl.sh
===
RCS file: /cvs/src/regress/usr.bin/ssh/krl.sh,v
retrieving revision 1.12
diff -u -p -r1.12 krl.sh
--- krl.sh  16 Jan 2023 04:11:29 -  1.12
+++ krl.sh  16 Jan 2023 08:00:35 -
@@ -1,4 +1,4 @@
-#  $OpenBSD: krl.sh,v 1.12 2023/01/16 04:11:29 djm Exp $
+#  $OpenBSD: krl.sh,v 1.11 2019/12/16 02:39:05 djm Exp $
 #  Placed in the Public Domain.
 
 tid="key revocation lists"
@@ -22,7 +22,16 @@ done
 # Old keys will interfere with ssh-keygen.
 rm -f $OBJ/revoked-* $OBJ/krl-*
 
-# Generate a CA key
+# Generate some KRL signing keys
+$SSHKEYGEN -t ed25519 -f $OBJ/krl-sign  -C "" -N "" > /dev/null ||
+   fatal "$SSHKEYGEN signing key failed"
+$SSHKEYGEN -t ed25519 -f $OBJ/krl-sign-wrong  -C "" -N "" > /dev/null ||
+   fatal "$SSHKEYGEN signing key-wrong failed"
+$SSHKEYGEN -t ed25519 -f $OBJ/krl-sign2 -C "" -N "" > /dev/null ||
+   fatal "$SSHKEYGEN signing key2 failed"
+$SSHKEYGEN -t ed25519 -f $OBJ/krl-sign3 -C "" -N "" > /dev/null ||
+   fatal "$SSHKEYGEN signing key3 failed"
+# Generate some CA keys
 $SSHKEYGEN -t $ktype1 -f $OBJ/revoked-ca  -C "" -N "" > /dev/null ||
fatal "$SSHKEYGEN CA failed"
 $SSHKEYGEN -t $ktype2 -f $OBJ/revoked-ca2  -C "" -N "" > /dev/null ||
@@ -108,7 +117,14 @@ for rkey in $RKEYS; do
 done
 
 genkrls() {
-   OPTS=$1
+   #OPTS="-vvv $@"
+   OPTS="$@"
+
+$SSHKEYGEN $OPTS -kf $OBJ/krl-revoked-signing $OBJ/krl-sign2.pub \
+   >/dev/null || fatal "$SSHKEYGEN KRL failed"
+$SSHKEYGEN $OPTS -kf $OBJ/krl-revoked-signing2 \
+$OBJ/krl-sign2.pub $OBJ/krl-sign3.pub \
+   >/dev/null || fatal "$SSHKEYGEN KRL failed"
 $SSHKEYGEN $OPTS -kf $OBJ/krl-empty - /dev/null || fatal "$SSHKEYGEN KRL failed"
 $SSHKEYGEN $OPTS -kf $OBJ/krl-keys $RKEYS \
@@ -136,9 +152,9 @@ $SSHKEYGEN $OPTS -kf $OBJ/krl-serial -s 
 $SSHKEYGEN $OPTS -kf $OBJ/krl-keyid -s $OBJ/revoked-ca.pub \
$OBJ/revoked-keyid >/dev/null || fatal "$SSHKEYGEN KRL failed"
 # These should succeed; they specify an wildcard CA key.
-$SSHKEYGEN $OPTS -kf $OBJ/krl-serial-wild -s NONE $OBJ/revoked-serials \
+$SSHKEYGEN $OPTS -kf $OBJ/krl-srl-wild -s NONE $OBJ/revoked-serials \
>/dev/null || fatal "$SSHKEYGEN KRL failed"
-$SSHKEYGEN $OPTS -kf $OBJ/krl-keyid-wild -s NONE $OBJ/revoked-keyid \
+$SSHKEYGEN $OPTS -kf $OBJ/krl-id-wild -s NONE $OBJ/revoked-keyid \
>/dev/null || fatal "$SSHKEYGEN KRL failed"
 # Revoke the same serials with the second CA key to ensure a multi-CA
 # KRL is generated.
@@ -149,16 +165,18 @@ $SSHKEYGEN $OPTS -kf $OBJ/krl-serial -u 
 ## XXX dump with trace and grep for set cert serials
 ## XXX test ranges near (u64)-1, etc.
 
-verbose "$tid: generating KRLs"
-genkrls
-
 check_krl() {
KEY=$1
KRL=$2
EXPECT_REVOKED=$3
TAG=$4
-   $SSHKEYGEN -Qf $KRL $KEY >/dev/null
+   ARG=$5
+   $SSHKEYGEN $ARG -Qf $KRL $KEY >/dev/null 2>&1
result=$?
+   case "x$EXPECT_REVOKED" in
+   xx|xy) ;;
+   default) fatal "bad expectation $EXPECT_REVOKED"
+   esac
if test "x$EXPECT_REVOKED" = "xy" -a $result -eq 0 ; then
fatal "key $KEY not revoked by KRL $KRL: $TAG"
elif test "x$EXPECT_REVOKED" = "xn" -a $result -ne 0 ; then
@@ -177,41 +195,107 @@ test_rev() {
CA_RESULT=$9
SERIAL_WRESULT=${10}
KEYID_WRESULT=${11}
+   ARG=${12}
verbose "$tid: checking revocations for $TAG"
for f in $FILES ; do
-   check_krl $f $OBJ/krl-empty no  "$TAG"
-   check_krl $f $OBJ/krl-keys  $KEYS_RESULT"$TAG"
-   check_krl $f $OBJ/krl-all   $ALL_RESULT "$TAG"
-   check_krl $f $OBJ/krl-sha1  $HASH_RESULT"$TAG"
-   check_krl $f $OBJ/krl-sha256$HASH_RESULT"$TAG"
-   check_krl $f $OBJ/krl-hash  $HASH_RESULT"$TAG"
-   check_krl $f $OBJ/krl-serial$SERIAL_RESULT  "$TAG"
-   check_krl $f $OBJ/krl-keyid $KEYID_RESULT   "$TAG"
-   check_krl $f $OBJ/krl-cert  $CERTS_RESULT   "$TAG"
-   check_krl $f $OBJ/krl-ca$CA_RESULT  "$TAG"
-   check_krl $f $OBJ/krl-serial-wild   $SERIAL_WRESULT "$TAG"
-   check_krl $f $OBJ/krl-keyid-wild$KEYID_WRESULT  "$TAG"
+   check_krl $f $OBJ/krl-empty no  "$TAG" "$ARG"
+   check_krl $f $OBJ/krl-keys  $KEYS_RESULT"$TAG" "$ARG"
+   check_krl $f $OBJ/krl-all   $ALL_RESULT "$TAG" "$ARG"
+   check_krl $f $OBJ/krl-sha1  $HASH_RESULT"$TAG" "$ARG"
+   

KRL 3/4: plumb in signing and verification to ssh-keygen

2023-01-16 Thread Damien Miller
Hi,

This is another OpenSSH key revocation list (KRL) change: to support KRL
signing and verification in ssh-keygen(1).

The KRL format has supported signing of KRLs and verification of KRL
signatures for a long time, but there is currently no way to generate a
signed KRL or check the signature on one. The final patch hooks this up
for ssh-keygen(1) at least.

ok?

---
 ssh-keygen.c | 62 ++--
 1 file changed, 51 insertions(+), 11 deletions(-)

diff --git a/ssh-keygen.c b/ssh-keygen.c
index 869b675..29b8aec 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -2179,7 +2179,8 @@ do_show_cert(struct passwd *pw)
 }
 
 static void
-load_krl(const char *path, struct ssh_krl **krlp)
+load_krl(const char *path, struct ssh_krl **krlp,
+struct sshkey **trusted_keys, size_t ntrusted_keys)
 {
struct sshbuf *krlbuf;
int r;
@@ -2187,7 +2188,8 @@ load_krl(const char *path, struct ssh_krl **krlp)
if ((r = sshbuf_load_file(path, )) != 0)
fatal_r(r, "Unable to load KRL %s", path);
/* XXX check sigs */
-   if ((r = ssh_krl_from_blob(krlbuf, krlp, NULL, 0)) != 0 ||
+   if ((r = ssh_krl_from_blob(krlbuf, krlp,
+   trusted_keys, ntrusted_keys)) != 0 ||
*krlp == NULL)
fatal_r(r, "Invalid KRL file %s", path);
sshbuf_free(krlbuf);
@@ -2378,20 +2380,54 @@ update_krl_from_file(struct passwd *pw, const char 
*file, int wild_ca,
free(path);
 }
 
+static void
+krl_process_opts(struct passwd *pw, char * const *opts, size_t nopts,
+struct sshkey ***sig_keys, size_t *nsig_keys)
+{
+   struct sshkey *key;
+   char *path;
+   size_t i;
+
+   for (i = 0; i < nopts; i++) {
+   if (strncasecmp(opts[i], "signing-key=", 12) == 0) {
+   path = tilde_expand_filename(opts[i] + 12, pw->pw_uid);
+   key = load_identity(path, NULL);
+   free(path);
+   *sig_keys = xrecallocarray(*sig_keys, *nsig_keys,
+   *nsig_keys + 1, sizeof(*sig_keys));
+   (*sig_keys)[(*nsig_keys)++] = key;
+   } else
+   fatal("Invalid option \"%s\"", opts[i]);
+   }
+}
+
+static void
+free_sig_keys(struct sshkey **sig_keys, size_t nsig_keys)
+{
+   size_t i;
+
+   for (i = 0; i < nsig_keys; i++)
+   sshkey_free(sig_keys[i]);
+   free(sig_keys);
+}
+
 static void
 do_gen_krl(struct passwd *pw, int updating, const char *ca_key_path,
 unsigned long long krl_version, const char *krl_comment,
+char * const *opts, size_t nopts,
 int argc, char **argv)
 {
struct ssh_krl *krl;
struct stat sb;
-   struct sshkey *ca = NULL;
+   struct sshkey *ca = NULL, **sig_keys = NULL;
int i, r, wild_ca = 0;
+   size_t nsig_keys = 0;
char *tmp;
struct sshbuf *kbuf;
 
if (*identity_file == '\0')
fatal("KRL generation requires an output file");
+   krl_process_opts(pw, opts, nopts, _keys, _keys);
if (stat(identity_file, ) == -1) {
if (errno != ENOENT)
fatal("Cannot access KRL \"%s\": %s",
@@ -2411,7 +2447,7 @@ do_gen_krl(struct passwd *pw, int updating, const char 
*ca_key_path,
}
 
if (updating)
-   load_krl(identity_file, );
+   load_krl(identity_file, , NULL, 0);
else if ((krl = ssh_krl_init()) == NULL)
fatal("couldn't create KRL");
 
@@ -2425,26 +2461,30 @@ do_gen_krl(struct passwd *pw, int updating, const char 
*ca_key_path,
 
if ((kbuf = sshbuf_new()) == NULL)
fatal("sshbuf_new failed");
-   if (ssh_krl_to_blob(krl, kbuf, NULL, 0) != 0)
+   if (ssh_krl_to_blob(krl, kbuf, sig_keys, nsig_keys) != 0)
fatal("Couldn't generate KRL");
if ((r = sshbuf_write_file(identity_file, kbuf)) != 0)
fatal("write %s: %s", identity_file, strerror(errno));
sshbuf_free(kbuf);
ssh_krl_free(krl);
sshkey_free(ca);
+   free_sig_keys(sig_keys, nsig_keys);
 }
 
 static void
-do_check_krl(struct passwd *pw, int print_krl, int argc, char **argv)
+do_check_krl(struct passwd *pw, int print_krl,
+char * const *opts, size_t nopts, int argc, char **argv)
 {
int i, r, ret = 0;
char *comment;
struct ssh_krl *krl;
-   struct sshkey *k;
+   struct sshkey *k = NULL, **sig_keys = NULL;
+   size_t nsig_keys = 0;
 
if (*identity_file == '\0')
fatal("KRL checking requires an input file");
-   load_krl(identity_file, );
+   krl_process_opts(pw, opts, nopts, _keys, _keys);
+   load_krl(identity_file, , sig_keys, nsig_keys);
if (print_krl)
krl_dump(krl, stdout);
for (i = 0; i < argc; i++) {
@@ -2460,6 +2500,7 @@ do_check_krl(struct passwd *pw, int print_krl, int argc, 

KRL 2/4: Refactor parsing and signature verification

2023-01-16 Thread Damien Miller
Hi,

This is the second of the OpenSSH key revocation list (KRL) diffs.
This one refactors KRL parsing, and particularly signature verification.

It splits the KRL parsing logic into three phases: signature
verification, key trust verification and everything else. The idea is
to make this easier to read and verify, but also to allow reuse of the
first two steps in other contexts (e.g. a KRL parser that just tests a
single key, instead of trying to load the whole KRL into RAM).

Also at https://github.com/djmdjm/openssh-wip/pull/19

ok?

(this is a bit of a mess to read as a diff; easier to look at before
and after files or use the github view)

---
 krl.c | 270 +++---
 krl.h |   2 +-
 2 files changed, 165 insertions(+), 107 deletions(-)

diff --git a/krl.c b/krl.c
index a8a6018..e5ef0bf 100644
--- a/krl.c
+++ b/krl.c
@@ -1054,87 +1054,81 @@ extension_section(struct sshbuf *sect, struct ssh_krl 
*krl)
return r;
 }
 
-/* Attempt to parse a KRL, checking its signature (if any) with sign_ca_keys. 
*/
-int
-ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp,
-const struct sshkey **sign_ca_keys, size_t nsign_ca_keys)
+/*
+ * Checks whether this is a KRL (correct magic) with supported version and
+ * verified signatures if present. NB. does not check whether keys are trusted
+ * to sign the KRL, caller must do by checking contents of returned key list.
+ * Does not modify buffer.
+ */
+static int
+check_krl_signature_header(struct sshbuf *buf,
+struct sshkey ***signing_keysp, size_t *nsigning_keysp)
 {
-   struct sshbuf *copy = NULL, *sect = NULL;
-   struct ssh_krl *krl = NULL;
-   char timestamp[64];
-   int r = SSH_ERR_INTERNAL_ERROR, sig_seen;
-   struct sshkey *key = NULL, **ca_used = NULL, **tmp_ca_used;
-   u_char type;
-   const u_char *blob;
-   size_t i, j, sig_off, sects_off, blen, nca_used;
+   int r = SSH_ERR_INTERNAL_ERROR;
+   struct sshbuf *copy = NULL;
u_int format_version;
+   u_char type;
+   const u_char *blob;
+   struct sshkey *key = NULL, **tmp, **sig_keys = NULL;
+   size_t i, sig_off, blen, nsig_keys = 0;
 
-   nca_used = 0;
-   *krlp = NULL;
-   if (sshbuf_len(buf) < sizeof(KRL_MAGIC) - 1 ||
-   memcmp(sshbuf_ptr(buf), KRL_MAGIC, sizeof(KRL_MAGIC) - 1) != 0) {
-   debug3_f("not a KRL");
-   return SSH_ERR_KRL_BAD_MAGIC;
+   if (signing_keysp == NULL || nsigning_keysp == NULL)
+   return SSH_ERR_INTERNAL_ERROR;
+   *signing_keysp = NULL;
+   *nsigning_keysp = 0;
+
+   /* KRL must begin with magic string */
+   if ((r = sshbuf_cmp(buf, 0, KRL_MAGIC, sizeof(KRL_MAGIC) - 1)) != 0) {
+   debug2_f("bad KRL magic header");
+   goto out;
}
-
-   /* Take a copy of the KRL buffer so we can verify its signature later */
+   /* Don't modify original buffer */
if ((copy = sshbuf_fromb(buf)) == NULL) {
-   r = SSH_ERR_ALLOC_FAIL;
+   error_f("sshbuf_fromb failed");
goto out;
}
-   if ((r = sshbuf_consume(copy, sizeof(KRL_MAGIC) - 1)) != 0)
-   goto out;
-
-   if ((krl = ssh_krl_init()) == NULL) {
-   error_f("alloc failed");
-   goto out;
-   }
-
-   if ((r = sshbuf_get_u32(copy, _version)) != 0)
+   if ((r = sshbuf_consume(copy, sizeof(KRL_MAGIC) - 1)) != 0 ||
+   (r = sshbuf_get_u32(copy, _version)) != 0)
goto out;
if (format_version != KRL_FORMAT_VERSION) {
+   error_f("unsupported KRL format version %u", format_version);
r = SSH_ERR_INVALID_FORMAT;
goto out;
}
-   if ((r = sshbuf_get_u64(copy, >krl_version)) != 0 ||
-   (r = sshbuf_get_u64(copy, >generated_date)) != 0 ||
-   (r = sshbuf_get_u64(copy, >flags)) != 0 ||
+   /* Skip header contents */
+   if ((r = sshbuf_get_u64(copy, NULL)) != 0 ||
+   (r = sshbuf_get_u64(copy, NULL)) != 0 ||
+   (r = sshbuf_get_u64(copy, NULL)) != 0 ||
(r = sshbuf_skip_string(copy)) != 0 ||
-   (r = sshbuf_get_cstring(copy, >comment, NULL)) != 0)
+   (r = sshbuf_get_cstring(copy, NULL, NULL)) != 0) {
+   error_fr(r, "parse KRL header");
goto out;
-
-   format_timestamp(krl->generated_date, timestamp, sizeof(timestamp));
-   debug("KRL version %llu generated at %s%s%s",
-   (long long unsigned)krl->krl_version, timestamp,
-   *krl->comment ? ": " : "", krl->comment);
-
+   }
/*
-* 1st pass: verify signatures, if any. This is done to avoid
-* detailed parsing of data whose provenance is unverified.
+* Perform minimal parsing (section type/len only) to reach signature
+* sections, if present.
 */
-   sig_seen = 0;
if (sshbuf_len(buf) <