Skip to content

Comments

tailscale: fix broken DNS on IPv6 only tailnets#1246

Open
nolith wants to merge 1 commit intonix-darwin:masterfrom
nolith:fix-tailscale-ipv6
Open

tailscale: fix broken DNS on IPv6 only tailnets#1246
nolith wants to merge 1 commit intonix-darwin:masterfrom
nolith:fix-tailscale-ipv6

Conversation

@nolith
Copy link

@nolith nolith commented Dec 31, 2024

When a tailnet has the disableIPv4 settings it will not deploy IPv4, resolving any ts.net address is broken because 100.100.100.100 is not reachable.

fd7a:115c:a1e0::53 is the magic dns address for IPv6 tailnets.

https://tailscale.com/kb/1337/acl-syntax#disableipv4

@Samasaur1
Copy link
Member

oops i screwed up that review a little. anyway that's a minor nit and this otherwise looks good. I grepped for 100.100.100.100 and the only other matches were in comments

@Samasaur1
Copy link
Member

ack it won't let me leave an inline comment on an unchanged part of the file, but the comment on line 36 should be updated as well:

https://github.com/LnL7/nix-darwin/blob/71a3a075e3229a7518d76636bb762aef2bcb73ac/modules/services/tailscale.nix#L36

@nolith nolith force-pushed the fix-tailscale-ipv6 branch from b712f1c to cc95d5c Compare January 1, 2025 17:14
@nolith
Copy link
Author

nolith commented Jan 1, 2025

Thank you all, I made all the suggested edit.

I'll test it tomorrow on my other machine that is using this option.

config = mkIf cfg.enable {
assertions = [{
assertion = !cfg.overrideLocalDns || config.networking.dns == [ "100.100.100.100" ];
assertion = cfg.overrideLocalDns -> (builtins.elem config.networking.dns "100.100.100.100" || builtins.elem config.networking.dns "fd7a:115c:a1e0::53");
Copy link
Member

@Enzime Enzime Jan 1, 2025

Choose a reason for hiding this comment

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

Suggested change
assertion = cfg.overrideLocalDns -> (builtins.elem config.networking.dns "100.100.100.100" || builtins.elem config.networking.dns "fd7a:115c:a1e0::53");
assertion = cfg.overrideLocalDns -> (builtins.any (x: x != "100.100.100.100" && x != "fd7a:115c:a1e0::53") config.networking.dns);

Copy link
Author

Choose a reason for hiding this comment

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

Hello, I'm not an expert of nix but I don't think this suggestion has the same behaviour of the existing implementation.

The original implementation was !cfg.overrideLocalDns || config.networking.dns == [ "100.100.100.100" ] and we are swapping it with an arrow operator.

b1 -> b2 == !b1 || b2

nix-repl> networking_dns = [ "100.100.100.100" "fd7a:115c:a1e0::53" ]
nix-repl> networking_dns ==  [ "100.100.100.100" "fd7a:115c:a1e0::53" ]
true

nix-repl> builtins.elem "100.100.100.100" networking_dns || builtins.elem "fd7a:115c:a1e0::53" networking_dns
true

nix-repl> builtins.any (x: x != "100.100.100.100" && x != "fd7a:115c:a1e0::53") networking_dns
false

nix-repl> builtins.elem "100.100.100.100" networking_dns && builtins.elem "fd7a:115c:a1e0::53" networking_dns
true

nix-repl> builtins.any (x: x != "100.100.100.100" || x != "fd7a:115c:a1e0::53") networking_dns
true

Copy link
Member

Choose a reason for hiding this comment

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

x != 5 || x != 6 is always true

The original condition we’re looking to verify is that at least one element in the list is neither 100.100.100.100 nor fd7a:115c:a1e0::53 so we want to use &&

Copy link
Author

Choose a reason for hiding this comment

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

x != 5 || x != 6 is always true

🤦

The original condition we’re looking to verify is that at least one element in the list is neither 100.100.100.100 nor fd7a:115c:a1e0::53 so we want to use &&

But this is not what the original condition was doing !cfg.overrideLocalDns || config.networking.dns == [ "100.100.100.100" ].

If we analyse cfg.overrideLocalDns -> (builtins.any (x: x != "100.100.100.100" && x != "fd7a:115c:a1e0::53") config.networking.dns):

  • when cfg.overrideLocalDns == false the assertion returns true - same as before
  • when cfg.overrideLocalDns == true we return the result of (builtins.any (x: x != "100.100.100.100" && x != "fd7a:115c:a1e0::53") config.networking.dns), but this is checking a different condition compared to config.networking.dns == [ "100.100.100.100" ]. When using magicDNS you want to set a global nameserver in the admin panel, not adding another nameserver to /etc/resolv.conf. I think the original intention of that assertion was to make sure you are not adding an extra nameserver that may result in unexpected results depending on which server is selected for the resolution.

Copy link
Author

Choose a reason for hiding this comment

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

hello @Enzime, what do you think of my last comment?

Copy link
Author

Choose a reason for hiding this comment

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

@Enzime did you had any chance to read my comment?

I'd like to close this PR and get rid of the override in my flake.

@nolith nolith force-pushed the fix-tailscale-ipv6 branch from cc95d5c to 8c73b37 Compare January 2, 2025 08:51
When a tailnet has the disableIPv4 settings it will not deploy IPv4, resolving any ts.net address is broken because 100.100.100.100 is not reachable.

https://tailscale.com/kb/1337/acl-syntax#disableipv4

Co-authored-by: Michael Hoang <Enzime@users.noreply.github.com>
Co-authored-by: Sam <30577766+Samasaur1@users.noreply.github.com>
@nolith nolith force-pushed the fix-tailscale-ipv6 branch from 8c73b37 to 801e0cb Compare March 27, 2025 14:38
@nolith
Copy link
Author

nolith commented May 21, 2025

I'm pretty sure the intersection of nix-darwing users and tailscale users with an ipv6 only tailnet is very small (likely only myself).

But dear reader, if you are like me, this is how I circumvented this bug without loading a forked flake of nix-darwin.

  # overriding DNS doesn't work with IPv6-only networks - https://github.com/nix-darwin/nix-darwin/pull/1246
  services.tailscale.overrideLocalDns = false;
  environment.etc."resolver/ts.net".text = lib.mkForce "nameserver fd7a:115c:a1e0::53";

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