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

Qubes Manager: Firewall Rules: Temporarily allowing full access does not revoke it after the time runs out #1173

Closed
3hhh opened this Issue Sep 6, 2015 · 20 comments

Comments

Projects
None yet
6 participants
@3hhh

3hhh commented Sep 6, 2015

How to re-produce:

  1. Create a VM_A with firewall settings "Deny all except... ", add some valid network connection.
  2. Start VM_A.
  3. Try to ping some host from inside VM_A, it won't work.
  4. Open Qubes Manager, right click on the running VM_A, select "Edit VM firewall rules" and allow full access for 1 minute.
  5. Run date inside VM_A and keep pinging the previously pinged host. It'll work.
  6. Wait for a minute.
  7. The ping will continue to work. <-- Not expected. This shouldn't work as you only allowed full access for one minute.
@3hhh

This comment has been minimized.

Show comment
Hide comment
@3hhh

3hhh Sep 6, 2015

tested under R3
debian template VM kernel: 3.12.23-1.pvops.qubes.x86_64
dom0 kernel: 3.18.10-2.pvops.qubes.x86_64, Xen 4.4.2

3hhh commented Sep 6, 2015

tested under R3
debian template VM kernel: 3.12.23-1.pvops.qubes.x86_64
dom0 kernel: 3.18.10-2.pvops.qubes.x86_64, Xen 4.4.2

@unman

This comment has been minimized.

Show comment
Hide comment
@unman

unman Sep 7, 2015

Member

If you look at iptables in the fw, you'll see that the FORWARD chain allows RELATED/ESTABLISHED traffic before the individual VM rules are set.
ESTABLISHED means that a packet is associated with a connection which has seen packets in both directions. RELATED means a new connection associated with an existing one.
After you allow full access, a FORWARD rule is set allowing traffic from VM_A: the first packet will be allowed outbound under this rule, but all subsequent traffic is allowed under the ESTABLISHED rule. Because you are continuously pinging, the traffic is allowed, even after the time limit expires and the permissive rule is deleted from the chain.

This isn't, I think, documented at all, but it's standard firewall behaviour, not to kill existing connections. In most cases this is just the behavior you want. For example, you want to download a file, and open the firewall for 5 mins: even if the transfer takes 20 minutes, you still get the file. No new connections would have been allowed after the 5 minutes expired.

Member

unman commented Sep 7, 2015

If you look at iptables in the fw, you'll see that the FORWARD chain allows RELATED/ESTABLISHED traffic before the individual VM rules are set.
ESTABLISHED means that a packet is associated with a connection which has seen packets in both directions. RELATED means a new connection associated with an existing one.
After you allow full access, a FORWARD rule is set allowing traffic from VM_A: the first packet will be allowed outbound under this rule, but all subsequent traffic is allowed under the ESTABLISHED rule. Because you are continuously pinging, the traffic is allowed, even after the time limit expires and the permissive rule is deleted from the chain.

This isn't, I think, documented at all, but it's standard firewall behaviour, not to kill existing connections. In most cases this is just the behavior you want. For example, you want to download a file, and open the firewall for 5 mins: even if the transfer takes 20 minutes, you still get the file. No new connections would have been allowed after the 5 minutes expired.

@3hhh

This comment has been minimized.

Show comment
Hide comment
@3hhh

3hhh Sep 7, 2015

@unman: No, it's not the ESTABLISHED or RELATED rules.

You can also check with "wget somesite", allow full access, check "wget somesite" again (it then works) and check "wget someOtherSite" after the time window expired. You'll be able to retrieve someOtherSite.

It's because the resepctive rule in the FORWARD chain is not removed, i.e. it's definitely a bug.

I can still see
21 1202 ACCEPT all -- any any 10.137.X.X anywhere
in the firewall VM iptables FORWARD chain after waiting for one minute. Might be because I use a debian firewall VM though.

3hhh commented Sep 7, 2015

@unman: No, it's not the ESTABLISHED or RELATED rules.

You can also check with "wget somesite", allow full access, check "wget somesite" again (it then works) and check "wget someOtherSite" after the time window expired. You'll be able to retrieve someOtherSite.

It's because the resepctive rule in the FORWARD chain is not removed, i.e. it's definitely a bug.

