From e556871e8b2445f643101669b6b3aa540f59f0e3 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Fri, 11 Aug 2023 10:42:44 +0000 Subject: [PATCH] change(dns): `DNS_KEEP_NAMESERVER` leaves DNS fully untouched --- cmd/gluetun/main.go | 2 +- internal/configuration/settings/dns.go | 17 ++++++++++++----- .../configuration/settings/settings_test.go | 2 +- internal/dns/plaintext.go | 6 ++++-- internal/dns/run.go | 17 ++++++++++++----- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/cmd/gluetun/main.go b/cmd/gluetun/main.go index f9c4fd61..5b43ae04 100644 --- a/cmd/gluetun/main.go +++ b/cmd/gluetun/main.go @@ -381,7 +381,7 @@ func _main(ctx context.Context, buildInfo models.BuildInformation, "port forwarding", goroutine.OptionTimeout(time.Second)) go portForwardLooper.Run(portForwardCtx, portForwardDone) - unboundLogger := logger.New(log.SetComponent("dns over tls")) + unboundLogger := logger.New(log.SetComponent("dns")) unboundLooper := dns.NewLoop(dnsConf, allSettings.DNS, httpClient, unboundLogger) dnsHandler, dnsCtx, dnsDone := goshutdown.NewGoRoutineHandler( diff --git a/internal/configuration/settings/dns.go b/internal/configuration/settings/dns.go index c48b951c..b307f0df 100644 --- a/internal/configuration/settings/dns.go +++ b/internal/configuration/settings/dns.go @@ -16,10 +16,14 @@ type DNS struct { // DoT server. It cannot be the zero value in the internal // state. ServerAddress netip.Addr - // KeepNameserver is true if the Docker DNS server - // found in /etc/resolv.conf should be kept. - // Note settings this to true will go around the - // DoT server blocking. + // KeepNameserver is true if the existing DNS server + // found in /etc/resolv.conf should be used + // Note setting this to true will likely DNS traffic + // outside the VPN tunnel since it would go through + // the local DNS server of your Docker/Kubernetes + // configuration, which is likely not going through the tunnel. + // This will also disable the DNS over TLS server and the + // `ServerAddress` field will be ignored. // It defaults to false and cannot be nil in the // internal state. KeepNameserver *bool @@ -75,8 +79,11 @@ func (d DNS) String() string { func (d DNS) toLinesNode() (node *gotree.Node) { node = gotree.New("DNS settings:") - node.Appendf("DNS server address to use: %s", d.ServerAddress) node.Appendf("Keep existing nameserver(s): %s", gosettings.BoolToYesNo(d.KeepNameserver)) + if *d.KeepNameserver { + return node + } + node.Appendf("DNS server address to use: %s", d.ServerAddress) node.AppendNode(d.DoT.toLinesNode()) return node } diff --git a/internal/configuration/settings/settings_test.go b/internal/configuration/settings/settings_test.go index 1fc62241..3bbaa24d 100644 --- a/internal/configuration/settings/settings_test.go +++ b/internal/configuration/settings/settings_test.go @@ -38,8 +38,8 @@ func Test_Settings_String(t *testing.T) { | ├── Run OpenVPN as: root | └── Verbosity level: 1 ├── DNS settings: -| ├── DNS server address to use: 127.0.0.1 | ├── Keep existing nameserver(s): no +| ├── DNS server address to use: 127.0.0.1 | └── DNS over TLS settings: | ├── Enabled: yes | ├── Update period: every 24h0m0s diff --git a/internal/dns/plaintext.go b/internal/dns/plaintext.go index 57165b57..09eb8277 100644 --- a/internal/dns/plaintext.go +++ b/internal/dns/plaintext.go @@ -19,7 +19,8 @@ func (l *Loop) useUnencryptedDNS(fallback bool) { l.logger.Info("using plaintext DNS at address " + targetIP.String()) } nameserver.UseDNSInternally(targetIP.AsSlice()) - err := nameserver.UseDNSSystemWide(l.resolvConf, targetIP.AsSlice(), *settings.KeepNameserver) + const keepNameserver = false + err := nameserver.UseDNSSystemWide(l.resolvConf, targetIP.AsSlice(), keepNameserver) if err != nil { l.logger.Error(err.Error()) } @@ -39,7 +40,8 @@ func (l *Loop) useUnencryptedDNS(fallback bool) { l.logger.Info("using plaintext DNS at address " + targetIP.String()) } nameserver.UseDNSInternally(targetIP.AsSlice()) - err = nameserver.UseDNSSystemWide(l.resolvConf, targetIP.AsSlice(), *settings.KeepNameserver) + const keepNameserver = false + err = nameserver.UseDNSSystemWide(l.resolvConf, targetIP.AsSlice(), keepNameserver) if err != nil { l.logger.Error(err.Error()) } diff --git a/internal/dns/run.go b/internal/dns/run.go index 4a8ea264..c6a52bba 100644 --- a/internal/dns/run.go +++ b/internal/dns/run.go @@ -10,9 +10,14 @@ import ( func (l *Loop) Run(ctx context.Context, done chan<- struct{}) { defer close(done) - const fallback = false - l.useUnencryptedDNS(fallback) // TODO remove? Use default DNS by default for Docker resolution? - // TODO this one is kept if DNS_KEEP_NAMESERVER=on and should be replaced + if *l.GetSettings().KeepNameserver { + l.logger.Warn("⚠️⚠️⚠️ keeping the default container nameservers, " + + "this will likely leak DNS traffic outside the VPN " + + "and go through your container network DNS outside the VPN tunnel!") + } else { + const fallback = false + l.useUnencryptedDNS(fallback) + } select { case <-l.start: @@ -27,7 +32,8 @@ func (l *Loop) Run(ctx context.Context, done chan<- struct{}) { unboundCancel := func() { waitError <- nil } closeStreams := func() {} - for *l.GetSettings().DoT.Enabled { + settings := l.GetSettings() + for !*settings.KeepNameserver && *settings.DoT.Enabled { var err error unboundCancel, waitError, closeStreams, err = l.setupUnbound(ctx) if err == nil { @@ -50,7 +56,8 @@ func (l *Loop) Run(ctx context.Context, done chan<- struct{}) { l.logAndWait(ctx, err) } - if !*l.GetSettings().DoT.Enabled { + settings = l.GetSettings() + if !*settings.KeepNameserver && !*settings.DoT.Enabled { const fallback = false l.useUnencryptedDNS(fallback) }