Re: [PATCH] [src] usr.bin/sendbug/sendbug.c - update categories
On 2022/03/29 21:05, Theo Buehler wrote:
> On Tue, Mar 29, 2022 at 08:51:25PM +0200, Jeremie Courreges-Anglas wrote:
> > On Tue, Mar 29 2022, Raf Czlonka wrote:
> > > Hello,
> > >
> > > sparc and vax ports have been retired a while back; add riscv64
> > > while there.
> >
> > Committed, thanks.
> >
> > There are more missing entries I think. If I follow the existing
> > pattern, naming the cpu architectures and not the platforms:
> > - aarch64 is missing
> > - powerpc64 is missing
> > - mips64el is missing
>
> Perhaps we can ditch the architectures list that seems to have been out
> of sync more often than not and reduce categories to something like
> these?
>
> documentation userland kernel
+1
> If somebody is smart enough to narrow a bug down to something
> arch-specific, they're hopefully smart enough to mention that in their
> bug report.
>
> > and after folding the line because of line wrapping:
> >
> > ok?
> >
> >
> > Index: sendbug.c
> > ===
> > RCS file: /cvs/src/usr.bin/sendbug/sendbug.c,v
> > retrieving revision 1.79
> > diff -u -p -r1.79 sendbug.c
> > --- sendbug.c 29 Mar 2022 18:44:12 - 1.79
> > +++ sendbug.c 29 Mar 2022 18:49:41 -
> > @@ -44,7 +44,8 @@ void template(FILE *);
> > void usbdevs(FILE *);
> >
> > const char *categories = "system user library documentation kernel "
> > -"alpha amd64 arm hppa i386 m88k mips64 powerpc riscv64 sh sparc64";
> > +"alpha aarch64 amd64 arm hppa i386 m88k mips64 mips64el powerpc
> > powerpc64 "
> > +"riscv64 sh sparc64";
> > const char *comment[] = {
> > "",
> > "",
> >
> > --
> > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
> >
>
Re: [PATCH] [src] usr.bin/sendbug/sendbug.c - update categories
On Tue, Mar 29, 2022 at 08:05:25PM BST, Theo Buehler wrote:
> On Tue, Mar 29, 2022 at 08:51:25PM +0200, Jeremie Courreges-Anglas wrote:
> > On Tue, Mar 29 2022, Raf Czlonka wrote:
> > > Hello,
> > >
> > > sparc and vax ports have been retired a while back; add riscv64
> > > while there.
> >
> > Committed, thanks.
> >
> > There are more missing entries I think. If I follow the existing
> > pattern, naming the cpu architectures and not the platforms:
> > - aarch64 is missing
> > - powerpc64 is missing
> > - mips64el is missing
>
> Perhaps we can ditch the architectures list that seems to have been out
> of sync more often than not and reduce categories to something like
> these?
>
> documentation userland kernel
>
> If somebody is smart enough to narrow a bug down to something
> arch-specific, they're hopefully smart enough to mention that in their
> bug report.
Hi Theo,
My initial thought was to simply remove the whole second line leaving only:
const char *categories = "system user library documentation kernel"
Regards,
Raf
Index: usr.bin/sendbug/sendbug.c
===
RCS file: /cvs/src/usr.bin/sendbug/sendbug.c,v
retrieving revision 1.78
diff -u -p -r1.78 sendbug.c
--- usr.bin/sendbug/sendbug.c 21 Aug 2017 21:41:13 - 1.78
+++ usr.bin/sendbug/sendbug.c 29 Mar 2022 21:21:28 -
@@ -43,8 +43,7 @@ int sendmail(const char *);
void template(FILE *);
void usbdevs(FILE *);
-const char *categories = "system user library documentation kernel "
-"alpha amd64 arm hppa i386 m88k mips64 powerpc sh sparc sparc64 vax";
+const char *categories = "system user library documentation kernel"
const char *comment[] = {
"",
"",
>
> > and after folding the line because of line wrapping:
> >
> > ok?
> >
> >
> > Index: sendbug.c
> > ===
> > RCS file: /cvs/src/usr.bin/sendbug/sendbug.c,v
> > retrieving revision 1.79
> > diff -u -p -r1.79 sendbug.c
> > --- sendbug.c 29 Mar 2022 18:44:12 - 1.79
> > +++ sendbug.c 29 Mar 2022 18:49:41 -
> > @@ -44,7 +44,8 @@ void template(FILE *);
> > void usbdevs(FILE *);
> >
> > const char *categories = "system user library documentation kernel "
> > -"alpha amd64 arm hppa i386 m88k mips64 powerpc riscv64 sh sparc64";
> > +"alpha aarch64 amd64 arm hppa i386 m88k mips64 mips64el powerpc
> > powerpc64 "
> > +"riscv64 sh sparc64";
> > const char *comment[] = {
> > "",
> > "",
> >
> > --
> > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
> >
Re: [PATCH] [src] usr.bin/sendbug/sendbug.c - update categories
On Tue, Mar 29, 2022 at 08:51:25PM +0200, Jeremie Courreges-Anglas wrote:
> On Tue, Mar 29 2022, Raf Czlonka wrote:
> > Hello,
> >
> > sparc and vax ports have been retired a while back; add riscv64
> > while there.
>
> Committed, thanks.
>
> There are more missing entries I think. If I follow the existing
> pattern, naming the cpu architectures and not the platforms:
> - aarch64 is missing
> - powerpc64 is missing
> - mips64el is missing
Perhaps we can ditch the architectures list that seems to have been out
of sync more often than not and reduce categories to something like
these?
documentation userland kernel
If somebody is smart enough to narrow a bug down to something
arch-specific, they're hopefully smart enough to mention that in their
bug report.
> and after folding the line because of line wrapping:
>
> ok?
>
>
> Index: sendbug.c
> ===
> RCS file: /cvs/src/usr.bin/sendbug/sendbug.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 sendbug.c
> --- sendbug.c 29 Mar 2022 18:44:12 - 1.79
> +++ sendbug.c 29 Mar 2022 18:49:41 -
> @@ -44,7 +44,8 @@ voidtemplate(FILE *);
> void usbdevs(FILE *);
>
> const char *categories = "system user library documentation kernel "
> -"alpha amd64 arm hppa i386 m88k mips64 powerpc riscv64 sh sparc64";
> +"alpha aarch64 amd64 arm hppa i386 m88k mips64 mips64el powerpc
> powerpc64 "
> +"riscv64 sh sparc64";
> const char *comment[] = {
> "",
> "",
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>
Re: [PATCH] [src] usr.bin/sendbug/sendbug.c - update categories
On Tue, Mar 29, 2022 at 07:51:25PM BST, Jeremie Courreges-Anglas wrote:
> On Tue, Mar 29 2022, Raf Czlonka wrote:
> > Hello,
> >
> > sparc and vax ports have been retired a while back; add riscv64
> > while there.
>
> Committed, thanks.
>
> There are more missing entries I think. If I follow the existing
> pattern, naming the cpu architectures and not the platforms:
> - aarch64 is missing
> - powerpc64 is missing
> - mips64el is missing
Hi Jeremie,
Yeah, I though about but wasn't sure which way to go - in the end,
I followed each link under supported platform from plat.html[0] and
used the descriptions found there.
A small comment in sendbug.c might help I guess.
Either way, whichever works best :^)
[0] https://www.openbsd.org/plat.html
Regards,
Raf
> and after folding the line because of line wrapping:
>
> ok?
>
>
> Index: sendbug.c
> ===
> RCS file: /cvs/src/usr.bin/sendbug/sendbug.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 sendbug.c
> --- sendbug.c 29 Mar 2022 18:44:12 - 1.79
> +++ sendbug.c 29 Mar 2022 18:49:41 -
> @@ -44,7 +44,8 @@ voidtemplate(FILE *);
> void usbdevs(FILE *);
>
> const char *categories = "system user library documentation kernel "
> -"alpha amd64 arm hppa i386 m88k mips64 powerpc riscv64 sh sparc64";
> +"alpha aarch64 amd64 arm hppa i386 m88k mips64 mips64el powerpc
> powerpc64 "
> +"riscv64 sh sparc64";
> const char *comment[] = {
> "",
> "",
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: [PATCH] [src] usr.bin/sendbug/sendbug.c - update categories
On Tue, Mar 29 2022, Raf Czlonka wrote:
> Hello,
>
> sparc and vax ports have been retired a while back; add riscv64
> while there.
Committed, thanks.
There are more missing entries I think. If I follow the existing
pattern, naming the cpu architectures and not the platforms:
- aarch64 is missing
- powerpc64 is missing
- mips64el is missing
and after folding the line because of line wrapping:
ok?
Index: sendbug.c
===
RCS file: /cvs/src/usr.bin/sendbug/sendbug.c,v
retrieving revision 1.79
diff -u -p -r1.79 sendbug.c
--- sendbug.c 29 Mar 2022 18:44:12 - 1.79
+++ sendbug.c 29 Mar 2022 18:49:41 -
@@ -44,7 +44,8 @@ void template(FILE *);
void usbdevs(FILE *);
const char *categories = "system user library documentation kernel "
-"alpha amd64 arm hppa i386 m88k mips64 powerpc riscv64 sh sparc64";
+"alpha aarch64 amd64 arm hppa i386 m88k mips64 mips64el powerpc powerpc64 "
+"riscv64 sh sparc64";
const char *comment[] = {
"",
"",
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: "evp_aead_ctx_st" missing ? when using "EVP_AEAD_CTX" functionality.
On Tue, Mar 29, 2022 at 06:24:09PM +0200, Remco wrote:
> I get a "forward declaration of 'struct evp_aead_ctx_st'" message, trying to
> use the "EVP_AEAD_CTX" functions on:
>
> "OpenBSD 7.1 (GENERIC.MP) #448: Tue Mar 29 00:13:47 MDT 2022
> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP"
>
> Some definitions, e.g. "evp_aead_ctx_st", appear to have moved from
> to "evp_locl.h", since revision 1.98 of .
>
> Is the EVP_AEAD_CTX functionality still supposed to be available, and how ?
>
> The "evp_locl.h" header does not appear installed, find /usr/include -name
> "evp_locl.h", shows nothing.
You need to move the EVP_AEAD_CTX from the stack to the heap and
allocate/free it with EVP_AEAD_CTX_{new,free}(). See the example in
EVP_AEAD_CTX_init(3) - unfortunately written without proper error
checking (I should fix this).
Instead of
EVP_AEAD_CTX ctx;
if (!EVP_AEAD_CTX_init(, ...))
...
You'd now write
EVP_AEAD_CTX *ctx;
ctx = EVP_AEAD_CTX_new();
if (ctx == NULL)
// error
if (!EVP_AEAD_CTX_init(ctx, ...))
// error
...
EVP_AEAD_CTX_free(ctx);
[PATCH] [src] usr.bin/sendbug/sendbug.c - update categories
Hello,
sparc and vax ports have been retired a while back; add riscv64
while there.
Regards,
Raf
Index: usr.bin/sendbug/sendbug.c
===
RCS file: /cvs/src/usr.bin/sendbug/sendbug.c,v
retrieving revision 1.78
diff -u -p -r1.78 sendbug.c
--- usr.bin/sendbug/sendbug.c 21 Aug 2017 21:41:13 - 1.78
+++ usr.bin/sendbug/sendbug.c 29 Mar 2022 17:39:38 -
@@ -44,7 +44,7 @@ void template(FILE *);
void usbdevs(FILE *);
const char *categories = "system user library documentation kernel "
-"alpha amd64 arm hppa i386 m88k mips64 powerpc sh sparc sparc64 vax";
+"alpha amd64 arm hppa i386 m88k mips64 powerpc riscv64 sh sparc64";
const char *comment[] = {
"",
"",
"evp_aead_ctx_st" missing ? when using "EVP_AEAD_CTX" functionality.
I get a "forward declaration of 'struct evp_aead_ctx_st'" message, trying to use the "EVP_AEAD_CTX" functions on: "OpenBSD 7.1 (GENERIC.MP) #448: Tue Mar 29 00:13:47 MDT 2022 dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP" Some definitions, e.g. "evp_aead_ctx_st", appear to have moved from to "evp_locl.h", since revision 1.98 of . Is the EVP_AEAD_CTX functionality still supposed to be available, and how ? The "evp_locl.h" header does not appear installed, find /usr/include -name "evp_locl.h", shows nothing.
Re: Security support status of xnf(4) and xbf(4)
On 3/29/22 04:16, Claudio Jeker wrote: > On Mon, Mar 28, 2022 at 04:38:33PM -0400, Demi Marie Obenour wrote: >> On 3/28/22 10:39, Mark Kettenis wrote: Date: Mon, 28 Mar 2022 09:51:22 -0400 From: Demi Marie Obenour On 3/27/22 21:45, Damien Miller wrote: > On Fri, 25 Mar 2022, Demi Marie Obenour wrote: > >> Linux’s netfront and blkfront drivers recently had a security >> vulnerability (XSA-396) that allowed a malicious backend to potentially >> compromise them. In follow-up audits, I found that OpenBSD’s xnf(4) >> currently trusts the backend domain. I reported this privately to Theo >> de Raadt, who indicated that OpenBSD does not consider this to be a >> security concern. >> >> This is obviously a valid position for the OpenBSD project to take, but >> it is surprising to some (such as myself) from the broader Xen >> ecosystem. Standard practice in the Xen world is that bugs in frontends >> that allow a malicious backend to cause mischief *are* considered >> security bugs unless there is explicit documentation to the contrary. >> As such, I believe this deserves to be noted in xnf(4) and xbf(4)’s man >> pages. If the OpenBSD project agrees, I am willing to write a patch, >> but I have no experience with mandoc so it might take a few tries. > > Hang on, what is a "malicious backend" in this context? Is it something > other than the Xen Hypervisor? If not, then it seems not to be a useful > attack model, as the hypervisor typically has near-complete access to > guests' memory and CPU state. The backend can run in any Xen VM. It often runs in dom0, but it is not required to, and in Qubes OS the network backend never runs in dom0. Unless it runs in dom0, it has no access to frontend memory, except for memory the frontend has explicitly given it access to via grant tables. >>> >>> So this is somewhat similar to the situation on sun4v (Sun's >>> virtualization of the SPARC architecture). When writing the vnet(4) >>> and vdsk(4) drivers for OpenBSD, I did consider the implications of >>> those drivers talking to a "malicious" domain. the SPARC hypervisor >>> implements a concept similar to grant tables. It is fairly obvious >>> that any memory you grant access to should be considered insecure. >>> This means that you either have to make a copy of the data or revoke >>> access to the shared memory through some sort of Hypervisor call that >>> implements a synchronization point of some sorts. Otherwise you and >>> up TOCTOU issues all over the place. But this obviously has >>> significant performance consequences. For vnet(4) I decided that an >>> extra copy was worth doing and the only reasonable way of doing things >>> given how OpenBSD's mbuf layer works. But for vdsk(4) I decided to >>> trust the other domain as there is no way to prevent it from feeding >>> you compromised data. Full disk encryption doesn't really solve the >>> problem unless you have a way to securely verify the bootloader. >> >> In Qubes OS, xbf(4) devices are configurable. While all of them are >> provided by dom0 (which is trusted) by default, it is possible to >> attach devices that are *not* provided by dom0, and these devices >> should not be trusted. >> >>> Personally I think it might be beneficial for us to turn xnf(4) into >>> what we colloquially call a "bcopy" network driver. But folks who >>> actually use xen may find the performance impact of doing this >>> unacceptable and decide to trust the backend instead. >> >> You actually don’t have to do that. The Xen network protocol >> requires the backend to drop access to the buffer before giving it >> to the frontend, so the frontend only needs to ensure that it cannot >> regain access. This will fail if the backend still has access, but >> that is a bug in the backend, in which case you should shut down the >> interface. So there should not be any significant performance impact. >> >> If you are curious about how Linux does this, you can look at >> drivers/xen/grant-table.c, drivers/net/xen-netfront.c, and >> drivers/block/xen-blkfront.c from the Linux source. They are >> dual licensed GPL/MIT so there should not be licensing issues there. >> Be sure to use a version at or after “xen/netfront: react properly to >> failing gnttab_end_foreign_access_ref()” and the other XSA-396 patches. > > So how does xen manage to limit access to less than a page size? > The hardware on x86 does not give you byte precise mappings for granting > memory. > An mbuf is 256 bytes and of those 256 bytes less then that is used for > data. Still for dma the full 4k page needs to be granted to the host. > The only way this can be done is by memcpy all data into individual pages. > The same is true for the most common mbuf cluster size of 2k. I was not aware that the OpenBSD mbuf layer could not handle the approach I described. Sorry for the
Re: amd64: do CPU identification before TSC sync test
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.
Uh, identifycpu() calls tsc_identify() calls tsc_freq_cpuid(), which
quietly sets lapic_per_second (a variable in amd64/lapic.c) based on
the core crystal frequency reported via CPUID.
If lapic_per_second is non-zero when lapic_calibrate_timer() is
called, the BP skips manual calibration of the LAPIC counter with
the i8254.
It might be less surprising if lapic_calibrate_timer() grabbed the
core crystal frequency via a function in amd64/tsc.c. Then the order
of TSC-sync-test and LAPIC setup wouldn't matter. We disable
interrupts when doing TSC sync anyway, so even if the LAPIC were
running we wouldn't see the interrupts.
...
But maybe you're talking about some other issue :)
Index: cpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
retrieving revision 1.155
diff -u -p -r1.155 cpu.c
--- cpu.c 21 Feb 2022 11:03:39 - 1.155
+++ cpu.c 29 Mar 2022 15:19:54 -
@@ -842,20 +842,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) {
@@ -865,11 +852,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);
@@ -930,18 +934,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);
@@ -960,6 +954,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: wg(4): 'Address already in use' when wgrtable is changed
Thanks for the answer.
There is one thing I'm worrying about.
Ifconfig doesn't show wgrtable value with your patch.
In my use case as follows, it seems that setting `wgrtable 1` is ignored.
```
# route -T1 add default `cat /etc/mygate`
# ifconfig wg0 create wgport 7111 wgkey `cat /etc/mykey.wg0`
# ifconfig wg0 up
# ifconfig wg0 wgrtable 1
# ifconfig wg0
wg0: flags=80c3 mtu 1420
index 6 priority 0 llprio 3
wgport 7111
wgpubkey e/CYTG1RGqT4jmrY0Fom8cAdtOWP7F/gBVwamyINRlg=
groups: wg
```
On 3/28/22 15:59, YASUOKA Masahiko wrote:
On Mon, 28 Mar 2022 15:20:02 +0900
Yuichiro NAITO wrote:
Thanks for the explanation.
I understand how your patch works.
I want to ask the goal of your patch.
It seems just removing 'Address already in use' message.
Is my guessing right?
Yes. There is nothing to do, since the command is to bind the same
port, protocol, and domain of prevous.
The code seems to do such the skip already, but it lacks consideration
for rtable_l2(rtable) != rtable case.
On 3/28/22 14:01, YASUOKA Masahiko wrote:
Hi,
On Mon, 28 Mar 2022 12:12:39 +0900
Yuichiro NAITO wrote:
On 3/27/22 18:25, YASUOKA Masahiko wrote:
Hi,
On Wed, 9 Mar 2022 15:28:44 +0900
Yuichiro NAITO wrote:
I see 'Address already in use' message,
when I change wgrtable for a running wg interface.
It doesn't make sense to me.
It can be reproduced by the following command sequence.
```
# route -T1 add default `cat /etc/mygate`
# ifconfig wg0 create wgport 7111 wgkey `cat /etc/mykey.wg0`
# ifconfig wg0 up
# ifconfig wg0 wgrtable 1
ifconfig: SIOCSWG: Address already in use
```
When I down wg0 interface before changing wgrtable,
It succeeds and no messages are shown.
I investigated the reason why 'Address already in use' is shown.
If wgrtable is specified by ifconfig argument,
`wg_ioctl_set` function in `sys/net/if_wg.c` is called.
And if the wg interface is running, `wg_bind` function is called.
`wg_bind` creates new sockets (IPv4 and 6) and replace them from old
ones.
If only wgrtable is changed, `wg_bind` binds as same port as existing
sockets.
So 'Address already in use' is shown.
Here is a simple patch to close existing sockets before `wg_bind`.
It works for me but I'm not 100% sure this is right fix.
Any other ideas?
```
diff --git a/sys/net/if_wg.c b/sys/net/if_wg.c
index 4dae3e3c976..0159664fb34 100644
--- a/sys/net/if_wg.c
+++ b/sys/net/if_wg.c
@@ -2253,11 +2253,14 @@ wg_ioctl_set(struct wg_softc *sc, struct
wg_data_io *data)
if (port != sc->sc_udp_port || rtable != sc->sc_udp_rtable) {
TAILQ_FOREACH(peer, >sc_peer_seq, p_seq_entry)
wg_peer_clear_src(peer);
- if (sc->sc_if.if_flags & IFF_RUNNING)
+ if (sc->sc_if.if_flags & IFF_RUNNING) {
+ if (port == sc->sc_udp_port)
+ wg_unbind(sc);
if ((ret = wg_bind(sc, , )) != 0)
goto error;
+ }
sc->sc_udp_port = port;
sc->sc_udp_rtable = rtable;
}
```
If rdomain 1 exists, the error will not shown.
# ifconfig vether0 rdomain 1 up
# ifconfig wg0 create wgport 7111 wgkey `openssl rand -base64 32` up
# ifconfig wg0 wgrtable 1
#
Yes, if rdomain 1 is created before `ifconfig wg0 wgrtable 1`,
setting wgrtable succeeds and there is no problem.
In the case which you reported to, it is supposed that rtable 1 exists
but rdomain 1 doesn't exist.
Even when "wgtable 1" is configured, becase there is no dedicated
rdomain, rdomain 0 will be used to bind the UDP port.
Exactly, it's the case that I reported and want to fix.
So what wg(4) should do for this case is "nothing".
I'm a little bit confused.
As you said, I can confirm your patch doesn't set wgrtable in my use
case.
It is not the result that I wanted.
# ifconfig wg0 create wgport 7111 wgkey `openssl rand -base64 32` up
-> bind 7111/udp on rdomain 0 (1)
is expected. (1)
# ifconfig wg0 wgrtable 1
-> bind 7111/udp on rdomain 0 (2)
is expected, since there is no "domain 1".
If trying to do (1) and (2), then it causes EADDRINUSE since it is to
bind the same port, proto, and domain. The latest diff is skip (2)
properly.
Previous
- if (port != sc->sc_udp_port || rtable != sc->sc_udp_rtable) {
"rtable != sc->sc_udp_rtable" was wrong since rdomain for rtable may
not exist. This is the cause of EADDRINUSE.
So the diff is updated.
ok?
Index: sys/net/if_wg.c
===
RCS file: /disk/cvs/openbsd/src/sys/net/if_wg.c,v
retrieving revision 1.22
diff -u -p -r1.22 if_wg.c
--- sys/net/if_wg.c 22 Feb 2022 01:15:02 - 1.22
+++ sys/net/if_wg.c 27 Mar 2022 09:17:08 -
@@ -2250,7 +2250,8 @@ wg_ioctl_set(struct wg_softc *sc, struct
else
rtable = sc->sc_udp_rtable;
- if (port != sc->sc_udp_port || rtable !=
Re: Security support status of xnf(4) and xbf(4)
On Mon, Mar 28, 2022 at 04:38:33PM -0400, Demi Marie Obenour wrote: > On 3/28/22 10:39, Mark Kettenis wrote: > >> Date: Mon, 28 Mar 2022 09:51:22 -0400 > >> From: Demi Marie Obenour > >> > >> On 3/27/22 21:45, Damien Miller wrote: > >>> On Fri, 25 Mar 2022, Demi Marie Obenour wrote: > >>> > Linux’s netfront and blkfront drivers recently had a security > vulnerability (XSA-396) that allowed a malicious backend to potentially > compromise them. In follow-up audits, I found that OpenBSD’s xnf(4) > currently trusts the backend domain. I reported this privately to Theo > de Raadt, who indicated that OpenBSD does not consider this to be a > security concern. > > This is obviously a valid position for the OpenBSD project to take, but > it is surprising to some (such as myself) from the broader Xen > ecosystem. Standard practice in the Xen world is that bugs in frontends > that allow a malicious backend to cause mischief *are* considered > security bugs unless there is explicit documentation to the contrary. > As such, I believe this deserves to be noted in xnf(4) and xbf(4)’s man > pages. If the OpenBSD project agrees, I am willing to write a patch, > but I have no experience with mandoc so it might take a few tries. > >>> > >>> Hang on, what is a "malicious backend" in this context? Is it something > >>> other than the Xen Hypervisor? If not, then it seems not to be a useful > >>> attack model, as the hypervisor typically has near-complete access to > >>> guests' memory and CPU state. > >> > >> The backend can run in any Xen VM. It often runs in dom0, but it > >> is not required to, and in Qubes OS the network backend never runs > >> in dom0. Unless it runs in dom0, it has no access to frontend memory, > >> except for memory the frontend has explicitly given it access to via > >> grant tables. > > > > So this is somewhat similar to the situation on sun4v (Sun's > > virtualization of the SPARC architecture). When writing the vnet(4) > > and vdsk(4) drivers for OpenBSD, I did consider the implications of > > those drivers talking to a "malicious" domain. the SPARC hypervisor > > implements a concept similar to grant tables. It is fairly obvious > > that any memory you grant access to should be considered insecure. > > This means that you either have to make a copy of the data or revoke > > access to the shared memory through some sort of Hypervisor call that > > implements a synchronization point of some sorts. Otherwise you and > > up TOCTOU issues all over the place. But this obviously has > > significant performance consequences. For vnet(4) I decided that an > > extra copy was worth doing and the only reasonable way of doing things > > given how OpenBSD's mbuf layer works. But for vdsk(4) I decided to > > trust the other domain as there is no way to prevent it from feeding > > you compromised data. Full disk encryption doesn't really solve the > > problem unless you have a way to securely verify the bootloader. > > In Qubes OS, xbf(4) devices are configurable. While all of them are > provided by dom0 (which is trusted) by default, it is possible to > attach devices that are *not* provided by dom0, and these devices > should not be trusted. > > > Personally I think it might be beneficial for us to turn xnf(4) into > > what we colloquially call a "bcopy" network driver. But folks who > > actually use xen may find the performance impact of doing this > > unacceptable and decide to trust the backend instead. > > You actually don’t have to do that. The Xen network protocol > requires the backend to drop access to the buffer before giving it > to the frontend, so the frontend only needs to ensure that it cannot > regain access. This will fail if the backend still has access, but > that is a bug in the backend, in which case you should shut down the > interface. So there should not be any significant performance impact. > > If you are curious about how Linux does this, you can look at > drivers/xen/grant-table.c, drivers/net/xen-netfront.c, and > drivers/block/xen-blkfront.c from the Linux source. They are > dual licensed GPL/MIT so there should not be licensing issues there. > Be sure to use a version at or after “xen/netfront: react properly to > failing gnttab_end_foreign_access_ref()” and the other XSA-396 patches. So how does xen manage to limit access to less than a page size? The hardware on x86 does not give you byte precise mappings for granting memory. An mbuf is 256 bytes and of those 256 bytes less then that is used for data. Still for dma the full 4k page needs to be granted to the host. The only way this can be done is by memcpy all data into individual pages. The same is true for the most common mbuf cluster size of 2k. So yes, this will be a bcopy ethernet driver and by that will be on the same level of crappyness as bce(4) and old old old realtek. If you can trust the host don't run your vm on that