I can still see
21 1202 ACCEPT all -- any any 10.137.X.X anywhere
in the firewall VM iptables FORWARD chain after waiting for one minute. Might be because I use a debian firewall VM though.

@unman

This comment has been minimized.

Show comment
Hide comment
@unman

unman Sep 7, 2015

Member

You're absolutely right. Despite my plausible explanation,(which DOES apply to fedora based firewalls), the debian fw doesn't reset the permissive rule.

Member

unman commented Sep 7, 2015

You're absolutely right. Despite my plausible explanation,(which DOES apply to fedora based firewalls), the debian fw doesn't reset the permissive rule.

@unman

This comment has been minimized.

Show comment
Hide comment
@unman

unman Sep 7, 2015

Member

This also affects the fedora templates - the permissive rule doesn't get deleted, and the firewall remains wide open.
I see the qubes-reload-firewall service in state "elapsed" after the timer ends, but it isn't triggering reload of the rules.

Member

unman commented Sep 7, 2015

This also affects the fedora templates - the permissive rule doesn't get deleted, and the firewall remains wide open.
I see the qubes-reload-firewall service in state "elapsed" after the timer ends, but it isn't triggering reload of the rules.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Sep 7, 2015

Member

On Mon, Sep 07, 2015 at 01:39:15PM -0700, unman wrote:

This also affects the fedora templates - the permissive rule doesn't get deleted, and the firewall remains wide open.
I see the qubes-reload-firewall service in state "elapsed" after the timer ends, but it isn't triggering reload of the rules.

Actually reload is triggered (you can see this in firewallvm logs), but
with the "allow all" rule still present.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Member

marmarek commented Sep 7, 2015

On Mon, Sep 07, 2015 at 01:39:15PM -0700, unman wrote:

This also affects the fedora templates - the permissive rule doesn't get deleted, and the firewall remains wide open.
I see the qubes-reload-firewall service in state "elapsed" after the timer ends, but it isn't triggering reload of the rules.

Actually reload is triggered (you can see this in firewallvm logs), but
with the "allow all" rule still present.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

@unman

This comment has been minimized.

Show comment
Hide comment
@unman

unman Sep 7, 2015

Member

Well, it's intermittent. On my test machine most of the time the rule gets deleted, but when it doesn't, as far as I can see I don't see the reload being triggered, the service stays in the "elapsed" state, and I don't see any entry in the logs.

Member

unman commented Sep 7, 2015

Well, it's intermittent. On my test machine most of the time the rule gets deleted, but when it doesn't, as far as I can see I don't see the reload being triggered, the service stays in the "elapsed" state, and I don't see any entry in the logs.

@unman

This comment has been minimized.

Show comment
Hide comment
@unman

unman Apr 16, 2017

Member

@andrewdavidwong Confirmed this issue still arises in 3.2 milestone

Member

unman commented Apr 16, 2017

@andrewdavidwong Confirmed this issue still arises in 3.2 milestone

@donob4n

This comment has been minimized.

Show comment
Hide comment
@donob4n

donob4n Jan 22, 2018

Hi, I was investigating this issue and found this opened ticket. As @unman said I had it on elapsed state when I've noticed the problem. I did some tests and all failed and the timer still elapsed. Then I stopped it manually and tested again. Since this, it seems running fine and after the timer fires it goes to inactive state.

I found this on https://www.freedesktop.org/software/systemd/man/systemd.timer.html#Description

RemainAfterElapse
Takes a boolean argument. If true, an elapsed timer will stay loaded, and its state remains queriable. If false, an elapsed timer unit that cannot elapse anymore is unloaded. Turning this off is particularly useful for transient timer units that shall disappear after they first elapse. Note that this setting has an effect on repeatedly starting a timer unit that only elapses once: if RemainAfterElapse= is on, it will not be started again, and is guaranteed to elapse only once. However, if RemainAfterElapse= is off, it might be started again if it is already elapsed, and thus be triggered multiple times. Defaults to yes.

I'm gonna try disabling it and check if I don't have this problem again.

donob4n commented Jan 22, 2018

Hi, I was investigating this issue and found this opened ticket. As @unman said I had it on elapsed state when I've noticed the problem. I did some tests and all failed and the timer still elapsed. Then I stopped it manually and tested again. Since this, it seems running fine and after the timer fires it goes to inactive state.

I found this on https://www.freedesktop.org/software/systemd/man/systemd.timer.html#Description

RemainAfterElapse
Takes a boolean argument. If true, an elapsed timer will stay loaded, and its state remains queriable. If false, an elapsed timer unit that cannot elapse anymore is unloaded. Turning this off is particularly useful for transient timer units that shall disappear after they first elapse. Note that this setting has an effect on repeatedly starting a timer unit that only elapses once: if RemainAfterElapse= is on, it will not be started again, and is guaranteed to elapse only once. However, if RemainAfterElapse= is off, it might be started again if it is already elapsed, and thus be triggered multiple times. Defaults to yes.

I'm gonna try disabling it and check if I don't have this problem again.

@donob4n

This comment has been minimized.

Show comment
Hide comment
@donob4n

donob4n Jan 23, 2018

Ouch

After doing it I'm getting an "Unknown lvalue 'RemainAfterElapse'"

"man systemd.timer" doesn't mention the option so probably is not available in fedora-23, I didn't test it on another OS.

After playing a little it get stuck on elapsed state again and it started to fail again. I manually stopped it and start to work fine again (except I noticed it's pretty inaccurate, for 1min it took near 2min for stop).

While running ok it goes dead after deleting the rules. I will look for another way to avoid the elapsed status.

donob4n commented Jan 23, 2018

Ouch

After doing it I'm getting an "Unknown lvalue 'RemainAfterElapse'"

"man systemd.timer" doesn't mention the option so probably is not available in fedora-23, I didn't test it on another OS.

After playing a little it get stuck on elapsed state again and it started to fail again. I manually stopped it and start to work fine again (except I noticed it's pretty inaccurate, for 1min it took near 2min for stop).

While running ok it goes dead after deleting the rules. I will look for another way to avoid the elapsed status.

@donob4n

This comment has been minimized.

Show comment
Hide comment
@donob4n

donob4n Jan 23, 2018

Ok I found the problem,

I am not sure if it's the expected behaviour of systemd, but with "OnActiveSec=1m" it's only activating the reload service once. Then it's stopped if the full access expired (because the service stops it), or it goes elapsed (forever?). Testing seems working because we usually use 1min which is the timer expiration after enabling it, and luckily it removes the rule on the first activation.

I've added "OnUnitActiveSec=1m" and it seems to do the expected behaviour, it activates the service each 60seconds until the full access expires and the service stops the timer.

If someone wants to test it add OnUnitActiveSec=1m on /usr/lib/systemd/system/qubes-reload-firewall@.timer

donob4n commented Jan 23, 2018

Ok I found the problem,

I am not sure if it's the expected behaviour of systemd, but with "OnActiveSec=1m" it's only activating the reload service once. Then it's stopped if the full access expired (because the service stops it), or it goes elapsed (forever?). Testing seems working because we usually use 1min which is the timer expiration after enabling it, and luckily it removes the rule on the first activation.

I've added "OnUnitActiveSec=1m" and it seems to do the expected behaviour, it activates the service each 60seconds until the full access expires and the service stops the timer.

If someone wants to test it add OnUnitActiveSec=1m on /usr/lib/systemd/system/qubes-reload-firewall@.timer

@3hhh

This comment has been minimized.

Show comment
Hide comment
@3hhh

3hhh Feb 4, 2018

Some further discussion: https://groups.google.com/forum/#!topic/qubes-users/SFZrJxG46NY

Key points:

  • In 3.2 there seems to be a 50:50 chance for the feature to work correctly.

  • The issue is also present in 4.0. There both the GUI and qvm-firewall list will show that the temporary allow all rule was removed, but the VM still has full internet access.

From my point of view it would make sense to remove the feature from both 3.2 and 4.0 entirely if it's not being fixed as it simply doesn't do what it advertises. Users can do "allow all" by implementing a respective rule themselves.

3hhh commented Feb 4, 2018

Some further discussion: https://groups.google.com/forum/#!topic/qubes-users/SFZrJxG46NY

Key points:

  • In 3.2 there seems to be a 50:50 chance for the feature to work correctly.

  • The issue is also present in 4.0. There both the GUI and qvm-firewall list will show that the temporary allow all rule was removed, but the VM still has full internet access.

From my point of view it would make sense to remove the feature from both 3.2 and 4.0 entirely if it's not being fixed as it simply doesn't do what it advertises. Users can do "allow all" by implementing a respective rule themselves.

@donob4n

This comment has been minimized.

Show comment
Hide comment
@donob4n

donob4n Feb 4, 2018

@3hhh have you tested it with "OnUnitActiveSec=1m" ? I hadn't get any failure since I've added it.

donob4n commented Feb 4, 2018

@3hhh have you tested it with "OnUnitActiveSec=1m" ? I hadn't get any failure since I've added it.

@3hhh

This comment has been minimized.

Show comment
Hide comment
@3hhh

3hhh Feb 6, 2018

I tested that in 4.0 as well, yes, but the timer in 4.0 triggers regardless of that setting (I could verify that from journalctl as you suggested).

The problem in 4.0 is simply that the iptables ACCEPT all rule in sys-firewall or the proxy VM is not removed after the timer triggers - the ruleset looks exactly identical during and after the timer runs out.

So probably the changes from qvm-firewall are not propagated somewhere (qvm-firewall changes, but iptables not).

Btw 3hhh = tripleh.

3hhh commented Feb 6, 2018

I tested that in 4.0 as well, yes, but the timer in 4.0 triggers regardless of that setting (I could verify that from journalctl as you suggested).

The problem in 4.0 is simply that the iptables ACCEPT all rule in sys-firewall or the proxy VM is not removed after the timer triggers - the ruleset looks exactly identical during and after the timer runs out.

So probably the changes from qvm-firewall are not propagated somewhere (qvm-firewall changes, but iptables not).

Btw 3hhh = tripleh.

@donob4n

This comment has been minimized.

Show comment
Hide comment
@donob4n

donob4n Feb 6, 2018

I am testing on Qubes 4.0 rc-4

The are more problems here, qubes-reload-firewall service execs:
qvm-firewall VmName
but in Qubes 4 this prints nothing, it should be:
qvm-firewall VmName list

also it tries to grep 'expires at' and the full access rule has not any 'expires at' comment or message.

This is totally broken in Qubes 4 but I think it should be easy to fix.

@marmarek could you take a look on this? What do you think about avoiding systemd for this and using:

sleep X && reload rules

or this other way for avoid problems with suspend/resume:

while true
if (expiration done)
reload rules
exit
else
sleep X

I don't see too much benefit from using sytemd here.

donob4n commented Feb 6, 2018

I am testing on Qubes 4.0 rc-4

The are more problems here, qubes-reload-firewall service execs:
qvm-firewall VmName
but in Qubes 4 this prints nothing, it should be:
qvm-firewall VmName list

also it tries to grep 'expires at' and the full access rule has not any 'expires at' comment or message.

This is totally broken in Qubes 4 but I think it should be easy to fix.

@marmarek could you take a look on this? What do you think about avoiding systemd for this and using:

sleep X && reload rules

or this other way for avoid problems with suspend/resume:

while true
if (expiration done)
reload rules
exit
else
sleep X

I don't see too much benefit from using sytemd here.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Feb 6, 2018

Member

@marmarek could you take a look on this? What do you think about avoiding systemd for this and using:

There were no long-running Qubes specific daemon in R3.2, that's why systemd is used. In R4.0 there is qubesd, which can handle this similarly to what you propose.

Member

marmarek commented Feb 6, 2018

@marmarek could you take a look on this? What do you think about avoiding systemd for this and using:

There were no long-running Qubes specific daemon in R3.2, that's why systemd is used. In R4.0 there is qubesd, which can handle this similarly to what you propose.

marmarek added a commit to QubesOS/qubes-core-admin that referenced this issue Feb 6, 2018

@donob4n

This comment has been minimized.

Show comment
Hide comment
@donob4n

donob4n Feb 6, 2018

Well, in Qubes 3.2 it's fixed just adding "OnUnitActiveSec=1m" to the timer.

For Qubes 4.0 it needs some rewrite, "qvm-firewall vm list" should tell that the rule has expiration time.

I don't know about qubesd, but why to use a daemon for this? We just need to call "qvm-firewall --reload" after the full access time expired. I would change this:
https://github.com/QubesOS/qubes-core-admin/blob/682d9503ee5f2cacca55eb09bb742d10472cdc34/qubes/firewall.py#L581
with some of the methods (sleep x && reload or while true....) and remove both qubes-reload-firewall service and timer (I don't see them used for another purpose).

Since "qvm-firewall --reload" always will set the rules to a right status, there is no problem if a user sets the full access before another ended, and current code of firewall settings should remain working fine.

If you like the solution I will test and do a pull request.

donob4n commented Feb 6, 2018

Well, in Qubes 3.2 it's fixed just adding "OnUnitActiveSec=1m" to the timer.

For Qubes 4.0 it needs some rewrite, "qvm-firewall vm list" should tell that the rule has expiration time.

I don't know about qubesd, but why to use a daemon for this? We just need to call "qvm-firewall --reload" after the full access time expired. I would change this:
https://github.com/QubesOS/qubes-core-admin/blob/682d9503ee5f2cacca55eb09bb742d10472cdc34/qubes/firewall.py#L581
with some of the methods (sleep x && reload or while true....) and remove both qubes-reload-firewall service and timer (I don't see them used for another purpose).

Since "qvm-firewall --reload" always will set the rules to a right status, there is no problem if a user sets the full access before another ended, and current code of firewall settings should remain working fine.

If you like the solution I will test and do a pull request.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Feb 6, 2018

Member

That "sleep x && and reload" needs to live in some process. We definitely don't want to hold qvm-firewall adding a rule (or any other CLI/GUI tool doing that) until the time expire.
But the line you've pointed is exactly the place that should be modified. And yes, qubes-reload-firewall service and timer should be gone in R4.0. I'm on it.

Member

marmarek commented Feb 6, 2018

That "sleep x && and reload" needs to live in some process. We definitely don't want to hold qvm-firewall adding a rule (or any other CLI/GUI tool doing that) until the time expire.
But the line you've pointed is exactly the place that should be modified. And yes, qubes-reload-firewall service and timer should be gone in R4.0. I'm on it.

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Feb 7, 2018

firewall: use asyncio's call_later instead of systemd to reload rules
When some expiring rules are present, it is necessary to reload firewall
when those rules expire. Previously systemd timer was used to trigger
this action, but since we have own daemon now, it isn't necessary
anymore - use this daemon for that.
Additionally automatically removing expired rules was completely broken
in R4.0.

Fixes QubesOS/qubes-issues#1173

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Feb 7, 2018

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Feb 7, 2018

@marmarek marmarek referenced this issue in QubesOS/qubes-core-admin Feb 7, 2018

Merged

Fix removing expired firewall rules #193

@qubesos-bot

This comment has been minimized.

Show comment
Hide comment
@qubesos-bot

qubesos-bot Mar 4, 2018

Automated announcement from builder-github

The package qubes-core-dom0-4.0.24-1.fc25 has been pushed to the r4.0 testing repository for dom0.
To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update

Automated announcement from builder-github

The package qubes-core-dom0-4.0.24-1.fc25 has been pushed to the r4.0 testing repository for dom0.
To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update

@qubesos-bot qubesos-bot referenced this issue in QubesOS/updates-status Mar 4, 2018

Closed

core-admin v4.0.24 (r4.0) #451

@qubesos-bot

This comment has been minimized.

Show comment
Hide comment
@qubesos-bot

qubesos-bot Mar 12, 2018

Automated announcement from builder-github

The package qubes-core-dom0-4.0.24-1.fc25 has been pushed to the r4.0 stable repository for dom0.
To install this update, please use the standard update command:

sudo qubes-dom0-update

Or update dom0 via Qubes Manager.

Changes included in this update

Automated announcement from builder-github

The package qubes-core-dom0-4.0.24-1.fc25 has been pushed to the r4.0 stable repository for dom0.
To install this update, please use the standard update command:

sudo qubes-dom0-update

Or update dom0 via Qubes Manager.

Changes included in this update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment