DNS_UPSTREAM_PLAIN_ADDRESSES option

- New CSV format with port, for example `ip1:port1,ip2:port2`
- retrocompatibility with `DNS_ADDRESS`. If set, force upstream type to plain and empty user-picked providers. 127.0.0.1 is now ignored since it's always set to this value internally.
- requires `DNS_UPSTREAM_TYPE=plain` must be set to use `DNS_UPSTREAM_PLAIN_ADDRESSES` (unless using retro `DNS_ADDRESS`)
- Warning log on using private upstream resolvers updated

Dockerfile

@@ -179,7 +179,7 @@ ENV VPN_SERVICE_PROVIDER=pia \
     DNS_UNBLOCK_HOSTNAMES= \
     DNS_REBINDING_PROTECTION_EXEMPT_HOSTNAMES= \
     DNS_UPDATE_PERIOD=24h \
-    DNS_ADDRESS=127.0.0.1 \
+    DNS_UPSTREAM_PLAIN_ADDRESSES= \
     # HTTP proxy
     HTTPPROXY= \
     HTTPPROXY_LOG=off \

internal/configuration/settings/dns.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"net/netip"
+	"slices"
 	"time"
 
 	"github.com/qdm12/dns/v2/pkg/provider"
@@ -13,16 +14,25 @@ import (
 	"github.com/qdm12/gotree"
 )
 
