fix: deprecate possibly unmaintained ipify (#270)

favonia · Nov 14, 2022 · 69b5d70 · 69b5d70
1 parent 4c68907
commit 69b5d70
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 22 deletions.
diff --git a/README.markdown b/README.markdown
@@ -39,12 +39,12 @@ Simply list all the domain names and you are done!
 
 ### 🕵️ Privacy
 
-By default, public IP addresses are obtained using the [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 could not update DNS records anyways. You can also configure the updater to use [ipify](https://www.ipify.org), which claims not to log any visitor information. [Open a GitHub issue](https://github.com/favonia/cloudflare-ddns/issues/new) to propose a new method to detect public IP addresses.
+By default, public IP addresses are obtained using the [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 could not update DNS records anyways.
 
 ### 🛡️ Security
 
 - 🛑 The superuser privileges are immediately dropped after the updater starts. This minimizes the impact of undiscovered security bugs in the updater.
-- 🛡️ The updater uses HTTPS (or [DNS over HTTPS (DoH)](https://en.wikipedia.org/wiki/DNS_over_HTTPS)) to detect public IP addresses, making it harder to tamper with the detection process. _(Due to the nature of address detection, it is impossible to protect the updater from an adversary who can modify the source IP address of the IP packages coming from your machine.)_
+- 🛡️ The updater uses HTTPS (or [DNS over HTTPS (DoH)](https://en.wikipedia.org/wiki/DNS_over_HTTPS)) to detect public IP addresses, making it harder to tamper with the detection process. _(Due to the nature of address detection, it is impossible to protect the updater from an adversary who can modify the source IP address of the IP packets coming from your machine.)_
 - 🖥️ Optionally, you can [monitor the updater via Healthchecks.io](https://healthchecks.io), which will notify you when the updating fails.
 - 📚 The updater uses only established open-source Go libraries.
   <details><summary>🔌 Full list of external Go libraries <em>(click to expand)</em></summary>
@@ -323,8 +323,8 @@ In most cases, `CF_ACCOUNT_ID` is not needed.
 | `DOMAINS`      | Comma-separated fully qualified domain names or wildcard domain names | The domains the updater should manage for both `A` and `AAAA` records | (See below) | (empty list)       |
 | `IP4_DOMAINS`  | Comma-separated fully qualified domain names or wildcard domain names | The domains the updater should manage for `A` records                 | (See below) | (empty list)       |
 | `IP6_DOMAINS`  | Comma-separated fully qualified domain names or wildcard domain names | The domains the updater should manage for `AAAA` records              | (See below) | (empty list)       |
-| `IP4_PROVIDER` | `cloudflare.doh`, `cloudflare.trace`, `ipify`, `local`, and `none`    | How to detect IPv4 addresses. (See below)                             | No          | `cloudflare.trace` |
-| `IP6_PROVIDER` | `cloudflare.doh`, `cloudflare.trace`, `ipify`, `local`, and `none`    | How to detect IPv6 addresses. (See below)                             | No          | `cloudflare.trace` |
+| `IP4_PROVIDER` | `cloudflare.doh`, `cloudflare.trace`, `local`, and `none`             | How to detect IPv4 addresses. (See below)                             | No          | `cloudflare.trace` |
+| `IP6_PROVIDER` | `cloudflare.doh`, `cloudflare.trace`, `local`, and `none`             | How to detect IPv6 addresses. (See below)                             | No          | `cloudflare.trace` |
 
 > <details>
 > <summary>📍 At least one of <code>DOMAINS</code> and <code>IP4/6_DOMAINS</code> must be non-empty.</summary>
@@ -340,8 +340,6 @@ In most cases, `CF_ACCOUNT_ID` is not needed.
 >   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.
-> - `ipify`\
->   Get the public IP address via [ipify’s public API](https://www.ipify.org/) 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 or `hostNetwork: true` in Kubernetes) for this policy, for otherwise the updater will detect the addresses inside the [bridge network in Docker](https://docs.docker.com/network/bridge/) or the [default namespaces in Kubernetes](https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/) instead of those in the host network.
 > - `none`\
@@ -457,7 +455,7 @@ _(Click to expand the following items.)_
 <details>
 <summary>I am migrating from <a href="https://hub.docker.com/r/oznu/cloudflare-ddns/">oznu/cloudflare-ddns.</a></summary>
 
-⚠️ [oznu/cloudflare-ddns](https://hub.docker.com/r/oznu/cloudflare-ddns/) relies on the insecure DNS protocol to obtain public IP addresses; a malicious hacker could forge DNS responses and trick it into updating your domain with any IP address. In comparison, we use only verified responses from Cloudflare or ipify, which makes the attack much more difficult.
+⚠️ [oznu/cloudflare-ddns](https://hub.docker.com/r/oznu/cloudflare-ddns/) relies on the insecure DNS protocol to obtain public IP addresses; a malicious hacker could more easily forge DNS responses and trick it into updating your domain with any IP address. In comparison, we use only verified responses from Cloudflare, which makes the attack much more difficult.
 
 | Old Parameter                          |     | New Paramater                                                                      |
 | -------------------------------------- | --- | ---------------------------------------------------------------------------------- |
@@ -470,7 +468,7 @@ _(Click to expand the following items.)_
 | `DELETE_ON_STOP=true`                  | ✔️  | Same                                                                               |
 | `INTERFACE=iface`                      | ✔️  | Not required for `local` providers; we can handle multiple network interfaces      |
 | `CUSTOM_LOOKUP_CMD=cmd`                | ❌  | _There is not even a shell in the minimum Docker image_                            |
-| `DNS_SERVER=server`                    | ❌  | _Only the secure Cloudflare and ipify are supported_                               |
+| `DNS_SERVER=server`                    | ❌  | _Only Cloudflare is supported_                                                     |
 
 </details>
 

diff --git a/internal/config/config_test.go b/internal/config/config_test.go
@@ -160,7 +160,6 @@ func TestReadProviderMap(t *testing.T) {
 		cloudflareTrace = provider.NewCloudflareTrace()
 		cloudflareDOH   = provider.NewCloudflareDOH()
 		local           = provider.NewLocal()
-		ipify           = provider.NewIpify()
 	)
 
 	for name, tc := range map[string]struct {
@@ -171,10 +170,10 @@ func TestReadProviderMap(t *testing.T) {
 		prepareMockPP func(*mocks.MockPP)
 	}{
 		"full": {
-			"cloudflare.trace", "ipify",
+			"cloudflare.trace", "local",
 			map[ipnet.Type]provider.Provider{
 				ipnet.IP4: cloudflareTrace,
-				ipnet.IP6: ipify,
+				ipnet.IP6: local,
 			},
 			true,
 			nil,

diff --git a/internal/config/env.go b/internal/config/env.go
@@ -147,7 +147,7 @@ func ReadProvider(ppfmt pp.PP, key, keyDeprecated string, field *provider.Provid
 		case "cloudflare":
 			ppfmt.Warningf(
 				pp.EmojiUserWarning,
-				`Parameter %s and provider "cloudflare" were deprecated; use %s=cloudflare.doh or %s=cloudflare.trace`,
+				`Parameter %s and provider "cloudflare" were deprecated; use %s=cloudflare.trace or %s=cloudflare.doh`,
 				keyDeprecated, key, key,
 			)
 			*field = provider.NewCloudflareTrace()
@@ -171,8 +171,8 @@ func ReadProvider(ppfmt pp.PP, key, keyDeprecated string, field *provider.Provid
 		case "ipify":
 			ppfmt.Warningf(
 				pp.EmojiUserWarning,
-				`Parameter %s was deprecated; use %s=%s`,
-				keyDeprecated, key, valPolicy,
+				`Parameter %s and provider "ipify" were deprecated; use %s=cloudflare.trace or %s=cloudflare.doh`,
+				keyDeprecated, key, key,
 			)
 			*field = provider.NewIpify()
 			return true
@@ -210,8 +210,8 @@ func ReadProvider(ppfmt pp.PP, key, keyDeprecated string, field *provider.Provid
 		case "cloudflare":
 			ppfmt.Errorf(
 				pp.EmojiUserError,
-				`Parameter %s does not accept "cloudflare"; use "cloudflare.doh" or "cloudflare.trace"`,
-				key,
+				`Parameter %s does not accept "cloudflare"; use %s=cloudflare.trace or %s=cloudflare.doh`,
+				key, key, key,
 			)
 			return false
 		case "cloudflare.trace":
@@ -221,6 +221,11 @@ func ReadProvider(ppfmt pp.PP, key, keyDeprecated string, field *provider.Provid
 			*field = provider.NewCloudflareDOH()
 			return true
 		case "ipify":
+			ppfmt.Warningf(
+				pp.EmojiUserWarning,
+				`Provider "ipify" was deprecated; use %s=cloudflare.trace or %s=cloudflare.doh`,
+				key, key,
+			)
 			*field = provider.NewIpify()
 			return true
 		case "local":

diff --git a/internal/config/env_test.go b/internal/config/env_test.go
@@ -464,7 +464,7 @@ func TestReadProvider(t *testing.T) {
 			func(m *mocks.MockPP) {
 				m.EXPECT().Warningf(
 					pp.EmojiUserWarning,
-					`Parameter %s and provider "cloudflare" were deprecated; use %s=cloudflare.doh or %s=cloudflare.trace`, //nolint:lll
+					`Parameter %s and provider "cloudflare" were deprecated; use %s=cloudflare.trace or %s=cloudflare.doh`, //nolint:lll
 					keyDeprecated, key, key,
 				)
 			},
@@ -521,10 +521,10 @@ func TestReadProvider(t *testing.T) {
 			func(m *mocks.MockPP) {
 				m.EXPECT().Warningf(
 					pp.EmojiUserWarning,
-					`Parameter %s was deprecated; use %s=%s`,
+					`Parameter %s and provider "ipify" were deprecated; use %s=cloudflare.trace or %s=cloudflare.doh`,
 					keyDeprecated,
 					key,
-					"ipify",
+					key,
 				)
 			},
 		},
@@ -555,16 +555,26 @@ func TestReadProvider(t *testing.T) {
 			func(m *mocks.MockPP) {
 				m.EXPECT().Errorf(
 					pp.EmojiUserError,
-					`Parameter %s does not accept "cloudflare"; use "cloudflare.doh" or "cloudflare.trace"`,
-					key,
+					`Parameter %s does not accept "cloudflare"; use %s=cloudflare.trace or %s=cloudflare.doh`,
+					key, key, key,
 				)
 			},
 		},
 		"cloudflare.trace": {true, " cloudflare.trace", false, "", none, cloudflareTrace, true, nil},
 		"cloudflare.doh":   {true, "    \tcloudflare.doh   ", false, "", none, cloudflareDOH, true, nil},
 		"none":             {true, "   none   ", false, "", cloudflareTrace, none, true, nil},
 		"local":            {true, "   local   ", false, "", cloudflareTrace, local, true, nil},
-		"ipify":            {true, "     ipify  ", false, "", cloudflareTrace, ipify, true, nil},
+		"ipify": {
+			true, "     ipify  ", false, "", cloudflareTrace, ipify, true,
+			func(m *mocks.MockPP) {
+				m.EXPECT().Warningf(
+					pp.EmojiUserWarning,
+					`Provider "ipify" was deprecated; use %s=cloudflare.trace or %s=cloudflare.doh`,
+					key,
+					key,
+				)
+			},
+		},
 		"others": {
 			true, "   something-else ", false, "", ipify, ipify, false,
 			func(m *mocks.MockPP) {