Bug 255852 - pf: set skip on: serious security hole
Summary: pf: set skip on: serious security hole
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 13.0-RELEASE
Hardware: Any Any
: --- Affects Many People
Assignee: Kristof Provost
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-13 21:17 UTC by rashey
Modified: 2024-08-28 08:40 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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(+)
Comment 10 emz 2024-08-28 08:34:39 UTC
So these commits above are already in my 13.2-RELEASE-p9 (I've confirmed it even visually). But I'm still experience the unability to unskip the once skipped interface from PF config, and I (still blaming myself) cannot explain this by anything except that those fixes do not fix the original issue (notice the vlan2 rmains always skipped):

[root@gw0:/etc]# egrep 'asif =|asif2 =|set skip on \$asif|set skip on vlan2' /etc/pf.gw0
asif = "vlan2"
asif2 = "vlan5"
#set skip on $asif
set skip on $asif2

[root@gw0:/etc]# pfctl -vsI                                                  
No ALTQ support in kernel
ALTQ related functions disabled
all
enc0 (skip)
igb0
igb1
lo
lo0 (skip)
ng0
ng1
ng10
ng11
ng12
ng13
ng14
ng15
ng16
ng17
ng18
ng19
ng2
ng20
ng21
ng22
ng3
ng4
ng5
ng6
ng7
ng8
ng9
pflog
pflog0
pfsync
pfsync0
r1g
r2g
tun
tun0
vlan
vlan1
vlan10 (skip)
vlan101
vlan11
vlan12
vlan13
vlan2 (skip)
vlan250
vlan251 (skip)
vlan252 (skip)
vlan3
vlan4
vlan434
vlan5 (skip)
vlan6
vlan8
vlan9
vpn

[root@gw0:/etc]# pfctl -f /etc/pf.gw0                                                   

[root@gw0:/etc]# pfctl -vsI          
No ALTQ support in kernel
ALTQ related functions disabled
all
enc0 (skip)
igb0
igb1
lo
lo0 (skip)
ng0
ng1
ng10
ng11
ng12
ng13
ng14
ng15
ng16
ng17
ng18
ng19
ng2
ng20
ng21
ng22
ng3
ng4
ng5
ng6
ng7
ng8
ng9
pflog
pflog0
pfsync
pfsync0
r1g
r2g
tun
tun0
vlan
vlan1
vlan10 (skip)
vlan101
vlan11
vlan12
vlan13
vlan2 (skip)
vlan250
vlan251 (skip)
vlan252 (skip)
vlan3
vlan4
vlan434
vlan5 (skip)
vlan6
vlan8
vlan9
vpn
Comment 11 Kristof Provost freebsd_committer freebsd_triage 2024-08-28 08:40:11 UTC
(In reply to emz from comment #10)
That's likely 280834
Fixed in https://cgit.freebsd.org/src/commit/?id=6a88e22728d285c4df17216515ce2b8d1e5a6835