Possible issue with "ip_source interface" with dual stack interfaces

[Monviech]

Hello,

I have posted this in the caddy.community before opening this issue: https://caddy.community/t/dynamic-dns-module-question-about-domain-configuration/22291/4

The gist of it is, when the ip_source interface has been set as IP source, it will read any assigned IP address from that interface.

IPv6 interfaces have a mandatory link local address (LLA), they may also have a unique local address (ULA), and also a global unicast address (GUA).

If that IP interface then has an additional IPv4 address in the RFC1918 space, e.g. 192.168.0.0/16, 172.16.0.0/12, 10.0.0.0/8, 127.0.0.0/8, it will also be read and processed.

As an example, take this interface:

hn1: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
        description: WAN (wan)
        options=80018<VLAN_MTU,VLAN_HWTAGGING,LINKSTATE>
        ether 00:15:5d:00:c9:8c
        inet 172.16.0.199 netmask 0xffffff00 broadcast 172.16.0.255
        inet6 fe80::215:5dff:fe00:c98c%hn1 prefixlen 64 scopeid 0x6
        inet6 2003:a:1704:63aa:215:5dff:fe00:c98c prefixlen 64 autoconf
        media: Ethernet autoselect (10Gbase-T <full-duplex>)
        status: active
        nd6 options=23<PERFORMNUD,ACCEPT_RTADV,AUTO_LINKLOCAL>

This is the Caddyfile configuration (just a test setup to trigger the logs):

