Bug 255852

Summary: pf: set skip on: serious security hole
Product: Base System Reporter: rashey
Component: kernAssignee: Kristof Provost <kp>
Status: Closed FIXED    
Severity: Affects Many People CC: kp, secteam
Priority: ---    
Version: 13.0-RELEASE   
Hardware: Any   
OS: Any   
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250994

Description rashey 2021-05-13 21:17:30 UTC
Once skipped interface cannot be unskipped till pf restart.

An oblivious administrators can make a hole in firewall by reloading ruleset without pf restart after network reconfiguration.

# ifconfig epair create
epair0a

# echo "set skip on { lo0, epair }" > /etc/pf.conf

# service pf reload
Reloading pf rules.

# pfctl -vsI
No ALTQ support in kernel
ALTQ related functions disabled
all
em0
em1
epair (skip)
epair0a (skip)
epair0b (skip)
lo
lo0 (skip)

echo "set skip on lo0" > /etc/pf.conf

# service pf reload
Reloading pf rules.

# pfctl -vsI
No ALTQ support in kernel
ALTQ related functions disabled
all
em0
em1
epair (skip)
epair0a (skip)
epair0b (skip)
lo
lo0 (skip)

# service pf restart
Disabling pf.
Enabling pf.

# pfctl -vsI
No ALTQ support in kernel
ALTQ related functions disabled
all
em0
em1
epair
epair0a
epair0b
lo
lo0 (skip)

# freebsd-version
13.0-RELEASE
Comment 1 Kristof Provost freebsd_committer 2021-05-14 12:29:08 UTC
That's the 'set skip on <ifgroup>' issue. I was under the impression that it was fixed in 13.0.

By the way, re-apply the ruleset will fix the skip configuration (and applying it again will break it again) on affected systems.
Comment 2 Kristof Provost freebsd_committer 2021-05-14 12:46:54 UTC
Sigh. I *thought* it was fixed everywhere, but apparently there's yet another manifestation of it. I am so very sick of this bug.

Confirmed to affect main as well.
Comment 3 Kristof Provost freebsd_committer 2021-05-16 10:48:23 UTC
Fix: https://reviews.freebsd.org/D30284
Test: https://reviews.freebsd.org/D30285

That's a really nasty fix, but it's probably the best we can do until I get around to nvlist-ifying the relevant ioctl call.
Comment 4 commit-hook freebsd_committer 2021-05-17 13:49:20 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=d69cc040147284c414dfd1c9f498dcc7c8291a8b

commit d69cc040147284c414dfd1c9f498dcc7c8291a8b
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-05-16 06:50:17 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-17 11:48:06 +0000

    pf: Set the pfik_group for userspace

    Userspace relies on this pointer to work out if the kif is a group or
    not. It can't use it for anything else, because it's a pointer to a
    kernel address. Substitute 0xfeedc0de for 'true', so that we don't leak
    kernel memory addresses to userspace.

    PR:             255852
    Reviewed by:    donner
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D30284

 sys/netpfil/pf/pf_if.c | 8 ++++++++
 1 file changed, 8 insertions(+)
Comment 5 commit-hook freebsd_committer 2021-05-17 13:49:21 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=45db38554517c7e1b0cc0265113c22f92a0eb494

commit 45db38554517c7e1b0cc0265113c22f92a0eb494
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-05-16 06:51:54 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-17 11:48:06 +0000

    pf tests: More set skip on <ifgroup> tests

    Test the specific case reported in PR 255852. Clearing the skip flag
    on groups was broken because pfctl couldn't work out if a kif was a
    group or not, because the kernel no longer set the pfik_group pointer.

    PR:             255852
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D30285

 tests/sys/netpfil/pf/set_skip.sh | 46 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
Comment 6 commit-hook freebsd_committer 2021-05-24 15:41:49 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=25fa4b8e668fed449e4c52f2a9be134c067d749c

commit 25fa4b8e668fed449e4c52f2a9be134c067d749c
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-05-16 06:51:54 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-24 15:38:22 +0000

    pf tests: More set skip on <ifgroup> tests

    Test the specific case reported in PR 255852. Clearing the skip flag
    on groups was broken because pfctl couldn't work out if a kif was a
    group or not, because the kernel no longer set the pfik_group pointer.

    PR:             255852
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D30285

    (cherry picked from commit 45db38554517c7e1b0cc0265113c22f92a0eb494)

 tests/sys/netpfil/pf/set_skip.sh | 46 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
Comment 7 commit-hook freebsd_committer 2021-05-24 15:41:50 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=31424ca0a1d02d10011f2d3c32d49c8e8a3e7c96

commit 31424ca0a1d02d10011f2d3c32d49c8e8a3e7c96
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-05-16 06:50:17 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-24 15:38:22 +0000

    pf: Set the pfik_group for userspace

    Userspace relies on this pointer to work out if the kif is a group or
    not. It can't use it for anything else, because it's a pointer to a
    kernel address. Substitute 0xfeedc0de for 'true', so that we don't leak
    kernel memory addresses to userspace.

    PR:             255852
    Reviewed by:    donner
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D30284

    (cherry picked from commit d69cc040147284c414dfd1c9f498dcc7c8291a8b)

 sys/netpfil/pf/pf_if.c | 8 ++++++++
 1 file changed, 8 insertions(+)
Comment 8 commit-hook freebsd_committer 2021-05-24 15:41:51 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b780106bba6ae8f0259c4d134908787ba58eac5b

commit b780106bba6ae8f0259c4d134908787ba58eac5b
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-05-16 06:51:54 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-24 15:40:28 +0000

    pf tests: More set skip on <ifgroup> tests

    Test the specific case reported in PR 255852. Clearing the skip flag
    on groups was broken because pfctl couldn't work out if a kif was a
    group or not, because the kernel no longer set the pfik_group pointer.

    PR:             255852
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D30285

    (cherry picked from commit 45db38554517c7e1b0cc0265113c22f92a0eb494)

 tests/sys/netpfil/pf/set_skip.sh | 46 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
Comment 9 commit-hook freebsd_committer 2021-05-24 15:41:51 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b34127064fde4d144b6d63dce0eada01563bc562

commit b34127064fde4d144b6d63dce0eada01563bc562
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-05-16 06:50:17 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-05-24 15:40:28 +0000

    pf: Set the pfik_group for userspace

    Userspace relies on this pointer to work out if the kif is a group or
    not. It can't use it for anything else, because it's a pointer to a
    kernel address. Substitute 0xfeedc0de for 'true', so that we don't leak
    kernel memory addresses to userspace.

    PR:             255852
    Reviewed by:    donner
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D30284

    (cherry picked from commit d69cc040147284c414dfd1c9f498dcc7c8291a8b)

 sys/netpfil/pf/pf_if.c | 8 ++++++++
 1 file changed, 8 insertions(+)