Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Direct passthroughs rules not removed on reload #959

Open
DevDorrejo opened this issue Apr 30, 2022 · 5 comments
Open

Direct passthroughs rules not removed on reload #959

DevDorrejo opened this issue Apr 30, 2022 · 5 comments
Labels
breaking Breaking change. Must be part of a major release. easy Effort is estimated to be easy.

Comments

@DevDorrejo
Copy link

What happened:
removing passthroughts and doing reload wont remove the rules

What you expected to happen:
on reload rules not appear

How to reproduce it (as minimally and precisely as possible):
firewall-cmd --permanent --direct --passthrough ipv4 -I FORWARD -o virbr0 -j ACCEPT firewall-cmd --direct --get-all-passthroughs firewall-cmd --permanent --direct --remove-passthrough ipv4 -I FORWARD -o virbr0 -j ACCEPT firewall-cmd --reload firewall-cmd --permanent --direct --remove-passthrough ipv4 -I FORWARD -o virbr0 -j ACCEPT

Anything else we need to know?:
workaround: systemctl restart firewalld.service

Environment:

  • Firewalld Version (if Fedora based dnf info firewalld or commit hash if developing from git git log -n1 --format=format:"%H"): 1.1.1
  • Firewalld Backend (cat /etc/firewalld/firewalld.conf | grep FirewallBackend): nftables
  • OS (e.g: cat /etc/os-release):
NAME="openSUSE Tumbleweed"
# VERSION="20220428"
ID="opensuse-tumbleweed"
ID_LIKE="opensuse suse"
VERSION_ID="20220428"
PRETTY_NAME="openSUSE Tumbleweed"
ANSI_COLOR="0;32"
CPE_NAME="cpe:/o:opensuse:tumbleweed:20220428"
BUG_REPORT_URL="https://bugs.opensuse.org"
HOME_URL="https://www.opensuse.org/"
DOCUMENTATION_URL="https://en.opensuse.org/Portal:Tumbleweed"
LOGO="distributor-logo-Tumbleweed
  • Others:
@erig0
Copy link
Collaborator

erig0 commented May 2, 2022

Do you have FlushAllOnReload=no in /etc/firewalld/firewalld.conf?

@erig0
Copy link
Collaborator

erig0 commented May 2, 2022

tl;dr don't use --permanent

You're using --permanent. It looks like when you use --permanent --direct --passthrough it is synonymous with --permanent --direct --add-passthrough. I don't know why.

Even the man pages say you shouldn't be able to use --permanent with --passthrough:

--direct --passthrough { ipv4 | ipv6 | eb } args
Pass a command through to the firewall. args can be all iptables,
ip6tables and ebtables command line arguments. This command is
untracked, which means that firewalld is not able to provide
information about this command later on, also not a listing of the
untracked passthoughs.

But, the code obviously allows it and aliases it to --add-passthrough:

firewalld/src/firewall-cmd.in

Lines 1888 to 1901 in 797234e

elif options_direct:
direct = fw.config().direct()
if a.passthrough:
if len(a.passthrough) < 2:
cmd.fail("usage: --permanent --direct --passthrough { ipv4 | ipv6 | eb } <args>")
cmd.print_msg(direct.addPassthrough(cmd.check_ipv(a.passthrough[0]),
splitArgs(a.passthrough[1])))
if a.add_passthrough:
if len(a.add_passthrough) < 2:
cmd.fail("usage: --permanent --direct --add-passthrough { ipv4 | ipv6 | eb } <args>")
cmd.print_msg(direct.addPassthrough(cmd.check_ipv(a.add_passthrough[0]),
splitArgs(a.add_passthrough[1])))

We can't fix this with out potentially breaking users that rely on --permanent --direct --passthrough being an alias for --permanent --direct --add-passthrough. It would be a breaking change.

@erig0 erig0 added can't fix Can't fix. Likely due to technical reasons. won't fix Won't fix. Out of scope or not useful. and removed can't fix Can't fix. Likely due to technical reasons. labels May 2, 2022
@erig0 erig0 added breaking Breaking change. Must be part of a major release. and removed won't fix Won't fix. Out of scope or not useful. labels May 2, 2022
@DevDorrejo
Copy link
Author

Thanks, understand, so i wont use --permanent, on this case

@erig0 erig0 added the easy Effort is estimated to be easy. label Sep 9, 2022
@erig0
Copy link
Collaborator

erig0 commented Oct 5, 2022

Maybe in the short term we should update the man page to indicate this is an alias for --permanent --add-passthrough... and that it will change in a future release, e.g. 2.0.0.

@DevDorrejo
Copy link
Author

👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change. Must be part of a major release. easy Effort is estimated to be easy.
Projects
None yet
Development

No branches or pull requests

2 participants