Repository: favonia/cloudflare-ddns
Pull request #487 (2023-05-18 17:02:33)

Revert "fix(provider): avoid static IPs such as 1.1.1.1"
Reverts favonia/cloudflare-ddns#484

I suppose I haven't done enough testing.


Patch:
diff --git a/README.markdown b/README.markdown
index 65e8040..a0d1504 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://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.
+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.
 
 ### 🛡️ 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://cloudflare-dns.com/cdn-cgi/trace) and update DNS records accordingly.
+>   Get the public IP address by parsing the [Cloudflare debugging page](https://1.1.1.1/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
deleted file mode 100644
index 0094fbd..0000000
--- a/internal/ipnet/forceresolve.go
+++ /dev/null
@@ -1,80 +0,0 @@
-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 5ca3a07..3b6b0bd 100644
--- a/internal/ipnet/ipnet.go
+++ b/internal/ipnet/ipnet.go
@@ -98,19 +98,7 @@ func (t Type) NormalizeDetectedIP(ppfmt pp.PP, ip netip.Addr) (netip.Addr, bool)
 	return ip, true
 }
 
-// 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.
+// UDPNetwork gives the network name for net.Dial.
 func (t Type) UDPNetwork() string {
 	switch t {
 	case IP4:
@@ -121,15 +109,3 @@ 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 c2134af..ed04e6b 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://cloudflare-dns.com/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS},
-			ipnet.IP6: {"https://cloudflare-dns.com/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS},
+			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},
 		},
 	}
 }
diff --git a/internal/provider/cloudflare_trace.go b/internal/provider/cloudflare_trace.go
index bf43e85..c501528 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://cloudflare-dns.com/cdn-cgi/trace", "ip"},
-			ipnet.IP6: {"https://cloudflare-dns.com/cdn-cgi/trace", "ip"},
+			ipnet.IP4: {"https://1.1.1.1/cdn-cgi/trace", "ip"},
+			ipnet.IP6: {"https://[2606:4700:4700::1111]/cdn-cgi/trace", "ip"},
 		},
 	}
 }
diff --git a/internal/provider/protocol/doh.go b/internal/provider/protocol/doh.go
index 1b672d6..13bdbfb 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,
-	ipNet ipnet.Type, url string, name string, class dnsmessage.Class,
+	url string, name string, class dnsmessage.Class,
 ) (netip.Addr, bool) {
 	var invalidIP netip.Addr
 
@@ -151,7 +151,6 @@ func getIPFromDNS(ctx context.Context, ppfmt pp.PP,
 	}
 
 	c := httpCore{
-		ipNet:       ipNet,
 		url:         url,
 		method:      http.MethodPost,
 		contentType: "application/dns-message",
@@ -188,7 +187,7 @@ func (p *DNSOverHTTPS) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type)
 		return netip.Addr{}, false
 	}
 
-	ip, ok := getIPFromDNS(ctx, ppfmt, ipNet, param.URL, param.Name, param.Class)
+	ip, ok := getIPFromDNS(ctx, ppfmt, 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 f2a4d11..f45afeb 100644
--- a/internal/provider/protocol/field.go
+++ b/internal/provider/protocol/field.go
@@ -10,9 +10,8 @@ import (
 	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
-func getIPFromField(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, url string, field string) (netip.Addr, bool) {
+func getIPFromField(ctx context.Context, ppfmt pp.PP, url string, field string) (netip.Addr, bool) {
 	c := httpCore{
-		ipNet:       ipNet,
 		url:         url,
 		method:      http.MethodGet,
 		contentType: "",
@@ -62,7 +61,7 @@ func (p *Field) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type) (netip
 		return netip.Addr{}, false
 	}
 
-	ip, ok := getIPFromField(ctx, ppfmt, ipNet, param.URL, param.Field)
+	ip, ok := getIPFromField(ctx, ppfmt, 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 2d9cf75..0724c3c 100644
--- a/internal/provider/protocol/http.go
+++ b/internal/provider/protocol/http.go
@@ -9,9 +9,8 @@ import (
 	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
-func getIPFromHTTP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, url string) (netip.Addr, bool) {
+func getIPFromHTTP(ctx context.Context, ppfmt pp.PP, url string) (netip.Addr, bool) {
 	c := httpCore{
-		ipNet:       ipNet,
 		url:         url,
 		method:      http.MethodGet,
 		contentType: "",
@@ -50,7 +49,7 @@ func (p *HTTP) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type) (netip.
 		return netip.Addr{}, false
 	}
 
-	ip, ok := getIPFromHTTP(ctx, ppfmt, ipNet, url)
+	ip, ok := getIPFromHTTP(ctx, ppfmt, url)
 	if !ok {
 		return netip.Addr{}, false
 	}
diff --git a/internal/provider/protocol/httpcore.go b/internal/provider/protocol/httpcore.go
index 32ebb65..f285998 100644
--- a/internal/provider/protocol/httpcore.go
+++ b/internal/provider/protocol/httpcore.go
@@ -7,12 +7,10 @@ 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
@@ -21,28 +19,21 @@ type httpCore struct {
 	extract     func(pp.PP, []byte) (netip.Addr, bool)
 }
 
-func (h *httpCore) getIP(ctx context.Context, ppfmt pp.PP) (netip.Addr, bool) {
+func (d *httpCore) getIP(ctx context.Context, ppfmt pp.PP) (netip.Addr, bool) {
 	var invalidIP netip.Addr
 
-	req, err := retryablehttp.NewRequestWithContext(ctx, h.method, h.url, h.reader)
+	req, err := retryablehttp.NewRequestWithContext(ctx, d.method, d.url, d.reader)
 	if err != nil {
-		ppfmt.Warningf(pp.EmojiImpossible, "Failed to prepare HTTP(S) request to %q: %v", h.url, err)
+		ppfmt.Warningf(pp.EmojiImpossible, "Failed to prepare HTTP(S) request to %q: %v", d.url, err)
 		return invalidIP, false
 	}
 
-	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.contentType != "" {
+		req.Header.Set("Content-Type", d.contentType)
 	}
 
-	if h.accept != "" {
-		req.Header.Set("Accept", h.accept)
+	if d.accept != "" {
+		req.Header.Set("Accept", d.accept)
 	}
 
 	c := retryablehttp.NewClient()
@@ -50,16 +41,16 @@ func (h *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", h.url, err)
+		ppfmt.Warningf(pp.EmojiError, "Failed to send HTTP(S) request to %q: %v", d.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", h.url, err)
+		ppfmt.Warningf(pp.EmojiError, "Failed to read HTTP(S) response from %q: %v", d.url, err)
 		return invalidIP, false
 	}
 
-	return h.extract(ppfmt, body)
+	return d.extract(ppfmt, body)
 }


Test patch:
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
index 18811ca..16b4dbf 100644
--- a/.github/workflows/test.yaml
+++ b/.github/workflows/test.yaml
@@ -19,12 +19,10 @@ 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
deleted file mode 100644
index 1ff0984..0000000
--- a/internal/ipnet/forceresolve_test.go
+++ /dev/null
@@ -1,85 +0,0 @@
-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 e482082..76f0a91 100644
--- a/internal/ipnet/ipnet_test.go
+++ b/internal/ipnet/ipnet_test.go
@@ -135,27 +135,6 @@ 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 {
@@ -173,21 +152,3 @@ 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 22942e6..a6af34e 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 force resolve the host of %q as an %s address",
+						"Failed to send HTTP(S) request to %q: %v",
 						"",
-						"IPv4",
+						gomock.Any(),
 					)
 				},
 			},
@@ -106,9 +106,9 @@ func TestFieldGetIP(t *testing.T) {
 				func(m *mocks.MockPP) {
 					m.EXPECT().Warningf(
 						pp.EmojiError,
-						"Failed to force resolve the host of %q as an %s address",
+						"Failed to send HTTP(S) request to %q: %v",
 						"",
-						"IPv6",
+						gomock.Any(),
 					)
 				},
 			},
diff --git a/internal/provider/protocol/http_test.go b/internal/provider/protocol/http_test.go
index 992faec..e6ffa01 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 force resolve the host of %q as an %s address",
+						"Failed to send HTTP(S) request to %q: %v",
 						"",
-						"IPv4",
+						gomock.Any(),
 					)
 				},
 			},
@@ -116,9 +116,9 @@ func TestHTTPGetIP(t *testing.T) {
 				func(m *mocks.MockPP) {
 					m.EXPECT().Warningf(
 						pp.EmojiError,
-						"Failed to force resolve the host of %q as an %s address",
+						"Failed to send HTTP(S) request to %q: %v",
 						"",
-						"IPv6",
+						gomock.Any(),
 					)
 				},
 			},
@@ -134,17 +134,6 @@ 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) {
@@ -167,8 +156,8 @@ func TestHTTPGetIP(t *testing.T) {
 					tc.prepareMockPP(mockPP)
 				}
 				ip, ok := provider.GetIP(ctx, mockPP, tc.ipNet)
-				require.Equal(t, tc.expected.IsValid(), ok)
 				require.Equal(t, tc.expected, ip)
+				require.Equal(t, tc.expected.IsValid(), ok)
 			})
 		}
 	})