+const (
+	dnsUpstreamTypeDot   = "dot"
+	dnsUpstreamTypeDoh   = "doh"
+	dnsUpstreamTypePlain = "plain"
+)
+
 // DNS contains settings to configure DNS.
 type DNS struct {
-	// UpstreamType can be dot or plain, and defaults to dot.
+	// UpstreamType can be [dnsUpstreamTypeDot], [dnsUpstreamTypeDoh]
+	// or [dnsUpstreamTypePlain]. It defaults to [dnsUpstreamTypeDot].
 	UpstreamType string `json:"upstream_type"`
 	// UpdatePeriod is the period to update DNS block lists.
 	// It can be set to 0 to disable the update.
 	// It defaults to 24h and cannot be nil in
 	// the internal state.
 	UpdatePeriod *time.Duration
-	// Providers is a list of DNS providers
+	// Providers is a list of DNS providers.
+	// It defaults to either ["cloudflare"] or [] if the
+	// UpstreamPlainAddresses field is set.
 	Providers []string `json:"providers"`
 	// Caching is true if the server should cache
 	// DNS responses.
@@ -32,21 +42,23 @@ type DNS struct {
 	// Blacklist contains settings to configure the filter
 	// block lists.
 	Blacklist DNSBlacklist
-	// ServerAddress is the DNS server to use inside
+	// UpstreamPlainAddresses are the upstream plaintext DNS resolver
-	// the Go program and for the system.
+	// addresses to use by the built-in DNS server forwarder.
-	// It defaults to '127.0.0.1' to be used with the
+	// Note, if the upstream type is [dnsUpstreamTypePlain] these are merged
-	// local server. It cannot be the zero value in the internal
+	// together with provider names set in the Providers field.
-	// state.
+	// If this field is set, the Providers field will default to the empty slice.
-	ServerAddress netip.Addr
+	UpstreamPlainAddresses []netip.AddrPort
 }
 
 var (
 	ErrDNSUpstreamTypeNotValid = errors.New("DNS upstream type is not valid")
 	ErrDNSUpdatePeriodTooShort = errors.New("update period is too short")
+	ErrDNSUpstreamPlainNoIPv6  = errors.New("upstream plain addresses do not contain any IPv6 address")
+	ErrDNSUpstreamPlainNoIPv4  = errors.New("upstream plain addresses do not contain any IPv4 address")
 )
 
 func (d DNS) validate() (err error) {
-	if !helpers.IsOneOf(d.UpstreamType, "dot", "doh", "plain") {
+	if !helpers.IsOneOf(d.UpstreamType, dnsUpstreamTypeDot, dnsUpstreamTypeDoh, dnsUpstreamTypePlain) {
 		return fmt.Errorf("%w: %s", ErrDNSUpstreamTypeNotValid, d.UpstreamType)
 	}
 
@@ -64,6 +76,18 @@ func (d DNS) validate() (err error) {
 		}
 	}
 
+	if d.UpstreamType == dnsUpstreamTypePlain {
+		if *d.IPv6 && !slices.ContainsFunc(d.UpstreamPlainAddresses, func(addrPort netip.AddrPort) bool {
+			return addrPort.Addr().Is6()
+		}) {
+			return fmt.Errorf("%w: in %d addresses", ErrDNSUpstreamPlainNoIPv6, len(d.UpstreamPlainAddresses))
+		} else if !slices.ContainsFunc(d.UpstreamPlainAddresses, func(addrPort netip.AddrPort) bool {
+			return addrPort.Addr().Is4()
+		}) {
+			return fmt.Errorf("%w: in %d addresses", ErrDNSUpstreamPlainNoIPv4, len(d.UpstreamPlainAddresses))
+		}
+	}
+
 	err = d.Blacklist.validate()
 	if err != nil {
 		return err
@@ -80,7 +104,7 @@ func (d *DNS) Copy() (copied DNS) {
 		Caching:                gosettings.CopyPointer(d.Caching),
 		IPv6:                   gosettings.CopyPointer(d.IPv6),
 		Blacklist:              d.Blacklist.copy(),
-		ServerAddress: d.ServerAddress,
+		UpstreamPlainAddresses: d.UpstreamPlainAddresses,
 	}
 }
 
@@ -94,11 +118,11 @@ func (d *DNS) overrideWith(other DNS) {
 	d.Caching = gosettings.OverrideWithPointer(d.Caching, other.Caching)
 	d.IPv6 = gosettings.OverrideWithPointer(d.IPv6, other.IPv6)
 	d.Blacklist.overrideWith(other.Blacklist)
-	d.ServerAddress = gosettings.OverrideWithValidator(d.ServerAddress, other.ServerAddress)
+	d.UpstreamPlainAddresses = gosettings.OverrideWithSlice(d.UpstreamPlainAddresses, other.UpstreamPlainAddresses)
 }
 
 func (d *DNS) setDefaults() {
-	d.UpstreamType = gosettings.DefaultComparable(d.UpstreamType, "dot")
+	d.UpstreamType = gosettings.DefaultComparable(d.UpstreamType, dnsUpstreamTypeDot)
 	const defaultUpdatePeriod = 24 * time.Hour
 	d.UpdatePeriod = gosettings.DefaultPointer(d.UpdatePeriod, defaultUpdatePeriod)
 	d.Providers = gosettings.DefaultSlice(d.Providers, []string{
@@ -107,26 +131,54 @@ func (d *DNS) setDefaults() {
 	d.Caching = gosettings.DefaultPointer(d.Caching, true)
 	d.IPv6 = gosettings.DefaultPointer(d.IPv6, false)
 	d.Blacklist.setDefaults()
-	d.ServerAddress = gosettings.DefaultValidator(d.ServerAddress,
+	d.UpstreamPlainAddresses = gosettings.DefaultSlice(d.UpstreamPlainAddresses, []netip.AddrPort{})
-		netip.AddrFrom4([4]byte{127, 0, 0, 1}))
+}
+
+func defaultDNSProviders() []string {
+	return []string{
+		provider.Cloudflare().Name,
+	}
 }
 
 func (d DNS) GetFirstPlaintextIPv4() (ipv4 netip.Addr) {
-	localhost := netip.AddrFrom4([4]byte{127, 0, 0, 1})
+	if d.UpstreamType == dnsUpstreamTypePlain {
-	if d.ServerAddress.Compare(localhost) != 0 && d.ServerAddress.Is4() {
+		for _, addrPort := range d.UpstreamPlainAddresses {
-		return d.ServerAddress
+			if addrPort.Addr().Is4() {
+				return addrPort.Addr()
+			}
+		}
 	}
 
+	ipv4 = findPlainIPv4InProviders(d.Providers)
+	if ipv4.IsValid() {
+		return ipv4
+	}
+
+	// Either:
+	// - all upstream plain addresses are IPv6 and no provider is set
+	// - all providers set do not have a plaintext IPv4 address
+	ipv4 = findPlainIPv4InProviders(defaultDNSProviders())
+	if !ipv4.IsValid() {
+		panic("no plaintext IPv4 address found in default DNS providers")
+	}
+	return ipv4
+}
+
+func findPlainIPv4InProviders(providerNames []string) netip.Addr {
 	providers := provider.NewProviders()
-	provider, err := providers.Get(d.Providers[0])
+	for _, name := range providerNames {
+		provider, err := providers.Get(name)
 		if err != nil {
 			// Settings should be validated before calling this function,
 			// so an error happening here is a programming error.
 			panic(err)
 		}
-
+		if len(provider.Plain.IPv4) > 0 {
 			return provider.Plain.IPv4[0].Addr()
 		}
+	}
+	return netip.Addr{}
+}
 
 func (d DNS) String() string {
 	return d.toLinesNode().String()
@@ -134,14 +186,23 @@ 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("Upstream resolver type: %s", d.UpstreamType)
 
 	upstreamResolvers := node.Append("Upstream resolvers:")
+	if len(d.UpstreamPlainAddresses) > 0 {
+		if d.UpstreamType == dnsUpstreamTypePlain {
+			for _, addr := range d.UpstreamPlainAddresses {
+				upstreamResolvers.Append(addr.String())
+			}
+		} else {
+			node.Appendf("Upstream plain addresses: ignored because upstream type is not plain")
+		}
+	} else {
 		for _, provider := range d.Providers {
 			upstreamResolvers.Append(provider)
 		}
+	}
 
 	node.Appendf("Caching: %s", gosettings.BoolToYesNo(d.Caching))
 	node.Appendf("IPv6: %s", gosettings.BoolToYesNo(d.IPv6))
@@ -182,10 +243,43 @@ func (d *DNS) read(r *reader.Reader) (err error) {
 		return err
 	}
 
-	d.ServerAddress, err = r.NetipAddr("DNS_ADDRESS", reader.RetroKeys("DNS_PLAINTEXT_ADDRESS"))
+	err = d.readUpstreamPlainAddresses(r)
 	if err != nil {
 		return err
 	}
 
 	return nil
 }
+
+func (d *DNS) readUpstreamPlainAddresses(r *reader.Reader) (err error) {
+	// If DNS_UPSTREAM_PLAIN_ADDRESSES is set, the user must also set DNS_UPSTREAM_TYPE=plain
+	// for these to be used. This is an added safety measure to reduce misunderstandings, and
+	// reduce odd settings overrides.
+	d.UpstreamPlainAddresses, err = r.CSVNetipAddrPorts("DNS_UPSTREAM_PLAIN_ADDRESSES")
+	if err != nil {
+		return err
+	}
+
+	// Retro-compatibility - remove in v4
+	// If DNS_ADDRESS is set to a non-localhost address, append it to the other
+	// upstream plain addresses, assuming port 53, and force the upstream type to plain AND
+	// clear any user picked providers, to maintain retro-compatibility behavior.
+	serverAddress, err := r.NetipAddr("DNS_ADDRESS",
+		reader.RetroKeys("DNS_PLAINTEXT_ADDRESS"),
+		reader.IsRetro("DNS_UPSTREAM_PLAIN_ADDRESSES"))
+	if err != nil {
+		return err
+	} else if !serverAddress.IsValid() {
+		return nil
+	}
+	isLocalhost := serverAddress.Compare(netip.AddrFrom4([4]byte{127, 0, 0, 1})) == 0
+	if isLocalhost {
+		return nil
+	}
+	const defaultPlainPort = 53
+	addrPort := netip.AddrPortFrom(serverAddress, defaultPlainPort)
+	d.UpstreamPlainAddresses = append(d.UpstreamPlainAddresses, addrPort)
+	d.UpstreamType = dnsUpstreamTypePlain
+	d.Providers = []string{}
+	return nil
+}

internal/configuration/settings/dns_test.go

@@ -0,0 +1,26 @@
+package settings
+
+import (
+	"testing"
+
+	"github.com/qdm12/dns/v2/pkg/provider"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_defaultDNSProviders(t *testing.T) {
+	t.Parallel()
+
+	names := defaultDNSProviders()
+
+	found := false
+	providers := provider.NewProviders()
+	for _, name := range names {
+		provider, err := providers.Get(name)
+		require.NoError(t, err)
+		if len(provider.Plain.IPv4) > 0 {
+			found = true
+			break
+		}
+	}
+	require.True(t, found, "no default DNS provider has a plaintext IPv4 address")
+}

internal/configuration/settings/settings.go

@@ -2,7 +2,6 @@ package settings
 
 import (
 	"fmt"
-	"net/netip"
 
 	"github.com/qdm12/gluetun/internal/configuration/settings/helpers"
 	"github.com/qdm12/gluetun/internal/constants/providers"
@@ -174,13 +173,11 @@ func (s Settings) Warnings() (warnings []string) {
 			"by creating an issue, attaching the new certificate and we will update Gluetun.")
 	}
 
-	// TODO remove in v4
+	for _, upstreamAddress := range s.DNS.UpstreamPlainAddresses {
-	if s.DNS.ServerAddress.Unmap().Compare(netip.AddrFrom4([4]byte{127, 0, 0, 1})) != 0 {
+		if upstreamAddress.Addr().IsPrivate() {
-		warnings = append(warnings, "DNS address is set to "+s.DNS.ServerAddress.String()+
+			warnings = append(warnings, "DNS upstream address "+upstreamAddress.String()+" is private: "+
-			" so the local forwarding DNS server will not be used."+ // xxx
+				"DNS traffic might leak out of the VPN tunnel to that address.")
-			" The default value changed to 127.0.0.1 so it uses the internal DNS server."+
+		}
-			" If this server fails to start, the IPv4 address of the first plaintext DNS server"+
-			" corresponding to the first DNS provider chosen is used.")
 	}
 
 	return warnings

internal/configuration/settings/settings_test.go

@@ -38,7 +38,6 @@ 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
 |   ├── Upstream resolver type: dot
 |   ├── Upstream resolvers:
 |   |   └── Cloudflare

internal/dns/settings.go

@@ -33,15 +33,7 @@ func buildServerSettings(settings settings.DNS,
 ) {
 	serverSettings.Logger = logger
 
-	providersData := provider.NewProviders()
+	upstreamResolvers := buildProviders(settings)
-	upstreamResolvers := make([]provider.Provider, len(settings.Providers))
-	for i := range settings.Providers {
-		var err error
-		upstreamResolvers[i], err = providersData.Get(settings.Providers[i])
-		if err != nil {
-			panic(err) // this should already had been checked
-		}
-	}
 
 	ipVersion := "ipv4"
 	if *settings.IPv6 {
@@ -123,3 +115,47 @@ func buildServerSettings(settings settings.DNS,
 
 	return serverSettings, nil
 }
+
+func buildProviders(settings settings.DNS) []provider.Provider {
+	if settings.UpstreamType == "plain" && len(settings.UpstreamPlainAddresses) > 0 {
+		providers := make([]provider.Provider, len(settings.UpstreamPlainAddresses))
+		for i, addrPort := range settings.UpstreamPlainAddresses {
+			providers[i] = provider.Provider{
+				Name: addrPort.String(),
+			}
+			if addrPort.Addr().Is4() {
+				providers[i].Plain.IPv4 = []netip.AddrPort{addrPort}
+			} else {
+				providers[i].Plain.IPv6 = []netip.AddrPort{addrPort}
+			}
+		}
+	}
+
+	providersData := provider.NewProviders()
+	providers := make([]provider.Provider, 0, len(settings.Providers)+len(settings.UpstreamPlainAddresses))
+	for _, providerName := range settings.Providers {
+		provider, err := providersData.Get(providerName)
+		if err != nil {
+			panic(err) // this should already had been checked
+		}
+		providers = append(providers, provider)
+	}
+
+	if settings.UpstreamType != "plain" {
+		return providers
+	}
+
+	for _, addrPort := range settings.UpstreamPlainAddresses {
+		newProvider := provider.Provider{
+			Name: addrPort.String(),
+		}
+		if addrPort.Addr().Is4() {
+			newProvider.Plain.IPv4 = []netip.AddrPort{addrPort}
+		} else {
+			newProvider.Plain.IPv6 = []netip.AddrPort{addrPort}
+		}
+		providers = append(providers, newProvider)
+	}
+
+	return providers
+}

internal/dns/setup.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net/netip"
 
 	"github.com/qdm12/dns/v2/pkg/check"
 	"github.com/qdm12/dns/v2/pkg/nameserver"
@@ -38,12 +37,8 @@ func (l *Loop) setupServer(ctx context.Context) (runError <-chan error, err erro
 	l.server = server
 
 	// use internal DNS server
-	const defaultDNSPort = 53
+	nameserver.UseDNSInternally(nameserver.SettingsInternalDNS{})
-	nameserver.UseDNSInternally(nameserver.SettingsInternalDNS{
-		AddrPort: netip.AddrPortFrom(settings.ServerAddress, defaultDNSPort),
-	})
 	err = nameserver.UseDNSSystemWide(nameserver.SettingsSystemDNS{
-		IPs:        []netip.Addr{settings.ServerAddress},
 		ResolvPath: l.resolvConf,
 	})
 	if err != nil {