Remove unnecessary NOWITNESS kludge
Initialize stack-based mutexed using mtx_init(). This removes the need
of the NOWITNESS kludge and lets the lock checker do its job with these
mutexes.
At the moment, static initialization of locks inside functions does not
work correctly with WITNESS. A lock initializer sets up a struct that
gets permanently referenced by the lock checker. Inside a function,
the static initializers put these structs on the stack, which causes
trouble when the function returns. In principle, this might be solvable
by using a compile-time expression that chooses the correct way of
initialization based on the scope of usage.
Index: dev/ic/mfi.c
===
RCS file: src/sys/dev/ic/mfi.c,v
retrieving revision 1.189
diff -u -p -r1.189 mfi.c
--- dev/ic/mfi.c25 May 2023 19:35:58 - 1.189
+++ dev/ic/mfi.c5 Jul 2023 02:56:57 -
@@ -925,8 +925,9 @@ mfi_poll(struct mfi_softc *sc, struct mf
void
mfi_exec(struct mfi_softc *sc, struct mfi_ccb *ccb)
{
- struct mutex m = MUTEX_INITIALIZER_FLAGS(IPL_BIO, __MTX_NAME,
- MTX_NOWITNESS);
+ struct mutex m;
+
+ mtx_init(, IPL_BIO);
#ifdef DIAGNOSTIC
if (ccb->ccb_cookie != NULL || ccb->ccb_done != NULL)
Index: dev/ic/mpi.c
===
RCS file: src/sys/dev/ic/mpi.c,v
retrieving revision 1.225
diff -u -p -r1.225 mpi.c
--- dev/ic/mpi.c25 May 2023 19:35:58 - 1.225
+++ dev/ic/mpi.c5 Jul 2023 02:56:57 -
@@ -1263,10 +1263,11 @@ mpi_poll_done(struct mpi_ccb *ccb)
void
mpi_wait(struct mpi_softc *sc, struct mpi_ccb *ccb)
{
- struct mutexcookie = MUTEX_INITIALIZER_FLAGS(
- IPL_BIO, __MTX_NAME, MTX_NOWITNESS);
+ struct mutexcookie;
void(*done)(struct mpi_ccb *);
+ mtx_init(, IPL_BIO);
+
done = ccb->ccb_done;
ccb->ccb_done = mpi_wait_done;
ccb->ccb_cookie =
Index: dev/pci/mfii.c
===
RCS file: src/sys/dev/pci/mfii.c,v
retrieving revision 1.88
diff -u -p -r1.88 mfii.c
--- dev/pci/mfii.c 25 May 2023 19:35:58 - 1.88
+++ dev/pci/mfii.c 5 Jul 2023 02:56:57 -
@@ -1764,8 +1764,9 @@ mfii_poll_done(struct mfii_softc *sc, st
int
mfii_exec(struct mfii_softc *sc, struct mfii_ccb *ccb)
{
- struct mutex m = MUTEX_INITIALIZER_FLAGS(IPL_BIO, __MTX_NAME,
- MTX_NOWITNESS);
+ struct mutex m;
+
+ mtx_init(, IPL_BIO);
#ifdef DIAGNOSTIC
if (ccb->ccb_cookie != NULL || ccb->ccb_done != NULL)
Index: dev/pci/mpii.c
===
RCS file: src/sys/dev/pci/mpii.c,v
retrieving revision 1.145
diff -u -p -r1.145 mpii.c
--- dev/pci/mpii.c 25 May 2023 19:35:58 - 1.145
+++ dev/pci/mpii.c 5 Jul 2023 02:56:57 -
@@ -2857,11 +2857,12 @@ mpii_init_queues(struct mpii_softc *sc)
void
mpii_wait(struct mpii_softc *sc, struct mpii_ccb *ccb)
{
- struct mutexmtx = MUTEX_INITIALIZER_FLAGS(IPL_BIO,
- __MTX_NAME, MTX_NOWITNESS);
+ struct mutexmtx;
void(*done)(struct mpii_ccb *);
void*cookie;
+ mtx_init(, IPL_BIO);
+
done = ccb->ccb_done;
cookie = ccb->ccb_cookie;
Index: scsi/scsi_base.c
===
RCS file: src/sys/scsi/scsi_base.c,v
retrieving revision 1.281
diff -u -p -r1.281 scsi_base.c
--- scsi/scsi_base.c25 May 2023 19:35:58 - 1.281
+++ scsi/scsi_base.c5 Jul 2023 02:56:57 -
@@ -1497,10 +1497,11 @@ scsi_done(struct scsi_xfer *xs)
int
scsi_xs_sync(struct scsi_xfer *xs)
{
- struct mutexcookie = MUTEX_INITIALIZER_FLAGS(IPL_BIO, __MTX_NAME,
- MTX_NOWITNESS);
+ struct mutexcookie;
int error;
+ mtx_init(, IPL_BIO);
+
#ifdef DIAGNOSTIC
if (xs->cookie != NULL)
panic("xs->cookie != NULL in scsi_xs_sync");
Re: pf.os database /p0f
Hello, the empty section yes would need to be pre-populated. Thanks for adding visibility to this as I noticed OpenBSD has p0f as well as FreeBSD the FreeBSD PfSense is being used as an example. Yes this database is starting to show it’s age. > On Jul 4, 2023, at 1:22 AM, Stuart Henderson wrote: > > On 2023/07/04 09:48, Solène Rapenne wrote: >> On Tue, 2023-07-04 at 03:39 +, Lee, Jonathan D wrote: >>> [cid:cd2efd41-42cb-4d83-9173-521bbb8f4539@namprd04.prod.outlook.com] >>> >>> Hello fellow software developers, >>> >>> I have noticed that p0f database files are not being updated. Many >>> new operating systems fingerprints are missing within the pf.os >>> database file that your software uses. I have added a section in >>> pf.os for Docker containers see the below diff checker output. Yes >>> this is unorthadox for the diff file again it is only a blank area >>> for new OS entries and it helps bring to lite that containers can >>> also be fingerprinted. The docx that is attached helps to showcase >>> the Kali penetration software running inside of a docker container. >>> The container was spun up and spun down and also deleted. I have >>> fingerprinted this docker container with the program p0f. I noticed >>> that p0f is used with pfSense and is used with access control lists >>> for source address OS see attached photos. Again for this to function >>> correctly it needs the database updated and new catagories like many >>> of the mainstream containers. We can fingerprint them like other OS >>> systems. >>> >> >> It seems you are using PFSense, which is based on FreeBSD. >> You are on the OpenBSD mailing list. >> >> Even if we update our fingerprint database to add docker like you >> suggest, this won't reflect in the product you are using. > > If somebody is able to send working TCP SYN signatures for the old > version of p0f that's used in PF (note that the separate p0f program > has changed quite a lot in the meantime and uses a different database > format), that don't cause problems with false detection, they could be > added. But there's no value in adding an empty placeholder section. > I'm a bit unsure whether this is going to be possible though (in > particular that they can be reliably identified separate to the > container's base OS). >
Re: Diff for evaluation (WACOM tablet driver)
On Mon, Jul 03, 2023 at 11:22:45AM +0200, Marc Espie wrote:
> I hope Vladimir will find the time to complete this answer.
>
> As far as Vlad's work goes, he did a presentation last week-end:
> https://www.lre.epita.fr/news_content/SS_summer_week_pres/Vladimir_Driver_OpenBSD.pptx
>
> (sorry for the medium, fortunately we have libreoffice)
>
> In the mean time, here is an updated diff.
>
> I removed the Gaomon stuff, which if anything should be a different patch.
>
> And I cleaned up the 20+ minor style violations I could find...
> (tabs instead of +4 spaces for continued lines, a few non-style compliant
> function declarations and/or code blocks, oh well)
>
> plus an extra malloc.h that snuck in and is not at all needed.
>
> And some typos in comments.
> And a C++ style comment. Oh well
>
> I would really for some version of this to get in soonish.
> I can vouch that my tablet "works" with it (well, as good as it can work
> within the limitations of wscons not allowing it to be easily differentiated
> from the normal mouse, which is really a pain for programs like gimp)
Thanks, I built a kernel with this and no issues observed. I have a
Wacom Bamboo (CTH-470, product id 0x00de) that doesn't attach yet, but
this can be left for future work. I don't see anything glaring. hid.c
could probably use some refactoring at some point in the future because
at least 3 functions now share 99% identical code (hid_is_collection,
hid_get_collection_data, hid_get_id_of_collection).
I recommend getting an ok from someone with more track record with
dev/hid and/or dev/usb, but from my side this is ok thfr@.
>
> dmesg for the tablet with the diff
> | uhidev1 at uhub1 port 4 configuration 1 interface 0 "Wacom Co.,Ltd. Intuos
> S" rev 2.00/1.07 addr 6
> | uhidev1: iclass 3/0, 228 report ids
> | uwacom0 at uhidev1: 9 buttons, Z and W dir, tip, barrel
> | wsmouse5 at uwacom0 mux 0
> | uwacom1 at uhidev1: 9 buttons, Z and W dir, tip, barrel
> | wsmouse6 at uwacom1 mux 0
> | uwacom2 at uhidev1: 9 buttons, Z and W dir, tip, barrel
> | wsmouse7 at uwacom2 mux 0
>
> as far as I understand, it appears as several mice because the stylus
> acts as totally different devices depending on the mode/end used
> (stuff that wscons completely hides from us).
>
> Without the patch, that tablet appears as 42 different uhid devices (!)
>
> The idea is that the parser for collections was really primitive. The
> debug stuff can show the details of various collection. There is the actual
> tablet mechanisms (which becomes one device) including scale, stylus, etc,
> and some other wacky collections (!): a debug collection that the wacom guys
> told us "oh some of our hw team needs that, but don't ever touch" and
> some other stuff we can't support yet (like battery support for some
> advanced models of stylus)
>
> Index: dev/hid/hid.c
> ===
> RCS file: /cvs/src/sys/dev/hid/hid.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 hid.c
> --- dev/hid/hid.c 20 May 2022 05:03:45 - 1.5
> +++ dev/hid/hid.c 3 Jul 2023 09:04:50 -
> @@ -657,3 +657,51 @@ hid_is_collection(const void *desc, int
> hid_end_parse(hd);
> return (0);
> }
> +
> +struct hid_data *
> +hid_get_collection_data(const void *desc, int size, int32_t usage,
> +uint32_t collection)
> +{
> + struct hid_data *hd;
> + struct hid_item hi;
> +
> + hd = hid_start_parse(desc, size, hid_all);
> +
> + DPRINTF("%s: usage=0x%x\n", __func__, usage);
> + while (hid_get_item(hd, )) {
> + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__,
> + hi.kind, hi.report_ID, hi.usage, usage);
> + if (hi.kind == hid_collection &&
> + hi.collection == collection && hi.usage == usage){
> + DPRINTF("%s: found\n", __func__);
> + return hd;
> + }
> + }
> + DPRINTF("%s: not found\n", __func__);
> + hid_end_parse(hd);
> + return NULL;
> +}
> +
> +int
> +hid_get_id_of_collection(const void *desc, int size, int32_t usage,
> +uint32_t collection)
> +{
> + struct hid_data *hd;
> + struct hid_item hi;
> +
> + hd = hid_start_parse(desc, size, hid_all);
> +
> + DPRINTF("%s: id=%d usage=0x%x\n", __func__, id, usage);
> + while (hid_get_item(hd, )) {
> + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__,
> + hi.kind, hi.report_ID, hi.usage, usage);
> + if (hi.kind == hid_collection &&
> + hi.collection == collection && hi.usage == usage){
> + DPRINTF("%s: found\n", __func__);
> + return hi.report_ID;
> + }
> + }
> + DPRINTF("%s: not found\n", __func__);
> + hid_end_parse(hd);
> + return 0;
> +}
> Index: dev/hid/hid.h
> ===
> RCS file:
Re: pf.os database /p0f
Lee, Jonathan D wrote: > Hello, the empty section yes, I agree would still need to be populated. > Thanks for adding some fresh visibility to this problem as I noticed OpenBSD > has p0f as well as FreeBSD the FreeBSD is being used as an example with > PfSense. > > The p0f database is starting to show its age. That sentence is the key. If it is old, you can stop using it. Or, you can be a partner in fixing it, and validating the changes, entirely. Not in a small way, but completely ensuring that what you propose has no downside. You in? > I am just researching a way to compartmentalize containers because their > abilities to perform data marshaling over the host's NIC. Furthermore, there > is many different vendors of containers from bsdJAILs to Kerbenets and many > others. My goal for the original email is just visibility for the need to > develop software or improve the older fingerprinting software that way it can > fingerprint and detect container signatures. The concrete example of Kali's > bleeding edge docker container is shown for more understanding on the > movement of this sector. Unfortunately noone else is willing to invest the time to rebuild the entire system (in an incredibly careful way) for you.
Re: pf(4) should mention DIOCXEND
On Tue, Jul 04, 2023 at 04:35:23PM +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> diff below updates pf(4) manpage to reflect changes [1] which
> were committed earlier today.
>
> does update to pf(4) read OK?
>
> thanks and
> regards
> sashan
>
> [1] https://marc.info/?l=openbsd-cvs=168848058603797=2
> https://marc.info/?l=openbsd-cvs=168847042626997=2
>
> 8<---8<---8<--8<
>
> diff --git a/share/man/man4/pf.4 b/share/man/man4/pf.4
> index 92eeb45f657..305c536b137 100644
> --- a/share/man/man4/pf.4
> +++ b/share/man/man4/pf.4
> @@ -48,12 +48,25 @@ and retrieve statistics.
> The most commonly used functions are covered by
> .Xr pfctl 8 .
> .Pp
> -Manipulations like loading a ruleset that involve more than a single
> +Operations like loading or reading a ruleset that involve more than a single
you probably don;t need to add "or reading", since you already indicate
that it is just an example ("like"), not an exhaustive list. or is there
a specific reason to list reading a ruleset?
> .Xr ioctl 2
> call require a so-called
> .Em ticket ,
should probably be Sy rather than Em, but don;t sweat it if such a
change would make the rest of the manual inconsistent.
> -which prevents the occurrence of
> -multiple concurrent manipulations.
> +which allows
> +.Xr pf 4
> +to deal with concurrent operations.
> +For certain
> +.Xr ioctl 2
> +commands (currently
> +.Dv DIOCGETRULES )
> +the number of tickets application can obtain is limited.
i'm not sure what this means. tickets per application? "tickets
application" does not read correctly.
> +The application must explicitly release the ticket using
s/using/using the/
or "using DIOCXEND to avoid..."
> +.Dv DIOCXEND
> +command to avoid hitting the limit.
> +All tickets which are not freed by
> +.Dv DIOCXEND
> +are released when application closes
s/application/the application/
> +.Pa /dev/pf .
> .Pp
> Fields of
> .Xr ioctl 2
> @@ -132,6 +145,9 @@ for subsequent
> calls and the number
> .Va nr
> of rules in the active ruleset.
> +The ticket should be released by
s/by/by the/
or maybe just "released by DIOCXEND".
> +.Dv DIOCXEND
> +command.
> .It Dv DIOCGETRULE Fa "struct pfioc_rule *pr"
> Get a
> .Va rule
> @@ -792,6 +808,10 @@ inactive rulesets since the last
> .Dv DIOCXBEGIN .
> .Dv DIOCXROLLBACK
> will silently ignore rulesets for which the ticket is invalid.
> +.It Dv DIOCXEND Fa "u_int32_t *ticket"
> +Release ticket obtained by
> +.Dv DIOCGETRULES
> +command.
again, either "by the XXX command" or "by XXX".
> .It Dv DIOCSETHOSTID Fa "u_int32_t *hostid"
> Set the host ID, which is used by
> .Xr pfsync 4
>
jmc
Re: pf.os database /p0f
Hello, the empty section yes, I agree would still need to be populated. Thanks for adding some fresh visibility to this problem as I noticed OpenBSD has p0f as well as FreeBSD the FreeBSD is being used as an example with PfSense. The p0f database is starting to show its age. I am just researching a way to compartmentalize containers because their abilities to perform data marshaling over the host's NIC. Furthermore, there is many different vendors of containers from bsdJAILs to Kerbenets and many others. My goal for the original email is just visibility for the need to develop software or improve the older fingerprinting software that way it can fingerprint and detect container signatures. The concrete example of Kali's bleeding edge docker container is shown for more understanding on the movement of this sector. On Jul 4, 2023, at 1:22 AM, Stuart Henderson wrote: On 2023/07/04 09:48, Solène Rapenne wrote: On Tue, 2023-07-04 at 03:39 +, Lee, Jonathan D wrote: [cid:cd2efd41-42cb-4d83-9173-521bbb8f4539@namprd04.prod.outlook.com] Hello fellow software developers, I have noticed that p0f database files are not being updated. Many new operating systems fingerprints are missing within the pf.os database file that your software uses. I have added a section in pf.os for Docker containers see the below diff checker output. Yes this is unorthadox for the diff file again it is only a blank area for new OS entries and it helps bring to lite that containers can also be fingerprinted. The docx that is attached helps to showcase the Kali penetration software running inside of a docker container. The container was spun up and spun down and also deleted. I have fingerprinted this docker container with the program p0f. I noticed that p0f is used with pfSense and is used with access control lists for source address OS see attached photos. Again for this to function correctly it needs the database updated and new catagories like many of the mainstream containers. We can fingerprint them like other OS systems. It seems you are using PFSense, which is based on FreeBSD. You are on the OpenBSD mailing list. Even if we update our fingerprint database to add docker like you suggest, this won't reflect in the product you are using. If somebody is able to send working TCP SYN signatures for the old version of p0f that's used in PF (note that the separate p0f program has changed quite a lot in the meantime and uses a different database format), that don't cause problems with false detection, they could be added. But there's no value in adding an empty placeholder section. I'm a bit unsure whether this is going to be possible though (in particular that they can be reliably identified separate to the container's base OS).
Re: validate vm.conf local prefixes in parser
On Tue, Jul 04, 2023 at 11:39:19AM -0400, Dave Voutila wrote:
> vmd's doing something close to shotgun parsing of the "local prefix" and
> "local inet6 prefix" settings in vm.conf(5). The parser intermixes ipv4
> and ipv6 parsing even when we know which one is valid in the parsing
> context. This makes me sad.
>
> Even worse, we're not validating the inputs at time of parsing and
> deferring to vm creation time. This makes me even sadder.
>
> The diff below:
> - splits parsing ipv4 and ipv6 into distinct functions
> - puts the validation into those functions (e.g prefix length, prefix
>has properly zero'd octets)
> - does *not* muck (yet) with the existing validation sprinkled in
>priv.c or config.c
>
> I thought about making pulling apart the prefix from prefix length
> parsing, but getaddrinfo(3) appears to not like parsing addresses like
> "192.168.0.0" vs "192.168.0.0/16". (I'm not a network person...maybe I'm
> being dumb here.)
>
> kn: this addresses some of your feedback on my previous diff from a few
> weeks ago.
>
> ok?
Most of these issues have been solved in for example bgpd.
The code there is able to parse most address forms also shortcuts like
192.168/24. Have a look at bgpd/config.c::host() and host_ip() on how it
works.
> diff refs/heads/master refs/heads/vmd-parse
> commit - 5d90c77abd2d7447f16f88ac9ea9e0485eac9f73
> commit + 228fe48802ec6250e3487aa791daceba4626b03f
> blob - b538d40be1a1e600c1021d95e3fadd310079fa7a
> blob + f5a2507ff5742ea3a62b0112e14d17aa8cbdf99d
> --- usr.sbin/vmd/config.c
> +++ usr.sbin/vmd/config.c
> @@ -49,7 +49,7 @@ config_init_localprefix(struct vmd_config *cfg)
> {
> struct sockaddr_in6 *sin6;
>
> - if (host(VMD_DHCP_PREFIX, >cfg_localprefix) == -1)
> + if (parse_prefix4(VMD_DHCP_PREFIX, >cfg_localprefix, NULL) == -1)
> return (-1);
>
> /* IPv6 is disabled by default */
> @@ -58,7 +58,7 @@ config_init_localprefix(struct vmd_config *cfg)
> /* Generate random IPv6 prefix only once */
> if (cfg->cfg_flags & VMD_CFG_AUTOINET6)
> return (0);
> - if (host(VMD_ULA_PREFIX, >cfg_localprefix6) == -1)
> + if (parse_prefix6(VMD_ULA_PREFIX, >cfg_localprefix6, NULL) == -1)
> return (-1);
> /* Randomize the 56 bits "Global ID" and "Subnet ID" */
> sin6 = ss2sin6(>cfg_localprefix6.ss);
> blob - 09468e3fe2c9f4f9193710c65667132f79a90df3
> blob + 3d030c201db3e8167831846cb1c8f6e3facc40fc
> --- usr.sbin/vmd/parse.y
> +++ usr.sbin/vmd/parse.y
> @@ -190,31 +190,30 @@ main: LOCAL INET6 {
> }
> | LOCAL INET6 PREFIX STRING {
> struct address h;
> + const char *err;
>
> - if (host($4, ) == -1 ||
> - h.ss.ss_family != AF_INET6 ||
> - h.prefixlen > 64 || h.prefixlen < 0) {
> - yyerror("invalid local inet6 prefix: %s", $4);
> - free($4);
> + if (parse_prefix6($4, , )) {
> + yyerror("invalid local inet6 prefix: %s", err);
> YYERROR;
> + } else {
> + env->vmd_cfg.cfg_flags |= VMD_CFG_INET6;
> + env->vmd_cfg.cfg_flags &= ~VMD_CFG_AUTOINET6;
> + memcpy(>vmd_cfg.cfg_localprefix6, ,
> + sizeof(h));
> }
> -
> - env->vmd_cfg.cfg_flags |= VMD_CFG_INET6;
> - env->vmd_cfg.cfg_flags &= ~VMD_CFG_AUTOINET6;
> - memcpy(>vmd_cfg.cfg_localprefix6, , sizeof(h));
> + free($4);
> }
> | LOCAL PREFIX STRING {
> struct address h;
> + const char *err;
>
> - if (host($3, ) == -1 ||
> - h.ss.ss_family != AF_INET ||
> - h.prefixlen > 32 || h.prefixlen < 0) {
> - yyerror("invalid local prefix: %s", $3);
> - free($3);
> + if (parse_prefix4($3, , )) {
> + yyerror("invalid local prefix: %s", err);
> YYERROR;
> - }
> -
> - memcpy(>vmd_cfg.cfg_localprefix, , sizeof(h));
> + } else
> + memcpy(>vmd_cfg.cfg_localprefix, ,
> + sizeof(h));
> + free($3);
> }
> | SOCKET OWNER owner_id {
> env->vmd_ps.ps_csock.cs_uid = $3.uid;
> @@ -1404,42 +1403,119 @@ int
> return (0);
> }
>
> +/*
> + * Parse an ipv4 address and prefix for local interfaces and validate
> + * constraints for vmd networking.
> + */
> int
> -host(const
validate vm.conf local prefixes in parser
vmd's doing something close to shotgun parsing of the "local prefix" and
"local inet6 prefix" settings in vm.conf(5). The parser intermixes ipv4
and ipv6 parsing even when we know which one is valid in the parsing
context. This makes me sad.
Even worse, we're not validating the inputs at time of parsing and
deferring to vm creation time. This makes me even sadder.
The diff below:
- splits parsing ipv4 and ipv6 into distinct functions
- puts the validation into those functions (e.g prefix length, prefix
has properly zero'd octets)
- does *not* muck (yet) with the existing validation sprinkled in
priv.c or config.c
I thought about making pulling apart the prefix from prefix length
parsing, but getaddrinfo(3) appears to not like parsing addresses like
"192.168.0.0" vs "192.168.0.0/16". (I'm not a network person...maybe I'm
being dumb here.)
kn: this addresses some of your feedback on my previous diff from a few
weeks ago.
ok?
diff refs/heads/master refs/heads/vmd-parse
commit - 5d90c77abd2d7447f16f88ac9ea9e0485eac9f73
commit + 228fe48802ec6250e3487aa791daceba4626b03f
blob - b538d40be1a1e600c1021d95e3fadd310079fa7a
blob + f5a2507ff5742ea3a62b0112e14d17aa8cbdf99d
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -49,7 +49,7 @@ config_init_localprefix(struct vmd_config *cfg)
{
struct sockaddr_in6 *sin6;
- if (host(VMD_DHCP_PREFIX, >cfg_localprefix) == -1)
+ if (parse_prefix4(VMD_DHCP_PREFIX, >cfg_localprefix, NULL) == -1)
return (-1);
/* IPv6 is disabled by default */
@@ -58,7 +58,7 @@ config_init_localprefix(struct vmd_config *cfg)
/* Generate random IPv6 prefix only once */
if (cfg->cfg_flags & VMD_CFG_AUTOINET6)
return (0);
- if (host(VMD_ULA_PREFIX, >cfg_localprefix6) == -1)
+ if (parse_prefix6(VMD_ULA_PREFIX, >cfg_localprefix6, NULL) == -1)
return (-1);
/* Randomize the 56 bits "Global ID" and "Subnet ID" */
sin6 = ss2sin6(>cfg_localprefix6.ss);
blob - 09468e3fe2c9f4f9193710c65667132f79a90df3
blob + 3d030c201db3e8167831846cb1c8f6e3facc40fc
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -190,31 +190,30 @@ main : LOCAL INET6 {
}
| LOCAL INET6 PREFIX STRING {
struct address h;
+ const char *err;
- if (host($4, ) == -1 ||
- h.ss.ss_family != AF_INET6 ||
- h.prefixlen > 64 || h.prefixlen < 0) {
- yyerror("invalid local inet6 prefix: %s", $4);
- free($4);
+ if (parse_prefix6($4, , )) {
+ yyerror("invalid local inet6 prefix: %s", err);
YYERROR;
+ } else {
+ env->vmd_cfg.cfg_flags |= VMD_CFG_INET6;
+ env->vmd_cfg.cfg_flags &= ~VMD_CFG_AUTOINET6;
+ memcpy(>vmd_cfg.cfg_localprefix6, ,
+ sizeof(h));
}
-
- env->vmd_cfg.cfg_flags |= VMD_CFG_INET6;
- env->vmd_cfg.cfg_flags &= ~VMD_CFG_AUTOINET6;
- memcpy(>vmd_cfg.cfg_localprefix6, , sizeof(h));
+ free($4);
}
| LOCAL PREFIX STRING {
struct address h;
+ const char *err;
- if (host($3, ) == -1 ||
- h.ss.ss_family != AF_INET ||
- h.prefixlen > 32 || h.prefixlen < 0) {
- yyerror("invalid local prefix: %s", $3);
- free($3);
+ if (parse_prefix4($3, , )) {
+ yyerror("invalid local prefix: %s", err);
YYERROR;
- }
-
- memcpy(>vmd_cfg.cfg_localprefix, , sizeof(h));
+ } else
+ memcpy(>vmd_cfg.cfg_localprefix, ,
+ sizeof(h));
+ free($3);
}
| SOCKET OWNER owner_id {
env->vmd_ps.ps_csock.cs_uid = $3.uid;
@@ -1404,42 +1403,119 @@ int
return (0);
}
+/*
+ * Parse an ipv4 address and prefix for local interfaces and validate
+ * constraints for vmd networking.
+ */
int
-host(const char *str, struct address *h)
+parse_prefix4(const char *str, struct address *h, const char **errstr)
{
- struct addrinfo hints, *res;
- int prefixlen;
- char*s, *p;
- const char *errstr;
+ struct addrinfo hints, *res = NULL;
+ in_addr_taddr;
+ int
pf(4) should mention DIOCXEND
Hello, diff below updates pf(4) manpage to reflect changes [1] which were committed earlier today. does update to pf(4) read OK? thanks and regards sashan [1] https://marc.info/?l=openbsd-cvs=168848058603797=2 https://marc.info/?l=openbsd-cvs=168847042626997=2 8<---8<---8<--8< diff --git a/share/man/man4/pf.4 b/share/man/man4/pf.4 index 92eeb45f657..305c536b137 100644 --- a/share/man/man4/pf.4 +++ b/share/man/man4/pf.4 @@ -48,12 +48,25 @@ and retrieve statistics. The most commonly used functions are covered by .Xr pfctl 8 . .Pp -Manipulations like loading a ruleset that involve more than a single +Operations like loading or reading a ruleset that involve more than a single .Xr ioctl 2 call require a so-called .Em ticket , -which prevents the occurrence of -multiple concurrent manipulations. +which allows +.Xr pf 4 +to deal with concurrent operations. +For certain +.Xr ioctl 2 +commands (currently +.Dv DIOCGETRULES ) +the number of tickets application can obtain is limited. +The application must explicitly release the ticket using +.Dv DIOCXEND +command to avoid hitting the limit. +All tickets which are not freed by +.Dv DIOCXEND +are released when application closes +.Pa /dev/pf . .Pp Fields of .Xr ioctl 2 @@ -132,6 +145,9 @@ for subsequent calls and the number .Va nr of rules in the active ruleset. +The ticket should be released by +.Dv DIOCXEND +command. .It Dv DIOCGETRULE Fa "struct pfioc_rule *pr" Get a .Va rule @@ -792,6 +808,10 @@ inactive rulesets since the last .Dv DIOCXBEGIN . .Dv DIOCXROLLBACK will silently ignore rulesets for which the ticket is invalid. +.It Dv DIOCXEND Fa "u_int32_t *ticket" +Release ticket obtained by +.Dv DIOCGETRULES +command. .It Dv DIOCSETHOSTID Fa "u_int32_t *hostid" Set the host ID, which is used by .Xr pfsync 4
Re: cksum remove redundant code
ok jmatthew@
On Tue, Jul 04, 2023 at 12:20:32PM +0300, Alexander Bluhm wrote:
> anyone?
>
> On Fri, May 26, 2023 at 06:44:25PM +0200, Alexander Bluhm wrote:
> > Hi,
> >
> > in_ifcap_cksum() checks ifp == NULL
> > in_hdr_cksum_out() sets ip_sum = 0
> > in_proto_cksum_out() and in6_proto_cksum_out() always write
> > th_sum if M_TCP_CSUM_OUT is set and proto is IPPROTO_TCP.
> >
> > ok?
> >
> > bluhm
> >
> > Index: netinet/ip_output.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
> > retrieving revision 1.388
> > diff -u -p -r1.388 ip_output.c
> > --- netinet/ip_output.c 22 May 2023 16:08:34 - 1.388
> > +++ netinet/ip_output.c 26 May 2023 11:55:49 -
> > @@ -1801,7 +1801,7 @@ in_hdr_cksum_out(struct mbuf *m, struct
> > struct ip *ip = mtod(m, struct ip *);
> >
> > ip->ip_sum = 0;
> > - if (ifp && in_ifcap_cksum(m, ifp, IFCAP_CSUM_IPv4)) {
> > + if (in_ifcap_cksum(m, ifp, IFCAP_CSUM_IPv4)) {
> > SET(m->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT);
> > } else {
> > ipstat_inc(ips_outswcsum);
> > Index: netinet/tcp_output.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_output.c,v
> > retrieving revision 1.138
> > diff -u -p -r1.138 tcp_output.c
> > --- netinet/tcp_output.c15 May 2023 16:34:56 - 1.138
> > +++ netinet/tcp_output.c26 May 2023 15:19:12 -
> > @@ -1295,7 +1295,6 @@ tcp_chopper(struct mbuf *m0, struct mbuf
> >
> > /* copy and adjust IP header, calculate checksum */
> > SET(m->m_pkthdr.csum_flags, M_TCP_CSUM_OUT);
> > - mhth->th_sum = 0;
> > if (ip) {
> > struct ip *mhip;
> >
> > @@ -1328,10 +1327,8 @@ tcp_chopper(struct mbuf *m0, struct mbuf
> > }
> > /* adjust IP header, calculate checksum */
> > SET(m0->m_pkthdr.csum_flags, M_TCP_CSUM_OUT);
> > - th->th_sum = 0;
> > if (ip) {
> > ip->ip_len = htons(m0->m_pkthdr.len);
> > - ip->ip_sum = 0;
> > in_hdr_cksum_out(m0, ifp);
> > in_proto_cksum_out(m0, ifp);
> > }
>
Re: sec(4): route based ipsec vpns
On Tue, Jul 04, 2023 at 03:26:30PM +1000, David Gwynne wrote:
> tl;dr: this adds sec(4) p2p ip interfaces. Traffic in and out of these
> interfaces is protected by IPsec security associations (SAs), but
> there's no flows (security policy database (SPD) entries) associated
> with these SAs. The policy for using the sec(4) interfaces and their
> SAs is route-based instead.
>
Hi,
I don't like this sleep with netlock held. sec_send() handler has the
netlock provided sleep point, so concurrent sec_ioctl() could grab
netlock and call sec_down() while sec_send() awaiting netlock release.
So sec_down() will sleep with netlock held awaiting sec_send() to finish
and sec_send() will sleep awaiting netlock acquisition. Smells like
deadlock.
Could you move taskq_del_barrier() and refcnt_finalize() to
sec_clone_destroy() to prevent this?
> +static int
> +sec_down(struct sec_softc *sc)
> +{
> + struct ifnet *ifp = >sc_if;
> + unsigned int idx = stoeplitz_h32(sc->sc_unit) % nitems(sec_map);
> +
> + NET_ASSERT_LOCKED();
> +
> + CLR(ifp->if_flags, IFF_RUNNING);
> +
> + SMR_SLIST_REMOVE_LOCKED(_map[idx], sc, sec_softc, sc_entry);
> +
> + smr_barrier();
> + taskq_del_barrier(systq, >sc_send);
> +
> + refcnt_finalize(>sc_refs, "secdown");
> +
> + return (0);
> +}
> +
> +static void
> +sec_send(void *arg)
> +{
> + struct sec_softc *sc = arg;
> + struct ifnet *ifp = >sc_if;
> + struct ifqueue *ifq = >if_snd;
> + struct tdb *tdb;
> + struct mbuf *m;
> + int error;
> +
> + if (!ISSET(ifp->if_flags, IFF_RUNNING))
> + return;
> +
> + tdb = sec_tdb_get(sc->sc_unit);
> + if (tdb == NULL)
> + goto purge;
> +
> + NET_LOCK();
> + while ((m = ifq_dequeue(ifq)) != NULL) {
> + CLR(m->m_flags, M_BCAST|M_MCAST);
> +
> +#if NPF > 0
> + pf_pkt_addr_changed(m);
> +#endif
> +
> + error = ipsp_process_packet(m, tdb,
> + m->m_pkthdr.ph_family, /* already tunnelled? */ 0);
> + if (error != 0)
> + counters_inc(ifp->if_counters, ifc_oerrors);
> + }
> + NET_UNLOCK();
> +
> + tdb_unref(tdb);
> + return;
> +
> +purge:
> + counters_add(ifp->if_counters, ifc_oerrors, ifq_purge(ifq));
> +}
> +
Nothing denies sec_start() be MP safe. No reason to wait or stop the
rest of kernel locked code here.
> +static void
> +sec_start(struct ifnet *ifp)
> +{
> + counters_add(ifp->if_counters, ifc_oerrors, ifq_purge(>if_snd));
> +}
Re: cksum remove redundant code
anyone?
On Fri, May 26, 2023 at 06:44:25PM +0200, Alexander Bluhm wrote:
> Hi,
>
> in_ifcap_cksum() checks ifp == NULL
> in_hdr_cksum_out() sets ip_sum = 0
> in_proto_cksum_out() and in6_proto_cksum_out() always write
> th_sum if M_TCP_CSUM_OUT is set and proto is IPPROTO_TCP.
>
> ok?
>
> bluhm
>
> Index: netinet/ip_output.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.388
> diff -u -p -r1.388 ip_output.c
> --- netinet/ip_output.c 22 May 2023 16:08:34 - 1.388
> +++ netinet/ip_output.c 26 May 2023 11:55:49 -
> @@ -1801,7 +1801,7 @@ in_hdr_cksum_out(struct mbuf *m, struct
> struct ip *ip = mtod(m, struct ip *);
>
> ip->ip_sum = 0;
> - if (ifp && in_ifcap_cksum(m, ifp, IFCAP_CSUM_IPv4)) {
> + if (in_ifcap_cksum(m, ifp, IFCAP_CSUM_IPv4)) {
> SET(m->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT);
> } else {
> ipstat_inc(ips_outswcsum);
> Index: netinet/tcp_output.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_output.c,v
> retrieving revision 1.138
> diff -u -p -r1.138 tcp_output.c
> --- netinet/tcp_output.c 15 May 2023 16:34:56 - 1.138
> +++ netinet/tcp_output.c 26 May 2023 15:19:12 -
> @@ -1295,7 +1295,6 @@ tcp_chopper(struct mbuf *m0, struct mbuf
>
> /* copy and adjust IP header, calculate checksum */
> SET(m->m_pkthdr.csum_flags, M_TCP_CSUM_OUT);
> - mhth->th_sum = 0;
> if (ip) {
> struct ip *mhip;
>
> @@ -1328,10 +1327,8 @@ tcp_chopper(struct mbuf *m0, struct mbuf
> }
> /* adjust IP header, calculate checksum */
> SET(m0->m_pkthdr.csum_flags, M_TCP_CSUM_OUT);
> - th->th_sum = 0;
> if (ip) {
> ip->ip_len = htons(m0->m_pkthdr.len);
> - ip->ip_sum = 0;
> in_hdr_cksum_out(m0, ifp);
> in_proto_cksum_out(m0, ifp);
> }
tcp timer wrap around, use 64 bit
Hi,
After changing tcp now tick to milliseconds, it will wrap around
after 49 days of uptime. That may be a problem in some places of
our stack. Better use a 64 bit counter.
As timestamp option is 32 bit in TCP protocol, we have to use the
lower 32 bit there. There are casts to 32 bits that should behave
correctly. More eyes to review would be helpful.
As a bonus, start with random 63 bit offset to avoid uptime leakage.
I am not aware that we leak anywhere, but more random is always
good. 2^63 milliseconds gives us 2.9*10^8 years of possible uptime.
ok?
bluhm
Index: netinet/tcp_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.388
diff -u -p -r1.388 tcp_input.c
--- netinet/tcp_input.c 30 May 2023 19:32:57 - 1.388
+++ netinet/tcp_input.c 4 Jul 2023 08:49:47 -
@@ -130,8 +130,8 @@ struct timeval tcp_ackdrop_ppslim_last;
#define TCP_PAWS_IDLE TCP_TIME(24 * 24 * 60 * 60)
/* for modulo comparisons of timestamps */
-#define TSTMP_LT(a,b) ((int)((a)-(b)) < 0)
-#define TSTMP_GEQ(a,b) ((int)((a)-(b)) >= 0)
+#define TSTMP_LT(a,b) ((int32_t)((a)-(b)) < 0)
+#define TSTMP_GEQ(a,b) ((int32_t)((a)-(b)) >= 0)
/* for TCP SACK comparisons */
#defineSEQ_MIN(a,b)(SEQ_LT(a,b) ? (a) : (b))
@@ -190,7 +190,7 @@ void tcp_newreno_partialack(struct tcpc
voidsyn_cache_put(struct syn_cache *);
voidsyn_cache_rm(struct syn_cache *);
-int syn_cache_respond(struct syn_cache *, struct mbuf *, uint32_t);
+int syn_cache_respond(struct syn_cache *, struct mbuf *, uint64_t);
voidsyn_cache_timer(void *);
voidsyn_cache_reaper(void *);
voidsyn_cache_insert(struct syn_cache *, struct tcpcb *);
@@ -198,10 +198,10 @@ void syn_cache_reset(struct sockaddr *,
struct tcphdr *, u_int);
int syn_cache_add(struct sockaddr *, struct sockaddr *, struct tcphdr *,
unsigned int, struct socket *, struct mbuf *, u_char *, int,
- struct tcp_opt_info *, tcp_seq *, uint32_t);
+ struct tcp_opt_info *, tcp_seq *, uint64_t);
struct socket *syn_cache_get(struct sockaddr *, struct sockaddr *,
struct tcphdr *, unsigned int, unsigned int, struct socket *,
- struct mbuf *, uint32_t);
+ struct mbuf *, uint64_t);
struct syn_cache *syn_cache_lookup(struct sockaddr *, struct sockaddr *,
struct syn_cache_head **, u_int);
@@ -375,7 +375,7 @@ tcp_input(struct mbuf **mp, int *offp, i
short ostate;
caddr_t saveti;
tcp_seq iss, *reuse = NULL;
- uint32_t now;
+ uint64_t now;
u_long tiwin;
struct tcp_opt_info opti;
struct tcphdr *th;
@@ -885,7 +885,7 @@ findpcb:
goto drop;
if (opti.ts_present && opti.ts_ecr) {
- int rtt_test;
+ int32_t rtt_test;
/* subtract out the tcp timestamp modulator */
opti.ts_ecr -= tp->ts_modulate;
@@ -1272,7 +1272,7 @@ trimthenstep6:
TSTMP_LT(opti.ts_val, tp->ts_recent)) {
/* Check to see if ts_recent is over 24 days old. */
- if ((int)(now - tp->ts_recent_age) > TCP_PAWS_IDLE) {
+ if (now - tp->ts_recent_age > TCP_PAWS_IDLE) {
/*
* Invalidate ts_recent. If this segment updates
* ts_recent, the age will be reset later and ts_recent
@@ -2120,7 +2120,7 @@ drop:
int
tcp_dooptions(struct tcpcb *tp, u_char *cp, int cnt, struct tcphdr *th,
struct mbuf *m, int iphlen, struct tcp_opt_info *oi,
-u_int rtableid, uint32_t now)
+u_int rtableid, uint64_t now)
{
u_int16_t mss = 0;
int opt, optlen;
@@ -2686,7 +2686,7 @@ tcp_pulloutofband(struct socket *so, u_i
* and update averages and current timeout.
*/
void
-tcp_xmit_timer(struct tcpcb *tp, int rtt)
+tcp_xmit_timer(struct tcpcb *tp, int32_t rtt)
{
int delta, rttmin;
@@ -3335,7 +3335,7 @@ void
syn_cache_timer(void *arg)
{
struct syn_cache *sc = arg;
- uint32_t now;
+ uint64_t now;
NET_LOCK();
if (sc->sc_flags & SCF_DEAD)
@@ -3469,7 +3469,7 @@ syn_cache_lookup(struct sockaddr *src, s
*/
struct socket *
syn_cache_get(struct sockaddr *src, struct sockaddr *dst, struct tcphdr *th,
-u_int hlen, u_int tlen, struct socket *so, struct mbuf *m, uint32_t now)
+u_int hlen, u_int tlen, struct socket *so, struct mbuf *m, uint64_t now)
{
struct syn_cache *sc;
struct syn_cache_head *scp;
@@ -3744,7 +3744,7 @@ syn_cache_unreach(struct sockaddr *src,
int
syn_cache_add(struct sockaddr *src, struct sockaddr *dst, struct tcphdr *th,
u_int iphlen, struct socket *so, struct mbuf *m, u_char *optp, int optlen,
-struct tcp_opt_info *oi, tcp_seq *issp, uint32_t now)
+struct tcp_opt_info *oi, tcp_seq
Re: pf.os database /p0f
On 2023/07/04 09:48, Solène Rapenne wrote: > On Tue, 2023-07-04 at 03:39 +, Lee, Jonathan D wrote: > > [cid:cd2efd41-42cb-4d83-9173-521bbb8f4539@namprd04.prod.outlook.com] > > > > Hello fellow software developers, > > > > I have noticed that p0f database files are not being updated. Many > > new operating systems fingerprints are missing within the pf.os > > database file that your software uses. I have added a section in > > pf.os for Docker containers see the below diff checker output. Yes > > this is unorthadox for the diff file again it is only a blank area > > for new OS entries and it helps bring to lite that containers can > > also be fingerprinted. The docx that is attached helps to showcase > > the Kali penetration software running inside of a docker container. > > The container was spun up and spun down and also deleted. I have > > fingerprinted this docker container with the program p0f. I noticed > > that p0f is used with pfSense and is used with access control lists > > for source address OS see attached photos. Again for this to function > > correctly it needs the database updated and new catagories like many > > of the mainstream containers. We can fingerprint them like other OS > > systems. > > > > It seems you are using PFSense, which is based on FreeBSD. > You are on the OpenBSD mailing list. > > Even if we update our fingerprint database to add docker like you > suggest, this won't reflect in the product you are using. If somebody is able to send working TCP SYN signatures for the old version of p0f that's used in PF (note that the separate p0f program has changed quite a lot in the meantime and uses a different database format), that don't cause problems with false detection, they could be added. But there's no value in adding an empty placeholder section. I'm a bit unsure whether this is going to be possible though (in particular that they can be reliably identified separate to the container's base OS).
Re: Add ethernet type check in ifsetlro()
On Mon, Jul 03, 2023 at 11:12:17PM +0200, Jan Klemkow wrote:
> bluhm pointed out that the ether_brport_isset() check it just allowed on
> ethernet devices. Thus, I put an additional ethernet check in the
> condition. This also fixes EBUSY errors of "ifconfig lo0 tcplro" calls
> in my setup.
>
> ok?
OK bluhm@
> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.702
> diff -u -p -r1.702 if.c
> --- net/if.c 2 Jul 2023 19:59:15 - 1.702
> +++ net/if.c 3 Jul 2023 20:58:32 -
> @@ -3206,7 +3206,7 @@ ifsetlro(struct ifnet *ifp, int on)
> KERNEL_ASSERT_LOCKED(); /* for if_flags */
>
> if (on && !ISSET(ifp->if_xflags, IFXF_LRO)) {
> - if (ether_brport_isset(ifp)) {
> + if (ifp->if_type == IFT_ETHER && ether_brport_isset(ifp)) {
> error = EBUSY;
> goto out;
> }
Re: pf.os database /p0f
On Tue, 2023-07-04 at 03:39 +, Lee, Jonathan D wrote: > [cid:cd2efd41-42cb-4d83-9173-521bbb8f4539@namprd04.prod.outlook.com] > > Hello fellow software developers, > > I have noticed that p0f database files are not being updated. Many > new operating systems fingerprints are missing within the pf.os > database file that your software uses. I have added a section in > pf.os for Docker containers see the below diff checker output. Yes > this is unorthadox for the diff file again it is only a blank area > for new OS entries and it helps bring to lite that containers can > also be fingerprinted. The docx that is attached helps to showcase > the Kali penetration software running inside of a docker container. > The container was spun up and spun down and also deleted. I have > fingerprinted this docker container with the program p0f. I noticed > that p0f is used with pfSense and is used with access control lists > for source address OS see attached photos. Again for this to function > correctly it needs the database updated and new catagories like many > of the mainstream containers. We can fingerprint them like other OS > systems. > It seems you are using PFSense, which is based on FreeBSD. You are on the OpenBSD mailing list. Even if we update our fingerprint database to add docker like you suggest, this won't reflect in the product you are using.
