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

Changing firewall rules does not block already-established connections #4141

Open
t4777sd opened this issue Jul 24, 2018 · 10 comments
Open

Changing firewall rules does not block already-established connections #4141

t4777sd opened this issue Jul 24, 2018 · 10 comments
Assignees
Labels
C: manager/widget help wanted This issue will probably not get done in a timely fashion without help from community contributors. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. ux User experience

Comments

@t4777sd
Copy link

t4777sd commented Jul 24, 2018

I know this is going to sound crazy, but I have tested it on various VMs with freshly installed templates provided from Qubes with freshly cloned AppVMs and it is reproducible.

Qubes OS version:

4.0

Affected component(s):

sys-firewall I assume

Steps to reproduce the behavior:

  1. Download / install default fedora28 templates via the qubes-update command
  2. Assign sys-net, sys-firewall, and an appVM to the freshly installed fedora28 template
  3. In the AppVM: open up firefox and browse the web. Open the browser to reddit.com, and it will be working. Keep reddit.com open
  4. In the AppVM Qubes Configure: go to Firewall tab and click "Limit outgoing connects to ..." and Apply
  5. Continue browser reddit.com. You will observe that reddit.com still is browsable
  6. Open up a new Tab and go to another domain such as ford.com or whatever. This request will be blocked
  7. Open the terminal in the AppVM. Type ping reddit.com and addresses can still ping
  8. Go back to Qubes firewall config and configure it to allow otugoing connections
  9. Reload the previously blocked ford.com and it will work now

Expected behavior:

All internet should be cut from the VM. That includes previously connected IP addresses, future connected IP addresses, and different internet protocols

Actual behavior:

Previously connected IPs not blocked. Only HTTP/HTTPS traffic to new IPs is blocked.

@unman
Copy link
Member

unman commented Jul 25, 2018

@t4777sd This isn't crazy, and it isn't new behaviour.
Exactly this issue has been raised before.

The other behaviour you have noted is expected for stateful firewalls.
The problem is that your expectations are misplaced.

The "NOTE" at the bottom of the firewall rules pane, which you seem to have missed, explicitly refers to ICMP being allowed, and points the user to use of qvm-firewall for greater control.
It also explains how to block all network access.

It was previously mooted that some explanation should be included on the firewall tab, or in the FAQ, or somewhere else. I dont know if that would help.

@andrewdavidwong
Copy link
Member

This isn't crazy, and it isn't new behaviour.
Exactly this issue has been raised before.

If you believe this is a duplicate of an existing issue, please let me know. I wasn't able to find one.

If it was raised on the MLs, a thread link would also be helpful.

It was previously mooted that some explanation should be included on the firewall tab, or in the FAQ, or somewhere else. I dont know if that would help.

Classifying as a documentation issue for now.

CC: @marmarek

@andrewdavidwong andrewdavidwong added C: doc T: task Type: task. An action item that is neither a bug nor an enhancement. labels Jul 25, 2018
@andrewdavidwong andrewdavidwong added this to the Ongoing milestone Jul 25, 2018
@andrewdavidwong andrewdavidwong changed the title Security (High): Firewall not fully working in 4.0 Changing firewall rules does not block already-established connections Jul 25, 2018
@t4777sd
Copy link
Author

t4777sd commented Jul 25, 2018

Perhaps it should be discussed if DNS / ICMP should be blocked or make it checkable with a check box instead of like that by default. I think most people's expectations is that a whitelist only allows what is in the whitelist and not the specififed whitelist + stuff from a hidden whitelist. It could even be that DNS traffic is auto-added to the rule-set when originally clicking to use a whitelist, so someone can remove it if they want.

The way it works now is a pretty big security risk. Imagine a qube meant to work with a VPN or with a normal proxy and as a result of this whitelist but not a whitelist firewall rules it is now having lots of DNS leaks everywhere.

Considering Qubes is meant to be a secure OS I think it's default behavior should work to maintain security.

@t4777sd
Copy link
Author

t4777sd commented Jul 25, 2018

The code base is very clean and easy to read. Thank you for that! I am looking at firewall.py.

If I change firewall.py to no longer automatically add those rules will it break something in Qubes? It seems strange to me that those rules were auto-added which basically breaks the firewall configuration functionality, so there must have been a good reason it was done that way.

Does anyone know the reason? I want to know before I make the changes

@unman
Copy link
Member

unman commented Jul 25, 2018

There used to be DNS/ICMP checkbox in previous versions. I doubt it will be reverted.
The current GUI version tries to strike a balance between usability and detailed control. It explicitly refers to the things you are worried about, tells the user how to block all outgoing traffic, and points the user toward qvm-firewall.
Note that the current treatment isn't unique to Qubes - I've seen other firewall controls aimed at ordinary users which also make assumptions about what should be whitelisted.

