From 69b5d706cf0c1e6696685d1569934c67676242d1 Mon Sep 17 00:00:00 2001 From: favonia Date: Mon, 14 Nov 2022 10:10:06 -0600 Subject: [PATCH] fix: deprecate possibly unmaintained ipify (#270) --- README.markdown | 14 ++++++-------- internal/config/config_test.go | 5 ++--- internal/config/env.go | 15 ++++++++++----- internal/config/env_test.go | 22 ++++++++++++++++------ 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/README.markdown b/README.markdown index 11a30ba9..68cfcfc9 100644 --- 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.
🔌 Full list of external Go libraries (click to expand) @@ -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` | >
> 📍 At least one of DOMAINS and IP4/6_DOMAINS must be non-empty. @@ -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.)_
I am migrating from oznu/cloudflare-ddns. -⚠️ [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_ |
diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 730d4d01..e488e00f 100644 --- 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 index b6877fa2..0e87d43e 100644 --- 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 index 87230322..7764d712 100644 --- 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,8 +555,8 @@ 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, ) }, }, @@ -564,7 +564,17 @@ func TestReadProvider(t *testing.T) { "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) {