KRL 1/4: extension mechanism

2023-01-15 Thread Damien Miller
Hi,

This is the first of a few changes to krl.c and related code.

This defines and implements an extension mechanism for KRLs.

This takes the form of new (sub-)section types that contain named
extensions. These may be flagged as "critical" which causes the KRL
parser to treat them as mandatory-to implement. If they aren't flagged
as critical then they are ignored.

I honestly feel kind of stupid for not doing this when I wrote the
format...

Unfortunately KRLs with extensions are not backwards-compatible, as
the parser treats unknown section types as a fatal error. They are
forwards-compatible though.

I didn't update the KRL format version because I couldn't see any
practical difference between "KRL parsing fails because of an unknown
section type" and "KRL parsing fails because the format version is
wrong", except that the latter also makes life harder for the no-
extension case.

This doesn't add support for any extensions, just the mechanism itself.

ok?

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

 PROTOCOL.krl | 49 +-
 krl.c| 84 
 krl.h|  2 ++
 3 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/PROTOCOL.krl b/PROTOCOL.krl
index 115f80e..0bd0a82 100644
--- a/PROTOCOL.krl
+++ b/PROTOCOL.krl
@@ -37,6 +37,7 @@ The available section types are:
 #define KRL_SECTION_FINGERPRINT_SHA1   3
 #define KRL_SECTION_SIGNATURE  4
 #define KRL_SECTION_FINGERPRINT_SHA256 5
+#define KRL_SECTION_EXTENSION  255
 
 2. Certificate section
 
@@ -64,6 +65,7 @@ The certificate section types are:
 #define KRL_SECTION_CERT_SERIAL_RANGE  0x21
 #define KRL_SECTION_CERT_SERIAL_BITMAP 0x22
 #define KRL_SECTION_CERT_KEY_ID0x23
+#define KRL_SECTION_CERT_EXTENSION 0x39
 
 2.1 Certificate serial list section
 
@@ -114,6 +116,29 @@ associated with a particular identity, e.g. a host or a 
user.
 This section must contain at least one "key_id". This section may appear
 multiple times.
 
+2.5. Certificate Extension subsections
+
+This subsection type provides a generic extension mechanism to the
+certificates KRL section that may be used to provide optional or critical
+data.
+
+Extensions are stored in subsections of type
+KRL_SECTION_CERT_EXTENSION with the following contents:
+
+   string  extension_name
+   boolean is_critical
+   string  extension_contents.
+
+Where "extension_name" describes the type of extension. It is
+recommended that user extensions follow "cert-n...@domain.org" naming.
+
+The "is_critical" indicates whether this extension is mandatory or
+optional. If true, then any unsupported extension encountered should
+result in KRL parsing failure. If false, then it may be safely be
+ignored.
+
+The "extension_contents" contains the body of the extension.
+
 3. Explicit key sections
 
 These sections, identified as KRL_SECTION_EXPLICIT_KEY, revoke keys
@@ -144,7 +169,29 @@ as a big-endian integer.
 
 This section may appear multiple times.
 
-5. KRL signature sections
+5. Extension sections
+
+This section type provides a generic extension mechanism to the KRL
+format that may be used to provide optional or critical data.
+
+Extensions are recorded in sections of type KRL_SECTION_EXTENSION
+with the following contents:
+
+   string  extension_name
+   boolean is_critical
+   string  extension_contents.
+
+Where "extension_name" describes the type of extension. It is
+recommended that user extensions follow "n...@domain.org" naming.
+
+The "is_critical" indicates whether this extension is mandatory or
+optional. If true, then any unsupported extension encountered should
+result in KRL parsing failure. If false, then it may be safely be
+ignored.
+
+The "extension_contents" contains the body of the extension.
+
+6. KRL signature sections
 
 The KRL_SECTION_SIGNATURE section serves a different purpose to the
 preceding ones: to provide cryptographic authentication of a KRL that
diff --git a/krl.c b/krl.c
index f491c24..a8a6018 100644
--- a/krl.c
+++ b/krl.c
@@ -837,6 +837,45 @@ format_timestamp(u_int64_t timestamp, char *ts, size_t nts)
}
 }
 
