tasket / Qubes-vpn-support

VPN configuration in Qubes OS
GNU General Public License v3.0
126 stars 28 forks source link

unsafe firewall loading mechanism risk of leaks #67

Closed adrelanos closed 1 year ago

adrelanos commented 1 year ago

I don't think /rw/config/rc.local is suitable. Networking might be up and functional before rc.local is done. That could lead to a leak.

The safe way do to would be something like netfilter-persistent. Scripts that run before networking comes up. The firewall rules need to be load before networking comes up. And if the firewall script fails, networking should not come up.

related: https://github.com/QubesOS/qubes-issues/issues/7801

tasket commented 1 year ago

IIRC I inquired about this with Marek in a Qubes issue years ago, and the answer was its considered safe because traffic (forwarding?) isn't enabled until the related qubes service has finished startup. Though it would be a good idea to check it again.

tasket commented 1 year ago

Also note the firewall rules are not enabled from rc.local (this only starts the vpn) and the the qubes-vpn-handler.service keeps disables forwarding if it sees the fw script hasn't run. EDIT: the service setup script refrains from enabling forwarding in this case.

tasket commented 1 year ago

@adrelanos I've looked at the status of '/proc/sys/net/ipv4/ip_forward' from inside the '/rw/config/qubes-firewall.d/90_tunnel-restrict' script and it is turned off. Although it was my understanding that networking in general is off until the firewall service finishes.

tasket commented 1 year ago

Per comment in https://github.com/QubesOS/qubes-issues/issues/7801

For reference, refer to qubes-firewall.service which has dependency Before=qubes-network.service and runs the code in '/lib/python3/dist-packages/qubesagent/firewall.py'. Within main() the two firewall execution handlers are called synchronously before sending a 'READY' signal to systemd.

This should take care of the issue, at least on this end. The qubes-vpn-handler.service script does change _ipforward setting, but only as a stop-gap if the Qubes service somehow malfunctions.

adrelanos commented 1 year ago

Would it be possible to tell users in documentation not to enable "provides network" and handle enabling IP forwarding in qubes-vpn-support scripts when appropriate?

tasket commented 1 year ago

I don't think there is any scripting an outside project can do to affect this; even setting up another service very early in the vm startup process will kick the can further down the road of "did user properly install it; is it really running?". I have made some suggestions in the Qubes issue.