From 320d91d8e3e3d7848e7559779a71715702449d5e Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Sat, 17 Jun 2023 13:31:28 +0000 Subject: [PATCH] change(publicip/dns): use DNS over TLS only - Fix critical issue #492 - Remove `google` dns provider since it does not support DNS over TLS --- README.md | 1 - cmd/updater/main.go | 4 +- go.mod | 2 +- go.sum | 4 +- internal/config/settings/pubip.go | 2 +- internal/config/settings/settings_test.go | 2 +- internal/config/sources/env/pubip.go | 12 +++++ pkg/publicip/dns/dns.go | 30 ++---------- pkg/publicip/dns/dns_test.go | 3 -- pkg/publicip/dns/fetch.go | 21 +++++++-- pkg/publicip/dns/fetch_test.go | 15 +++--- pkg/publicip/dns/integration_test.go | 9 +--- pkg/publicip/dns/ip.go | 25 +++++++--- pkg/publicip/dns/options_test.go | 10 ++-- pkg/publicip/dns/providers.go | 57 +++++++++++++---------- pkg/publicip/dns/providers_test.go | 34 +++++++------- 16 files changed, 124 insertions(+), 107 deletions(-) diff --git a/README.md b/README.md index 779e3569f..fb76f03d5 100644 --- a/README.md +++ b/README.md @@ -255,7 +255,6 @@ You can otherwise customize it with the following: - `noip` using [http://ip1.dynupdate6.no-ip.com](http://ip1.dynupdate6.no-ip.com) - You can also specify an HTTPS URL such as `https://ipinfo.io/ip` - `PUBLICIP_DNS_PROVIDERS` gets your public IPv4 address only or IPv6 address only or one of them (see #136). It can be one or more of the following: - - `google` - `cloudflare` - `opendns` diff --git a/cmd/updater/main.go b/cmd/updater/main.go index 905e8ba47..0fefe5c6a 100644 --- a/cmd/updater/main.go +++ b/cmd/updater/main.go @@ -107,7 +107,7 @@ func _main(ctx context.Context, settingsSource SettingsSource, args []string, lo return client.Query(ctx, *healthSettings.ServerAddress) } - announcementExp, err := time.Parse(time.RFC3339, "2023-06-30T00:00:00Z") + announcementExp, err := time.Parse(time.RFC3339, "2023-07-15T00:00:00Z") if err != nil { return err } @@ -118,7 +118,7 @@ func _main(ctx context.Context, settingsSource SettingsSource, args []string, lo Version: buildInfo.Version, Commit: buildInfo.Commit, BuildDate: buildInfo.Created, - Announcement: "Environment variables parsing was changed on 12 June, please report any issue you might have", + Announcement: "Public IP dns provider GOOGLE, see https://github.com/qdm12/ddns-updater/issues/492", AnnounceExp: announcementExp, // Sponsor information PaypalUser: "qmcgaw", diff --git a/go.mod b/go.mod index beb94bbb3..ec1b99476 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/go-chi/chi v4.1.2+incompatible github.com/golang/mock v1.6.0 github.com/miekg/dns v1.1.54 - github.com/qdm12/gosettings v0.3.0 + github.com/qdm12/gosettings v0.4.0-rc1 github.com/qdm12/goshutdown v0.3.0 github.com/qdm12/gosplash v0.1.0 github.com/qdm12/gotree v0.2.0 diff --git a/go.sum b/go.sum index a36749a46..bcea92d9e 100644 --- a/go.sum +++ b/go.sum @@ -519,8 +519,8 @@ github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsT github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A= github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4OA4YeYWdaU= github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA= -github.com/qdm12/gosettings v0.3.0 h1:YutcgQzVaOB3LuLj+Smtoy90JOH/5B5p2IH3BvV3ra4= -github.com/qdm12/gosettings v0.3.0/go.mod h1:JRV3opOpHvnKlIA29lKQMdYw1WSMVMfHYLLHPHol5ME= +github.com/qdm12/gosettings v0.4.0-rc1 h1:UYA92yyeDPbmZysIuG65yrpZVPtdIoRmtEHft/AyI38= +github.com/qdm12/gosettings v0.4.0-rc1/go.mod h1:JRV3opOpHvnKlIA29lKQMdYw1WSMVMfHYLLHPHol5ME= github.com/qdm12/goshutdown v0.3.0 h1:pqBpJkdwlZlfTEx4QHtS8u8CXx6pG0fVo6S1N0MpSEM= github.com/qdm12/goshutdown v0.3.0/go.mod h1:EqZ46No00kCTZ5qzdd3qIzY6ayhMt24QI8Mh8LVQYmM= github.com/qdm12/gosplash v0.1.0 h1:Sfl+zIjFZFP7b0iqf2l5UkmEY97XBnaKkH3FNY6Gf7g= diff --git a/internal/config/settings/pubip.go b/internal/config/settings/pubip.go index 1a2c9ea37..a5ed18ebc 100644 --- a/internal/config/settings/pubip.go +++ b/internal/config/settings/pubip.go @@ -98,7 +98,7 @@ func (p *PubIP) toLinesNode() (node *gotree.Node) { node.Appendf("DNS enabled: %s", gosettings.BoolToYesNo(p.DNSEnabled)) if *p.DNSEnabled { node.Appendf("DNS timeout: %s", p.DNSTimeout) - childNode := node.Appendf("DNS providers") + childNode := node.Appendf("DNS over TLS providers") for _, provider := range p.DNSProviders { childNode.Appendf(provider) } diff --git a/internal/config/settings/settings_test.go b/internal/config/settings/settings_test.go index 06946d6b9..f518af5af 100644 --- a/internal/config/settings/settings_test.go +++ b/internal/config/settings/settings_test.go @@ -30,7 +30,7 @@ func Test_Settings_String(t *testing.T) { | | └── all | ├── DNS enabled: yes | ├── DNS timeout: 3s -| └── DNS providers +| └── DNS over TLS providers | └── all ├── Resolver: use Go default resolver ├── IPv6 diff --git a/internal/config/sources/env/pubip.go b/internal/config/sources/env/pubip.go index 1e9c7655e..608618e79 100644 --- a/internal/config/sources/env/pubip.go +++ b/internal/config/sources/env/pubip.go @@ -46,6 +46,18 @@ func (s *Source) readPubIP() (settings settings.PubIP, err error) { } settings.DNSProviders = s.env.CSV("PUBLICIP_DNS_PROVIDERS") + + // Retro-compatibility + for i, provider := range settings.DNSProviders { + if provider == "google" { + s.warner.Warnf("dns provider google will be ignored " + + "since it is no longer supported, " + + "see https://github.com/qdm12/ddns-updater/issues/492") + settings.DNSProviders[i] = settings.DNSProviders[len(settings.DNSProviders)-1] + settings.DNSProviders = settings.DNSProviders[:len(settings.DNSProviders)-1] + } + } + settings.DNSTimeout, err = s.env.Duration("PUBLICIP_DNS_TIMEOUT") if err != nil { return settings, err diff --git a/pkg/publicip/dns/dns.go b/pkg/publicip/dns/dns.go index f216d8e73..f703ea0da 100644 --- a/pkg/publicip/dns/dns.go +++ b/pkg/publicip/dns/dns.go @@ -1,16 +1,10 @@ package dns -import ( - "net" - - "github.com/miekg/dns" -) +import "time" type Fetcher struct { ring ring - client Client - client4 Client - client6 Client + timeout time.Duration } type ring struct { @@ -28,29 +22,11 @@ func New(options ...Option) (f *Fetcher, err error) { } } - dialer := &net.Dialer{ - Timeout: settings.timeout, - } - return &Fetcher{ ring: ring{ counter: new(uint32), providers: settings.providers, }, - client: &dns.Client{ - Net: "udp", - Dialer: dialer, - Timeout: settings.timeout, - }, - client4: &dns.Client{ - Net: "udp4", - Dialer: dialer, - Timeout: settings.timeout, - }, - client6: &dns.Client{ - Net: "udp6", - Dialer: dialer, - Timeout: settings.timeout, - }, + timeout: settings.timeout, }, nil } diff --git a/pkg/publicip/dns/dns_test.go b/pkg/publicip/dns/dns_test.go index daee881ea..c5ee85f5d 100644 --- a/pkg/publicip/dns/dns_test.go +++ b/pkg/publicip/dns/dns_test.go @@ -16,7 +16,4 @@ func Test_New(t *testing.T) { assert.NotNil(t, impl.ring.counter) assert.NotEmpty(t, impl.ring.providers) - assert.NotNil(t, impl.client) - assert.NotNil(t, impl.client4) - assert.NotNil(t, impl.client6) } diff --git a/pkg/publicip/dns/fetch.go b/pkg/publicip/dns/fetch.go index ae03f498c..a395d7d73 100644 --- a/pkg/publicip/dns/fetch.go +++ b/pkg/publicip/dns/fetch.go @@ -4,12 +4,14 @@ import ( "context" "errors" "fmt" + "net" "net/netip" "github.com/miekg/dns" ) var ( + ErrNetworkNotSupported = errors.New("network not supported") ErrAnswerNotReceived = errors.New("response answer not received") ErrAnswerTypeMismatch = errors.New("answer type is not expected") ErrAnswerTypeNotSupported = errors.New("answer type not supported") @@ -17,8 +19,21 @@ var ( ErrIPMalformed = errors.New("IP address malformed") ) -func fetch(ctx context.Context, client Client, providerData providerData) ( - publicIPs []netip.Addr, err error) { +func fetch(ctx context.Context, client Client, network string, + providerData providerData) (publicIPs []netip.Addr, err error) { + var serverHost string + switch network { + case "tcp": + serverHost = providerData.TLSName + case "tcp4": + serverHost = providerData.IPv4.String() + case "tcp6": + serverHost = providerData.IPv6.String() + default: + return nil, fmt.Errorf("%w: %s", ErrNetworkNotSupported, network) + } + serverAddress := net.JoinHostPort(serverHost, "853") + message := &dns.Msg{ MsgHdr: dns.MsgHdr{ Opcode: dns.OpcodeQuery, @@ -32,7 +47,7 @@ func fetch(ctx context.Context, client Client, providerData providerData) ( }, } - r, _, err := client.ExchangeContext(ctx, message, providerData.nameserver) + r, _, err := client.ExchangeContext(ctx, message, serverAddress) if err != nil { return nil, err } diff --git a/pkg/publicip/dns/fetch_test.go b/pkg/publicip/dns/fetch_test.go index afd98b37b..e00c9f5e3 100644 --- a/pkg/publicip/dns/fetch_test.go +++ b/pkg/publicip/dns/fetch_test.go @@ -3,6 +3,7 @@ package dns import ( "context" "errors" + "net" "net/netip" "testing" "time" @@ -18,10 +19,10 @@ func Test_fetch(t *testing.T) { t.Parallel() providerData := providerData{ - nameserver: "nameserver", - fqdn: "record", - class: dns.ClassNONE, - qType: dns.Type(dns.TypeTXT), + TLSName: "nameserver", + fqdn: "record", + class: dns.ClassNONE, + qType: dns.Type(dns.TypeTXT), } expectedMessage := &dns.Msg{ @@ -101,11 +102,13 @@ func Test_fetch(t *testing.T) { ctx := context.Background() client := mock_dns.NewMockClient(ctrl) + expectedAddress := net.JoinHostPort(providerData.TLSName, "853") client.EXPECT(). - ExchangeContext(ctx, expectedMessage, providerData.nameserver). + ExchangeContext(ctx, expectedMessage, expectedAddress). Return(testCase.response, time.Millisecond, testCase.exchangeErr) - publicIPs, err := fetch(ctx, client, providerData) + const network = "tcp" // so it picks the TLSName field as the address + publicIPs, err := fetch(ctx, client, network, providerData) if testCase.err != nil { require.Error(t, err) diff --git a/pkg/publicip/dns/integration_test.go b/pkg/publicip/dns/integration_test.go index 4349fc017..e0db6b44d 100644 --- a/pkg/publicip/dns/integration_test.go +++ b/pkg/publicip/dns/integration_test.go @@ -14,7 +14,7 @@ import ( func Test_integration(t *testing.T) { t.Parallel() - fetcher, err := New(SetProviders(Google, Cloudflare, OpenDNS)) + fetcher, err := New(SetProviders(Cloudflare, OpenDNS)) require.NoError(t, err) ctx := context.Background() @@ -27,12 +27,7 @@ func Test_integration(t *testing.T) { require.NoError(t, err) assert.NotNil(t, publicIP2) - publicIP3, err := fetcher.IP4(ctx) - require.NoError(t, err) - assert.NotNil(t, publicIP2) - - assert.Equal(t, publicIP1, publicIP2) - assert.Equal(t, publicIP1, publicIP3) + assert.Equal(t, publicIP1.String(), publicIP2.String()) t.Logf("Public IP is %s", publicIP1) } diff --git a/pkg/publicip/dns/ip.go b/pkg/publicip/dns/ip.go index 3b369f53d..d442ac23d 100644 --- a/pkg/publicip/dns/ip.go +++ b/pkg/publicip/dns/ip.go @@ -2,10 +2,13 @@ package dns import ( "context" + "crypto/tls" "errors" "fmt" "net/netip" "sync/atomic" + + "github.com/miekg/dns" ) var ( @@ -13,7 +16,7 @@ var ( ) func (f *Fetcher) IP(ctx context.Context) (publicIP netip.Addr, err error) { - publicIPs, err := f.ip(ctx, f.client) + publicIPs, err := f.ip(ctx, "tcp") if err != nil { return netip.Addr{}, err } @@ -21,7 +24,7 @@ func (f *Fetcher) IP(ctx context.Context) (publicIP netip.Addr, err error) { } func (f *Fetcher) IP4(ctx context.Context) (publicIP netip.Addr, err error) { - publicIPs, err := f.ip(ctx, f.client4) + publicIPs, err := f.ip(ctx, "tcp4") if err != nil { return netip.Addr{}, err } @@ -35,7 +38,7 @@ func (f *Fetcher) IP4(ctx context.Context) (publicIP netip.Addr, err error) { } func (f *Fetcher) IP6(ctx context.Context) (publicIP netip.Addr, err error) { - publicIPs, err := f.ip(ctx, f.client6) + publicIPs, err := f.ip(ctx, "tcp6") if err != nil { return netip.Addr{}, err } @@ -48,9 +51,19 @@ func (f *Fetcher) IP6(ctx context.Context) (publicIP netip.Addr, err error) { return netip.Addr{}, fmt.Errorf("%w: ipv6", ErrIPNotFoundForVersion) } -func (f *Fetcher) ip(ctx context.Context, client Client) ( +func (f *Fetcher) ip(ctx context.Context, network string) ( publicIPs []netip.Addr, err error) { index := int(atomic.AddUint32(f.ring.counter, 1)) % len(f.ring.providers) - provider := f.ring.providers[index] - return fetch(ctx, client, provider.data()) + providerData := f.ring.providers[index].data() + + client := &dns.Client{ + Net: network + "-tls", + Timeout: f.timeout, + TLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + ServerName: providerData.TLSName, + }, + } + + return fetch(ctx, client, network, providerData) } diff --git a/pkg/publicip/dns/options_test.go b/pkg/publicip/dns/options_test.go index 0d4f6913b..7c9af4612 100644 --- a/pkg/publicip/dns/options_test.go +++ b/pkg/publicip/dns/options_test.go @@ -31,18 +31,18 @@ func Test_SetProviders(t *testing.T) { initialSettings: settings{ providers: []Provider{Cloudflare}, }, - providers: []Provider{Google}, + providers: []Provider{OpenDNS}, expectedSettings: settings{ - providers: []Provider{Google}, + providers: []Provider{OpenDNS}, }, }, - "Google and Cloudflare": { + "OpenDNS and Cloudflare": { initialSettings: settings{ providers: []Provider{Cloudflare}, }, - providers: []Provider{Google, Cloudflare}, + providers: []Provider{OpenDNS, Cloudflare}, expectedSettings: settings{ - providers: []Provider{Cloudflare, Google}, + providers: []Provider{Cloudflare, OpenDNS}, }, }, "invalid provider": { diff --git a/pkg/publicip/dns/providers.go b/pkg/publicip/dns/providers.go index 2742622d1..71faed4ae 100644 --- a/pkg/publicip/dns/providers.go +++ b/pkg/publicip/dns/providers.go @@ -3,6 +3,7 @@ package dns import ( "errors" "fmt" + "net/netip" "github.com/miekg/dns" ) @@ -11,14 +12,12 @@ type Provider string const ( Cloudflare Provider = "cloudflare" - Google Provider = "google" OpenDNS Provider = "opendns" ) func ListProviders() []Provider { return []Provider{ Cloudflare, - Google, OpenDNS, } } @@ -35,35 +34,45 @@ func ValidateProvider(provider Provider) error { } type providerData struct { - nameserver string - fqdn string - class dns.Class - qType dns.Type + // Address for IPv4 or IPv6. + Address string + IPv4 netip.Addr + IPv6 netip.Addr + TLSName string + fqdn string + class dns.Class + qType dns.Type } -func (provider Provider) data() providerData { - switch provider { - case Google: - return providerData{ - nameserver: "ns1.google.com:53", - fqdn: "o-o.myaddr.l.google.com.", - class: dns.ClassINET, - qType: dns.Type(dns.TypeTXT), - } +func (p Provider) data() providerData { + switch p { + // Note on deprecating Google: + // Only their nameserver ns1.google.com returns your public IP address. + // All their other nameservers return the closest Google datacenter IP. + // Unfortunately, ns1.google.com is not compatible with DNS over TLS, + // and dns.google.com is but does not echo your IP address. + // dig TXT @ns1.google.com o-o.myaddr.l.google.com +tls + // dig TXT @dns.google.com o-o.myaddr.l.google.com +tls case Cloudflare: return providerData{ - nameserver: "one.one.one.one:53", - fqdn: "whoami.cloudflare.", - class: dns.ClassCHAOS, - qType: dns.Type(dns.TypeTXT), + Address: "1dot1dot1dot1.cloudflare-dns.com", + IPv4: netip.AddrFrom4([4]byte{1, 1, 1, 1}), + IPv6: netip.AddrFrom16([16]byte{0x26, 0x6, 0x47, 0x0, 0x47, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x11, 0x11}), //nolint:lll + TLSName: "cloudflare-dns.com", + fqdn: "whoami.cloudflare.", + class: dns.ClassCHAOS, + qType: dns.Type(dns.TypeTXT), } case OpenDNS: return providerData{ - nameserver: "resolver1.opendns.com:53", - fqdn: "myip.opendns.com.", - class: dns.ClassINET, - qType: dns.Type(dns.TypeANY), + Address: "dns.opendns.com", + IPv4: netip.AddrFrom4([4]byte{208, 67, 222, 222}), + IPv6: netip.AddrFrom16([16]byte{0x26, 0x20, 0x1, 0x19, 0x0, 0x35, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x35}), //nolint:lll + TLSName: "dns.opendns.com", + fqdn: "myip.opendns.com.", + class: dns.ClassINET, + qType: dns.Type(dns.TypeANY), } } - panic(`provider unknown: "` + string(provider) + `"`) + panic(`provider unknown: "` + string(p) + `"`) } diff --git a/pkg/publicip/dns/providers_test.go b/pkg/publicip/dns/providers_test.go index c00d90e08..bfda27b63 100644 --- a/pkg/publicip/dns/providers_test.go +++ b/pkg/publicip/dns/providers_test.go @@ -2,6 +2,7 @@ package dns import ( "errors" + "net/netip" "testing" "github.com/miekg/dns" @@ -17,7 +18,7 @@ func Test_ValidateProvider(t *testing.T) { err error }{ "valid provider": { - provider: Google, + provider: Cloudflare, }, "invalid provider": { provider: Provider("invalid"), @@ -49,31 +50,28 @@ func Test_data(t *testing.T) { data providerData panicMessage string }{ - "google": { - provider: Google, - data: providerData{ - nameserver: "ns1.google.com:53", - fqdn: "o-o.myaddr.l.google.com.", - class: dns.ClassINET, - qType: dns.Type(dns.TypeTXT), - }, - }, "cloudflare": { provider: Cloudflare, data: providerData{ - nameserver: "one.one.one.one:53", - fqdn: "whoami.cloudflare.", - class: dns.ClassCHAOS, - qType: dns.Type(dns.TypeTXT), + Address: "1dot1dot1dot1.cloudflare-dns.com", + IPv4: netip.AddrFrom4([4]byte{1, 1, 1, 1}), + IPv6: netip.AddrFrom16([16]byte{0x26, 0x6, 0x47, 0x0, 0x47, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x11, 0x11}), //nolint:lll + TLSName: "cloudflare-dns.com", + fqdn: "whoami.cloudflare.", + class: dns.ClassCHAOS, + qType: dns.Type(dns.TypeTXT), }, }, "opendns": { provider: OpenDNS, data: providerData{ - nameserver: "resolver1.opendns.com:53", - fqdn: "myip.opendns.com.", - class: dns.ClassINET, - qType: dns.Type(dns.TypeANY), + Address: "dns.opendns.com", + IPv4: netip.AddrFrom4([4]byte{208, 67, 222, 222}), + IPv6: netip.AddrFrom16([16]byte{0x26, 0x20, 0x1, 0x19, 0x0, 0x35, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x35}), //nolint:lll + TLSName: "dns.opendns.com", + fqdn: "myip.opendns.com.", + class: dns.ClassINET, + qType: dns.Type(dns.TypeANY), }, }, "invalid provider": {