{
        storage file_system {
                root /usr/local/etc/caddy
        }
        log {
                output net unixgram//var/caddy/var/run/log {
                }
                format json {
                        time_format rfc3339
                }
        }

        servers {
                trusted_proxies static 192.168.1.1/32
                log_credentials
        }

        dynamic_dns {
                provider cloudflare sfdfsdfsdfs
                domains {
                        example.net @
                }
                ip_source interface hn1
                check_interval 5m
                ttl 1h
        }

        import /usr/local/etc/caddy/caddy.d/*.global
}

example.net {
        abort
}

These are the emitted logs:

2024-01-11T21:05:46 Informational   caddy   "info","ts":"2024-01-11T21:05:46Z","logger":"dynamic_dns","msg":"updating DNS record","zone":"example.net","type":"AAAA","name":"@","value":"2003:a:1704:63aa:215:5dff:fe00:c98c","ttl":3600}   
2024-01-11T21:05:46 Informational   caddy   "info","ts":"2024-01-11T21:05:46Z","logger":"dynamic_dns","msg":"updating DNS record","zone":"example.net","type":"AAAA","name":"@","value":"fe80::215:5dff:fe00:c98c","ttl":3600}  
2024-01-11T21:05:46 Informational   caddy   "info","ts":"2024-01-11T21:05:46Z","logger":"dynamic_dns","msg":"updating DNS record","zone":"example.net","type":"A","name":"@","value":"172.16.0.199","ttl":3600}

Expected behavior: IPv4 RFC1918 addresses and IPv6 non-GUA addresses should be ignored by default.

Logged behavior: An AAAA record request for the IPv6 link local address (LLA) fe80::215:5dff:fe00:c98c and also an A record request for the IPv4 RFC1918 172.16.0.199 has been issued additionally to the GUA AAAA-Record request.

Suggestion: Maybe there can be an additional option that will filter these events from happening. Like include_private_ips on and include_private_ips off, where off is the default since it would be the expected behavior.

Thank you a lot :)

[jm355]

I'm also running into this. Here's a script that gets only one ipv6 address, for me it gets the temporary privacy address, which is the desired behavior for me (it's also the address return by https://api64.ipify.org). It would be cool to see a ip_source script option, or have a script like this built in.

#!/bin/dash
/sbin/ip -6 addr | grep inet6 | awk -F '[ \t]+|/' '{print $3}' | grep -v ^::1 | grep -v ^fe80 | grep -v ^fd5e | grep -v "<postfix of my permanent address>$" | head -1

[mholt]

Thanks for this issue -- you're both right, we should probably manage that in this code. Can someone submit a PR? My plate is a bit full right now.

[Monviech]

@mholt Thanks for your reply. When I submitted this Issue I actually spend some time to look at the code. But for me golang is too convoluted right now to get into. I hope somebody else can fix this eventually.

[jm355]

currently the interface source appends all the ips it finds. This is fine-ish however, it doesn't check https://pkg.go.dev/net/netip#Addr.IsPrivate for ipv4, and and doesn't check https://pkg.go.dev/net/netip#Addr.IsGlobalUnicast for ipv6.

For ipv6, if there are multiple ip addresses, do we want to submit all of them? It seems like the ideal is no, especially if someone has privacy addresses enabled. It seems like a better solution would be to only return one ipv4 and one ipv6. Maybe we can just assume the first one is the one we want?

Another solution would be something like using a bash script, I'm much more familiar with bash than go so it's a lot easier for me to think of a solution there:

cmd, err := exec.Command("/bin/sh", "ip -6 addr | grep inet6 | grep global | grep -v deprecated | grep temporary | awk -F '[ \t]+|/' '{print $3}' | head -1").Output()
if err != nil {
  fmt.Printf("error %s", err)
}
if string(cmd) == "" {
  cmd, err := exec.Command("/bin/sh", "ip -6 addr | grep inet6 | grep global | grep -v deprecated | awk -F '[ \t]+|/' '{print $3}' | head -1").Output()
  if err != nil {
    fmt.Printf("error %s", err)
  }
}
output := string(cmd)
return output

(adapted from https://stackoverflow.com/questions/25834277/executing-a-bash-script-from-golang, temporary is the label my privacy address has so prioritize that, and if it doesn't exist, just use whichever global ipv6 addr it can)

[jm355]

I could pretty easily add the checks for IsPrivate and IsGlobalUnicast and make a pr for those, @mholt would you like that or would you prefer to wait until a more robust solution is found? I don't really have time to set up a way to test out changes I make, so I could submit a pr but not make sure it works right, unless it's super easy

[francislavoie]

I wrote the interface module, but I just did it cause I was bored and there was an open feature request. I don't use it myself, and I don't have any motivation to play with it for that reason. So PRs welcome if you think those improvements make sense, if users of it agree the improvements would be a positive change.

I'm not sure what you mean by not being able to test - but it's trivial to build a fork of the plugin using xcaddy, you just need to add --with github.com/mholt/caddy-dynamicdns=<path-to-your-fork>

[jm355]

Oh, good to know on adding the fork. I'm pretty sure the IsPrivate and IsGlobalUnicast checks would be net positive changes, however, do people who use a vpn or something like tailscale use ddns? If so, this might mess up their configuration

[francislavoie]

Could just change it to be smart by default, with an option to turn it off. Or make the smarts opt-in. Whatever. The interface module could take a block of options, boolean flags or w/e.

[jm355]

Not sure I understand the fork syntax. My fork's at https://github.com/jm355/caddy-dynamicdns, would I do --with github.com/mholt/caddy-dynamicdns=../../jm355/caddy-dynamicdns? (All the fork examples at https://github.com/caddyserver/xcaddy do the ../../ part)

[jm355]

I tried it a few different ways and the build always fails. I don't have my fork checked out on my server, I'm using docker compose for everything so idk where to even clone it to

[francislavoie]

The relative path syntax assumes you do have it checked out. If you don't, then use --with github.com/mholt/caddy-dynamicdns=github.com/jm355/caddy-dynamicdns@master or something like that (where the @ is a git ref, you can use a specific commit hash etc)

[Monviech]

@jm355 Thank you so much for looking into this. I appreciate it. ^^

@francislavoie Thank you for writing this, I'm offering it downstream in the opnsense plugin, on a firewall this option is very nifty. :)

[mholt]

Thank you both for working on this! :blush: