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

[Bug]: Iptables firewall rules update to block traffic on abended tunnel #1649

Closed
8 tasks done
frankozland opened this issue Aug 24, 2024 · 5 comments
Closed
8 tasks done

Comments

@frankozland
Copy link

Code of Conduct

  • I agree to follow this project's Code of Conduct.

Issue reporting checklist

Operating System

Raspberry Pi OS (64-bit) Lite Bookworm

Quick install or Manual setup?

Quick install

Onboard wireless chipset or external adapter?

Onboard wireless chipset

Hardware

Raspberry Pi 4 Model B

RaspAP version

3.1.3 (Latest)

Other software or services running with RaspAP?

Yes (specify below)

Contact details (optional)

frankoz95967943@gmail.com

Bug description

config/iptables_rules.json

        {
            "name": "firewall policies",
            "fw-state": true,
            "comment": "Policy rules (firewall)",
            "rules": [
                "-P INPUT DROP",
                "-P FORWARD ACCEPT",
                "-P OUTPUT ACCEPT",
                "-t nat -P PREROUTING ACCEPT",
                "-t nat -P POSTROUTING ACCEPT",
                "-t nat -P INPUT ACCEPT",
                "-t nat -P OUTPUT ACCEPT"
            ]

With '-P FORWARD ACCEPT' and any cli/openvpn/wg goes down, packets will still flow.
Masquerade means "FORWARD ANY MEANS POSSIBLE"

So if tunnel dies, packets still traverse.

This should be changed to '-P FORWARD DROP'

2ndly - if i have already applied custom firewall rules, they should be honored vs raspap changing them.
For example, in my firewall i specificed '-P FORWARD DROP'. RASPAP ignored what i configured and changed it to '-P FORWARD ACCEPT'.

I think Raspap should check to see if i have any rules that its about to change and give option not to change them if already set.

Steps to reproduce

install raspap
apply personal custom firewall rules
enable tunnel

Screenshots

No response

Additional context

No response

Relevant log output

No response

@frankozland
Copy link
Author

There is a 2nd bug as well - wireguard only uses udp - so its ok to say -A INPUT -p UDP.

But openvpn allows both UDP and TCP. But i dont think -A INPUT -P is needed at all - in rules below, i left it in just for consistency, but removed the -p UDP.

What is happening is very easy to reproduce on either wireguard or openvpn.
Simply stop wg if thats the tunnel or stop openvpn if thats the vpn tunnel.

Traffic will still flow.

Below is a better kill switch for both.
If the tunnel drops, traffic stops.

The basic jist - MASQUERADE is a-ok but ONLY if traversing the vpn tunnel.
If vpn tunnel isnt there? Any forwarded packet is dropped.
If tunnel re-established?
Packets flow.

In current ruleset as deployed - traffic will flow if the tunnel is there or not.
For most users this will be not desired.

I think you also want to be sure to drop any tun+ to tun+ traffic (thats an attack),
and wg+ to wg+ (same attack).

For custom CLI vpn, if you can identify the tunnel name from 'ip a' (or known from publisher), same rules will protect traffic to be sure it flows thru whatever tunnel name they create.

Confirmed with iptraf (sudo apt-get install iptraf)

            "name": "openvpn",
            "comment": "Rules for tunnel device (tun)",
            "ip-version": 4,
            "dependson": [
                  { "var": "openvpn-enable", "type": "bool" },
                  { "var": "openvpn-serverip", "type": "string", "replace": "$IPADDRESS$" },
                  { "var": "ap-device", "type": "string", "replace": "$INTERFACE$" }
            ],
            "rules": [
                "-A INPUT -s $IPADDRESS$ -j ACCEPT",
                "-A FORWARD -i tun+ -o wlan+ -j ACCEPT",
                "-A FORWARD -i tun+ -o tun+ -j DROP",
                "-A FORWARD -i wlan+ -o tun+ -j ACCEPT",
                "-A FORWARD -i eth+ -o tun+ -j ACCEPT",
                "-A FORWARD -i tun+ -o eth+ -j ACCEPT",
                "-t nat -A POSTROUTING -o $INTERFACE$ -j MASQUERADE",
                "-P FORWARD DROP"
            ]

        },
        {
            "name": "wireguard",
            "comment": "Rules for wireguard device (wg)",
            "ip-version": 4,
            "dependson": [
                  { "var": "wireguard-enable", "type": "bool" },
                  { "var": "wireguard-serverip", "type": "string", "replace": "$IPADDRESS$" },
                  { "var": "client-device", "type": "string", "replace": "$INTERFACE$" }
            ],
            "rules": [
                "-A INPUT -p udp -s $IPADDRESS$ -j ACCEPT",
                "-A FORWARD -i wg+ -o wlan+ -j ACCEPT",
                "-A FORWARD -i wg+ -o wg+ -j DROP",
                "-A FORWARD -i wlan+ -o wg+ -j ACCEPT",
                "-A FORWARD -i eth+ -o wg+ -j ACCEPT",
                "-A FORWARD -i wg+ -o eth+ -j ACCEPT",
                "-t nat -A POSTROUTING -o $INTERFACE$ -j MASQUERADE",
                "-P FORWARD DROP"
            ]
        }

@frankozland
Copy link
Author

I dont know if i did this right - but i updated the json file with changes.
I confirmed changes - stopping wireguard stopped all traffic.
Restarted wireguard traffic flowed again.

#1650

@frankozland frankozland changed the title [Bug]: Iptables should not default to "FORWARD ACCEPT" [Bug]: Iptables firewall rules update to block traffic on abended tunnel Aug 24, 2024
@billz
Copy link
Member

billz commented Aug 25, 2024

@frankozland thanks for this. the iptables_rules.json file is used by the Insiders edition, so the PR would need to be applied to this repo. The file was committed to this repo at some stage but is unused by the code. WG has an optional kill switch (also Insiders, for now) that's implemented with PostUp/PreDown scripts that apply identical iptables rules. Maybe the best move here is to implement an OpenVPN kill switch in the public repo.

Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Sep 25, 2024
Copy link

This issue was closed because it has been inactive for 14 days since being marked as stale.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants