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

Add Virtual DNS for IPv6 #558

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

1cho1ce
Copy link

@1cho1ce 1cho1ce commented Oct 5, 2023

Add Virtual IPv6 DNS for ipv6-enabled qubes.
The IPv6 address for DNS is using the same logic as IPv4:
10.137.x.x - normal qubes
10.138.x.x - disposable qubes
10.139.x.x - Virtual DNS
fd09:24ef:4179::a89:x - normal qubes
fd09:24ef:4179::a8a:x - disposable qubes
fd09:24ef:4179::a8b:x - Virtual DNS

…eed to only add Virtual DNS servers to qubes that have netvm set and don't add DNS to every qube that provides network (sys-net/sys-usb/etc).
@@ -2251,8 +2251,9 @@ def create_qdb_entries(self):
str(self.gateway6))
self.untrusted_qdb.write('/qubes-netvm-netmask', str(self.netmask))

for i, addr in zip(('primary', 'secondary'), self.dns):
self.untrusted_qdb.write('/qubes-netvm-{}-dns'.format(i), addr)
if self.netvm is not None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change made you hardcode virtual DNS addresses in the other PR. If you drop this condition, you will always have them available.

'10.139.1.1',
'10.139.1.2',
))
if self.netvm is not None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was self.netvm is not None or self.provides_network specifically to have virtual DNS addresses in sys-net too. Since you use self.netvm.features in a line below, you should check for self.features in case of no netvm. This asymmetric handling of sys-net is a bit weird, I agree, but since the addresses are constant anyway, it works just fine.

Copy link
Author

@1cho1ce 1cho1ce Oct 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to check if the qube has netvm or not from inside it?
This change was mainly so I could add reject rules to sys-net here based on whatever this qube's netvm DNS servers are set. Without it I couldn't find a way to check if qube is connected to a netvm to determine if it should pass or reject the "virtual" DNS queries.

Copy link
Author

@1cho1ce 1cho1ce Oct 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note the reason as to why did I check self.netvm.features.check_with_netvm is because I wanted to pass the qube usable primary and secondary virtual DNS servers.
For example, with ipv6 feature enabled only in sys-vpn and all qubes connected to it, if I just check if sys-vpn has ipv6 feature and give it IPv6 primary and IPv4 secondary virtual DNS servers then it won't be able to use the primary IPv6 server because the sys-vpn's netvm doesn't support IPv6 so it's better to give sys-vpn two IPv4 virtual DNS from netvm that are supported by the qube's netvm.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to check if the qube has netvm or not from inside it?

I guess I should just check if /qubes-ip is set.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, with ipv6 feature enabled only in sys-vpn and all qubes connected to it, if I just check if sys-vpn has ipv6 feature and give it IPv6 primary and IPv4 secondary virtual DNS servers then it won't be able to use the primary IPv6 server because the sys-vpn's netvm doesn't support IPv6 so it's better to give sys-vpn two IPv4 virtual DNS from netvm that are supported by the qube's netvm.

That very much depends what your VPN does. If it tunnels all the traffic (IPv4 + IPv6) inside a IPv4 tunnel, then IPv6 DNS should also work. And in that case, the VPN software will set DNS in sys-vpn (in /etc/resolv.conf, or systemd-resolved) to something else than virtual DNS, so all should be fine.

We have two sets of DNS addresses in the qubesdb:

  1. /qubes-primary-dns, /qubes-secondary-dns - this is what qube itself should use (and when providing network to others - where should redirect DNS traffic to)
  2. /qubes-netvm-primary-dns, /qubes-netvm-secondary-dns - this is what addresses should be intercepted and redirected to the actual DNS (which may be the virtual one in connected netvm)

The first set in a qube should match the second set in the qube's netvm. I wouldn't be surprised if it's mixed up right now (because we had just one possibility there).