@t4777sd
Copy link
Author

t4777sd commented Jul 25, 2018

I get it. However, the very feature we are discussing is a usability issue, Especially the DNS whitelisting. That can be done by the user very easily within the current GUI by any user that wants to whitelist DNS. Instead of crippling the firewall and making the whole interface unusable the note at the bottom can read "whitelist port 53 for DNS" or auto-add such rules when originally clicked.

If you look at how Qubes is actually used you can see the automatic DNS whitelisting is Anti-User. You can verify this by looking at the popularity around VPN and Whonix whose specific purpose is create a more secure environment to stop DNS leaks and other such leaks. We are not talking about some obscure thing, but something that a large portion of Qubes users want to do as evidenced by their demand for anti-leak solutions.

So, given the obvious demand, why would the default behavior of the firewall to create such leaks?

To make something more "usable" it should serve more people's needs in an accessible way. In other words, reduce complexity so that more people can utilize said feature. This is not the case here. Complexity was not reduced at all in the case of DNS rules. The interface was not simplified at all. It merely pushes anyone who wants to restrict traffic to specific DNS servers or to no DNS servers have to use more complex tools.

The very opposite of usability.

@marmarek
Copy link
Member

If you believe this is a duplicate of an existing issue, please let me know. I wasn't able to find one.

The previous discussion existing connections: #2731
There is already a tooltip about this.

As for DNS/ICMP and all kind of VPNs - you need some kind of extra firewall rules anyway, To avoid leaks when your VPN connection breaks. See documentation:

#    Block forwarding of connections through upstream network device
#    (in case the vpn tunnel breaks):
iptables -I FORWARD -o eth0 -j DROP
iptables -I FORWARD -i eth0 -j DROP

You get other leaks prevention here for free.
Other than that, cutting your VM from DNS access but still having some network access there I'd call specialized enough that can be handled with command line.

Anyway, if you still think such option in GUI tool would be useful, feel free to add it and open a PR to https://github.com/qubesos/qubes-manager/.

@andrewdavidwong andrewdavidwong added T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. help wanted This issue will probably not get done in a timely fashion without help from community contributors. C: manager/widget P: minor Priority: minor. The lowest priority, below "default." and removed C: doc T: task Type: task. An action item that is neither a bug nor an enhancement. labels Jul 26, 2018
@andrewdavidwong andrewdavidwong modified the milestones: Ongoing, Far in the future Jul 26, 2018
@t4777sd
Copy link
Author

t4777sd commented Jul 26, 2018

@marmarek I would love to do a pull request. I can see by looking at screenshots of 3.2 that the firewall was previously fully functional and then later was made less functional: https://www.qubes-os.org/attachment/wiki/QubesFirewall/r2b1-manager-firewall.png

I just need to revert the changes that made it less functional to get it back to that screenshot.

My Questions

I assume it was changed for a reason. If it was due to a philosophy change, then will a pull request be accepted that returns power back to the user?

If it was due to a technical issue that resulted from re-enabling those rules, then can you point me to that issue so that I can try to solve it as part of the PR?

@marmarta
Copy link
Member

The reason was pretty simple: for a less-technically inclined user (the target for GUI config tools), configuring firewall to "block everything except this address" would actually not work like they think it should, because just whitelisting an address doesn't mean it will actually work (because if DNS is not allowed, they will not be able to access it anyway). So if you can figure out a way to present it clearly enough for a non-technical user, it would be very welcome; but it should work even for someone who does not know what DNS is.

@andrewdavidwong andrewdavidwong added ux User experience and removed P: minor Priority: minor. The lowest priority, below "default." labels Jul 27, 2018
@andrewdavidwong andrewdavidwong modified the milestones: Far in the future, Release 4.1 Jul 27, 2018
@marmarek
Copy link
Member

The feature described in the original post can be implemented by purging no longer allowed entries from conntrack. We do that already when dynamically changing netvm: https://github.com/QubesOS/qubes-core-agent-linux/blob/master/network/vif-route-qubes#L50, so that code can be used as an inspiration.

mati7337 added a commit to mati7337/qubes-core-agent-linux that referenced this issue Dec 12, 2022
Previously adding a firewall rule didn't close already established connections:
QubesOS/qubes-issues#4141
mati7337 added a commit to mati7337/qubes-core-agent-linux that referenced this issue Dec 14, 2022
Previously adding a firewall rule didn't close already established connections:
QubesOS/qubes-issues#4141
@andrewdavidwong andrewdavidwong removed this from the Release 4.2 milestone Aug 13, 2023
@andrewdavidwong andrewdavidwong added the P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. label Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: manager/widget help wanted This issue will probably not get done in a timely fashion without help from community contributors. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. ux User experience
Projects
None yet
Development

No branches or pull requests

5 participants