+static int
+cert_extension_subsection(struct sshbuf *subsect, struct ssh_krl *krl)
+{
+   int r = SSH_ERR_INTERNAL_ERROR;
+   u_char critical = 1;
+   struct sshbuf *value = NULL;
+   char *name = NULL;
+
+   if ((r = sshbuf_get_cstring(subsect, , NULL)) != 0 ||
+   (r = sshbuf_get_u8(subsect, )) != 0 ||
+   (r = sshbuf_froms(subsect, )) != 0) {
+   debug_fr(r, "parse");
+   error("KRL has invalid certificate extension subsection");
+   r = SSH_ERR_INVALID_FORMAT;
+   goto out;
+   }
+   if (sshbuf_len(subsect) != 0) {
+   error("KRL has invalid certificate extension subsection: "
+   "trailing data");
+   r = SSH_ERR_INVALID_FORMAT;

Re: [updated] console enhancement patchset

2023-01-15 Thread Daniel Dickman



On Sun, 15 Jan 2023, Crystal Kolipe wrote:

> Hi Daniel,
> 
> On Sun, Jan 15, 2023 at 05:54:39PM -0500, Daniel Dickman wrote:
> > Hi Crystal,
> > 
> > I tried your patch but it seems to tickle something on my lenovo laptop.
> > 
> > I see this message:
> > entry point at 0x1001000
> > 
> > and then nothing further seems to show up.
> 
> Thanks for testing.
> 
> * Is this the first version of the patchset that you've tested?

Yep, did not get a chance anything before this one.

> * If not, have previous versions worked on this hardware?
> 
> As of three days ago, -current now includes some of the earlier work I did on
> this, but I'm assuming that an unpatched -current kernel is running fine for
> you?
> 

That's right, unpatched -current with your earlier bits works fine.

The kernel that doesn't work for me is the very latest -current with 
only this diff.



Re: [updated] console enhancement patchset

2023-01-15 Thread Crystal Kolipe
Hi Daniel,

On Sun, Jan 15, 2023 at 05:54:39PM -0500, Daniel Dickman wrote:
> Hi Crystal,
> 
> I tried your patch but it seems to tickle something on my lenovo laptop.
> 
> I see this message:
> entry point at 0x1001000
> 
> and then nothing further seems to show up.

Thanks for testing.

* Is this the first version of the patchset that you've tested?
* If not, have previous versions worked on this hardware?

As of three days ago, -current now includes some of the earlier work I did on
this, but I'm assuming that an unpatched -current kernel is running fine for
you?



Re: [updated] console enhancement patchset

2023-01-15 Thread Daniel Dickman
Hi Crystal,

I tried your patch but it seems to tickle something on my lenovo laptop.

I see this message:
entry point at 0x1001000

and then nothing further seems to show up.

In case helpful, dmesg from this laptop is shown below.



OpenBSD 7.2-current (GENERIC.MP) #0: Sun Jan 15 17:08:05 EST 2023
root@XXX:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 16452091904 (15689MB)
avail mem = 15934099456 (15195MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.3 @ 0xca50a000 (35 entries)
bios0: vendor LENOVO version "H6CN16WW(V1.09)" date 10/12/2022
bios0: LENOVO 82ND
efi0 at bios0: UEFI 2.7
efi0: INSYDE Corp. rev 0x60324016
acpi0 at bios0: ACPI 5.0
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP UEFI SSDT SSDT IVRS SSDT SSDT TPM2 MSDM ASF! BOOT HPET 
APIC MCFG SLIC SSDT VFCT SSDT SSDT SSDT SSDT SSDT CRAT CDIT SSDT SSDT SSDT SSDT 
SSDT SSDT FPDT WSMT SSDT SSDT SSDT SSDT SSDT SSDT BGRT
acpi0: wakeup devices GPP0(S4) GPP1(S4) GP17(S0)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpihpet0 at acpi0: 14318180 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD Ryzen 7 5700U with Radeon Graphics, 1800.00 MHz, 17-68-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 
8-way L2 cache, 4MB 64b/line 16-way L3 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 100MHz
cpu0: mwait min=64, max=64, C-substates=1.1, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: AMD Ryzen 7 5700U with Radeon Graphics, 1800.00 MHz, 17-68-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu1: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 
8-way L2 cache, 4MB 64b/line 16-way L3 cache
cpu1: smt 1, core 0, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: AMD Ryzen 7 5700U with Radeon Graphics, 1800.00 MHz, 17-68-01
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu2: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 
8-way L2 cache, 4MB 64b/line 16-way L3 cache
cpu2: smt 0, core 1, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: AMD Ryzen 7 5700U with Radeon Graphics, 1800.00 MHz, 17-68-01
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu3: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 
8-way L2 cache, 4MB 64b/line 16-way L3 cache
cpu3: smt 1, core 1, package 0
cpu4 at mainbus0: apid 4 (application processor)
cpu4: AMD Ryzen 7 5700U with Radeon Graphics, 1800.00 MHz, 17-68-01
cpu4: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu4: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 
8-way L2 cache, 4MB 64b/line 16-way L3 cache
cpu4: smt 

Re: don't remove known vmd vm's on failure

2023-01-15 Thread Dave Voutila


Dave Voutila  writes:

> It turns out not only does vmd have numerous error paths for handling
> when something is amiss with a guest, most of the paths don't check if
> it's a known vm defined in vm.conf.
>
> As a result, vmd often removes the vm from the SLIST of vm's meaning
> one can't easily attempt to start it again or see it in vmctl's status
> output.
>
> A simple reproduction:
>
>   1. define a vm with memory > 4gb in vm.conf
>   2. run vmd in the foreground (doas vmd -d) so it's not started by rc.d
>   3. try to start with `vmctl start -c ${vm_name}`, you should trigger
>  an ENOMEM and get the "Cannot allocate memory" message from vmctl.
>   4. try to start the same vm again...now you get EPERM!
>   5. the vm is no longer visible in the output from `vmctl status` :(
>
> The problem is most of the error paths call vm_remove, which not only
> tears down the vm via vm_stop, but also removes it from the vm list and
> frees it. Only clean stops or restarts seem to perform this check
> currently.
>
> Below diff refactors into checking if the vm is defined in the global
> config before deciding to call vm_stop or vm_remove.

Slight tweak... __func__->caller to actually pass the correct name to
vm_{stop,remove}() from vm_terminate()


diff refs/heads/master refs/heads/vmd-accounting
commit - d4e23fe7544b01187ebf3ac8ae32e955445ee666
commit + 46503195403bfab50cd34bd8682f35a17d54d03d
blob - 6bffb2519a31464836aa573dbccb7aa14ea97722
blob + f30dc14de1ff9d5cf121cbc08b6db183a06d0c07
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -67,6 +67,8 @@ struct vmd*env;
 int vm_claimid(const char *, int, uint32_t *);
 voidstart_vm_batch(int, short, void*);

+static inline void vm_terminate(struct vmd_vm *, const char *);
+
 struct vmd *env;

 static struct privsep_proc procs[] = {
@@ -395,14 +397,14 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
errno = vmr.vmr_result;
log_warn("%s: failed to forward vm result",
vcp->vcp_name);
-   vm_remove(vm, __func__);
+   vm_terminate(vm, __func__);
return (-1);
}
}

if (vmr.vmr_result) {
log_warnx("%s: failed to start vm", vcp->vcp_name);
-   vm_remove(vm, __func__);
+   vm_terminate(vm, __func__);
errno = vmr.vmr_result;
break;
}
@@ -410,7 +412,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
/* Now configure all the interfaces */
if (vm_priv_ifconfig(ps, vm) == -1) {
log_warn("%s: failed to configure vm", vcp->vcp_name);
-   vm_remove(vm, __func__);
+   vm_terminate(vm, __func__);
break;
}

@@ -441,10 +443,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
log_info("%s: sent vm %d successfully.",
vm->vm_params.vmc_params.vcp_name,
vm->vm_vmid);
-   if (vm->vm_from_config)
-   vm_stop(vm, 0, __func__);
-   else
-   vm_remove(vm, __func__);
+   vm_terminate(vm, __func__);
}

/* Send a response if a control client is waiting for it */
@@ -470,10 +469,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
}
if (vmr.vmr_result != EAGAIN ||
vm->vm_params.vmc_bootdevice) {
-   if (vm->vm_from_config)
-   vm_stop(vm, 0, __func__);
-   else
-   vm_remove(vm, __func__);
+   vm_terminate(vm, __func__);
} else {
/* Stop VM instance but keep the tty open */
vm_stop(vm, 1, __func__);
@@ -509,7 +505,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
imsg->hdr.peerid, -1, , sizeof(vir)) == -1) {
log_debug("%s: GET_INFO_VM failed for vm %d, removing",
__func__, vm->vm_vmid);
-   vm_remove(vm, __func__);
+   vm_terminate(vm, __func__);
return (-1);
}
break;
@@ -545,7 +541,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
sizeof(vir)) == -1) {
log_debug("%s: GET_INFO_VM_END failed",
__func__);
-   vm_remove(vm, __func__);
+