So, lets suppose setup like this: sys-net (no ipv6) -> sys-vpn (ipv6) -> some-vm (ipv6 too, because connected to sys-vpn)

  • sys-net should have both sets IPv4-only
  • sys-vpn should have first set IPv4-only, but second IPv6+IPv4
  • some-vm should have the first set IPv6+IPv4 (and no second set because it doesn't provide network to others)

In sys-vpn, if it doesn't tunnel IPv6 DNS into VPN (to some real DNS server), it should have a reject rule on IPv6 virtual DNS address, because sys-net doesn't support it. This can be done by having virtual IPv6 DNS reject rule in postrouting instead of prerouting - then if VPN redirects the IPv6 DNS to somewhere else, it will bypass the reject rule and will work; otherwise it will get rejected (and hopefully the ICMP response will be forwarded back to the client - requires testing).

Is there a way to check if the qube has netvm or not from inside it?

I guess I should just check if /qubes-ip is set.

Yes, that works.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In sys-vpn, if it doesn't tunnel IPv6 DNS into VPN (to some real DNS server), it should have a reject rule on IPv6 virtual DNS address, because sys-net doesn't support it. This can be done by having virtual IPv6 DNS reject rule in postrouting instead of prerouting - then if VPN redirects the IPv6 DNS to somewhere else, it will bypass the reject rule and will work; otherwise it will get rejected (and hopefully the ICMP response will be forwarded back to the client - requires testing).

I've decided to add jump from dnat-dns to separate chain custom-dnat-dns for VPN rules instead of adding reject rules in postrouting or forward chains for the sake of uniformity in the same way as chain input has chain custom-input and chain forward has chain custom-forward.
VPN should add its DNS redirects to custom-dnat-dns chain.

I think the PR is ready for a review now.

@marmarek
Copy link
Member

marmarek commented Oct 7, 2023

See also CI failures - both pylint and unit tests (the latter failed due to changed condition for setting dns property).
You may also want to add a test for this new case, should be quite easy based on an existing test + setting ipv6 feature.

@marmarek
Copy link
Member

marmarek commented Oct 7, 2023

And finally, this need to be guarded with some condition "if qube knows how to handle IPv6 DNS". Otherwise setting IPv6 DNS address will confuse network-related startup scripts.
I think you can use qvm-features-request framework. Basically announce supported-feature.ipv6dns=1 in https://github.com/QubesOS/qubes-core-agent-linux/blob/main/qubes-rpc/post-install.d/10-qubes-core-agent-features.sh, and check for it (features.check_with_template("supported-feature.ipv6dns", False)) here, in addition to checking for ipv6 itself.

@1cho1ce
Copy link
Author

1cho1ce commented Oct 9, 2023

And finally, this need to be guarded with some condition "if qube knows how to handle IPv6 DNS". Otherwise setting IPv6 DNS address will confuse network-related startup scripts.
I think you can use qvm-features-request framework. Basically announce supported-feature.ipv6dns=1 in https://github.com/QubesOS/qubes-core-agent-linux/blob/main/qubes-rpc/post-install.d/10-qubes-core-agent-features.sh, and check for it (features.check_with_template("supported-feature.ipv6dns", False)) here, in addition to checking for ipv6 itself.

I can't think of a proper universal way to check if just IPv6 DNS handling is supported. I think that just checking if the system supports IPv6 is enough. But in that case this IPv6 support check should be done for all IPv6 networking and not just for DNS.
Should I add supported-feature.ipv6 instead?

@marmarek
Copy link
Member

I can't think of a proper universal way to check if just IPv6 DNS handling is supported. I think that just checking if the system supports IPv6 is enough. But in that case this IPv6 support check should be done for all IPv6 networking and not just for DNS.

The point is that adding virtual IPv6 DNS without regression in case of only IPv4 real DNS requires the reject rules you add in the other repo. If appropriate reject rules are added, it shouldn't matter if real IPv6 DNS is present or not. And to know that, you just need to know if core-agent-linux is new enough in relevant qube - which can be done with the mechanism I proposed above (core-agent-linux announcing support for ipv6dns feature).

@1cho1ce
Copy link
Author

1cho1ce commented Oct 10, 2023

Oh, I got it now.
What is the proper way to check the qubes-core-agent-linux version?
I can see that it gets the qubes-agents version from the /usr/share/qubes/marker-vm file here:
https://github.com/QubesOS/qubes-core-agent-linux/blob/main/qubes-rpc/post-install.d/10-qubes-core-agent-features.sh#L5
But it has only major version (4.2) and not minor one (like 4.2.24).
Or is it enough to just check for next major version 4.3 since this feature won't be included before Qubes OS 4.3?

@marmarek
Copy link
Member

What is the proper way to check the qubes-core-agent-linux version?

The idea is not to check explicit version, but to check if the version installed there announced appropriate feature.

  1. core-agent-linux that has ipv6dns support should announce supported-feature.ipv6dns=1 (add it to https://github.com/QubesOS/qubes-core-agent-linux/blob/main/qubes-rpc/post-install.d/10-qubes-core-agent-features.sh)
  2. code here checks if supported-feature.ipv6dns was announced by the qube using features.check_with_template("supported-feature.ipv6dns", False)

@1cho1ce
Copy link
Author

1cho1ce commented Oct 10, 2023

Got it, that was obvious and I was just dumbed out.

@1cho1ce
Copy link
Author

1cho1ce commented Oct 10, 2023

PR is ready for a review along with this one QubesOS/qubes-core-agent-linux#462

UPD:
I've missed your comment above, I'll move reject rules to postrouting instead of prerouting and test it out. After this it'll be ready for a review.

@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #558 (751518b) into main (ba5efc8) will increase coverage by 0.04%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #558      +/-   ##
==========================================
+ Coverage   67.13%   67.17%   +0.04%     
==========================================
  Files          55       55              
  Lines       11125    11142      +17     
==========================================
+ Hits         7469     7485      +16     
- Misses       3656     3657       +1     
Flag Coverage Δ
unittests 67.17% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
qubes/vm/mix/net.py 73.37% <100.00%> (+0.43%) ⬆️
qubes/vm/qubesvm.py 52.06% <100.00%> (+0.31%) ⬆️

... and 1 file with indirect coverage changes

@marmarek
Copy link
Member

I'll run this (and the other PR) through openQA. For now just to test if IPv4 DNS still works properly. IPv6 DNS support will need additional test, but I'll add it later.

@qubesos-bot
Copy link

qubesos-bot commented Oct 20, 2023

OpenQA test summary

Complete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.2&build=2024040404-4.2&flavor=pull-requests

Installing updates failed, skipping the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants