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

firewall rules should be removed for a VM that is shutdown #164

Closed
marmarek opened this Issue Mar 8, 2015 · 11 comments

Comments

Projects
None yet
2 participants
@marmarek
Member

marmarek commented Mar 8, 2015

Reported by joanna on 29 Mar 2011 14:32 UTC
Currently the iptables rules remain enforced after I shutdown the VM for which they were created. This might get us into troubles when the user start another VM that will be assigned the same xid (so same vif interface in proxyvm).

Migrated-From: https://wiki.qubes-os.org/ticket/164

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Modified by joanna on 31 Mar 2011 10:17 UTC

Member

marmarek commented Mar 8, 2015

Modified by joanna on 31 Mar 2011 10:17 UTC

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by rafal on 31 Mar 2011 10:54 UTC
xid should never be reused. It increases for each created domain.
In fact, in a few places we rely on it.
Anyway, fw rules for nonexistent vif just waste resources, and should be removed. The tiny issue is how to do this.
The only relevant piece of code that will be triggered when a domain is shutdown, is the hotplug script
/etc/xen/scripts/vif-route-qubes. Correct me if I am wrong.
This script could parse fw rules and remove any that include -i vifX.0. But this is ugly.
The better way would be to change write_iptables_xenstore_entry() so that instead adding every rule directly to the FORWARD chain, it would create a chain RULES_FOR_VIFX.0. BTW, it would make the rules more readable. Then, when vifX.0 goes down, vif-route-qubes could just delete this chain and the reference to it in FORWARD - this would be constant command.
Let me know if this is correct.

Member

marmarek commented Mar 8, 2015

Comment by rafal on 31 Mar 2011 10:54 UTC
xid should never be reused. It increases for each created domain.
In fact, in a few places we rely on it.
Anyway, fw rules for nonexistent vif just waste resources, and should be removed. The tiny issue is how to do this.
The only relevant piece of code that will be triggered when a domain is shutdown, is the hotplug script
/etc/xen/scripts/vif-route-qubes. Correct me if I am wrong.
This script could parse fw rules and remove any that include -i vifX.0. But this is ugly.
The better way would be to change write_iptables_xenstore_entry() so that instead adding every rule directly to the FORWARD chain, it would create a chain RULES_FOR_VIFX.0. BTW, it would make the rules more readable. Then, when vifX.0 goes down, vif-route-qubes could just delete this chain and the reference to it in FORWARD - this would be constant command.
Let me know if this is correct.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by rafal on 31 Mar 2011 10:57 UTC
One more thing - would the rules be automatically corrected after any domain start ? It seems so; write_iptables_xenstore_entry() rebuilds each rules. In such case, I would not touch it at all.

Member

marmarek commented Mar 8, 2015

Comment by rafal on 31 Mar 2011 10:57 UTC
One more thing - would the rules be automatically corrected after any domain start ? It seems so; write_iptables_xenstore_entry() rebuilds each rules. In such case, I would not touch it at all.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by smoku on 31 Mar 2011 11:08 UTC
Yes. That's the idea - after any domain start ALL the rules are rebuilt, so things fix automagically.
If we care about rules for non existant domains, we may add the rebuild to the domain shutdown event too.

Member

marmarek commented Mar 8, 2015

Comment by smoku on 31 Mar 2011 11:08 UTC
Yes. That's the idea - after any domain start ALL the rules are rebuilt, so things fix automagically.
If we care about rules for non existant domains, we may add the rebuild to the domain shutdown event too.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by rafal on 31 Mar 2011 11:28 UTC
There is no spoon Ctrl-W "domain shutdown event".
Thus, I would simply let these stale rules remain until a new domain is started.

Member

marmarek commented Mar 8, 2015

Comment by rafal on 31 Mar 2011 11:28 UTC
There is no spoon Ctrl-W "domain shutdown event".
Thus, I would simply let these stale rules remain until a new domain is started.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by joanna on 31 Mar 2011 11:38 UTC

There is no spoon Ctrl-W

Gee, what editor are you using? ;)

Thus, I would simply let these stale rules remain until a new domain is started.
But they are not disappearing, are they?

Member

marmarek commented Mar 8, 2015

Comment by joanna on 31 Mar 2011 11:38 UTC

There is no spoon Ctrl-W

Gee, what editor are you using? ;)

Thus, I would simply let these stale rules remain until a new domain is started.
But they are not disappearing, are they?

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by smoku on 31 Mar 2011 14:21 UTC
Replying to rafal:

There is no spoon Ctrl-W "domain shutdown event".

Yet qubes-manager is able to "gray out" domains that stopped working.

Replying to joanna:

But they are not disappearing, are they?

They will disappear once any other domain is started.

Member

marmarek commented Mar 8, 2015

Comment by smoku on 31 Mar 2011 14:21 UTC
Replying to rafal:

There is no spoon Ctrl-W "domain shutdown event".

Yet qubes-manager is able to "gray out" domains that stopped working.

Replying to joanna:

But they are not disappearing, are they?

They will disappear once any other domain is started.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by joanna on 31 Mar 2011 14:25 UTC
We use polling in qubes-manager...

Member

marmarek commented Mar 8, 2015

Comment by joanna on 31 Mar 2011 14:25 UTC
We use polling in qubes-manager...

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by joanna on 3 Apr 2011 09:23 UTC
#171 might be a side effect of this, especially when user starts and stops VMs frequently. For this reason I increase the priority of this ticket.

Member

marmarek commented Mar 8, 2015

Comment by joanna on 3 Apr 2011 09:23 UTC
#171 might be a side effect of this, especially when user starts and stops VMs frequently. For this reason I increase the priority of this ticket.

@marmarek marmarek added P: critical and removed P: major labels Mar 8, 2015

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Modified by joanna on 3 Apr 2011 23:24 UTC

Member

marmarek commented Mar 8, 2015

Modified by joanna on 3 Apr 2011 23:24 UTC

@marmarek marmarek self-assigned this Mar 8, 2015

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 8, 2015

Member

Comment by rafal on 4 Apr 2011 15:31 UTC
joanna> But they are not disappearing, are they?
smoku> They will disappear once any other domain is started.

Works for me - as expected, the rules disappear after any domain start, which is good enough; isn't it ?

If you still experience the problem, please provide detailed reproduction scenario.

Member

marmarek commented Mar 8, 2015

Comment by rafal on 4 Apr 2011 15:31 UTC
joanna> But they are not disappearing, are they?
smoku> They will disappear once any other domain is started.

Works for me - as expected, the rules disappear after any domain start, which is good enough; isn't it ?

If you still experience the problem, please provide detailed reproduction scenario.

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