feat(config): check 1.1.1.1 only when IPv4 is used (#494)

* docs(README): document the switch to 1.0.0.1 * feat(config): check 1.1.1.1 only when IPv4 is used * test(config): improve coverage * ci: make linter happy * test(protocol): improve coverage * ci: make linter happy
favonia · May 22, 2023 · d0db1be · d0db1be
1 parent 4524153
commit d0db1be
Show file tree

Hide file tree

Showing 32 changed files with 289 additions and 229 deletions.
diff --git 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://one.one.one.one/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,13 +227,15 @@ _(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://one.one.one.one/cdn-cgi/trace) and update DNS records accordingly. This is the default provider.
 > - `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`\
 >   Stop the DNS updating completely. Existing DNS records will not be removed.
 >
-> The option `IP4_PROVIDER` is governing IPv4 addresses and `A`-type records, while the option `IP6_PROVIDER` is governing IPv6 addresses and `AAAA`-type records. The two options act independently of each other.
+> The option `IP4_PROVIDER` is governing IPv4 addresses and `A`-type records, while the option `IP6_PROVIDER` is governing IPv6 addresses and `AAAA`-type records. The two options act independently of each other; that is, you can specify different address providers for IPv4 and IPv6.
+>
+> Some technical details: For the providers `cloudflare.doh` and `cloudflare.trace`, the updater will connect to the servers `1.1.1.1` for IPv4 and `2606:4700:4700::1111` for IPv6. Since version 1.9.3, the updater will switch to `1.0.0.1` for IPv4 if `1.1.1.1` appears to be blocked or intercepted by your ISP or your router (which is still not uncommon).
 >
 > </details>
 

diff --git a/cmd/ddns/ddns.go b/cmd/ddns/ddns.go
@@ -28,11 +28,10 @@ func formatName() string {
 }
 
 func initConfig(ctx context.Context, ppfmt pp.PP) (*config.Config, setter.Setter, bool) {
-	use1001 := config.ShouldWeUse1001(ctx, ppfmt)
-	c := config.Default(use1001)
+	c := config.Default()
 
 	// Read the config
-	if !c.ReadEnv(ppfmt, use1001) || !c.NormalizeConfig(ppfmt) {
+	if !c.ReadEnv(ppfmt) || !c.NormalizeConfig(ppfmt) || !c.ShouldWeUse1001(ctx, ppfmt) {
 		return c, nil, false
 	}
 

diff --git a/internal/config/config.go b/internal/config/config.go
@@ -17,6 +17,7 @@ import (
 type Config struct {
 	Auth             api.Auth
 	Provider         map[ipnet.Type]provider.Provider
+	Use1001          bool
 	Domains          map[ipnet.Type][]domain.Domain
 	UpdateCron       cron.Schedule
 	UpdateOnStart    bool
@@ -30,14 +31,15 @@ type Config struct {
 	Monitor          monitor.Monitor
 }
 
-// Default gives the default configuration. If use1001 is true, 1.0.0.1 is used instead of 1.1.1.1.
-func Default(use1001 bool) *Config {
+// Default gives the default configuration.
+func Default() *Config {
 	return &Config{
 		Auth: nil,
 		Provider: map[ipnet.Type]provider.Provider{
-			ipnet.IP4: provider.NewCloudflareTrace(use1001),
-			ipnet.IP6: provider.NewCloudflareTrace(use1001),
+			ipnet.IP4: provider.NewCloudflareTrace(),
+			ipnet.IP6: provider.NewCloudflareTrace(),
 		},
+		Use1001: false,
 		Domains: map[ipnet.Type][]domain.Domain{
 			ipnet.IP4: nil,
 			ipnet.IP6: nil,

diff --git a/internal/config/config_print_test.go b/internal/config/config_print_test.go
@@ -78,7 +78,7 @@ func TestPrintDefault(t *testing.T) {
 		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "%-*s %s", 24, "IP detection:", "5s"),
 		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "%-*s %s", 24, "Record updating:", "30s"),
 	)
-	config.Default(false).Print(mockPP)
+	config.Default().Print(mockPP)
 }
 
 //nolint:paralleltest // changing the environment variable TZ
@@ -116,7 +116,7 @@ func TestPrintMaps(t *testing.T) {
 		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "%-*s %s", 24, "Healthchecks:", "(URL redacted)"),
 	)
 
-	c := config.Default(false)
+	c := config.Default()
 
 	c.Domains[ipnet.IP4] = []domain.Domain{domain.FQDN("test4.org"), domain.Wildcard("test4.org")}
 	c.Domains[ipnet.IP6] = []domain.Domain{domain.FQDN("test6.org"), domain.Wildcard("test6.org")}

diff --git a/internal/config/config_read.go b/internal/config/config_read.go
@@ -9,16 +9,16 @@ import (
 )
 
 // ReadEnv calls the relevant readers to read all relevant environment variables except TZ
-// and update relevant fields. One should subsequently call [Config.NormalizeConfig] to maintain
-// invariants across different fields. If use1001 is true, 1.0.0.1 is used instead of 1.1.1.1.
-func (c *Config) ReadEnv(ppfmt pp.PP, use1001 bool) bool {
+// and update relevant fields. One should subsequently call [Config.NormalizeConfig]
+// to maintain invariants across different fields.
+func (c *Config) ReadEnv(ppfmt pp.PP) bool {
 	if ppfmt.IsEnabledFor(pp.Info) {
 		ppfmt.Infof(pp.EmojiEnvVars, "Reading settings . . .")
 		ppfmt = ppfmt.IncIndent()
 	}
 
 	if !ReadAuth(ppfmt, &c.Auth) ||
-		!ReadProviderMap(ppfmt, use1001, &c.Provider) ||
+		!ReadProviderMap(ppfmt, &c.Provider) ||
 		!ReadDomainMap(ppfmt, &c.Domains) ||
 		!ReadCron(ppfmt, "UPDATE_CRON", &c.UpdateCron) ||
 		!ReadBool(ppfmt, "UPDATE_ON_START", &c.UpdateOnStart) ||

diff --git a/internal/config/config_read_test.go b/internal/config/config_read_test.go
@@ -1,7 +1,6 @@
 package config_test
 
 import (
-	"fmt"
 	"testing"
 	"time"
 
@@ -20,68 +19,60 @@ import (
 
 //nolint:paralleltest // environment variables are global
 func TestReadEnvWithOnlyToken(t *testing.T) {
-	for _, use1001 := range []bool{true, false} {
-		t.Run(fmt.Sprintf("use1001=%t", use1001), func(t *testing.T) {
-			mockCtrl := gomock.NewController(t)
+	mockCtrl := gomock.NewController(t)
 
-			unset(t,
-				"CF_API_TOKEN", "CF_API_TOKEN_FILE", "CF_ACCOUNT_ID",
-				"IP4_PROVIDER", "IP6_PROVIDER",
-				"DOMAINS", "IP4_DOMAINS", "IP6_DOMAINS",
-				"UPDATE_CRON", "UPDATE_ON_START", "DELETE_ON_STOP", "CACHE_EXPIRATION", "TTL", "PROXIED", "DETECTION_TIMEOUT")
+	unset(t,
+		"CF_API_TOKEN", "CF_API_TOKEN_FILE", "CF_ACCOUNT_ID",
+		"IP4_PROVIDER", "IP6_PROVIDER",
+		"DOMAINS", "IP4_DOMAINS", "IP6_DOMAINS",
+		"UPDATE_CRON", "UPDATE_ON_START", "DELETE_ON_STOP", "CACHE_EXPIRATION", "TTL", "PROXIED", "DETECTION_TIMEOUT")
 
-			store(t, "CF_API_TOKEN", "deadbeaf")
+	store(t, "CF_API_TOKEN", "deadbeaf")
 
-			var cfg config.Config
-			mockPP := mocks.NewMockPP(mockCtrl)
-			innerMockPP := mocks.NewMockPP(mockCtrl)
-			gomock.InOrder(
-				mockPP.EXPECT().IsEnabledFor(pp.Info).Return(true),
-				mockPP.EXPECT().Infof(pp.EmojiEnvVars, "Reading settings . . ."),
-				mockPP.EXPECT().IncIndent().Return(innerMockPP),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%s", "IP4_PROVIDER", "none"),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%s", "IP6_PROVIDER", "none"),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%s", "UPDATE_CRON", "@disabled"),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%t", "UPDATE_ON_START", false),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%t", "DELETE_ON_STOP", false),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%v", "CACHE_EXPIRATION", time.Duration(0)),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%d", "TTL", api.TTL(0)),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%s", "PROXIED", ""),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%v", "DETECTION_TIMEOUT", time.Duration(0)),
-				innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%v", "UPDATE_TIMEOUT", time.Duration(0)),
-			)
-			ok := cfg.ReadEnv(mockPP, use1001)
-			require.True(t, ok)
-		})
-	}
+	var cfg config.Config
+	mockPP := mocks.NewMockPP(mockCtrl)
+	innerMockPP := mocks.NewMockPP(mockCtrl)
+	gomock.InOrder(
+		mockPP.EXPECT().IsEnabledFor(pp.Info).Return(true),
+		mockPP.EXPECT().Infof(pp.EmojiEnvVars, "Reading settings . . ."),
+		mockPP.EXPECT().IncIndent().Return(innerMockPP),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%s", "IP4_PROVIDER", "none"),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%s", "IP6_PROVIDER", "none"),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%s", "UPDATE_CRON", "@disabled"),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%t", "UPDATE_ON_START", false),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%t", "DELETE_ON_STOP", false),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%v", "CACHE_EXPIRATION", time.Duration(0)),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%d", "TTL", api.TTL(0)),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%s", "PROXIED", ""),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%v", "DETECTION_TIMEOUT", time.Duration(0)),
+		innerMockPP.EXPECT().Infof(pp.EmojiBullet, "Use default %s=%v", "UPDATE_TIMEOUT", time.Duration(0)),
+	)
+	ok := cfg.ReadEnv(mockPP)
+	require.True(t, ok)
 }
 
 //nolint:paralleltest // environment variables are global
 func TestReadEnvEmpty(t *testing.T) {
-	for _, use1001 := range []bool{true, false} {
-		t.Run(fmt.Sprintf("use1001=%t", use1001), func(t *testing.T) {
-			mockCtrl := gomock.NewController(t)
+	mockCtrl := gomock.NewController(t)
 
-			unset(t,
-				"CF_API_TOKEN", "CF_API_TOKEN_FILE", "CF_ACCOUNT_ID",
-				"IP4_PROVIDER", "IP6_PROVIDER",
-				"IP4_POLICY", "IP6_POLICY",
-				"DOMAINS", "IP4_DOMAINS", "IP6_DOMAINS",
-				"UPDATE_CRON", "UPDATE_ON_START", "DELETE_ON_STOP", "CACHE_EXPIRATION", "TTL", "PROXIED", "DETECTION_TIMEOUT")
+	unset(t,
+		"CF_API_TOKEN", "CF_API_TOKEN_FILE", "CF_ACCOUNT_ID",
+		"IP4_PROVIDER", "IP6_PROVIDER",
+		"IP4_POLICY", "IP6_POLICY",
+		"DOMAINS", "IP4_DOMAINS", "IP6_DOMAINS",
+		"UPDATE_CRON", "UPDATE_ON_START", "DELETE_ON_STOP", "CACHE_EXPIRATION", "TTL", "PROXIED", "DETECTION_TIMEOUT")
 
-			var cfg config.Config
-			mockPP := mocks.NewMockPP(mockCtrl)
-			innerMockPP := mocks.NewMockPP(mockCtrl)
-			gomock.InOrder(
-				mockPP.EXPECT().IsEnabledFor(pp.Info).Return(true),
-				mockPP.EXPECT().Infof(pp.EmojiEnvVars, "Reading settings . . ."),
-				mockPP.EXPECT().IncIndent().Return(innerMockPP),
-				innerMockPP.EXPECT().Errorf(pp.EmojiUserError, "Needs either CF_API_TOKEN or CF_API_TOKEN_FILE"),
-			)
-			ok := cfg.ReadEnv(mockPP, use1001)
-			require.False(t, ok)
-		})
-	}
+	var cfg config.Config
+	mockPP := mocks.NewMockPP(mockCtrl)
+	innerMockPP := mocks.NewMockPP(mockCtrl)
+	gomock.InOrder(
+		mockPP.EXPECT().IsEnabledFor(pp.Info).Return(true),
+		mockPP.EXPECT().Infof(pp.EmojiEnvVars, "Reading settings . . ."),
+		mockPP.EXPECT().IncIndent().Return(innerMockPP),
+		innerMockPP.EXPECT().Errorf(pp.EmojiUserError, "Needs either CF_API_TOKEN or CF_API_TOKEN_FILE"),
+	)
+	ok := cfg.ReadEnv(mockPP)
+	require.False(t, ok)
 }
 
 //nolint:funlen
@@ -131,8 +122,8 @@ func TestNormalizeConfig(t *testing.T) {
 		"empty-ip6": {
 			input: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP4: provider.NewCloudflareTrace(true),
-					ipnet.IP6: provider.NewCloudflareTrace(false),
+					ipnet.IP4: provider.NewCloudflareTrace(),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP4: {domain.FQDN("a.b.c")},
@@ -143,7 +134,7 @@ func TestNormalizeConfig(t *testing.T) {
 			ok: true,
 			expected: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP4: provider.NewCloudflareTrace(true),
+					ipnet.IP4: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP4: {domain.FQDN("a.b.c")},
@@ -168,7 +159,7 @@ func TestNormalizeConfig(t *testing.T) {
 		"empty-ip6-none-ip4": {
 			input: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(true),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP4: {domain.FQDN("a.b.c")},
@@ -194,7 +185,7 @@ func TestNormalizeConfig(t *testing.T) {
 		"ignored-ip4-domains": {
 			input: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(true),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP4: {domain.FQDN("a.b.c"), domain.FQDN("d.e.f")},
@@ -205,7 +196,7 @@ func TestNormalizeConfig(t *testing.T) {
 			ok: true,
 			expected: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(true),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP4: {domain.FQDN("a.b.c"), domain.FQDN("d.e.f")},
@@ -231,7 +222,7 @@ func TestNormalizeConfig(t *testing.T) {
 		"template": {
 			input: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(false),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP6: {domain.FQDN("a.b.c"), domain.FQDN("a.bb.c"), domain.FQDN("a.d.e.f")},
@@ -241,7 +232,7 @@ func TestNormalizeConfig(t *testing.T) {
 			ok: true,
 			expected: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(false),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP6: {domain.FQDN("a.b.c"), domain.FQDN("a.bb.c"), domain.FQDN("a.d.e.f")},
@@ -264,7 +255,7 @@ func TestNormalizeConfig(t *testing.T) {
 		"template/invalid/proxied": {
 			input: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(true),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP6: {domain.FQDN("a.b.c"), domain.FQDN("a.bb.c"), domain.FQDN("a.d.e.f")},
@@ -285,7 +276,7 @@ func TestNormalizeConfig(t *testing.T) {
 		"template/error/proxied": {
 			input: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(false),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP6: {domain.FQDN("a.b.c")},
@@ -306,7 +297,7 @@ func TestNormalizeConfig(t *testing.T) {
 		"template/error/proxied/ill-formed": {
 			input: &config.Config{ //nolint:exhaustruct
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(true),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP6: {domain.FQDN("a.b.c")},
@@ -344,7 +335,7 @@ func TestNormalizeConfig(t *testing.T) {
 				DeleteOnStop: true,
 				UpdateCron:   cron.MustNew("@every 5m"),
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(false),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP6: {domain.FQDN("a.b.c")},
@@ -356,7 +347,7 @@ func TestNormalizeConfig(t *testing.T) {
 				DeleteOnStop: true,
 				UpdateCron:   cron.MustNew("@every 5m"),
 				Provider: map[ipnet.Type]provider.Provider{
-					ipnet.IP6: provider.NewCloudflareTrace(false),
+					ipnet.IP6: provider.NewCloudflareTrace(),
 				},
 				Domains: map[ipnet.Type][]domain.Domain{
 					ipnet.IP6: {domain.FQDN("a.b.c")},

diff --git a/internal/config/config_test.go b/internal/config/config_test.go
@@ -11,6 +11,5 @@ import (
 func TestDefaultConfigNotNil(t *testing.T) {
 	t.Parallel()
 
-	require.NotNil(t, config.Default(true))
-	require.NotNil(t, config.Default(false))
+	require.NotNil(t, config.Default())
 }