Skip to content

Conversation

fichtner
Copy link

@fichtner fichtner commented Oct 9, 2023

As reported by an OPNsense user doing a security scan pf/pflog can drop e.g. invalid length packets under the default rule which also uses a -1 value like subrulenr.

Transform the displayed value from "4294967295" to "-1" in this case because it is more correct (although both are suboptimal for processing).

FreeBSD: https://cgit.freebsd.org/src/tree/sys/netpfil/pf/pf_ioctl.c?id=3347078000c078f2e67214ef1ba2e0bffe1aea4f#n349
OpenBSD: https://github.com/openbsd/src/blob/142580dd4dc788acb41545aca79c845e04d1cb7d/sys/net/pf_ioctl.c#L242

See also: opnsense/core#6800

As reported by an OPNsense user doing a security scan pf/pflog can
drop e.g. invalid length packets under the default rule which also
uses a -1 value like subrulenr.

Transform the displayed value from "4294967295" to "-1" in this case
because it is more correct (although both are suboptimal for processing).

FreeBSD: https://cgit.freebsd.org/src/tree/sys/netpfil/pf/pf_ioctl.c?id=3347078000c078f2e67214ef1ba2e0bffe1aea4f#n349
OpenBSD: https://github.com/openbsd/src/blob/142580dd4dc788acb41545aca79c845e04d1cb7d/sys/net/pf_ioctl.c#L242

See also: opnsense/core#6800
@fichtner
Copy link
Author

fichtner commented Oct 9, 2023

I don't mind changing the value to something else as requested as displaying "default" also makes sense, but it may have a bigger impact on parsers.

@bluhm
Copy link

bluhm commented Oct 9, 2023

OpenBSD prints "def" in tcpdump. This is easier to understand for the user than -1 in output.
https://github.com/openbsd/src/blob/de35ea824a17d664ed147ca87993647d140bcf42/usr.sbin/tcpdump/print-pflog.c#L98

@fichtner
Copy link
Author

fichtner commented Oct 9, 2023

@bluhm Thanks for the pointer. I've updated it accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants