Re: amd64: do CPU identification before TSC sync test
Hi, Scott.
After applying your patch, cpu1 is not identified on my current kernel.
Dmesg shows as follows. I'll see it further more.
OpenBSD 7.1-current (GENERIC.MP) #3: Wed May 11 12:27:53 JST 2022
yuich...@yuichiro-obsd.soum.co.jp:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 2129526784 (2030MB)
avail mem = 2047676416 (1952MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xe0010 (242 entries)
bios0: vendor Phoenix Technologies LTD version "6.00" date 12/12/2018
bios0: VMware, Inc. VMware Virtual Platform
acpi0 at bios0: ACPI 4.0
acpi0: sleep states S0 S1 S4 S5
acpi0: tables DSDT FACP BOOT APIC MCFG SRAT HPET WAET
acpi0: wakeup devices PCI0(S3) USB_(S1) P2P0(S3) S1F0(S3) S2F0(S3) S8F0(S3)
S16F(S3) S17F(S3) S18F(S3) S22F(S3) S23F(S3) S24F(S3) S25F(S3) PE40(S3)
S1F0(S3) PE50(S3) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD Ryzen 7 5700G with Radeon Graphics, 3792.28 MHz, 19-50-00
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,PCLMUL,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,PKU,IBPB,XSAVEOPT,XSAVEC,XSAVES
cpu0: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line
8-way L2 cache
cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 65MHz
cpu1 at mainbus0: apid 2 (application processor)
cpu1: failed to identify
On 5/11/22 11:04, Scott Cheloha wrote:
On Tue, Mar 29, 2022 at 10:24:03AM -0500, Scott Cheloha wrote:
On Tue, Mar 29, 2022 at 03:26:49PM +1100, Jonathan Gray wrote:
On Mon, Mar 28, 2022 at 10:52:09PM -0500, Scott Cheloha wrote:
I want to use the IA32_TSC_ADJUST MSR where available when testing TSC
synchronization. We note if it's available during CPU identification.
Can we do CPU identification earlier in cpu_hatch() and
cpu_start_secondary(), before we do the TSC sync testing?
This can wait until after release. I'm just trying to suss out
whether there is an order dependency I'm not seeing. My laptop
appears to boot and resume no differently with this patch.
Thoughts?
The rest aside, moving the cpu_ucode_apply() call to after the
identifycpu() call is wrong as microcode can add cpuid bits.
I would keep cpu_tsx_disable() before it as well.
Okay, moved them up.
I'm sure I've had problems trying to change the sequencing
of lapic, tsc freq and identify in the past. It caused problems
only on certain machines.
[...]
6 week bump + rebase.
Once again, I want to do CPU identification before the TSC sync test
so we can check for and use the IA32_TSC_ADJUST MSR during the sync
test.
Does anyone understand amd64 CPU startup well enough to say whether
this rearrangement is going to break something?
Is this ok?
Index: cpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
retrieving revision 1.156
diff -u -p -r1.156 cpu.c
--- cpu.c 26 Apr 2022 08:35:30 - 1.156
+++ cpu.c 11 May 2022 02:00:37 -
@@ -852,20 +852,7 @@ cpu_start_secondary(struct cpu_info *ci)
printf("dropping into debugger; continue from here to resume
boot\n");
db_enter();
#endif
- } else {
- /*
-* Synchronize time stamp counters. Invalidate cache and
-* synchronize twice (in tsc_sync_bp) to minimize possible
-* cache effects. Disable interrupts to try and rule out any
-* external interference.
-*/
- s = intr_disable();
- wbinvd();
- tsc_sync_bp(ci);
- intr_restore(s);
-#ifdef TSC_DEBUG
- printf("TSC skew=%lld\n", (long long)ci->ci_tsc_skew);
-#endif
+ goto cleanup;
}
if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
@@ -875,11 +862,28 @@ cpu_start_secondary(struct cpu_info *ci)
for (i = 200; (ci->ci_flags & CPUF_IDENTIFY) && i > 0; i--)
delay(10);
- if (ci->ci_flags & CPUF_IDENTIFY)
+ if (ci->ci_flags & CPUF_IDENTIFY) {
printf("%s: failed to identify\n",
ci->ci_dev->dv_xname);
+ goto cleanup;
+ }
}
+ /*
+* Synchronize time stamp counters. Invalidate cache and
+* synchronize twice (in tsc_sync_bp) to minimize possible
+*
Re: Mark pw_error __dead in util.h
On Tue, May 03, 2022 at 10:37:36PM -0500, Matthew Martin wrote: > The function is already marked __dead in passwd.c, so appears to just be > an oversight. ping diff --git util.h util.h index dd64f478e23..752f8bb9fc5 100644 --- util.h +++ util.h @@ -97,7 +97,7 @@ void pw_edit(int, const char *); void pw_prompt(void); void pw_copy(int, int, const struct passwd *, const struct passwd *); intpw_scan(char *, struct passwd *, int *); -void pw_error(const char *, int, int); +__dead voidpw_error(const char *, int, int); intgetptmfd(void); intopenpty(int *, int *, char *, const struct termios *, const struct winsize *);
Re: [v2] amd64: simplify TSC sync testing
On Wed, May 11, 2022 at 10:52:55AM +0900, Yuichiro NAITO wrote: > Hi, Scott. > > Recently I started running OpenBSD on ESXi. > I'm facing monotonic time going back problem as same as Yasuoka-san's report. > > https://marc.info/?l=openbsd-tech=161657532610882=2 > > I've tried your v2 patch. It seems the problem has been solved in my > enviroment. > But I'm a little bit confused about the patch. May I ask you goal of the > patch? The primary goal of the patch is to eliminate false positives when testing for TSC skew. NUMA lag seems to sometimes fool the current test. It would be nice to be able to use the TSC in userland on multisocket machines. > Is it indented to fix the problem or is it for collecting test cases and the > results? The intention of the patch was not to fix the problem Yasuoka is describing. If it fixes that problem it is unintentional. > Here is the dmesg shown in my patched environment. > > [...]
Re: amd64: do CPU identification before TSC sync test
On Tue, Mar 29, 2022 at 10:24:03AM -0500, Scott Cheloha wrote:
> On Tue, Mar 29, 2022 at 03:26:49PM +1100, Jonathan Gray wrote:
> > On Mon, Mar 28, 2022 at 10:52:09PM -0500, Scott Cheloha wrote:
> > > I want to use the IA32_TSC_ADJUST MSR where available when testing TSC
> > > synchronization. We note if it's available during CPU identification.
> > >
> > > Can we do CPU identification earlier in cpu_hatch() and
> > > cpu_start_secondary(), before we do the TSC sync testing?
> > >
> > > This can wait until after release. I'm just trying to suss out
> > > whether there is an order dependency I'm not seeing. My laptop
> > > appears to boot and resume no differently with this patch.
> > >
> > > Thoughts?
> >
> > The rest aside, moving the cpu_ucode_apply() call to after the
> > identifycpu() call is wrong as microcode can add cpuid bits.
> > I would keep cpu_tsx_disable() before it as well.
>
> Okay, moved them up.
>
> > I'm sure I've had problems trying to change the sequencing
> > of lapic, tsc freq and identify in the past. It caused problems
> > only on certain machines.
>
> [...]
6 week bump + rebase.
Once again, I want to do CPU identification before the TSC sync test
so we can check for and use the IA32_TSC_ADJUST MSR during the sync
test.
Does anyone understand amd64 CPU startup well enough to say whether
this rearrangement is going to break something?
Is this ok?
Index: cpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
retrieving revision 1.156
diff -u -p -r1.156 cpu.c
--- cpu.c 26 Apr 2022 08:35:30 - 1.156
+++ cpu.c 11 May 2022 02:00:37 -
@@ -852,20 +852,7 @@ cpu_start_secondary(struct cpu_info *ci)
printf("dropping into debugger; continue from here to resume
boot\n");
db_enter();
#endif
- } else {
- /*
-* Synchronize time stamp counters. Invalidate cache and
-* synchronize twice (in tsc_sync_bp) to minimize possible
-* cache effects. Disable interrupts to try and rule out any
-* external interference.
-*/
- s = intr_disable();
- wbinvd();
- tsc_sync_bp(ci);
- intr_restore(s);
-#ifdef TSC_DEBUG
- printf("TSC skew=%lld\n", (long long)ci->ci_tsc_skew);
-#endif
+ goto cleanup;
}
if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
@@ -875,11 +862,28 @@ cpu_start_secondary(struct cpu_info *ci)
for (i = 200; (ci->ci_flags & CPUF_IDENTIFY) && i > 0; i--)
delay(10);
- if (ci->ci_flags & CPUF_IDENTIFY)
+ if (ci->ci_flags & CPUF_IDENTIFY) {
printf("%s: failed to identify\n",
ci->ci_dev->dv_xname);
+ goto cleanup;
+ }
}
+ /*
+* Synchronize time stamp counters. Invalidate cache and
+* synchronize twice (in tsc_sync_bp) to minimize possible
+* cache effects. Disable interrupts to try and rule out any
+* external interference.
+*/
+ s = intr_disable();
+ wbinvd();
+ tsc_sync_bp(ci);
+ intr_restore(s);
+#ifdef TSC_DEBUG
+ printf("TSC skew=%lld\n", (long long)ci->ci_tsc_skew);
+#endif
+
+cleanup:
CPU_START_CLEANUP(ci);
pmap_kremove(MP_TRAMPOLINE, PAGE_SIZE);
@@ -940,18 +944,8 @@ cpu_hatch(void *v)
if (ci->ci_flags & CPUF_PRESENT)
panic("%s: already running!?", ci->ci_dev->dv_xname);
#endif
-
- /*
-* Synchronize the TSC for the first time. Note that interrupts are
-* off at this point.
-*/
- wbinvd();
ci->ci_flags |= CPUF_PRESENT;
- ci->ci_tsc_skew = 0;/* reset on resume */
- tsc_sync_ap(ci);
- lapic_enable();
- lapic_startclock();
cpu_ucode_apply(ci);
cpu_tsx_disable(ci);
@@ -970,6 +964,17 @@ cpu_hatch(void *v)
/* Prevent identifycpu() from running again */
atomic_setbits_int(>ci_flags, CPUF_IDENTIFIED);
}
+
+ /*
+* Synchronize the TSC for the first time. Note that interrupts are
+* off at this point.
+*/
+ wbinvd();
+ ci->ci_tsc_skew = 0;/* reset on resume */
+ tsc_sync_ap(ci);
+
+ lapic_enable();
+ lapic_startclock();
while ((ci->ci_flags & CPUF_GO) == 0)
delay(10);
Re: [v2] amd64: simplify TSC sync testing
Hi, Scott. Recently I started running OpenBSD on ESXi. I'm facing monotonic time going back problem as same as Yasuoka-san's report. https://marc.info/?l=openbsd-tech=161657532610882=2 I've tried your v2 patch. It seems the problem has been solved in my enviroment. But I'm a little bit confused about the patch. May I ask you goal of the patch? Is it indented to fix the problem or is it for collecting test cases and the results? Here is the dmesg shown in my patched environment. OpenBSD 7.1-current (GENERIC.MP) #2: Mon May 2 12:03:22 JST 2022 yuich...@yuichiro-obsd.soum.co.jp:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 2129526784 (2030MB) avail mem = 2047713280 (1952MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xe0010 (242 entries) bios0: vendor Phoenix Technologies LTD version "6.00" date 12/12/2018 bios0: VMware, Inc. VMware Virtual Platform acpi0 at bios0: ACPI 4.0 acpi0: sleep states S0 S1 S4 S5 acpi0: tables DSDT FACP BOOT APIC MCFG SRAT HPET WAET acpi0: wakeup devices PCI0(S3) USB_(S1) P2P0(S3) S1F0(S3) S2F0(S3) S8F0(S3) S16F(S3) S17F(S3) S18F(S3) S22F(S3) S23F(S3) S24F(S3) S25F(S3) PE40(S3) S1F0(S3) PE50(S3) [...] acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: AMD Ryzen 7 5700G with Radeon Graphics, 3792.27 MHz, 19-50-00 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,PCLMUL,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,PKU,IBPB,XSAVEOPT,XSAVEC,XSAVES cpu0: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 8-way L2 cache cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 65MHz cpu1 at mainbus0: apid 2 (application processor) cpu1: AMD Ryzen 7 5700G with Radeon Graphics, 3792.01 MHz, 19-50-00 cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,PCLMUL,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,PKU,IBPB,XSAVEOPT,XSAVEC,XSAVES cpu1: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 8-way L2 cache cpu1: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu1: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu1: smt 0, core 0, package 2 ioapic0 at mainbus0: apid 1 pa 0xfec0, version 20, 24 pins acpimcfg0 at acpi0 acpimcfg0: addr 0xf000, bus 0-127 acpihpet0 at acpi0: 14318179 Hz acpiprt0 at acpi0: bus 0 (PCI0) acpipci0 at acpi0 PCI0: 0x 0x0011 0x0001 acpicmos0 at acpi0 "PNP0A05" at acpi0 not configured acpiac0 at acpi0: AC unit online acpicpu0 at acpi0: C1(@1 halt!) acpicpu1 at acpi0: C1(@1 halt!) pvbus0 at mainbus0: VMware vmt0 at pvbus0 pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 "Intel 82443BX AGP" rev 0x01 ppb0 at pci0 dev 1 function 0 "Intel 82443BX AGP" rev 0x01 pci1 at ppb0 bus 1 pcib0 at pci0 dev 7 function 0 "Intel 82371AB PIIX4 ISA" rev 0x08 pciide0 at pci0 dev 7 function 1 "Intel 82371AB IDE" rev 0x01: DMA, channel 0 configured to compatibility, channel 1 configured to compatibility pciide0: channel 0 disabled (no drives) pciide0: channel 1 disabled (no drives) piixpm0 at pci0 dev 7 function 3 "Intel 82371AB Power" rev 0x08: SMBus disabled "VMware VMCI" rev 0x10 at pci0 dev 7 function 7 not configured vga1 at pci0 dev 15 function 0 "VMware SVGA II" rev 0x00 wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation) wsdisplay0: screen 1-5 added (80x25, vt100 emulation) mpi0 at pci0 dev 16 function 0 "Symbios Logic 53c1030" rev 0x01: apic 1 int 17 mpi0: 0, firmware 1.3.41.32 scsibus1 at mpi0: 16 targets, initiator 7 sd0 at scsibus1 targ 0 lun 0: sd0: 40960MB, 512 bytes/sector, 83886080 sectors mpi0: target 0 Sync at 160MHz width 16bit offset 127 QAS 1 DT 1 IU 1 ppb1 at pci0 dev 17 function 0 "VMware PCI" rev 0x02 pci2 at ppb1 bus 2 uhci0 at pci2 dev 0 function 0 "VMware UHCI" rev 0x00: apic 1 int 18 ehci0 at pci2 dev 1 function 0 "VMware EHCI" rev 0x00: apic 1 int 19 usb0 at ehci0: USB revision 2.0 uhub0 at usb0 configuration 1 interface 0 "VMware EHCI root hub" rev 2.00/1.00 addr 1 ahci0 at pci2 dev 3 function 0 "VMware AHCI" rev 0x00: msi, AHCI 1.3 ahci0: port 0: 6.0Gb/s scsibus2 at ahci0: 32 targets cd0 at scsibus2 targ 0 lun 0:
ssh: double fclose() in sshkey_save_public()
Hey,
it looks like in sshkey_save_public() the same fd will be closed twice
if the first fclose() returns something other than 0.
The patch below should make sure everything only gets closed once.
I moved the close() call and refactored a bit to improve readability.
Index: authfile.c
===
RCS file: /cvs/src/usr.bin/ssh/authfile.c,v
retrieving revision 1.142
diff -u -p -r1.142 authfile.c
--- authfile.c 1 Jan 2022 01:55:30 - 1.142
+++ authfile.c 10 May 2022 21:38:29 -
@@ -496,20 +496,25 @@ sshkey_save_public(const struct sshkey *
return SSH_ERR_SYSTEM_ERROR;
if ((f = fdopen(fd, "w")) == NULL) {
r = SSH_ERR_SYSTEM_ERROR;
+ close(fd);
goto fail;
}
if ((r = sshkey_write(key, f)) != 0)
goto fail;
fprintf(f, " %s\n", comment);
- if (ferror(f) || fclose(f) != 0) {
+ if (ferror(f)) {
r = SSH_ERR_SYSTEM_ERROR;
+ goto fail;
+ }
+ if (fclose(f) != 0) {
+ r = SSH_ERR_SYSTEM_ERROR;
+ f = NULL;
fail:
- oerrno = errno;
- if (f != NULL)
+ if (f != NULL) {
+ oerrno = errno;
fclose(f);
- else
- close(fd);
- errno = oerrno;
+ errno = oerrno;
+ }
return r;
}
return 0;
Add support for Advantech multi-port serial device to puc(4)
This patch adds support for the Advantech PCIE-1612B multi-port serial device
to puc(4). The device has an Exar XR17V354 UART and the changes use the
existing code that supports XR17V35x devices.
This patch was minimally tested. All four ports were tested with several
different serial devices. I only tested RS-232 and do not have the ability
to test RS-422 or RS-485.
dmesg snippet:
...
puc0 at pci4 dev 0 function 0 "Advantech PCIE-1612B-AE 4-port RS232/422/485"
rev 0x03: ports: 16 com
com4 at puc0 port 0 apic 2 int 17: xr17v35x, 256 byte fifo
com5 at puc0 port 1 apic 2 int 17: xr17v35x, 256 byte fifo
com6 at puc0 port 2 apic 2 int 17: xr17v35x, 256 byte fifo
com7 at puc0 port 3 apic 2 int 17: xr17v35x, 256 byte fifo
...
Index: pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1988
diff -u -p -u -p -r1.1988 pcidevs
--- pcidevs 11 Mar 2022 08:28:40 - 1.1988
+++ pcidevs 10 May 2022 15:38:10 -
@@ -244,6 +244,7 @@ vendor TECHSAN 0x13d0 Techsan Electroni
vendor ABOCOM 0x13d1 Abocom
vendor SUNDANCE0x13f0 Sundance
vendor CMI 0x13f6 C-Media Electronics
+vendor ADVANTECH 0x13fe Advantech
vendor LAVA0x1407 Lava
vendor SUNIX 0x1409 Sunix
vendor ICENSEMBLE 0x1412 IC Ensemble
@@ -512,6 +513,9 @@ product ACCTON EN1217 0x1217 EN1217
/* Acer products */
product ACER M1435 0x1435 M1435 VL-PCI
+
+/* Advantech products */
+product ADVANTECH 1612B0x000b PCIE-1612B-AE 4-port
RS232/422/485
/* Analog Devices */
product AD SP21535 0x1535 ADSP 21535 DSP
Index: pucdata.c
===
RCS file: /cvs/src/sys/dev/pci/pucdata.c,v
retrieving revision 1.116
diff -u -p -u -p -r1.116 pucdata.c
--- pucdata.c 9 Jan 2022 05:42:58 - 1.116
+++ pucdata.c 10 May 2022 15:38:10 -
@@ -2139,6 +2139,20 @@ const struct puc_device_description puc_
},
},
+ { /*
+* Advantech PCIE-1612B-AE 4-port RS232/422/485
+* Has a 4-channel Exar XR17V354 UART
+*/
+ { PCI_VENDOR_ADVANTECH, PCI_PRODUCT_ADVANTECH_1612B, 0, 0 },
+ { 0x, 0x, 0, 0 },
+ {
+ { PUC_PORT_COM_XR17V35X, 0x10, 0x },
+ { PUC_PORT_COM_XR17V35X, 0x10, 0x0400 },
+ { PUC_PORT_COM_XR17V35X, 0x10, 0x0800 },
+ { PUC_PORT_COM_XR17V35X, 0x10, 0x0C00 },
+ },
+ },
+
{ /* Dell DRAC 3 Virtual UART */
{ PCI_VENDOR_DELL, PCI_PRODUCT_DELL_DRAC_3_VUART, 0, 0 },
{ 0x, 0x, 0, 0 },
rpki-client: remove verify_cb()
With RSC and recent cert.c changes, rpki-client now has a hard
dependency on RFC 3779 support in libcrypto, so this code is no
longer useful.
Index: validate.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
retrieving revision 1.32
diff -u -p -r1.32 validate.c
--- validate.c 10 May 2022 07:41:37 - 1.32
+++ validate.c 10 May 2022 20:28:52 -
@@ -347,67 +347,6 @@ valid_origin(const char *uri, const char
}
/*
- * Callback for X509_verify_cert() to handle critical extensions in old
- * LibreSSL libraries or OpenSSL libs without RFC3779 support.
- */
-static int
-verify_cb(int ok, X509_STORE_CTX *store_ctx)
-{
- X509*cert;
- const STACK_OF(X509_EXTENSION) *exts;
- X509_EXTENSION *ext;
- ASN1_OBJECT *obj;
- char*file;
- int depth, error, i, nid;
-
- error = X509_STORE_CTX_get_error(store_ctx);
- depth = X509_STORE_CTX_get_error_depth(store_ctx);
-
- if (error != X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION)
- return ok;
-
- if ((file = X509_STORE_CTX_get_app_data(store_ctx)) == NULL)
- cryptoerrx("X509_STORE_CTX_get_app_data");
-
- if ((cert = X509_STORE_CTX_get_current_cert(store_ctx)) == NULL) {
- warnx("%s: got no current cert", file);
- return 0;
- }
- if ((exts = X509_get0_extensions(cert)) == NULL) {
- warnx("%s: got no cert extensions", file);
- return 0;
- }
-
- for (i = 0; i < sk_X509_EXTENSION_num(exts); i++) {
- ext = sk_X509_EXTENSION_value(exts, i);
-
- /* skip over non-critical and known extensions */
- if (!X509_EXTENSION_get_critical(ext))
- continue;
- if (X509_supported_extension(ext))
- continue;
-
- if ((obj = X509_EXTENSION_get_object(ext)) == NULL) {
- warnx("%s: got no extension object", file);
- return 0;
- }
-
- nid = OBJ_obj2nid(obj);
- switch (nid) {
- case NID_sbgp_ipAddrBlock:
- case NID_sbgp_autonomousSysNum:
- continue;
- default:
- warnx("%s: depth %d: unknown extension: nid %d",
- file, depth, nid);
- return 0;
- }
- }
-
- return 1;
-}
-
-/*
* Walk the certificate tree to the root and build a certificate
* chain from cert->x509. All certs in the tree are validated and
* can be loaded as trusted stack into the validator.
@@ -476,9 +415,6 @@ valid_x509(char *file, X509_STORE_CTX *s
if (!X509_VERIFY_PARAM_add0_policy(params, cp_oid))
cryptoerrx("X509_VERIFY_PARAM_add0_policy");
- X509_STORE_CTX_set_verify_cb(store_ctx, verify_cb);
- if (!X509_STORE_CTX_set_app_data(store_ctx, file))
- cryptoerrx("X509_STORE_CTX_set_app_data");
flags = X509_V_FLAG_CRL_CHECK;
flags |= X509_V_FLAG_EXPLICIT_POLICY;
flags |= X509_V_FLAG_INHIBIT_MAP;
Re: move vmm(4) spinout paranoia behind MP_LOCKDEBUG
ping
Dave Voutila writes:
> This tucks all the spinout paranoia behind `#ifdef MP_LOCKDEBUG` and
> uses the same approach used in amd64's pmap's TLB shootdown code.
>
> Part of me wants to remove this altogether, but I'm not sure it's
> outlived its usefulness quite yet.
>
> Three areas that busy wait on ipi's are modified:
>
> 1. vmm_start - performs ipi to enable vmm on all cpus
> 2. vmm_stop - performs ipi to disable vmm on all cpus
> 3. vmx_remote_vmclear - performs ipi to vmclear a cpu (only pertinent to
>Intel hosts)
>
> (3) is the most likely to spin out and prior to bumping the spinout to
> the current value (based on __mp_lock_spinout) we had reports from users
> of hitting it on slower/older MP hardware.
>
> For vmm_{start, stop}, I moved the current cpu start/stop routine to
> before performing the ipi broadcast because if we're going to fail to
> (dis)enable vmm we should fail fast. If we fail, there's no need to
> broadcast the ipi. This simplifies the code paths and removes a local
> variable.
>
> All three migrate to infinite busy waits and only have a spinout if
> built with MP_LOCKDEBUG. On a spinout, we enter ddb.
>
> Compiled on amd64 GENERIC, GENERIC.MP, and GENERIC.MP with
> MP_LOCKDEBUG. (This time I won't break GENERIC :)
>
> OK?
>
> -dv
>
> Index: sys/arch/amd64/amd64/vmm.c
> ===
> RCS file: /opt/cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.305
> diff -u -p -r1.305 vmm.c
> --- sys/arch/amd64/amd64/vmm.c28 Mar 2022 06:28:47 - 1.305
> +++ sys/arch/amd64/amd64/vmm.c16 Apr 2022 18:49:01 -
> @@ -43,6 +43,11 @@
> #include
> #include
>
> +#ifdef MP_LOCKDEBUG
> +#include
> +extern int __mp_lock_spinout;
> +#endif /* MP_LOCKDEBUG */
> +
> /* #define VMM_DEBUG */
>
> void *l1tf_flush_region;
> @@ -1328,17 +1333,26 @@ int
> vmm_start(void)
> {
> struct cpu_info *self = curcpu();
> - int ret = 0;
> #ifdef MULTIPROCESSOR
> struct cpu_info *ci;
> CPU_INFO_ITERATOR cii;
> - int i;
> -#endif
> +#ifdef MP_LOCKDEBUG
> + int nticks;
> +#endif /* MP_LOCKDEBUG */
> +#endif /* MULTIPROCESSOR */
>
> /* VMM is already running */
> if (self->ci_flags & CPUF_VMM)
> return (0);
>
> + /* Start VMM on this CPU */
> + start_vmm_on_cpu(self);
> + if (!(self->ci_flags & CPUF_VMM)) {
> + printf("%s: failed to enter VMM mode\n",
> + self->ci_dev->dv_xname);
> + return (EIO);
> + }
> +
> #ifdef MULTIPROCESSOR
> /* Broadcast start VMM IPI */
> x86_broadcast_ipi(X86_IPI_START_VMM);
> @@ -1346,25 +1360,23 @@ vmm_start(void)
> CPU_INFO_FOREACH(cii, ci) {
> if (ci == self)
> continue;
> - for (i = 10; (!(ci->ci_flags & CPUF_VMM)) && i>0;i--)
> - delay(10);
> - if (!(ci->ci_flags & CPUF_VMM)) {
> - printf("%s: failed to enter VMM mode\n",
> - ci->ci_dev->dv_xname);
> - ret = EIO;
> +#ifdef MP_LOCKDEBUG
> + nticks = __mp_lock_spinout;
> +#endif /* MP_LOCKDEBUG */
> + while (!(ci->ci_flags & CPUF_VMM)) {
> + CPU_BUSY_CYCLE();
> +#ifdef MP_LOCKDEBUG
> + if (--nticks <= 0) {
> + db_printf("%s: spun out", __func__);
> + db_enter();
> + nticks = __mp_lock_spinout;
> + }
> +#endif /* MP_LOCKDEBUG */
> }
> }
> #endif /* MULTIPROCESSOR */
>
> - /* Start VMM on this CPU */
> - start_vmm_on_cpu(self);
> - if (!(self->ci_flags & CPUF_VMM)) {
> - printf("%s: failed to enter VMM mode\n",
> - self->ci_dev->dv_xname);
> - ret = EIO;
> - }
> -
> - return (ret);
> + return (0);
> }
>
> /*
> @@ -1376,17 +1388,26 @@ int
> vmm_stop(void)
> {
> struct cpu_info *self = curcpu();
> - int ret = 0;
> #ifdef MULTIPROCESSOR
> struct cpu_info *ci;
> CPU_INFO_ITERATOR cii;
> - int i;
> -#endif
> +#ifdef MP_LOCKDEBUG
> + int nticks;
> +#endif /* MP_LOCKDEBUG */
> +#endif /* MULTIPROCESSOR */
>
> /* VMM is not running */
> if (!(self->ci_flags & CPUF_VMM))
> return (0);
>
> + /* Stop VMM on this CPU */
> + stop_vmm_on_cpu(self);
> + if (self->ci_flags & CPUF_VMM) {
> + printf("%s: failed to exit VMM mode\n",
> + self->ci_dev->dv_xname);
> + return (EIO);
> + }
> +
> #ifdef MULTIPROCESSOR
> /* Stop VMM on other CPUs */
> x86_broadcast_ipi(X86_IPI_STOP_VMM);
> @@ -1394,25 +1415,23 @@ vmm_stop(void)
> CPU_INFO_FOREACH(cii, ci) {
> if (ci == self)
> continue;
> - for (i = 10; (ci->ci_flags & CPUF_VMM) && i>0
rpki-client: deserialize ASIdentifiers in libcrypto
The ASIdentifiers code is a bit strangely factored presumably due to
constraints of the low-level shoveling. I kept the coarse structure
of the code and left some house keeping for later. The changes in
sbgp_asrange() and sbgp_asid() should be easy.
The in-tree sbgp_assysnum() is tricky to follow. The main thing to
note is that the for loop at the end really only inspects asnum (and
ignores rdi). Since RDI is forbidden in RFC 6487, I added a check
and error. All the complicated gymnastics can be dropped and what
remains is a bit stricter and what it does should be rather obvious.
With this diff, ASN1_TYPE and ASN1_frame() uses are gone from cert.c.
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.74
diff -u -p -r1.74 cert.c
--- cert.c 10 May 2022 16:43:53 - 1.74
+++ cert.c 10 May 2022 18:13:02 -
@@ -34,13 +34,6 @@
#include "extern.h"
/*
- * Type of ASIdentifier (RFC 3779, 3.2.3).
- */
-#defineASID_TYPE_ASNUM 0x00
-#define ASID_TYPE_RDI 0x01
-#define ASID_TYPE_MAX ASID_TYPE_RDI
-
-/*
* A parsing sequence of a file (which may just be ).
*/
struct parse {
@@ -128,70 +121,36 @@ sbgp_addr(struct parse *p, struct cert_i
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_asrange(struct parse *p, const unsigned char *d, size_t dsz)
+sbgp_asrange(struct parse *p, const ASRange *range)
{
struct cert_as as;
- ASN1_SEQUENCE_ANY *seq;
- const ASN1_TYPE *t;
- int rc = 0;
-
- if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, , dsz)) == NULL) {
- cryptowarnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
- "failed ASN.1 sequence parse", p->fn);
- goto out;
- }
- if (sk_ASN1_TYPE_num(seq) != 2) {
- warnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
- "want 2 elements, have %d", p->fn,
- sk_ASN1_TYPE_num(seq));
- goto out;
- }
memset(, 0, sizeof(struct cert_as));
as.type = CERT_AS_RANGE;
- t = sk_ASN1_TYPE_value(seq, 0);
- if (t->type != V_ASN1_INTEGER) {
- warnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
- "want ASN.1 integer, have %s (NID %d)",
- p->fn, ASN1_tag2str(t->type), t->type);
- goto out;
- }
- if (!as_id_parse(t->value.integer, )) {
+ if (!as_id_parse(range->min, )) {
warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
"malformed AS identifier", p->fn);
- goto out;
+ return 0;
}
- t = sk_ASN1_TYPE_value(seq, 1);
- if (t->type != V_ASN1_INTEGER) {
- warnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
- "want ASN.1 integer, have %s (NID %d)",
- p->fn, ASN1_tag2str(t->type), t->type);
- goto out;
- }
- if (!as_id_parse(t->value.integer, )) {
+ if (!as_id_parse(range->max, )) {
warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
"malformed AS identifier", p->fn);
- goto out;
+ return 0;
}
if (as.range.max == as.range.min) {
warnx("%s: RFC 3379 section 3.2.3.8: ASRange: "
"range is singular", p->fn);
- goto out;
+ return 0;
} else if (as.range.max < as.range.min) {
warnx("%s: RFC 3379 section 3.2.3.8: ASRange: "
"range is out of order", p->fn);
- goto out;
+ return 0;
}
- if (!append_as(p, ))
- goto out;
- rc = 1;
-out:
- sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
- return rc;
+ return append_as(p, );
}
/*
@@ -224,80 +183,46 @@ sbgp_asid(struct parse *p, const ASN1_IN
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_asnum(struct parse *p, const unsigned char *d, size_t dsz)
+sbgp_asnum(struct parse *p, const ASIdentifierChoice *aic)
{
struct cert_as as;
- ASN1_TYPE *t, *tt;
- ASN1_SEQUENCE_ANY *seq = NULL;
- int i, rc = 0;
- const unsigned char *sv = d;
-
- /* We can either be a null (inherit) or sequence. */
-
- if ((t = d2i_ASN1_TYPE(NULL, , dsz)) == NULL) {
- cryptowarnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: "
- "failed ASN.1 type parse", p->fn);
- goto out;
- }
-
- /*
-* Section 3779 3.2.3.3 is to inherit with an ASN.1 NULL type,
-* which is the easy case.
-*/
+ const ASIdOrRanges *aors = NULL;
+ const ASIdOrRange *aor;
+ int i;
- switch
Re: uvm_pagedequeue()
> Date: Tue, 10 May 2022 18:45:21 +0200
> From: Martin Pieuchot
>
> On 05/05/22(Thu) 14:54, Martin Pieuchot wrote:
> > Diff below introduces a new wrapper to manipulate active/inactive page
> > queues.
> >
> > ok?
>
> Anyone?
Sorry I started looking at this and got distracted.
I'm not sure about the changes to uvm_pageactivate(). It doesn't
quite match what NetBSD does, but I guess NetBSD assumes that
uvm_pageactiave() isn't called for a page that is already active? And
that's something we can't guarantee?
The diff is correct though in the sense that it is equivalent to the
code we already have. So if this definitely is the direction you want
to go:
ok kettenis@
> > Index: uvm/uvm_page.c
> > ===
> > RCS file: /cvs/src/sys/uvm/uvm_page.c,v
> > retrieving revision 1.165
> > diff -u -p -r1.165 uvm_page.c
> > --- uvm/uvm_page.c 4 May 2022 14:58:26 - 1.165
> > +++ uvm/uvm_page.c 5 May 2022 12:49:13 -
> > @@ -987,16 +987,7 @@ uvm_pageclean(struct vm_page *pg)
> > /*
> > * now remove the page from the queues
> > */
> > - if (pg->pg_flags & PQ_ACTIVE) {
> > - TAILQ_REMOVE(_active, pg, pageq);
> > - flags_to_clear |= PQ_ACTIVE;
> > - uvmexp.active--;
> > - }
> > - if (pg->pg_flags & PQ_INACTIVE) {
> > - TAILQ_REMOVE(_inactive, pg, pageq);
> > - flags_to_clear |= PQ_INACTIVE;
> > - uvmexp.inactive--;
> > - }
> > + uvm_pagedequeue(pg);
> >
> > /*
> > * if the page was wired, unwire it now.
> > @@ -1243,16 +1234,7 @@ uvm_pagewire(struct vm_page *pg)
> > MUTEX_ASSERT_LOCKED();
> >
> > if (pg->wire_count == 0) {
> > - if (pg->pg_flags & PQ_ACTIVE) {
> > - TAILQ_REMOVE(_active, pg, pageq);
> > - atomic_clearbits_int(>pg_flags, PQ_ACTIVE);
> > - uvmexp.active--;
> > - }
> > - if (pg->pg_flags & PQ_INACTIVE) {
> > - TAILQ_REMOVE(_inactive, pg, pageq);
> > - atomic_clearbits_int(>pg_flags, PQ_INACTIVE);
> > - uvmexp.inactive--;
> > - }
> > + uvm_pagedequeue(pg);
> > uvmexp.wired++;
> > }
> > pg->wire_count++;
> > @@ -1324,28 +1306,32 @@ uvm_pageactivate(struct vm_page *pg)
> > KASSERT(uvm_page_owner_locked_p(pg));
> > MUTEX_ASSERT_LOCKED();
> >
> > + uvm_pagedequeue(pg);
> > + if (pg->wire_count == 0) {
> > + TAILQ_INSERT_TAIL(_active, pg, pageq);
> > + atomic_setbits_int(>pg_flags, PQ_ACTIVE);
> > + uvmexp.active++;
> > +
> > + }
> > +}
> > +
> > +/*
> > + * uvm_pagedequeue: remove a page from any paging queue
> > + */
> > +void
> > +uvm_pagedequeue(struct vm_page *pg)
> > +{
> > + if (pg->pg_flags & PQ_ACTIVE) {
> > + TAILQ_REMOVE(_active, pg, pageq);
> > + atomic_clearbits_int(>pg_flags, PQ_ACTIVE);
> > + uvmexp.active--;
> > + }
> > if (pg->pg_flags & PQ_INACTIVE) {
> > TAILQ_REMOVE(_inactive, pg, pageq);
> > atomic_clearbits_int(>pg_flags, PQ_INACTIVE);
> > uvmexp.inactive--;
> > }
> > - if (pg->wire_count == 0) {
> > - /*
> > -* if page is already active, remove it from list so we
> > -* can put it at tail. if it wasn't active, then mark
> > -* it active and bump active count
> > -*/
> > - if (pg->pg_flags & PQ_ACTIVE)
> > - TAILQ_REMOVE(_active, pg, pageq);
> > - else {
> > - atomic_setbits_int(>pg_flags, PQ_ACTIVE);
> > - uvmexp.active++;
> > - }
> > -
> > - TAILQ_INSERT_TAIL(_active, pg, pageq);
> > - }
> > }
> > -
> > /*
> > * uvm_pagezero: zero fill a page
> > */
> > Index: uvm/uvm_page.h
> > ===
> > RCS file: /cvs/src/sys/uvm/uvm_page.h,v
> > retrieving revision 1.67
> > diff -u -p -r1.67 uvm_page.h
> > --- uvm/uvm_page.h 29 Jan 2022 06:25:33 - 1.67
> > +++ uvm/uvm_page.h 5 May 2022 12:49:13 -
> > @@ -224,6 +224,7 @@ boolean_t uvm_page_physget(paddr_t *);
> > #endif
> >
> > void uvm_pageactivate(struct vm_page *);
> > +void uvm_pagedequeue(struct vm_page *);
> > vaddr_tuvm_pageboot_alloc(vsize_t);
> > void uvm_pagecopy(struct vm_page *, struct vm_page *);
> > void uvm_pagedeactivate(struct vm_page *);
> >
>
>
rpki-client: shuffle two IP address functions down a bit
This moves two helper functions down so that the file starts with the
code parsing ASIdentifiers, then the code dealing with IPAddrBlocks.
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.74
diff -u -p -r1.74 cert.c
--- cert.c 10 May 2022 16:43:53 - 1.74
+++ cert.c 10 May 2022 16:44:44 -
@@ -54,34 +54,6 @@ extern ASN1_OBJECT *manifest_oid; /* 1.3
extern ASN1_OBJECT *notify_oid;/* 1.3.6.1.5.5.7.48.13 (rpkiNotify) */
/*
- * Append an IP address structure to our list of results.
- * This will also constrain us to having at most one inheritance
- * statement per AFI and also not have overlapping ranges (as prohibited
- * in section 2.2.3.6).
- * It does not make sure that ranges can't coalesce, that is, that any
- * two ranges abut each other.
- * This is warned against in section 2.2.3.6, but doesn't change the
- * semantics of the system.
- * Returns zero on failure (IP overlap) non-zero on success.
- */
-static int
-append_ip(struct parse *p, const struct cert_ip *ip)
-{
- struct cert *res = p->res;
-
- if (!ip_addr_check_overlap(ip, p->fn, p->res->ips, p->res->ipsz))
- return 0;
- if (res->ipsz >= MAX_IP_SIZE)
- return 0;
- res->ips = reallocarray(res->ips, res->ipsz + 1,
- sizeof(struct cert_ip));
- if (res->ips == NULL)
- err(1, NULL);
- res->ips[res->ipsz++] = *ip;
- return 1;
-}
-
-/*
* Append an AS identifier structure to our list of results.
* Makes sure that the identifiers do not overlap or improperly inherit
* as defined by RFC 3779 section 3.3.
@@ -102,28 +74,6 @@ append_as(struct parse *p, const struct
}
/*
- * Construct a RFC 3779 2.2.3.8 range from its bit string.
- * Returns zero on failure, non-zero on success.
- */
-static int
-sbgp_addr(struct parse *p, struct cert_ip *ip, const ASN1_BIT_STRING *bs)
-{
- if (!ip_addr_parse(bs, ip->afi, p->fn, >ip)) {
- warnx("%s: RFC 3779 section 2.2.3.8: IPAddress: "
- "invalid IP address", p->fn);
- return 0;
- }
-
- if (!ip_cert_compose_ranges(ip)) {
- warnx("%s: RFC 3779 section 2.2.3.8: IPAddress: "
- "IP address range reversed", p->fn);
- return 0;
- }
-
- return append_ip(p, ip);
-}
-
-/*
* Parse a range of addresses as in 3.2.3.8.
* Returns zero on failure, non-zero on success.
*/
@@ -414,6 +364,56 @@ out:
sk_ASN1_TYPE_pop_free(sseq, ASN1_TYPE_free);
free(sv);
return rc;
+}
+
+/*
+ * Append an IP address structure to our list of results.
+ * This will also constrain us to having at most one inheritance
+ * statement per AFI and also not have overlapping ranges (as prohibited
+ * in section 2.2.3.6).
+ * It does not make sure that ranges can't coalesce, that is, that any
+ * two ranges abut each other.
+ * This is warned against in section 2.2.3.6, but doesn't change the
+ * semantics of the system.
+ * Returns zero on failure (IP overlap) non-zero on success.
+ */
+static int
+append_ip(struct parse *p, const struct cert_ip *ip)
+{
+ struct cert *res = p->res;
+
+ if (!ip_addr_check_overlap(ip, p->fn, p->res->ips, p->res->ipsz))
+ return 0;
+ if (res->ipsz >= MAX_IP_SIZE)
+ return 0;
+ res->ips = reallocarray(res->ips, res->ipsz + 1,
+ sizeof(struct cert_ip));
+ if (res->ips == NULL)
+ err(1, NULL);
+ res->ips[res->ipsz++] = *ip;
+ return 1;
+}
+
+/*
+ * Construct a RFC 3779 2.2.3.8 range from its bit string.
+ * Returns zero on failure, non-zero on success.
+ */
+static int
+sbgp_addr(struct parse *p, struct cert_ip *ip, const ASN1_BIT_STRING *bs)
+{
+ if (!ip_addr_parse(bs, ip->afi, p->fn, >ip)) {
+ warnx("%s: RFC 3779 section 2.2.3.8: IPAddress: "
+ "invalid IP address", p->fn);
+ return 0;
+ }
+
+ if (!ip_cert_compose_ranges(ip)) {
+ warnx("%s: RFC 3779 section 2.2.3.8: IPAddress: "
+ "IP address range reversed", p->fn);
+ return 0;
+ }
+
+ return append_ip(p, ip);
}
/*
Re: uvm_pagedequeue()
On 05/05/22(Thu) 14:54, Martin Pieuchot wrote:
> Diff below introduces a new wrapper to manipulate active/inactive page
> queues.
>
> ok?
Anyone?
> Index: uvm/uvm_page.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_page.c,v
> retrieving revision 1.165
> diff -u -p -r1.165 uvm_page.c
> --- uvm/uvm_page.c4 May 2022 14:58:26 - 1.165
> +++ uvm/uvm_page.c5 May 2022 12:49:13 -
> @@ -987,16 +987,7 @@ uvm_pageclean(struct vm_page *pg)
> /*
>* now remove the page from the queues
>*/
> - if (pg->pg_flags & PQ_ACTIVE) {
> - TAILQ_REMOVE(_active, pg, pageq);
> - flags_to_clear |= PQ_ACTIVE;
> - uvmexp.active--;
> - }
> - if (pg->pg_flags & PQ_INACTIVE) {
> - TAILQ_REMOVE(_inactive, pg, pageq);
> - flags_to_clear |= PQ_INACTIVE;
> - uvmexp.inactive--;
> - }
> + uvm_pagedequeue(pg);
>
> /*
>* if the page was wired, unwire it now.
> @@ -1243,16 +1234,7 @@ uvm_pagewire(struct vm_page *pg)
> MUTEX_ASSERT_LOCKED();
>
> if (pg->wire_count == 0) {
> - if (pg->pg_flags & PQ_ACTIVE) {
> - TAILQ_REMOVE(_active, pg, pageq);
> - atomic_clearbits_int(>pg_flags, PQ_ACTIVE);
> - uvmexp.active--;
> - }
> - if (pg->pg_flags & PQ_INACTIVE) {
> - TAILQ_REMOVE(_inactive, pg, pageq);
> - atomic_clearbits_int(>pg_flags, PQ_INACTIVE);
> - uvmexp.inactive--;
> - }
> + uvm_pagedequeue(pg);
> uvmexp.wired++;
> }
> pg->wire_count++;
> @@ -1324,28 +1306,32 @@ uvm_pageactivate(struct vm_page *pg)
> KASSERT(uvm_page_owner_locked_p(pg));
> MUTEX_ASSERT_LOCKED();
>
> + uvm_pagedequeue(pg);
> + if (pg->wire_count == 0) {
> + TAILQ_INSERT_TAIL(_active, pg, pageq);
> + atomic_setbits_int(>pg_flags, PQ_ACTIVE);
> + uvmexp.active++;
> +
> + }
> +}
> +
> +/*
> + * uvm_pagedequeue: remove a page from any paging queue
> + */
> +void
> +uvm_pagedequeue(struct vm_page *pg)
> +{
> + if (pg->pg_flags & PQ_ACTIVE) {
> + TAILQ_REMOVE(_active, pg, pageq);
> + atomic_clearbits_int(>pg_flags, PQ_ACTIVE);
> + uvmexp.active--;
> + }
> if (pg->pg_flags & PQ_INACTIVE) {
> TAILQ_REMOVE(_inactive, pg, pageq);
> atomic_clearbits_int(>pg_flags, PQ_INACTIVE);
> uvmexp.inactive--;
> }
> - if (pg->wire_count == 0) {
> - /*
> - * if page is already active, remove it from list so we
> - * can put it at tail. if it wasn't active, then mark
> - * it active and bump active count
> - */
> - if (pg->pg_flags & PQ_ACTIVE)
> - TAILQ_REMOVE(_active, pg, pageq);
> - else {
> - atomic_setbits_int(>pg_flags, PQ_ACTIVE);
> - uvmexp.active++;
> - }
> -
> - TAILQ_INSERT_TAIL(_active, pg, pageq);
> - }
> }
> -
> /*
> * uvm_pagezero: zero fill a page
> */
> Index: uvm/uvm_page.h
> ===
> RCS file: /cvs/src/sys/uvm/uvm_page.h,v
> retrieving revision 1.67
> diff -u -p -r1.67 uvm_page.h
> --- uvm/uvm_page.h29 Jan 2022 06:25:33 - 1.67
> +++ uvm/uvm_page.h5 May 2022 12:49:13 -
> @@ -224,6 +224,7 @@ boolean_t uvm_page_physget(paddr_t *);
> #endif
>
> void uvm_pageactivate(struct vm_page *);
> +void uvm_pagedequeue(struct vm_page *);
> vaddr_t uvm_pageboot_alloc(vsize_t);
> void uvm_pagecopy(struct vm_page *, struct vm_page *);
> void uvm_pagedeactivate(struct vm_page *);
>
Re: rpki-client: deserialize IPAddrBlocks in libcrypto
On Tue, May 10, 2022 at 01:27:17PM +0200, Theo Buehler wrote:
> This is a straightforward conversion to letting libcrypto's RFC 3779
> code parse a cert's IPAddrBlocks. The magic happens in X509V3_EXT_d2i()
> in sbgp_ipaddrblk(). After that, we simply have to walk the returned
> structure. The fact that it parsed only means that it is well-formed as
> far as the templating goes. Notably, at this point we will not yet have
> called X509v3_addr_is_canonical(), so no additional validation of the
> extension has happened beforehand.
>
> This does not use any of the X509v3_addr_* API apart from the types.
>
> There's more cleanup that can be done, e.g. the reaching through and
> copying of struct ip is a bit weird. This also fixes at least one leak
> in sbgp_addr_range().
Lovley. The new code is so much shorter. Also the necessary sacrifice to
understand the code went from a rhino to a chicken. Big improvement.
OK claudio@
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 cert.c
> --- cert.c21 Apr 2022 12:59:03 - 1.71
> +++ cert.c10 May 2022 10:29:57 -
> @@ -423,49 +423,18 @@ out:
> */
> static int
> sbgp_addr_range(struct parse *p, struct cert_ip *ip,
> - const unsigned char *d, size_t dsz)
> +const IPAddressRange *range)
> {
> - ASN1_SEQUENCE_ANY *seq;
> - const ASN1_TYPE *t;
> - int rc = 0;
> -
> - if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, , dsz)) == NULL) {
> - cryptowarnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> - "failed ASN.1 sequence parse", p->fn);
> - goto out;
> - }
> - if (sk_ASN1_TYPE_num(seq) != 2) {
> - warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> - "want 2 elements, have %d", p->fn, sk_ASN1_TYPE_num(seq));
> - goto out;
> - }
> -
> - t = sk_ASN1_TYPE_value(seq, 0);
> - if (t->type != V_ASN1_BIT_STRING) {
> - warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> - "want ASN.1 bit string, have %s (NID %d)",
> - p->fn, ASN1_tag2str(t->type), t->type);
> - goto out;
> - }
> - if (!ip_addr_parse(t->value.bit_string,
> - ip->afi, p->fn, >range.min)) {
> + if (!ip_addr_parse(range->min, ip->afi, p->fn, >range.min)) {
> warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> "invalid IP address", p->fn);
> - goto out;
> + return 0;
> }
>
> - t = sk_ASN1_TYPE_value(seq, 1);
> - if (t->type != V_ASN1_BIT_STRING) {
> - warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> - "want ASN.1 bit string, have %s (NID %d)",
> - p->fn, ASN1_tag2str(t->type), t->type);
> - goto out;
> - }
> - if (!ip_addr_parse(t->value.bit_string,
> - ip->afi, p->fn, >range.max)) {
> + if (!ip_addr_parse(range->max, ip->afi, p->fn, >range.max)) {
> warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> "invalid IP address", p->fn);
> - goto out;
> + return 0;
> }
>
> if (!ip_cert_compose_ranges(ip)) {
> @@ -474,10 +443,7 @@ sbgp_addr_range(struct parse *p, struct
> return 0;
> }
>
> - rc = append_ip(p, ip);
> -out:
> - sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
> - return rc;
> + return append_ip(p, ip);
> }
>
> /*
> @@ -488,48 +454,35 @@ out:
> */
> static int
> sbgp_addr_or_range(struct parse *p, struct cert_ip *ip,
> - const unsigned char *d, size_t dsz)
> +const IPAddressOrRanges *aors)
> {
> struct cert_ip nip;
> - ASN1_SEQUENCE_ANY *seq;
> - const ASN1_TYPE *t;
> + const IPAddressOrRange *aor;
> int i, rc = 0;
>
> - if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, , dsz)) == NULL) {
> - cryptowarnx("%s: RFC 3779 section 2.2.3.7: IPAddressOrRange: "
> - "failed ASN.1 sequence parse", p->fn);
> - goto out;
> - }
> -
> - /* Either RFC 3779 2.2.3.8 or 2.2.3.9. */
> -
> - for (i = 0; i < sk_ASN1_TYPE_num(seq); i++) {
> + for (i = 0; i < sk_IPAddressOrRange_num(aors); i++) {
> nip = *ip;
> - t = sk_ASN1_TYPE_value(seq, i);
> - switch (t->type) {
> - case V_ASN1_BIT_STRING:
> + aor = sk_IPAddressOrRange_value(aors, i);
> + switch (aor->type) {
> + case IPAddressOrRange_addressPrefix:
> nip.type = CERT_IP_ADDR;
> - if (!sbgp_addr(p, , t->value.bit_string))
> + if (!sbgp_addr(p, , aor->u.addressPrefix))
> goto out;
> break;
> -
Re: rpki-client: three leaks in cert.c
On Tue, May 10, 2022 at 01:47:44PM +0200, Theo Buehler wrote:
> In sbgp_asrange() and sbgp_addr_range(), the ASN1_SEQUENCE_ANY *seq is
> potentially leaked due to early return 0 instead of goto out. The last
> hunk collides with my IPAddrBlocks diff. Sending this out so I don't
> forget.
>
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 cert.c
> --- cert.c21 Apr 2022 12:59:03 - 1.71
> +++ cert.c10 May 2022 11:35:57 -
> @@ -161,7 +161,7 @@ sbgp_asrange(struct parse *p, const unsi
> if (!as_id_parse(t->value.integer, )) {
> warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
> "malformed AS identifier", p->fn);
> - return 0;
> + goto out;
> }
>
> t = sk_ASN1_TYPE_value(seq, 1);
> @@ -174,7 +174,7 @@ sbgp_asrange(struct parse *p, const unsi
> if (!as_id_parse(t->value.integer, )) {
> warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
> "malformed AS identifier", p->fn);
> - return 0;
> + goto out;
> }
>
> if (as.range.max == as.range.min) {
> @@ -471,7 +471,7 @@ sbgp_addr_range(struct parse *p, struct
> if (!ip_cert_compose_ranges(ip)) {
> warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> "IP address range reversed", p->fn);
> - return 0;
> + goto out;
> }
>
> rc = append_ip(p, ip);
>
OK claudio@
--
:wq Claudio
Re: [External] : Re: move memory allocation in pfr_add_tables() outside of NET_LOCK()/PF_LOCK()
On Tue, May 10, 2022 at 12:40:39AM +0200, Alexandr Nedvedicky wrote:
> updated diff is below.
Although this makes the code more complex, I see no simple solution.
We must get the sleeps out of the pf lock. As seen from the mail
on bugs@, sleeping in pf is not a good thing. Also mbuhl@ is hunting
some sleeping bugs with copyin and copyout from syzkaller. So this
is the right direction.
OK bluhm@
> 8<---8<---8<--8<
> diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
> index 8315b115474..1f036e1368f 100644
> --- a/sys/net/pf_ioctl.c
> +++ b/sys/net/pf_ioctl.c
> @@ -2217,12 +2217,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> error = ENODEV;
> goto fail;
> }
> - NET_LOCK();
> - PF_LOCK();
> error = pfr_add_tables(io->pfrio_buffer, io->pfrio_size,
> >pfrio_nadd, io->pfrio_flags | PFR_FLAG_USERIOCTL);
> - PF_UNLOCK();
> - NET_UNLOCK();
> break;
> }
>
> diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
> index fb23bcabe04..d0e42ca62ba 100644
> --- a/sys/net/pf_table.c
> +++ b/sys/net/pf_table.c
> @@ -189,6 +189,7 @@ void pfr_clstats_ktable(struct
> pfr_ktable *, time_t, int);
> struct pfr_ktable*pfr_create_ktable(struct pfr_table *, time_t, int,
> int);
> void pfr_destroy_ktables(struct pfr_ktableworkq *, int);
> +void pfr_destroy_ktables_aux(struct pfr_ktableworkq *);
> void pfr_destroy_ktable(struct pfr_ktable *, int);
> int pfr_ktable_compare(struct pfr_ktable *,
> struct pfr_ktable *);
> @@ -1493,14 +1494,16 @@ pfr_clr_tables(struct pfr_table *filter, int *ndel,
> int flags)
> int
> pfr_add_tables(struct pfr_table *tbl, int size, int *nadd, int flags)
> {
> - struct pfr_ktableworkq addq, changeq;
> - struct pfr_ktable *p, *q, *r, key;
> + struct pfr_ktableworkq addq, changeq, auxq;
> + struct pfr_ktable *p, *q, *r, *n, *w, key;
> int i, rv, xadd = 0;
> time_t tzero = gettime();
>
> ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY);
> SLIST_INIT();
> SLIST_INIT();
> + SLIST_INIT();
> + /* pre-allocate all memory outside of locks */
> for (i = 0; i < size; i++) {
> YIELD(flags & PFR_FLAG_USERIOCTL);
> if (COPYIN(tbl+i, _t, sizeof(key.pfrkt_t), flags))
> @@ -1509,65 +1512,149 @@ pfr_add_tables(struct pfr_table *tbl, int size, int
> *nadd, int flags)
> flags & PFR_FLAG_USERIOCTL))
> senderr(EINVAL);
> key.pfrkt_flags |= PFR_TFLAG_ACTIVE;
> - p = RB_FIND(pfr_ktablehead, _ktables, );
> + p = pfr_create_ktable(_t, tzero, 0,
> + !(flags & PFR_FLAG_USERIOCTL));
> + if (p == NULL)
> + senderr(ENOMEM);
> +
> + /*
> + * Note: we also pre-allocate a root table here. We keep it
> + * at ->pfrkt_root, which we must not forget about.
> + */
> + key.pfrkt_flags = 0;
> + memset(key.pfrkt_anchor, 0, sizeof(key.pfrkt_anchor));
> + p->pfrkt_root = pfr_create_ktable(_t, 0, 0,
> + !(flags & PFR_FLAG_USERIOCTL));
> + if (p->pfrkt_root == NULL) {
> + pfr_destroy_ktable(p, 0);
> + senderr(ENOMEM);
> + }
> +
> + SLIST_FOREACH(q, , pfrkt_workq) {
> + if (!pfr_ktable_compare(p, q)) {
> + /*
> + * We need no lock here, because `p` is empty,
> + * there are no rules or shadow tables
> + * attached.
> + */
> + pfr_destroy_ktable(p->pfrkt_root, 0);
> + p->pfrkt_root = NULL;
> + pfr_destroy_ktable(p, 0);
> + p = NULL;
> + break;
> + }
> + }
> + if (q != NULL)
> + continue;
> +
> + SLIST_INSERT_HEAD(, p, pfrkt_workq);
> + }
> +
> + /*
> + * auxq contains freshly allocated tables with no dups.
> + * also note there are no rulesets attached, because
> + * the attach operation requires PF_LOCK().
> + */
> + NET_LOCK();
> + PF_LOCK();
> + SLIST_FOREACH_SAFE(n, , pfrkt_workq, w) {
> + p = RB_FIND(pfr_ktablehead, _ktables, n);
> if (p == NULL) {
> - p = pfr_create_ktable(_t, tzero, 1,
> - !(flags & PFR_FLAG_USERIOCTL));
> -
rpki-client: three leaks in cert.c
In sbgp_asrange() and sbgp_addr_range(), the ASN1_SEQUENCE_ANY *seq is
potentially leaked due to early return 0 instead of goto out. The last
hunk collides with my IPAddrBlocks diff. Sending this out so I don't
forget.
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.71
diff -u -p -r1.71 cert.c
--- cert.c 21 Apr 2022 12:59:03 - 1.71
+++ cert.c 10 May 2022 11:35:57 -
@@ -161,7 +161,7 @@ sbgp_asrange(struct parse *p, const unsi
if (!as_id_parse(t->value.integer, )) {
warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
"malformed AS identifier", p->fn);
- return 0;
+ goto out;
}
t = sk_ASN1_TYPE_value(seq, 1);
@@ -174,7 +174,7 @@ sbgp_asrange(struct parse *p, const unsi
if (!as_id_parse(t->value.integer, )) {
warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
"malformed AS identifier", p->fn);
- return 0;
+ goto out;
}
if (as.range.max == as.range.min) {
@@ -471,7 +471,7 @@ sbgp_addr_range(struct parse *p, struct
if (!ip_cert_compose_ranges(ip)) {
warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
"IP address range reversed", p->fn);
- return 0;
+ goto out;
}
rc = append_ip(p, ip);
rpki-client: deserialize IPAddrBlocks in libcrypto
This is a straightforward conversion to letting libcrypto's RFC 3779
code parse a cert's IPAddrBlocks. The magic happens in X509V3_EXT_d2i()
in sbgp_ipaddrblk(). After that, we simply have to walk the returned
structure. The fact that it parsed only means that it is well-formed as
far as the templating goes. Notably, at this point we will not yet have
called X509v3_addr_is_canonical(), so no additional validation of the
extension has happened beforehand.
This does not use any of the X509v3_addr_* API apart from the types.
There's more cleanup that can be done, e.g. the reaching through and
copying of struct ip is a bit weird. This also fixes at least one leak
in sbgp_addr_range().
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.71
diff -u -p -r1.71 cert.c
--- cert.c 21 Apr 2022 12:59:03 - 1.71
+++ cert.c 10 May 2022 10:29:57 -
@@ -423,49 +423,18 @@ out:
*/
static int
sbgp_addr_range(struct parse *p, struct cert_ip *ip,
- const unsigned char *d, size_t dsz)
+const IPAddressRange *range)
{
- ASN1_SEQUENCE_ANY *seq;
- const ASN1_TYPE *t;
- int rc = 0;
-
- if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, , dsz)) == NULL) {
- cryptowarnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
- "failed ASN.1 sequence parse", p->fn);
- goto out;
- }
- if (sk_ASN1_TYPE_num(seq) != 2) {
- warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
- "want 2 elements, have %d", p->fn, sk_ASN1_TYPE_num(seq));
- goto out;
- }
-
- t = sk_ASN1_TYPE_value(seq, 0);
- if (t->type != V_ASN1_BIT_STRING) {
- warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
- "want ASN.1 bit string, have %s (NID %d)",
- p->fn, ASN1_tag2str(t->type), t->type);
- goto out;
- }
- if (!ip_addr_parse(t->value.bit_string,
- ip->afi, p->fn, >range.min)) {
+ if (!ip_addr_parse(range->min, ip->afi, p->fn, >range.min)) {
warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
"invalid IP address", p->fn);
- goto out;
+ return 0;
}
- t = sk_ASN1_TYPE_value(seq, 1);
- if (t->type != V_ASN1_BIT_STRING) {
- warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
- "want ASN.1 bit string, have %s (NID %d)",
- p->fn, ASN1_tag2str(t->type), t->type);
- goto out;
- }
- if (!ip_addr_parse(t->value.bit_string,
- ip->afi, p->fn, >range.max)) {
+ if (!ip_addr_parse(range->max, ip->afi, p->fn, >range.max)) {
warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
"invalid IP address", p->fn);
- goto out;
+ return 0;
}
if (!ip_cert_compose_ranges(ip)) {
@@ -474,10 +443,7 @@ sbgp_addr_range(struct parse *p, struct
return 0;
}
- rc = append_ip(p, ip);
-out:
- sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
- return rc;
+ return append_ip(p, ip);
}
/*
@@ -488,48 +454,35 @@ out:
*/
static int
sbgp_addr_or_range(struct parse *p, struct cert_ip *ip,
- const unsigned char *d, size_t dsz)
+const IPAddressOrRanges *aors)
{
struct cert_ip nip;
- ASN1_SEQUENCE_ANY *seq;
- const ASN1_TYPE *t;
+ const IPAddressOrRange *aor;
int i, rc = 0;
- if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, , dsz)) == NULL) {
- cryptowarnx("%s: RFC 3779 section 2.2.3.7: IPAddressOrRange: "
- "failed ASN.1 sequence parse", p->fn);
- goto out;
- }
-
- /* Either RFC 3779 2.2.3.8 or 2.2.3.9. */
-
- for (i = 0; i < sk_ASN1_TYPE_num(seq); i++) {
+ for (i = 0; i < sk_IPAddressOrRange_num(aors); i++) {
nip = *ip;
- t = sk_ASN1_TYPE_value(seq, i);
- switch (t->type) {
- case V_ASN1_BIT_STRING:
+ aor = sk_IPAddressOrRange_value(aors, i);
+ switch (aor->type) {
+ case IPAddressOrRange_addressPrefix:
nip.type = CERT_IP_ADDR;
- if (!sbgp_addr(p, , t->value.bit_string))
+ if (!sbgp_addr(p, , aor->u.addressPrefix))
goto out;
break;
- case V_ASN1_SEQUENCE:
+ case IPAddressOrRange_addressRange:
nip.type = CERT_IP_RANGE;
- d = t->value.asn1_string->data;
- dsz = t->value.asn1_string->length;
- if (!sbgp_addr_range(p, , d,
Re: rpki-client: consider CA:FALSE certs to be invalid
On Tue, May 10, 2022 at 08:32:15AM +, Job Snijders wrote:
> Hi,
>
> Following this errata report https://www.rfc-editor.org/errata/eid6854
>
> If the Basic Constraints extension is present, and the certificate is
> *not* a CA - as determined by X509_check_ca(3), leave the purpose as
> CERT_PURPOSE_INVALID.
LibreSSL's X509_check_ca() has a bug that will make it return 1 if
x509v3_cache_extensions() fails... That shouldn't affect us here since
x should already have its extensions cached.
I think for your purposes it is better to check the extension flags
instead of parsing the basic constraints extension another time:
if (X509_get_extension_flags(x) & EXFLAG_BCONS) {
warnx("%s: Basic Constraints in non-CA cert", fn);
goto out;
}
and we should probably consider rewriting this function to inspect
extension flags also for the CA case.
>
> While there, use the value name rather than 0.
>
> OK?
>
> Kind regards,
>
> Job
>
> Index: x509.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 x509.c
> --- x509.c9 May 2022 17:13:06 - 1.42
> +++ x509.c10 May 2022 08:31:03 -
> @@ -182,15 +182,22 @@ out:
> enum cert_purpose
> x509_get_purpose(X509 *x, const char *fn)
> {
> + BASIC_CONSTRAINTS *bc = NULL;
> EXTENDED_KEY_USAGE *eku = NULL;
> int crit;
> - enum cert_purposepurpose = 0;
> + enum cert_purposepurpose = CERT_PURPOSE_INVALID;
>
> if (X509_check_ca(x) == 1) {
> purpose = CERT_PURPOSE_CA;
> goto out;
> }
>
> + bc = X509_get_ext_d2i(x, NID_basic_constraints, , NULL);
> + if (bc != NULL) {
> + warnx("%s: Basic Constraints ext in CA:FALSE cert", fn);
> + goto out;
> + }
> +
> eku = X509_get_ext_d2i(x, NID_ext_key_usage, , NULL);
> if (eku == NULL) {
> warnx("%s: EKU: extension missing", fn);
> @@ -212,6 +219,7 @@ x509_get_purpose(X509 *x, const char *fn
> }
>
> out:
> + BASIC_CONSTRAINTS_free(bc);
> EXTENDED_KEY_USAGE_free(eku);
> return purpose;
> }
>
rpki-client: consider CA:FALSE certs to be invalid
Hi,
Following this errata report https://www.rfc-editor.org/errata/eid6854
If the Basic Constraints extension is present, and the certificate is
*not* a CA - as determined by X509_check_ca(3), leave the purpose as
CERT_PURPOSE_INVALID.
While there, use the value name rather than 0.
OK?
Kind regards,
Job
Index: x509.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
retrieving revision 1.42
diff -u -p -r1.42 x509.c
--- x509.c 9 May 2022 17:13:06 - 1.42
+++ x509.c 10 May 2022 08:31:03 -
@@ -182,15 +182,22 @@ out:
enum cert_purpose
x509_get_purpose(X509 *x, const char *fn)
{
+ BASIC_CONSTRAINTS *bc = NULL;
EXTENDED_KEY_USAGE *eku = NULL;
int crit;
- enum cert_purposepurpose = 0;
+ enum cert_purposepurpose = CERT_PURPOSE_INVALID;
if (X509_check_ca(x) == 1) {
purpose = CERT_PURPOSE_CA;
goto out;
}
+ bc = X509_get_ext_d2i(x, NID_basic_constraints, , NULL);
+ if (bc != NULL) {
+ warnx("%s: Basic Constraints ext in CA:FALSE cert", fn);
+ goto out;
+ }
+
eku = X509_get_ext_d2i(x, NID_ext_key_usage, , NULL);
if (eku == NULL) {
warnx("%s: EKU: extension missing", fn);
@@ -212,6 +219,7 @@ x509_get_purpose(X509 *x, const char *fn
}
out:
+ BASIC_CONSTRAINTS_free(bc);
EXTENDED_KEY_USAGE_free(eku);
return purpose;
}
Re: I got the RALINK RT5372 usb wifi adapter working
On Mon, May 09, 2022 at 08:17:35PM +, molotov31337 wrote: > I recently picked up a Panda Wireless PAU06 and got it working, can this be > committed? > Here is the cvs diff Committed, thanks! > Index: usbdevs > === > RCS file: /cvs/src/sys/dev/usb/usbdevs,v > retrieving revision 1.745 > diff -u -p -u -r1.745 usbdevs > --- usbdevs 24 Dec 2021 06:18:11 - 1.745 > +++ usbdevs 9 May 2022 20:12:42 - > @@ -3731,6 +3731,7 @@ product RALINK RT3370 0x3370 RT3370 > product RALINK RT3572 0x3572 RT3572 > product RALINK RT3573 0x3573 RT3573 > product RALINK RT5370 0x5370 RT5370 > +product RALINK RT5372 0x5372 RT5372 > product RALINK RT5572 0x5572 RT5572 > product RALINK MT7601 0x7601 MT7601 > product RALINK MT7601_2 0x760a MT7601 > Index: if_run.c > === > RCS file: /cvs/src/sys/dev/usb/if_run.c,v > retrieving revision 1.135 > diff -u -p -u -r1.135 if_run.c > --- if_run.c 22 Nov 2021 10:17:14 - 1.135 > +++ if_run.c 9 May 2022 20:12:43 - > @@ -259,6 +259,7 @@ static const struct usb_devno run_devs[] > USB_ID(RALINK, RT3572), > USB_ID(RALINK, RT3573), > USB_ID(RALINK, RT5370), > + USB_ID(RALINK, RT5372), > USB_ID(RALINK, RT5572), > USB_ID(RALINK, RT8070), USB_ID(SAMSUNG, WIS09ABGN), > > And the device shows up in dmesg as > run0 at uhub0 port 4 configuration 1 interface 0 "Ralink 802.11 n WLAN" rev > 2.00/1.01 addr 2 > run0: MAC/BBP RT5392 (rev 0x0223), RF RT5372 (MIMO 2T2R), address > 9c:ef:d5:fa:4b:15 > > Sent with [ProtonMail](https://protonmail.com/) secure email.
fix mac address on iwx(4) AX210
As noticed by jsg@ and kevlo@ we use a bad MAC address on AX210 devices.
Patch below fixes the issue on AX210, and still works on AX200.
The old way of reading the MAC no longer works on AX210; apparently this
new way of reading the MAC was introduced in the 9k hw generation but the
old way was still working until now.
ok?
diff 04382ee86a01509dce834178b7ab1460d3539207
37ed6bbfb5c966e24cc7dbbe8dd40222aac69407
blob - b3ea3675ae9dc65b56a1c90ab7f92c1c0a4f3a8f
blob + 9e94f17a9fc41ca7c3ce1096497ccc478f958197
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -341,8 +341,9 @@ voidiwx_sta_tx_agg_start(struct iwx_softc *, struct
i
uint8_t);
void iwx_ba_task(void *);
-intiwx_set_mac_addr_from_csr(struct iwx_softc *, struct iwx_nvm_data *);
+void iwx_set_mac_addr_from_csr(struct iwx_softc *, struct iwx_nvm_data *);
intiwx_is_valid_mac_addr(const uint8_t *);
+void iwx_flip_hw_address(uint32_t, uint32_t, uint8_t *);
intiwx_nvm_get(struct iwx_softc *);
intiwx_load_firmware(struct iwx_softc *);
intiwx_start_fw(struct iwx_softc *);
@@ -3688,31 +3689,33 @@ iwx_ampdu_tx_start(struct ieee80211com *ic, struct iee
return EBUSY;
}
-/* Read the mac address from WFMP registers. */
-int
+void
iwx_set_mac_addr_from_csr(struct iwx_softc *sc, struct iwx_nvm_data *data)
{
- const uint8_t *hw_addr;
uint32_t mac_addr0, mac_addr1;
+ memset(data->hw_addr, 0, sizeof(data->hw_addr));
+
if (!iwx_nic_lock(sc))
- return EBUSY;
+ return;
- mac_addr0 = htole32(iwx_read_prph(sc, IWX_WFMP_MAC_ADDR_0));
- mac_addr1 = htole32(iwx_read_prph(sc, IWX_WFMP_MAC_ADDR_1));
+ mac_addr0 = htole32(IWX_READ(sc, IWX_CSR_MAC_ADDR0_STRAP(sc)));
+ mac_addr1 = htole32(IWX_READ(sc, IWX_CSR_MAC_ADDR1_STRAP(sc)));
- hw_addr = (const uint8_t *)_addr0;
- data->hw_addr[0] = hw_addr[3];
- data->hw_addr[1] = hw_addr[2];
- data->hw_addr[2] = hw_addr[1];
- data->hw_addr[3] = hw_addr[0];
+ iwx_flip_hw_address(mac_addr0, mac_addr1, data->hw_addr);
- hw_addr = (const uint8_t *)_addr1;
- data->hw_addr[4] = hw_addr[1];
- data->hw_addr[5] = hw_addr[0];
+ /* If OEM fused a valid address, use it instead of the one in OTP. */
+ if (iwx_is_valid_mac_addr(data->hw_addr)) {
+ iwx_nic_unlock(sc);
+ return;
+ }
+ mac_addr0 = htole32(IWX_READ(sc, IWX_CSR_MAC_ADDR0_OTP(sc)));
+ mac_addr1 = htole32(IWX_READ(sc, IWX_CSR_MAC_ADDR1_OTP(sc)));
+
+ iwx_flip_hw_address(mac_addr0, mac_addr1, data->hw_addr);
+
iwx_nic_unlock(sc);
- return 0;
}
int
@@ -3728,6 +3731,22 @@ iwx_is_valid_mac_addr(const uint8_t *addr)
!ETHER_IS_MULTICAST(addr));
}
+void
+iwx_flip_hw_address(uint32_t mac_addr0, uint32_t mac_addr1, uint8_t *dest)
+{
+ const uint8_t *hw_addr;
+
+ hw_addr = (const uint8_t *)_addr0;
+ dest[0] = hw_addr[3];
+ dest[1] = hw_addr[2];
+ dest[2] = hw_addr[1];
+ dest[3] = hw_addr[0];
+
+ hw_addr = (const uint8_t *)_addr1;
+ dest[4] = hw_addr[1];
+ dest[5] = hw_addr[0];
+}
+
int
iwx_nvm_get(struct iwx_softc *sc)
{
@@ -10654,6 +10673,8 @@ iwx_attach(struct device *parent, struct device *self,
}
}
+ sc->mac_addr_from_csr = 0x380; /* differs on BZ hw generation */
+
if (sc->sc_device_family >= IWX_DEVICE_FAMILY_AX210) {
sc->sc_umac_prph_offset = 0x30;
sc->max_tfd_queue_size = IWX_TFD_QUEUE_SIZE_MAX_GEN3;
blob - 6ddfb14e5eb9240cc82ea07d12a6e5c846167586
blob + 0f1948913a39bee6956603e3e322b9961615f038
--- sys/dev/pci/if_iwxreg.h
+++ sys/dev/pci/if_iwxreg.h
@@ -1160,6 +1160,12 @@ enum msix_ivar_for_cause {
#define IWX_MSIX_AUTO_CLEAR_CAUSE (0 << 7)
#define IWX_MSIX_NON_AUTO_CLEAR_CAUSE (1 << 7)
+#define IWX_CSR_ADDR_BASE(sc) ((sc)->mac_addr_from_csr)
+#define IWX_CSR_MAC_ADDR0_OTP(sc) (IWX_CSR_ADDR_BASE(sc) + 0x00)
+#define IWX_CSR_MAC_ADDR1_OTP(sc) (IWX_CSR_ADDR_BASE(sc) + 0x04)
+#define IWX_CSR_MAC_ADDR0_STRAP(sc)(IWX_CSR_ADDR_BASE(sc) + 0x08)
+#define IWX_CSR_MAC_ADDR1_STRAP(sc)(IWX_CSR_ADDR_BASE(sc) + 0x0c)
+
/**
* uCode API flags
* @IWX_UCODE_TLV_FLAGS_PAN: This is PAN capable microcode; this previously
blob - bec0758d890fb5939a291ed794a610f9e5ac37df
blob + 19f9a333d1a1e479845512439daaef8adc164ca4
--- sys/dev/pci/if_iwxvar.h
+++ sys/dev/pci/if_iwxvar.h
@@ -681,6 +681,7 @@ struct iwx_softc {
#define IWX_DEVICE_FAMILY_220001
#define IWX_DEVICE_FAMILY_AX2102
uint32_t sc_sku_id[3];
+ uint32_t mac_addr_from_csr;
struct iwx_dma_info ctxt_info_dma;
struct iwx_self_init_dram init_dram;
Re: rpki-client: enforce RSC compliant filenames
On Tue, May 10, 2022 at 08:38:32AM +0200, Theo Buehler wrote:
> On Tue, May 10, 2022 at 08:28:10AM +0200, Claudio Jeker wrote:
> > On Tue, May 10, 2022 at 08:05:00AM +0200, Theo Buehler wrote:
> > > This moves valid_filename() to validate.c and splits out a helper
> > > portable_filename() which can be used from the RSC code. While moving
> > > valid_filename() is not necessary, I thought it makes sense to keep the
> > > two functions next to each other.
> > >
> > > I could not find a short name with valid_ prefix for portable_filename()
> > > that I liked. valid_posix_filename()? valid_checklist_file()?
> > >
> > > Also, valid_filename() could be stricter and enforce that the unique '.'
> > > is followed by a 3-letter suffix.
> >
> > I would use valid_filename() for what you call portable_filename() and
> > then introduce a mft_filename() in mft.c that does the extra check.
> > Or maybe rename valid_filename() to valid_mft_filename().
>
> I like both suggestion. Went with the latter since it is clearer at the
> call site what it does. Thanks
>
> > I don't think we need to enforce the location of the dot. That is enforced
> > by rtype_from_file_extension() and rtype_from_mftfile().
>
> Right.
OK claudio@
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.134
> diff -u -p -r1.134 extern.h
> --- extern.h 9 May 2022 17:19:32 - 1.134
> +++ extern.h 10 May 2022 06:32:03 -
> @@ -502,6 +502,7 @@ intvalid_cert(const char *, struct au
> int valid_roa(const char *, struct auth *, struct roa *);
> int valid_filehash(int, const char *, size_t);
> int valid_hash(unsigned char *, size_t, const char *, size_t);
> +int valid_filename(const char *, size_t);
> int valid_uri(const char *, size_t, const char *);
> int valid_origin(const char *, const char *);
> int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,
> Index: mft.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 mft.c
> --- mft.c 9 May 2022 17:02:34 - 1.61
> +++ mft.c 10 May 2022 06:36:32 -
> @@ -129,16 +129,15 @@ rtype_from_file_extension(const char *fn
> /*
> * Validate that a filename listed on a Manifest only contains characters
> * permitted in draft-ietf-sidrops-6486bis section 4.2.2
> + * Also ensure that there is exactly one '.'.
> */
> static int
> -valid_filename(const char *fn, size_t len)
> +valid_mft_filename(const char *fn, size_t len)
> {
> const unsigned char *c;
> - size_t i;
>
> - for (c = fn, i = 0; i < len; i++, c++)
> - if (!isalnum(*c) && *c != '-' && *c != '_' && *c != '.')
> - return 0;
> + if (!valid_filename(fn, len))
> + return 0;
>
> c = memchr(fn, '.', len);
> if (c == NULL || c != memrchr(fn, '.', len))
> @@ -205,7 +204,7 @@ mft_parse_filehash(struct parse *p, cons
> p->fn, ASN1_tag2str(file->type), file->type);
> goto out;
> }
> - if (!valid_filename(file->value.ia5string->data,
> + if (!valid_mft_filename(file->value.ia5string->data,
> file->value.ia5string->length)) {
> warnx("%s: RFC 6486 section 4.2.2: bad filename", p->fn);
> goto out;
> @@ -482,7 +481,7 @@ mft_parse(X509 **x509, const char *fn, c
> goto out;
> }
> if ((crlfile = strrchr(crldp, '/')) == NULL ||
> - !valid_filename(crlfile + 1, strlen(crlfile + 1)) ||
> + !valid_mft_filename(crlfile + 1, strlen(crlfile + 1)) ||
> rtype_from_file_extension(crlfile + 1) != RTYPE_CRL) {
> warnx("%s: RFC 6487 section 4.8.6: CRL: "
> "bad CRL distribution point extension", fn);
> Index: rsc.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/rsc.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 rsc.c
> --- rsc.c 9 May 2022 17:02:34 - 1.1
> +++ rsc.c 10 May 2022 06:32:09 -
> @@ -147,6 +147,8 @@ rsc_parse_filenamehash(struct parse *p,
> }
>
> if (elemsz == 2) {
> + ASN1_IA5STRING *filename;
> +
> file = sk_ASN1_TYPE_value(seq, i++);
> if (file->type != V_ASN1_IA5STRING) {
> warnx("%s: RSC FileNameAndHash: want ASN.1 IA5 string,"
> @@ -154,25 +156,17 @@ rsc_parse_filenamehash(struct parse *p,
> ASN1_tag2str(file->type), file->type);
> goto out;
> }
> - fn = strndup((const char *)file->value.ia5string->data,
> - file->value.ia5string->length);
> - if (fn == NULL)
> - err(1, NULL);
>
Re: rpki-client: enforce RSC compliant filenames
On Tue, May 10, 2022 at 08:28:10AM +0200, Claudio Jeker wrote:
> On Tue, May 10, 2022 at 08:05:00AM +0200, Theo Buehler wrote:
> > This moves valid_filename() to validate.c and splits out a helper
> > portable_filename() which can be used from the RSC code. While moving
> > valid_filename() is not necessary, I thought it makes sense to keep the
> > two functions next to each other.
> >
> > I could not find a short name with valid_ prefix for portable_filename()
> > that I liked. valid_posix_filename()? valid_checklist_file()?
> >
> > Also, valid_filename() could be stricter and enforce that the unique '.'
> > is followed by a 3-letter suffix.
>
> I would use valid_filename() for what you call portable_filename() and
> then introduce a mft_filename() in mft.c that does the extra check.
> Or maybe rename valid_filename() to valid_mft_filename().
I like both suggestion. Went with the latter since it is clearer at the
call site what it does. Thanks
> I don't think we need to enforce the location of the dot. That is enforced
> by rtype_from_file_extension() and rtype_from_mftfile().
Right.
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.134
diff -u -p -r1.134 extern.h
--- extern.h9 May 2022 17:19:32 - 1.134
+++ extern.h10 May 2022 06:32:03 -
@@ -502,6 +502,7 @@ int valid_cert(const char *, struct au
int valid_roa(const char *, struct auth *, struct roa *);
int valid_filehash(int, const char *, size_t);
int valid_hash(unsigned char *, size_t, const char *, size_t);
+int valid_filename(const char *, size_t);
int valid_uri(const char *, size_t, const char *);
int valid_origin(const char *, const char *);
int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,
Index: mft.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
retrieving revision 1.61
diff -u -p -r1.61 mft.c
--- mft.c 9 May 2022 17:02:34 - 1.61
+++ mft.c 10 May 2022 06:36:32 -
@@ -129,16 +129,15 @@ rtype_from_file_extension(const char *fn
/*
* Validate that a filename listed on a Manifest only contains characters
* permitted in draft-ietf-sidrops-6486bis section 4.2.2
+ * Also ensure that there is exactly one '.'.
*/
static int
-valid_filename(const char *fn, size_t len)
+valid_mft_filename(const char *fn, size_t len)
{
const unsigned char *c;
- size_t i;
- for (c = fn, i = 0; i < len; i++, c++)
- if (!isalnum(*c) && *c != '-' && *c != '_' && *c != '.')
- return 0;
+ if (!valid_filename(fn, len))
+ return 0;
c = memchr(fn, '.', len);
if (c == NULL || c != memrchr(fn, '.', len))
@@ -205,7 +204,7 @@ mft_parse_filehash(struct parse *p, cons
p->fn, ASN1_tag2str(file->type), file->type);
goto out;
}
- if (!valid_filename(file->value.ia5string->data,
+ if (!valid_mft_filename(file->value.ia5string->data,
file->value.ia5string->length)) {
warnx("%s: RFC 6486 section 4.2.2: bad filename", p->fn);
goto out;
@@ -482,7 +481,7 @@ mft_parse(X509 **x509, const char *fn, c
goto out;
}
if ((crlfile = strrchr(crldp, '/')) == NULL ||
- !valid_filename(crlfile + 1, strlen(crlfile + 1)) ||
+ !valid_mft_filename(crlfile + 1, strlen(crlfile + 1)) ||
rtype_from_file_extension(crlfile + 1) != RTYPE_CRL) {
warnx("%s: RFC 6487 section 4.8.6: CRL: "
"bad CRL distribution point extension", fn);
Index: rsc.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/rsc.c,v
retrieving revision 1.1
diff -u -p -r1.1 rsc.c
--- rsc.c 9 May 2022 17:02:34 - 1.1
+++ rsc.c 10 May 2022 06:32:09 -
@@ -147,6 +147,8 @@ rsc_parse_filenamehash(struct parse *p,
}
if (elemsz == 2) {
+ ASN1_IA5STRING *filename;
+
file = sk_ASN1_TYPE_value(seq, i++);
if (file->type != V_ASN1_IA5STRING) {
warnx("%s: RSC FileNameAndHash: want ASN.1 IA5 string,"
@@ -154,25 +156,17 @@ rsc_parse_filenamehash(struct parse *p,
ASN1_tag2str(file->type), file->type);
goto out;
}
- fn = strndup((const char *)file->value.ia5string->data,
- file->value.ia5string->length);
- if (fn == NULL)
- err(1, NULL);
- /*
-* filename must confirm to portable file name character set
-* XXX: use valid_filename() instead
-*/
- if
Re: rpki-client: enforce RSC compliant filenames
On Tue, May 10, 2022 at 08:05:00AM +0200, Theo Buehler wrote:
> This moves valid_filename() to validate.c and splits out a helper
> portable_filename() which can be used from the RSC code. While moving
> valid_filename() is not necessary, I thought it makes sense to keep the
> two functions next to each other.
>
> I could not find a short name with valid_ prefix for portable_filename()
> that I liked. valid_posix_filename()? valid_checklist_file()?
>
> Also, valid_filename() could be stricter and enforce that the unique '.'
> is followed by a 3-letter suffix.
I would use valid_filename() for what you call portable_filename() and
then introduce a mft_filename() in mft.c that does the extra check.
Or maybe rename valid_filename() to valid_mft_filename().
I don't think we need to enforce the location of the dot. That is enforced
by rtype_from_file_extension() and rtype_from_mftfile().
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.134
> diff -u -p -r1.134 extern.h
> --- extern.h 9 May 2022 17:19:32 - 1.134
> +++ extern.h 10 May 2022 06:03:19 -
> @@ -502,6 +502,8 @@ intvalid_cert(const char *, struct au
> int valid_roa(const char *, struct auth *, struct roa *);
> int valid_filehash(int, const char *, size_t);
> int valid_hash(unsigned char *, size_t, const char *, size_t);
> +int portable_filename(const char *, size_t);
> +int valid_filename(const char *, size_t);
> int valid_uri(const char *, size_t, const char *);
> int valid_origin(const char *, const char *);
> int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,
> Index: mft.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 mft.c
> --- mft.c 9 May 2022 17:02:34 - 1.61
> +++ mft.c 10 May 2022 06:03:19 -
> @@ -127,27 +127,6 @@ rtype_from_file_extension(const char *fn
> }
>
> /*
> - * Validate that a filename listed on a Manifest only contains characters
> - * permitted in draft-ietf-sidrops-6486bis section 4.2.2
> - */
> -static int
> -valid_filename(const char *fn, size_t len)
> -{
> - const unsigned char *c;
> - size_t i;
> -
> - for (c = fn, i = 0; i < len; i++, c++)
> - if (!isalnum(*c) && *c != '-' && *c != '_' && *c != '.')
> - return 0;
> -
> - c = memchr(fn, '.', len);
> - if (c == NULL || c != memrchr(fn, '.', len))
> - return 0;
> -
> - return 1;
> -}
> -
> -/*
> * Check that the file is an ASPA, CER, CRL, GBR or a ROA.
> * Returns corresponding rtype or RTYPE_INVALID on error.
> */
> Index: rsc.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/rsc.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 rsc.c
> --- rsc.c 9 May 2022 17:02:34 - 1.1
> +++ rsc.c 10 May 2022 06:03:46 -
> @@ -147,6 +147,8 @@ rsc_parse_filenamehash(struct parse *p,
> }
>
> if (elemsz == 2) {
> + ASN1_IA5STRING *filename;
> +
> file = sk_ASN1_TYPE_value(seq, i++);
> if (file->type != V_ASN1_IA5STRING) {
> warnx("%s: RSC FileNameAndHash: want ASN.1 IA5 string,"
> @@ -154,25 +156,17 @@ rsc_parse_filenamehash(struct parse *p,
> ASN1_tag2str(file->type), file->type);
> goto out;
> }
> - fn = strndup((const char *)file->value.ia5string->data,
> - file->value.ia5string->length);
> - if (fn == NULL)
> - err(1, NULL);
>
> - /*
> - * filename must confirm to portable file name character set
> - * XXX: use valid_filename() instead
> - */
> - if (strchr(fn, '/') != NULL) {
> - warnx("%s: path components disallowed in filename: %s",
> - p->fn, fn);
> - goto out;
> - }
> - if (strchr(fn, '\n') != NULL) {
> - warnx("%s: newline disallowed in filename: %s",
> - p->fn, fn);
> + filename = file->value.ia5string;
> +
> + if (!portable_filename(filename->data, filename->length)) {
> + warnx("%s: RSC FileNameAndHash: bad filename", p->fn);
> goto out;
> }
> +
> + fn = strndup(filename->data, filename->length);
> + if (fn == NULL)
> + err(1, NULL);
> }
>
> /* Now hash value. */
> Index: validate.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
>
rpki-client: enforce RSC compliant filenames
This moves valid_filename() to validate.c and splits out a helper
portable_filename() which can be used from the RSC code. While moving
valid_filename() is not necessary, I thought it makes sense to keep the
two functions next to each other.
I could not find a short name with valid_ prefix for portable_filename()
that I liked. valid_posix_filename()? valid_checklist_file()?
Also, valid_filename() could be stricter and enforce that the unique '.'
is followed by a 3-letter suffix.
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.134
diff -u -p -r1.134 extern.h
--- extern.h9 May 2022 17:19:32 - 1.134
+++ extern.h10 May 2022 06:03:19 -
@@ -502,6 +502,8 @@ int valid_cert(const char *, struct au
int valid_roa(const char *, struct auth *, struct roa *);
int valid_filehash(int, const char *, size_t);
int valid_hash(unsigned char *, size_t, const char *, size_t);
+int portable_filename(const char *, size_t);
+int valid_filename(const char *, size_t);
int valid_uri(const char *, size_t, const char *);
int valid_origin(const char *, const char *);
int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,
Index: mft.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
retrieving revision 1.61
diff -u -p -r1.61 mft.c
--- mft.c 9 May 2022 17:02:34 - 1.61
+++ mft.c 10 May 2022 06:03:19 -
@@ -127,27 +127,6 @@ rtype_from_file_extension(const char *fn
}
/*
- * Validate that a filename listed on a Manifest only contains characters
- * permitted in draft-ietf-sidrops-6486bis section 4.2.2
- */
-static int
-valid_filename(const char *fn, size_t len)
-{
- const unsigned char *c;
- size_t i;
-
- for (c = fn, i = 0; i < len; i++, c++)
- if (!isalnum(*c) && *c != '-' && *c != '_' && *c != '.')
- return 0;
-
- c = memchr(fn, '.', len);
- if (c == NULL || c != memrchr(fn, '.', len))
- return 0;
-
- return 1;
-}
-
-/*
* Check that the file is an ASPA, CER, CRL, GBR or a ROA.
* Returns corresponding rtype or RTYPE_INVALID on error.
*/
Index: rsc.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/rsc.c,v
retrieving revision 1.1
diff -u -p -r1.1 rsc.c
--- rsc.c 9 May 2022 17:02:34 - 1.1
+++ rsc.c 10 May 2022 06:03:46 -
@@ -147,6 +147,8 @@ rsc_parse_filenamehash(struct parse *p,
}
if (elemsz == 2) {
+ ASN1_IA5STRING *filename;
+
file = sk_ASN1_TYPE_value(seq, i++);
if (file->type != V_ASN1_IA5STRING) {
warnx("%s: RSC FileNameAndHash: want ASN.1 IA5 string,"
@@ -154,25 +156,17 @@ rsc_parse_filenamehash(struct parse *p,
ASN1_tag2str(file->type), file->type);
goto out;
}
- fn = strndup((const char *)file->value.ia5string->data,
- file->value.ia5string->length);
- if (fn == NULL)
- err(1, NULL);
- /*
-* filename must confirm to portable file name character set
-* XXX: use valid_filename() instead
-*/
- if (strchr(fn, '/') != NULL) {
- warnx("%s: path components disallowed in filename: %s",
- p->fn, fn);
- goto out;
- }
- if (strchr(fn, '\n') != NULL) {
- warnx("%s: newline disallowed in filename: %s",
- p->fn, fn);
+ filename = file->value.ia5string;
+
+ if (!portable_filename(filename->data, filename->length)) {
+ warnx("%s: RSC FileNameAndHash: bad filename", p->fn);
goto out;
}
+
+ fn = strndup(filename->data, filename->length);
+ if (fn == NULL)
+ err(1, NULL);
}
/* Now hash value. */
Index: validate.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
retrieving revision 1.31
diff -u -p -r1.31 validate.c
--- validate.c 21 Apr 2022 09:53:07 - 1.31
+++ validate.c 10 May 2022 06:03:19 -
@@ -275,6 +275,42 @@ valid_hash(unsigned char *buf, size_t le
}
/*
+ * Validate that a filename only contains characters from the POSIX portable
+ * filename character set [A-Za-z0-9._-], see IEEE Std 1003.1-2013, 3.278.
+ */
+int
+portable_filename(const char *fn, size_t len)
+{
+ const unsigned char *c;
+ size_t i;
+
+ for (c = fn, i = 0; i
