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

qubes-firewall-user-script is ignored #3260

Closed
na-- opened this issue Oct 30, 2017 · 25 comments
Closed

qubes-firewall-user-script is ignored #3260

na-- opened this issue Oct 30, 2017 · 25 comments
Labels
affects-4.1 This issue affects Qubes OS 4.1. C: core C: networking diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. r4.0-buster-stable r4.0-centos7-stable r4.0-fc24-cur-test r4.0-fc25-cur-test r4.0-fc26-stable r4.0-jessie-stable r4.0-stretch-stable T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Milestone

Comments

@na--
Copy link

na-- commented Oct 30, 2017

Qubes OS version:

R 4.0 RC2

Affected TemplateVMs:

all


Steps to reproduce the behavior:

Try to set up a leak-proof VPN as described here or add some firwall rules as described here

Expected behavior:

/rw/config/qubes-firewall-user-script is called when there are changes in the rules

Actual behavior:

/rw/config/qubes-firewall-user-script is totally ignored

General notes:

I think this is the commit that killed it and I don't see any replacement that offers similar functions. Not sure if the new firewall architecture precludes something like it or not, but it seems that this is the place where such a hook should be added?


Related issues:

#1815

@andrewdavidwong andrewdavidwong added T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. C: core labels Oct 31, 2017
@andrewdavidwong andrewdavidwong added this to the Release 4.0 milestone Oct 31, 2017
@tasket
Copy link

tasket commented Oct 31, 2017

I ran into this, also.

Workaround: You can use /rw/config/qubes-ip-change-hook instead if all you need is for it to run once before forwarding is switched on.

@na--
Copy link
Author

na-- commented Oct 31, 2017

Thanks, that works, I can use /rw/config/rc.local for the first run.

@tasket
Copy link

tasket commented Nov 1, 2017

@na-- Keep in mind that rc.local isn't synchronized with qubes-firewall, so there may be a time gap between network availability and execution of a script from rc.local. Using qubes-ip-change-hook removes the gap.

@na--
Copy link
Author

na-- commented Nov 1, 2017

Oh, I totally misread your previous post, for some reason I thought qubes-ip-change-hook was executed every time the firewall settings were changed... I will use qubes-ip-change-hook instead of rc.local, but that means that there's no full substitute for qubes-firewall-user-script, right?

@tasket
Copy link

tasket commented Nov 1, 2017

IIRC @marmarek said something to the effect that 'qubes-firewall-user-script' is supposed to work (so this issue is a valid bug). But I'd say there is also a bug-like lack of clarity at this point in how the feature is expected to work, esp since the filename 'qubes-ip-change-hook' implies that it gets re-run on each change (which it currently does not). The behavior and the filename needs to be reviewed.

For the workaround, it depends on how you're trying to use the firewall. For example, the VPN doc script doesn't have to be re-run if the FORWARD chain isn't getting re-built. I've been testing it in R4rc2 and seems OK.

@marmarek
Copy link
Member

Previously qubes-firewall overwritten all the rules in FORWARD chain at each firewall change, so it was necessary to apply custom rules each time. Now qubes-firewall:

  • does not touch anything outside of QBS-FORWARD chain, especially custom rules in FORWARD chain are retained - so user changes do not need to be applied each time
  • does not reload the whole firewall, if only one set of rules have changed

How this should affect calling /rw/config/qubes-firewall-user-script? Previously a simple script with iptables -A FORWARD .... would work. Now, if that would be called after each firewall change, it would accumulate additional rules. Maybe the script should be called one time at VM boot (before enabling IP forward)?

@marmarek
Copy link
Member

Maybe the script should be called one time at VM boot (before enabling IP forward)?

Unless advised otherwise, I'm going to implement this approach.

@tasket
Copy link

tasket commented Nov 19, 2017

When I checked over a week ago, there was nothing written in python or *sh in R4 that called qubes-firewall-user-script. So there is that.

Apparently in its place is qubes-ip-change-hook. This descriptive name suggests it will run each time there is an IP address change (such as when downstream vifs are added/removed or when the upstream netvm for the vm is changed). In theory this could be really nice to have, in addition to call once before IP forward. You could even have the same script do both by passing options like update and init.

@marmarek
Copy link
Member

When I checked over a week ago, there was nothing written in python or *sh in R4 that called qubes-firewall-user-script. So there is that.

Yes, this bug is legitimate, the scripts currently is not called at all. I'm evaluating here what should be the correct fix.

This descriptive name suggests it will run each time there is an IP address change (such as when downstream vifs are added/removed or when the upstream netvm for the vm is changed).

This script is called when netvm is changed (which in Qubes 3.x caused IP change on eth0). But, in Qubes 4.0, VM's IP does no longer depend on its netvm, so the name is misleading now...

@tasket
Copy link

tasket commented Jan 28, 2018

@marmarek - Is there any update on how this will finally behave in 4.0?

People are looking for a VPN solution on Qubes 4.0 and I'd like to address that soon while also addressing some of the issues doc/vpn.md has in general.

@marmarek
Copy link
Member

See #3260 (comment) and #3260 (comment). Are you ok with this?

@tasket
Copy link

tasket commented Jan 29, 2018

@marmarek - That looks OK, similar to current behavior. But the actual qubes-firewall-user-script didn't work at startup the last time I checked; I have to use qubes-ip-change-hook instead. I also have code out there that symlinks one to the other... :)

marmarek added a commit to marmarek/qubes-core-agent-linux that referenced this issue Feb 5, 2018
Call it just after creating base chains in iptables/nftables. This allow
the user to modify how those rules are plugged in, add custom rules at
beginning/end etc.

Fixes QubesOS/qubes-issues#3260
marmarek added a commit to marmarek/qubes-core-agent-linux that referenced this issue Feb 5, 2018
@adubois
Copy link

adubois commented Feb 6, 2018

@marmarek not sure I understood your fix, a hook is required so that user can be notified when downstream system connects (new vifXX.0). This will allow rules specific to that qube to be added.
Please note that the IP is fixed but the vifXX.0 is not (between stops and starts). I will test if qubes-ip-change-hook provide the functionality I am after, if it does, it may be preferable to have it visible in /rw/config I was not aware of this hook. I may try to document the qube start-up process with the various hooks.

@marmarek
Copy link
Member

marmarek commented Feb 7, 2018

Please note that the IP is fixed but the vifXX.0 is not (between stops and starts).

That's correct. But there are already rules preventing IP spoofing (see raw table), so I recommend using IP address in custom rules. This way you can add them once and don't need to update on VM start/stop.
Note that using vifXX in custom rules is not only more inconvenient, but can easily lead to mistakes - for example HVM without PV drivers (like plain Windows) have two vif interfaces.

@adubois
Copy link

adubois commented Feb 7, 2018

OK understood. However I believe 2 hooks are required and maybe the file names should reflect on the reason for the trigger rather than the purpose of the script (i.e. after-first-qubes-firewall-update and after-each-qubes-firewall-update or whatever is the event which trigger the call on your side). From my side, I still need a hook to get notified when a new AppVM starts. I have about 30 VMs, not all started, and need the behaviour of the firewall to adapt to which "lab" (set of machines) I have running. I also have some "route add" commands in the script that would only be accepted as valid if the machine is up.

@qubesos-bot
Copy link

Automated announcement from builder-github

The package qubes-core-agent_4.0.21-1+deb10u1 has been pushed to the r4.0 testing repository for the Debian template.
To test this update, first enable the testing repository in /etc/apt/sources.list.d/qubes-*.list by uncommenting the line containing buster-testing (or appropriate equivalent for your template version), then use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package qubes-core-agent_4.0.21-1+deb9u1 has been pushed to the r4.0 testing repository for the Debian template.
To test this update, first enable the testing repository in /etc/apt/sources.list.d/qubes-*.list by uncommenting the line containing stretch-testing (or appropriate equivalent for your template version), then use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package python2-dnf-plugins-qubes-hooks-4.0.24-1.fc26) has been pushed to the r4.0 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo yum update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package qubes-core-agent_4.0.24-1+deb10u1 has been pushed to the r4.0 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package qubes-core-agent_4.0.24-1+deb9u1 has been pushed to the r4.0 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@andrewdavidwong
Copy link
Member

Reopening due to apparent regression reported in #8521.

@andrewdavidwong andrewdavidwong removed this from the Release 4.0 milestone Sep 18, 2023
@andrewdavidwong andrewdavidwong added needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. regression A bug in which a supported feature that worked in a prior Qubes OS release has stopped working. affects-4.1 This issue affects Qubes OS 4.1. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. C: networking labels Sep 18, 2023
@UndeadDevel
Copy link

This can actually be closed again; I misunderstood what this mechanism is supposed to do due to outdated community guides...I just tested and the script is called on VM start if the qubes-firewall service is enabled, so it works as it should as per marmarek's comments in this issue thread.

@andrewdavidwong andrewdavidwong added diagnosed Technical diagnosis has been performed (see issue comments). and removed needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. regression A bug in which a supported feature that worked in a prior Qubes OS release has stopped working. labels Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-4.1 This issue affects Qubes OS 4.1. C: core C: networking diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. r4.0-buster-stable r4.0-centos7-stable r4.0-fc24-cur-test r4.0-fc25-cur-test r4.0-fc26-stable r4.0-jessie-stable r4.0-stretch-stable T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Projects
None yet
Development

No branches or pull requests

7 participants