Repository: favonia/cloudflare-ddns
Pull request #484 (2023-05-17 12:14:55)

fix(provider): avoid static IPs such as 1.1.1.1
Some ISPs are still blocking 1.1.1.1, unfortunately. The new mechanism manually resolves the host `cloudflare-dns.com` as an IPv4 or IPv6 address, skipping the dual-stack switching in the Go standard library.

favonia: It _seems_ the security hardening prevents resolving the domain `one.one.one.one` to an IPv6 address after resolving it to an IPv4 address. I understand why there's such a restriction but I'm not sure how to elegantly work around it. :frowning_face:

Patch:
diff --git a/README.markdown b/README.markdown
index a0d1504..65e8040 100644
--- a/README.markdown
+++ b/README.markdown
@@ -26,7 +26,7 @@ A feature-rich and robust Cloudflare DDNS updater with a small footprint. The pr
 
 ### 🕵️ Privacy
 
-By default, public IP addresses are obtained via [Cloudflare debugging page](https://1.1.1.1/cdn-cgi/trace). This minimizes the impact on privacy because we are already using the Cloudflare API to update DNS records. Moreover, if Cloudflare servers are not reachable, chances are you cannot update DNS records anyways.
+By default, public IP addresses are obtained via [Cloudflare debugging page](https://cloudflare-dns.com/cdn-cgi/trace). This minimizes the impact on privacy because we are already using the Cloudflare API to update DNS records. Moreover, if Cloudflare servers are not reachable, chances are you cannot update DNS records anyways.
 
 ### 🛡️ Security
 
@@ -227,7 +227,7 @@ _(Click to expand the following items.)_
 > - `cloudflare.doh`\
 >   Get the public IP address by querying `whoami.cloudflare.` against [Cloudflare via DNS-over-HTTPS](https://developers.cloudflare.com/1.1.1.1/dns-over-https) and update DNS records accordingly.
 > - `cloudflare.trace`\
->   Get the public IP address by parsing the [Cloudflare debugging page](https://1.1.1.1/cdn-cgi/trace) and update DNS records accordingly.
+>   Get the public IP address by parsing the [Cloudflare debugging page](https://cloudflare-dns.com/cdn-cgi/trace) and update DNS records accordingly.
 > - `local`\
 >   Get the address via local network interfaces and update DNS records accordingly. When multiple local network interfaces or in general multiple IP addresses are present, the updater will use the address that would have been used for outbound UDP connections to Cloudflare servers. ⚠️ You need access to the host network (such as `network_mode: host` in Docker Compose) for this policy, for otherwise the updater will detect the addresses inside the [bridge network in Docker](https://docs.docker.com/network/bridge/) instead of those in the host network.
 > - `none`\
diff --git a/internal/ipnet/forceresolve.go b/internal/ipnet/forceresolve.go
new file mode 100644
index 0000000..0094fbd
--- /dev/null
+++ b/internal/ipnet/forceresolve.go
@@ -0,0 +1,80 @@
+package ipnet
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net"
+	"net/netip"
+	"net/url"
+
+	"github.com/hashicorp/go-retryablehttp"
+)
+
+var errNoIP = errors.New("no IP addresses were found")
+
+func ResolveHostname(ctx context.Context, ipNet Type, host string) (netip.Addr, error) {
+	ips, err := net.DefaultResolver.LookupNetIP(ctx, ipNet.IPNetwork(), host)
+	if err != nil {
+		return netip.Addr{}, fmt.Errorf("couldn't resolve %q: %w", host, err)
+	}
+	if len(ips) == 0 {
+		return netip.Addr{}, errNoIP
+	}
+
+	// The current Go runtime just try the list in sequence without randomization.
+	// Therefore, it is probably okay to just return the first IP address.
+	ip := ips[0]
+
+	// A temporary fix before https://go-review.googlesource.com/c/go/+/415580 fully lands
+	if ipNet == IP4 && ip.Is4In6() {
+		ip = ip.Unmap()
+	}
+
+	return ip, nil
+}
+
+func ForceResolveURLHost(ctx context.Context, ipNet Type, u *url.URL) bool {
+	hostname := u.Hostname()
+
+	// If it's already an IP address, skip the test.
+	// This does not take into account URLs such as https://0/
+	if _, err := netip.ParseAddr(hostname); err != nil {
+		// Resolve the host name.
+		ip, err := ResolveHostname(ctx, ipNet, hostname)
+		if err != nil {
+			return false
+		}
+		hostname = ip.String()
+	}
+
+	// If the port is empty, we have a few choices due to issue #14836:
+	//
+	// 1. Remove the trailing ':' after calling net.JoinHostPort
+	// 2. Reimplement net.JoinHostPort but handle the empty port specially
+	// 3. Look up the port number by the scheme
+	//
+	// The following code chooses method 2. The standard library removes
+	// the trailing ':' when net.NewRquestWithContext is called.
+	host := net.JoinHostPort(hostname, u.Port())
+	if host[len(host)-1] == ':' {
+		host = host[:len(host)-1]
+	}
+
+	// Replace the URL host with the resolved one.
+	u.Host = host
+	return true
+}
+
+func ForceResolveRetryableRequest(ctx context.Context, ipNet Type, r *retryablehttp.Request) bool {
+	originalHost := r.Host
+	if originalHost == "" {
+		originalHost = r.URL.Host
+	}
+
+	if !ForceResolveURLHost(ctx, ipNet, r.URL) {
+		return false
+	}
+	r.Host = originalHost
+	return true
+}
diff --git a/internal/ipnet/ipnet.go b/internal/ipnet/ipnet.go
index 3b6b0bd..5ca3a07 100644
--- a/internal/ipnet/ipnet.go
+++ b/internal/ipnet/ipnet.go
@@ -98,7 +98,19 @@ func (t Type) NormalizeDetectedIP(ppfmt pp.PP, ip netip.Addr) (netip.Addr, bool)
 	return ip, true
 }
 
-// UDPNetwork gives the network name for net.Dial.
+// Check IP address format.
+func (t Type) CheckIPFormat(ip netip.Addr) bool {
+	switch t {
+	case IP4:
+		return ip.Is4()
+	case IP6:
+		return ip.Is6()
+	default:
+		return false
+	}
+}
+
+// UDPNetwork gives the UDP name for net.Dial.
 func (t Type) UDPNetwork() string {
 	switch t {
 	case IP4:
@@ -109,3 +121,15 @@ func (t Type) UDPNetwork() string {
 		return ""
 	}
 }
+
+// IPNetwork gives IP network name for net.LookupNetIP.
+func (t Type) IPNetwork() string {
+	switch t {
+	case IP4:
+		return "ip4"
+	case IP6:
+		return "ip6"
+	default:
+		return ""
+	}
+}
diff --git a/internal/provider/cloudflare_doh.go b/internal/provider/cloudflare_doh.go
index ed04e6b..c2134af 100644
--- a/internal/provider/cloudflare_doh.go
+++ b/internal/provider/cloudflare_doh.go
@@ -16,8 +16,8 @@ func NewCloudflareDOH() Provider {
 			Name  string
 			Class dnsmessage.Class
 		}{
-			ipnet.IP4: {"https://1.1.1.1/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS},
-			ipnet.IP6: {"https://[2606:4700:4700::1111]/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS},
+			ipnet.IP4: {"https://cloudflare-dns.com/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS},
+			ipnet.IP6: {"https://cloudflare-dns.com/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS},
 		},
 	}
 }
diff --git a/internal/provider/cloudflare_trace.go b/internal/provider/cloudflare_trace.go
index c501528..bf43e85 100644
--- a/internal/provider/cloudflare_trace.go
+++ b/internal/provider/cloudflare_trace.go
@@ -13,8 +13,8 @@ func NewCloudflareTrace() Provider {
 			URL   string
 			Field string
 		}{
-			ipnet.IP4: {"https://1.1.1.1/cdn-cgi/trace", "ip"},
-			ipnet.IP6: {"https://[2606:4700:4700::1111]/cdn-cgi/trace", "ip"},
+			ipnet.IP4: {"https://cloudflare-dns.com/cdn-cgi/trace", "ip"},
+			ipnet.IP6: {"https://cloudflare-dns.com/cdn-cgi/trace", "ip"},
 		},
 	}
 }
diff --git a/internal/provider/protocol/doh.go b/internal/provider/protocol/doh.go
index 13bdbfb..1b672d6 100644
--- a/internal/provider/protocol/doh.go
+++ b/internal/provider/protocol/doh.go
@@ -138,7 +138,7 @@ func parseDNSResponse(ppfmt pp.PP, r []byte, id uint16, name string, class dnsme
 }
 
 func getIPFromDNS(ctx context.Context, ppfmt pp.PP,
-	url string, name string, class dnsmessage.Class,
+	ipNet ipnet.Type, url string, name string, class dnsmessage.Class,
 ) (netip.Addr, bool) {
 	var invalidIP netip.Addr
 
@@ -151,6 +151,7 @@ func getIPFromDNS(ctx context.Context, ppfmt pp.PP,
 	}
 
 	c := httpCore{
+		ipNet:       ipNet,
 		url:         url,
 		method:      http.MethodPost,
 		contentType: "application/dns-message",
@@ -187,7 +188,7 @@ func (p *DNSOverHTTPS) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type)
 		return netip.Addr{}, false
 	}
 
-	ip, ok := getIPFromDNS(ctx, ppfmt, param.URL, param.Name, param.Class)
+	ip, ok := getIPFromDNS(ctx, ppfmt, ipNet, param.URL, param.Name, param.Class)
 	if !ok {
 		return netip.Addr{}, false
 	}
diff --git a/internal/provider/protocol/field.go b/internal/provider/protocol/field.go
index f45afeb..f2a4d11 100644
--- a/internal/provider/protocol/field.go
+++ b/internal/provider/protocol/field.go
@@ -10,8 +10,9 @@ import (
 	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
-func getIPFromField(ctx context.Context, ppfmt pp.PP, url string, field string) (netip.Addr, bool) {
+func getIPFromField(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, url string, field string) (netip.Addr, bool) {
 	c := httpCore{
+		ipNet:       ipNet,
 		url:         url,
 		method:      http.MethodGet,
 		contentType: "",
@@ -61,7 +62,7 @@ func (p *Field) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type) (netip
 		return netip.Addr{}, false
 	}
 
-	ip, ok := getIPFromField(ctx, ppfmt, param.URL, param.Field)
+	ip, ok := getIPFromField(ctx, ppfmt, ipNet, param.URL, param.Field)
 	if !ok {
 		return netip.Addr{}, false
 	}
diff --git a/internal/provider/protocol/http.go b/internal/provider/protocol/http.go
index 0724c3c..2d9cf75 100644
--- a/internal/provider/protocol/http.go
+++ b/internal/provider/protocol/http.go
@@ -9,8 +9,9 @@ import (
 	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
-func getIPFromHTTP(ctx context.Context, ppfmt pp.PP, url string) (netip.Addr, bool) {
+func getIPFromHTTP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, url string) (netip.Addr, bool) {
 	c := httpCore{
+		ipNet:       ipNet,
 		url:         url,
 		method:      http.MethodGet,
 		contentType: "",
@@ -49,7 +50,7 @@ func (p *HTTP) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type) (netip.
 		return netip.Addr{}, false
 	}
 
-	ip, ok := getIPFromHTTP(ctx, ppfmt, url)
+	ip, ok := getIPFromHTTP(ctx, ppfmt, ipNet, url)
 	if !ok {
 		return netip.Addr{}, false
 	}
diff --git a/internal/provider/protocol/httpcore.go b/internal/provider/protocol/httpcore.go
index f285998..32ebb65 100644
--- a/internal/provider/protocol/httpcore.go
+++ b/internal/provider/protocol/httpcore.go
@@ -7,10 +7,12 @@ import (
 
 	"github.com/hashicorp/go-retryablehttp"
 
+	"github.com/favonia/cloudflare-ddns/internal/ipnet"
 	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
 type httpCore struct {
+	ipNet       ipnet.Type
 	url         string
 	method      string
 	contentType string
@@ -19,21 +21,28 @@ type httpCore struct {
 	extract     func(pp.PP, []byte) (netip.Addr, bool)
 }
 
-func (d *httpCore) getIP(ctx context.Context, ppfmt pp.PP) (netip.Addr, bool) {
+func (h *httpCore) getIP(ctx context.Context, ppfmt pp.PP) (netip.Addr, bool) {
 	var invalidIP netip.Addr
 
-	req, err := retryablehttp.NewRequestWithContext(ctx, d.method, d.url, d.reader)
+	req, err := retryablehttp.NewRequestWithContext(ctx, h.method, h.url, h.reader)
 	if err != nil {
-		ppfmt.Warningf(pp.EmojiImpossible, "Failed to prepare HTTP(S) request to %q: %v", d.url, err)
+		ppfmt.Warningf(pp.EmojiImpossible, "Failed to prepare HTTP(S) request to %q: %v", h.url, err)
 		return invalidIP, false
 	}
 
-	if d.contentType != "" {
-		req.Header.Set("Content-Type", d.contentType)
+	if !ipnet.ForceResolveRetryableRequest(ctx, h.ipNet, req) {
+		ppfmt.Warningf(pp.EmojiError,
+			"Failed to force resolve the host of %q as an %s address",
+			h.url, h.ipNet.Describe())
+		return invalidIP, false
+	}
+
+	if h.contentType != "" {
+		req.Header.Set("Content-Type", h.contentType)
 	}
 
-	if d.accept != "" {
-		req.Header.Set("Accept", d.accept)
+	if h.accept != "" {
+		req.Header.Set("Accept", h.accept)
 	}
 
 	c := retryablehttp.NewClient()
@@ -41,16 +50,16 @@ func (d *httpCore) getIP(ctx context.Context, ppfmt pp.PP) (netip.Addr, bool) {
 
 	resp, err := c.Do(req)
 	if err != nil {
-		ppfmt.Warningf(pp.EmojiError, "Failed to send HTTP(S) request to %q: %v", d.url, err)
+		ppfmt.Warningf(pp.EmojiError, "Failed to send HTTP(S) request to %q: %v", h.url, err)
 		return invalidIP, false
 	}
 	defer resp.Body.Close()
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		ppfmt.Warningf(pp.EmojiError, "Failed to read HTTP(S) response from %q: %v", d.url, err)
+		ppfmt.Warningf(pp.EmojiError, "Failed to read HTTP(S) response from %q: %v", h.url, err)
 		return invalidIP, false
 	}
 
-	return d.extract(ppfmt, body)
+	return h.extract(ppfmt, body)
 }


Test patch:
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
index 16b4dbf..18811ca 100644
--- a/.github/workflows/test.yaml
+++ b/.github/workflows/test.yaml
@@ -19,10 +19,12 @@ jobs:
           disable-sudo: true
           egress-policy: block
           allowed-endpoints: >
+            1dot1dot1dot1.cloudflare-ddns.com:443
             api.github.com:443
             codecov.io:443
             github.com:443
             objects.githubusercontent.com:443
+            one.one.one.one:443
             proxy.golang.org:443
             storage.googleapis.com:443
             sum.golang.org:443
diff --git a/internal/ipnet/forceresolve_test.go b/internal/ipnet/forceresolve_test.go
new file mode 100644
index 0000000..1ff0984
--- /dev/null
+++ b/internal/ipnet/forceresolve_test.go
@@ -0,0 +1,85 @@
+package ipnet_test
+
+import (
+	"context"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/favonia/cloudflare-ddns/internal/ipnet"
+)
+
+func mustURL(s string) *url.URL {
+	u, err := url.Parse(s)
+	if err != nil {
+		panic("mustURL")
+	}
+	return u
+}
+
+func TestResolveHostname(t *testing.T) {
+	t.Parallel()
+
+	check := func(ipNet ipnet.Type, hostname string, ok bool) {
+		t.Helper()
+
+		ip, err := ipnet.ResolveHostname(context.Background(), ipNet, hostname)
+		if ok {
+			require.Nil(t, err)
+			require.True(t, ipNet.CheckIPFormat(ip))
+		} else {
+			require.NotNil(t, err)
+			require.Zero(t, ip)
+		}
+	}
+
+	// The test cases are designed to work around the security hardening
+	// of GitHub Actions: an address cannot be resolved into two different
+	// IP addresses. An IPv4 address and an IPv6 one count as two addresses.
+	for name, tc := range map[string]struct {
+		hostname string
+		ipNet    ipnet.Type
+		ok       bool
+	}{
+		"one.one.one.one/4":                  {"one.one.one.one", ipnet.IP4, true},
+		"1dot1dot1dot1.cloudflare-dns.com/6": {"1dot1dot1dot1.cloudflare-dns.com", ipnet.IP6, true},
+		"1.1.1.1/4":                          {"1.1.1.1", ipnet.IP4, true},
+		"1.1.1.1/6":                          {"1.1.1.1", ipnet.IP6, false},
+		"::1/4":                              {"::1", ipnet.IP4, false},
+		"::1/6":                              {"::1", ipnet.IP6, true},
+		"..../4":                             {"....", ipnet.IP4, false},
+		"..../6":                             {"....", ipnet.IP6, false},
+	} {
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			check(tc.ipNet, tc.hostname, tc.ok)
+		})
+	}
+}
+
+func TestForceResolveURLHost(t *testing.T) {
+	t.Parallel()
+
+	for url, tc := range map[string]map[ipnet.Type][]string{
+		"https://one.one.one.one": {
+			ipnet.IP4: {"https://1.1.1.1", "https://1.0.0.1"},
+		},
+		"https://1dot1dot1dot1.cloudflare-dns.com": {
+			ipnet.IP6: {"https://[2606:4700:4700::1111]", "https://[2606:4700:4700::1001]"},
+		},
+	} {
+		url, tc := url, tc
+		t.Run(url, func(t *testing.T) {
+			t.Parallel()
+
+			for ipNet, urls := range tc {
+				u := mustURL(url)
+				ok := ipnet.ForceResolveURLHost(context.Background(), ipNet, u)
+				require.True(t, ok)
+				require.Contains(t, urls, u.String())
+			}
+		})
+	}
+}
diff --git a/internal/ipnet/ipnet_test.go b/internal/ipnet/ipnet_test.go
index 76f0a91..e482082 100644
--- a/internal/ipnet/ipnet_test.go
+++ b/internal/ipnet/ipnet_test.go
@@ -135,6 +135,27 @@ func TestNormalizeDetectedIP(t *testing.T) {
 	}
 }
 
+func TestCheckIPFormat(t *testing.T) {
+	t.Parallel()
+
+	for name, tc := range map[string]struct {
+		ip  netip.Addr
+		ok4 bool
+		ok6 bool
+	}{
+		"1::2":    {mustIP("1::2"), false, true},
+		"1.1.1.1": {mustIP("1.1.1.1"), true, false},
+	} {
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			require.Equal(t, tc.ok4, ipnet.IP4.CheckIPFormat(tc.ip))
+			require.Equal(t, tc.ok6, ipnet.IP6.CheckIPFormat(tc.ip))
+			require.False(t, ipnet.Type(0).CheckIPFormat(tc.ip))
+		})
+	}
+}
+
 func TestUDPNetwork(t *testing.T) {
 	t.Parallel()
 	for name, tc := range map[string]struct {
@@ -152,3 +173,21 @@ func TestUDPNetwork(t *testing.T) {
 		})
 	}
 }
+
+func TestIPNetwork(t *testing.T) {
+	t.Parallel()
+	for name, tc := range map[string]struct {
+		input    ipnet.Type
+		expected string
+	}{
+		"4":   {ipnet.IP4, "ip4"},
+		"6":   {ipnet.IP6, "ip6"},
+		"100": {ipnet.Type(100), ""},
+	} {
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			require.Equal(t, tc.expected, tc.input.IPNetwork())
+		})
+	}
+}
diff --git a/internal/provider/protocol/field_test.go b/internal/provider/protocol/field_test.go
index a6af34e..22942e6 100644
--- a/internal/provider/protocol/field_test.go
+++ b/internal/provider/protocol/field_test.go
@@ -95,9 +95,9 @@ func TestFieldGetIP(t *testing.T) {
 				func(m *mocks.MockPP) {
 					m.EXPECT().Warningf(
 						pp.EmojiError,
-						"Failed to send HTTP(S) request to %q: %v",
+						"Failed to force resolve the host of %q as an %s address",
 						"",
-						gomock.Any(),
+						"IPv4",
 					)
 				},
 			},
@@ -106,9 +106,9 @@ func TestFieldGetIP(t *testing.T) {
 				func(m *mocks.MockPP) {
 					m.EXPECT().Warningf(
 						pp.EmojiError,
-						"Failed to send HTTP(S) request to %q: %v",
+						"Failed to force resolve the host of %q as an %s address",
 						"",
-						gomock.Any(),
+						"IPv6",
 					)
 				},
 			},
diff --git a/internal/provider/protocol/http_test.go b/internal/provider/protocol/http_test.go
index e6ffa01..992faec 100644
--- a/internal/provider/protocol/http_test.go
+++ b/internal/provider/protocol/http_test.go
@@ -105,9 +105,9 @@ func TestHTTPGetIP(t *testing.T) {
 				func(m *mocks.MockPP) {
 					m.EXPECT().Warningf(
 						pp.EmojiError,
-						"Failed to send HTTP(S) request to %q: %v",
+						"Failed to force resolve the host of %q as an %s address",
 						"",
-						gomock.Any(),
+						"IPv4",
 					)
 				},
 			},
@@ -116,9 +116,9 @@ func TestHTTPGetIP(t *testing.T) {
 				func(m *mocks.MockPP) {
 					m.EXPECT().Warningf(
 						pp.EmojiError,
-						"Failed to send HTTP(S) request to %q: %v",
+						"Failed to force resolve the host of %q as an %s address",
 						"",
-						gomock.Any(),
+						"IPv6",
 					)
 				},
 			},
@@ -134,6 +134,17 @@ func TestHTTPGetIP(t *testing.T) {
 					m.EXPECT().Warningf(pp.EmojiImpossible, "Unhandled IP network: %s", "IPv4")
 				},
 			},
+			"localhost:0": {
+				false, ipnet.IP4, "https://127.0.0.1:0/", ipnet.IP4, invalidIP,
+				func(m *mocks.MockPP) {
+					m.EXPECT().Warningf(
+						pp.EmojiError,
+						"Failed to send HTTP(S) request to %q: %v",
+						"https://127.0.0.1:0/",
+						gomock.Any(),
+					)
+				},
+			},
 		} {
 			tc := tc
 			t.Run(name, func(t *testing.T) {
@@ -156,8 +167,8 @@ func TestHTTPGetIP(t *testing.T) {
 					tc.prepareMockPP(mockPP)
 				}
 				ip, ok := provider.GetIP(ctx, mockPP, tc.ipNet)
-				require.Equal(t, tc.expected, ip)
 				require.Equal(t, tc.expected.IsValid(), ok)
+				require.Equal(t, tc.expected, ip)
 			})
 		}
 	})