dnsproxy: bind dns proxy to localhost only

pkg/datapath/iptables/iptables.go

@@ -459,7 +459,8 @@ func (m *IptablesManager) iptIngressProxyRule(rules string, prog iptablesInterfa
 	ingressProxyMark := fmt.Sprintf("%#x", linux_defaults.MagicMarkIsToProxy)
 	ingressProxyPort := fmt.Sprintf("%d", proxyPort)
 
-	if strings.Contains(rules, fmt.Sprintf("CILIUM_PRE_mangle -p %s -m mark --mark %s", l4proto, ingressMarkMatch)) {
+	existingRuleRegex := regexp.MustCompile(fmt.Sprintf("CILIUM_PRE_mangle -p %s -m mark --mark %s.*--on-ip %s", l4proto, ingressMarkMatch, ip))
+	if existingRuleRegex.MatchString(rules) {
 		return nil
 	}
 
@@ -489,7 +490,8 @@ func (m *IptablesManager) iptEgressProxyRule(rules string, prog iptablesInterfac
 	egressProxyMark := fmt.Sprintf("%#x", linux_defaults.MagicMarkIsToProxy)
 	egressProxyPort := fmt.Sprintf("%d", proxyPort)
 
-	if strings.Contains(rules, fmt.Sprintf("-A CILIUM_PRE_mangle -p %s -m mark --mark %s", l4proto, egressMarkMatch)) {
+	existingRuleRegex := regexp.MustCompile(fmt.Sprintf("-A CILIUM_PRE_mangle -p %s -m mark --mark %s.*--on-ip %s", l4proto, egressMarkMatch, ip))
+	if existingRuleRegex.MatchString(rules) {
 		return nil
 	}
 
@@ -740,11 +742,11 @@ func (m *IptablesManager) addProxyRules(prog iptablesInterface, ip string, proxy
 
 	// Delete all other rules for this same proxy name
 	// These may accumulate if there is a bind failure on a previously used port
-	portMatch := fmt.Sprintf("TPROXY --on-port %d ", proxyPort)
+	portAndIPMatch := fmt.Sprintf("TPROXY --on-port %d --on-ip %s ", proxyPort, ip)
 	scanner := bufio.NewScanner(strings.NewReader(rules))
 	for scanner.Scan() {
 		rule := scanner.Text()
-		if !strings.Contains(rule, "-A CILIUM_PRE_mangle ") || !strings.Contains(rule, "cilium: TPROXY to host "+name) || strings.Contains(rule, portMatch) {
+		if !strings.Contains(rule, "-A CILIUM_PRE_mangle ") || !strings.Contains(rule, "cilium: TPROXY to host "+name) || strings.Contains(rule, portAndIPMatch) {
 			continue
 		}
 

pkg/datapath/iptables/iptables_test.go

@@ -297,6 +297,47 @@ func (s *iptablesTestSuite) TestAddProxyRulesv4(c *check.C) {
 	mockManager.addProxyRules(mockIp4tables, "0.0.0.0", 37379, false, "cilium-dns-egress")
 	err = mockIp4tables.checkExpectations()
 	c.Assert(err, check.IsNil)
+
+	mockIp4tables.expectations = []expectation{
+		{
+			args: "-t mangle -S",
+			out: []byte(
+				`-P PREROUTING ACCEPT
+-P INPUT ACCEPT
+-P FORWARD ACCEPT
+-P OUTPUT ACCEPT
+-P POSTROUTING ACCEPT
+-N OLD_CILIUM_POST_mangle
+-N OLD_CILIUM_PRE_mangle
+-N CILIUM_POST_mangle
+-N CILIUM_PRE_mangle
+-N KUBE-KUBELET-CANARY
+-N KUBE-PROXY-CANARY
+-A PREROUTING -m comment --comment "cilium-feeder: CILIUM_PRE_mangle" -j OLD_CILIUM_PRE_mangle
+-A POSTROUTING -m comment --comment "cilium-feeder: CILIUM_POST_mangle" -j OLD_CILIUM_POST_mangle
+-A PREROUTING -m comment --comment "cilium-feeder: CILIUM_PRE_mangle" -j CILIUM_PRE_mangle
+-A POSTROUTING -m comment --comment "cilium-feeder: CILIUM_POST_mangle" -j CILIUM_POST_mangle
+-A OLD_CILIUM_PRE_mangle -m socket --transparent -m comment --comment "cilium: any->pod redirect proxied traffic to host proxy" -j MARK --set-xmark 0x200/0xffffffff
+-A OLD_CILIUM_PRE_mangle -p tcp -m mark --mark 0x3920200 -m comment --comment "cilium: TPROXY to host cilium-dns-egress proxy" -j TPROXY --on-port 37379 --on-ip 0.0.0.0 --tproxy-mark 0x200/0xffffffff
+-A OLD_CILIUM_PRE_mangle -p udp -m mark --mark 0x3920200 -m comment --comment "cilium: TPROXY to host cilium-dns-egress proxy" -j TPROXY --on-port 37379 --on-ip 0.0.0.0 --tproxy-mark 0x200/0xffffffff
+-A CILIUM_PRE_mangle -p tcp -m mark --mark 0x3920200 -m comment --comment "cilium: TPROXY to host cilium-dns-egress proxy" -j TPROXY --on-port 37379 --on-ip 0.0.0.0 --tproxy-mark 0x200/0xffffffff
+-A CILIUM_PRE_mangle -p udp -m mark --mark 0x3920200 -m comment --comment "cilium: TPROXY to host cilium-dns-egress proxy" -j TPROXY --on-port 37379 --on-ip 0.0.0.0 --tproxy-mark 0x200/0xffffffff
+`),
+		}, {
+			args: "-t mangle -A CILIUM_PRE_mangle -p tcp -m mark --mark 0x3920200 -m comment --comment cilium: TPROXY to host cilium-dns-egress proxy -j TPROXY --tproxy-mark 0x200 --on-ip 127.0.0.1 --on-port 37379",
+		}, {
+			args: "-t mangle -A CILIUM_PRE_mangle -p udp -m mark --mark 0x3920200 -m comment --comment cilium: TPROXY to host cilium-dns-egress proxy -j TPROXY --tproxy-mark 0x200 --on-ip 127.0.0.1 --on-port 37379",
+		}, {
+			args: "-t mangle -D CILIUM_PRE_mangle -p tcp -m mark --mark 0x3920200 -m comment --comment cilium: TPROXY to host cilium-dns-egress proxy -j TPROXY --on-port 37379 --on-ip 0.0.0.0 --tproxy-mark 0x200/0xffffffff",
+		}, {
+			args: "-t mangle -D CILIUM_PRE_mangle -p udp -m mark --mark 0x3920200 -m comment --comment cilium: TPROXY to host cilium-dns-egress proxy -j TPROXY --on-port 37379 --on-ip 0.0.0.0 --tproxy-mark 0x200/0xffffffff",
+		},
+	}
+
+	// Same port number, new IP, adds new ones, deletes stale rules. Does not touch OLD_ chains
+	mockManager.addProxyRules(mockIp4tables, "127.0.0.1", 37379, false, "cilium-dns-egress")
+	err = mockIp4tables.checkExpectations()
+	c.Assert(err, check.IsNil)
 }
 
 func (s *iptablesTestSuite) TestGetProxyPort(c *check.C) {

pkg/fqdn/dnsproxy/ipfamily.go

@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Authors of Cilium
+
+package dnsproxy
+
+type ipFamily struct {
+	Name        string
+	IPv4Enabled bool
+	IPv6Enabled bool
+	UDPAddress  string
+	TCPAddress  string
+	Localhost   string
+}
+
+func ipv4Family() ipFamily {
+	return ipFamily{
+		Name:        "ipv4",
+		IPv4Enabled: true,
+		IPv6Enabled: false,
+		UDPAddress:  "udp4",
+		TCPAddress:  "tcp4",
+		Localhost:   "127.0.0.1",
+	}
+}
+
+func ipv6Family() ipFamily {
+	return ipFamily{
+		Name:        "ipv6",
+		IPv4Enabled: false,
+		IPv6Enabled: true,
+		UDPAddress:  "udp6",
+		TCPAddress:  "tcp6",
+		Localhost:   "::1",
+	}
+}

pkg/fqdn/dnsproxy/proxy.go

@@ -59,12 +59,6 @@ const (
 // A singleton is always running inside cilium-agent.
 // Note: All public fields are read only and do not require locking
 type DNSProxy struct {
-	// BindAddr is the local address the server is using to listen for DNS
-	// requests. This is a read-only value and reflects the actual value. Passing
-	// ":0" to StartDNSProxy will allow the kernel to set the port, and that can
-	// be read here.
-	BindAddr string
-
 	// BindPort is the port in BindAddr.
 	BindPort uint16
 
@@ -94,9 +88,11 @@ type DNSProxy struct {
 	// design now.
 	NotifyOnDNSMsg NotifyOnDNSMsgFunc
 
-	// UDPServer, TCPServer are the miekg/dns server instances. They handle DNS
-	// parsing etc. for us.
-	UDPServer, TCPServer *dns.Server
+	// DNSServers are the miekg/dns server instances.
+	// Depending on the configuration, these might be
+	// TCPv4, UDPv4, TCPv6 and/or UDPv4.
+	// They handle DNS parsing etc. for us.
+	DNSServers []*dns.Server
 
 	// EnableDNSCompression allows the DNS proxy to compress responses to
 	// endpoints that are larger than 512 Bytes or the EDNS0 option, if present.
@@ -638,60 +634,67 @@ func StartDNSProxy(
 	}
 	atomic.StoreInt32(&p.rejectReply, dns.RcodeRefused)
 
-	// Start the DNS listeners on UDP and TCP
+	// Start the DNS listeners on UDP and TCP for IPv4 and/or IPv6
 	var (
-		UDPConn     *net.UDPConn
-		TCPListener *net.TCPListener
-		err         error
-
-		EnableIPv4, EnableIPv6 = option.Config.EnableIPv4, option.Config.EnableIPv6
+		dnsServers []*dns.Server
+		bindPort   uint16
+		err        error
 	)
 
 	start := time.Now()
 	for time.Since(start) < ProxyBindTimeout {
-		UDPConn, TCPListener, err = bindToAddr(address, port, EnableIPv4, EnableIPv6)
+		dnsServers, bindPort, err = bindToAddr(address, port, p, option.Config.EnableIPv4, option.Config.EnableIPv6)
 		if err == nil {
 			break
 		}
 		log.WithError(err).Warnf("Attempt to bind DNS Proxy failed, retrying in %v", ProxyBindRetryInterval)
 		time.Sleep(ProxyBindRetryInterval)
 	}
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to bind DNS proxy: %w", err)
 	}
 
-	p.BindAddr = UDPConn.LocalAddr().String()
-	p.BindPort = uint16(UDPConn.LocalAddr().(*net.UDPAddr).Port)
-	p.UDPServer = &dns.Server{PacketConn: UDPConn, Addr: p.BindAddr, Net: "udp", Handler: p,
-		SessionUDPFactory: &sessionUDPFactory{ipv4Enabled: EnableIPv4, ipv6Enabled: EnableIPv6},
-	}
-	p.TCPServer = &dns.Server{Listener: TCPListener, Addr: p.BindAddr, Net: "tcp", Handler: p}
-	log.WithField("address", p.BindAddr).Debug("DNS Proxy bound to address")
+	p.BindPort = bindPort
+	p.DNSServers = dnsServers
+
+	log.WithField("port", bindPort).WithField("addresses", len(dnsServers)).Debug("DNS Proxy bound to addresses")
 
-	for _, s := range []*dns.Server{p.UDPServer, p.TCPServer} {
+	for _, s := range p.DNSServers {
 		go func(server *dns.Server) {
 			// try 5 times during a single ProxyBindTimeout period. We fatal here
 			// because we have no other way to indicate failure this late.
 			start := time.Now()
+			var err error
 			for time.Since(start) < ProxyBindTimeout {
-				if err := server.ActivateAndServe(); err != nil {
+				log.Debugf("Trying to start the %s DNS proxy on %s", server.Net, server.Addr)
+
+				if err = server.ActivateAndServe(); err != nil {
 					log.WithError(err).Errorf("Failed to start the %s DNS proxy on %s", server.Net, server.Addr)
+					time.Sleep(ProxyBindRetryInterval)
+					continue
 				}
-				time.Sleep(ProxyBindRetryInterval)
+				break // successful shutdown before timeout
+			}
+			if err != nil {
+				log.WithError(err).Fatalf("Failed to start the %s DNS proxy on %s", server.Net, server.Addr)
 			}
-			log.Fatalf("Failed to start %s DNS Proxy on %s", server.Net, server.Addr)
 		}(s)
 	}
 
 	// This function is called in proxy.Cleanup, which is added to Daemon cleanup module in bootstrapFQDN
-	p.unbindAddress = func() {
-		UDPConn.Close()
-		TCPListener.Close()
-	}
+	p.unbindAddress = func() { shutdownServers(p.DNSServers) }
 
 	return p, nil
 }
 
+func shutdownServers(dnsServers []*dns.Server) {
+	for _, s := range dnsServers {
+		if err := s.Shutdown(); err != nil {
+			log.WithError(err).Errorf("Failed to stop the %s DNS proxy on %s", s.Net, s.Addr)
+		}
+	}
+}
+
 // LookupEndpointByIP wraps LookupRegisteredEndpoint by falling back to an restored EP, if available
 func (p *DNSProxy) LookupEndpointByIP(ip net.IP) (endpoint *endpoint.Endpoint, err error) {
 	endpoint, err = p.LookupRegisteredEndpoint(ip)
@@ -1083,42 +1086,72 @@ func ExtractMsgDetails(msg *dns.Msg) (qname string, responseIPs []net.IP, TTL ui
 	return qname, responseIPs, TTL, CNAMEs, msg.Rcode, answerTypes, qTypes, nil
 }
 
-// bindToAddr attempts to bind to address and port for both UDP and TCP. If
-// port is 0 a random open port is assigned and the same one is used for UDP
-// and TCP.
+// bindToAddr attempts to bind to address and port for both UDP and TCP on IPv4 and/or IPv6.
+// If address is empty it automatically binds to the loopback interfaces on IPv4 and/or IPv6.
+// If port is 0 a random open port is assigned and the same one is used for UDP and TCP.
 // Note: This mimics what the dns package does EXCEPT for setting reuseport.
 // This is ok for now but it would simplify proxy management in the future to
 // have it set.
-func bindToAddr(address string, port uint16, ipv4, ipv6 bool) (*net.UDPConn, *net.TCPListener, error) {
-	var err error
-	var listener net.Listener
-	var conn net.PacketConn
+func bindToAddr(address string, port uint16, handler dns.Handler, ipv4, ipv6 bool) (dnsServers []*dns.Server, bindPort uint16, err error) {
 	defer func() {
 		if err != nil {
-			if listener != nil {
-				listener.Close()
-			}
-			if conn != nil {
-				conn.Close()
-			}
+			shutdownServers(dnsServers)
 		}
 	}()
 
-	bindAddr := net.JoinHostPort(address, strconv.Itoa(int(port)))
+	// Global singleton sessionUDPFactory which is used for IPv4 & IPv6
+	sessUdpFactory := &sessionUDPFactory{ipv4Enabled: ipv4, ipv6Enabled: ipv6}
 
-	listener, err = listenConfig(linux_defaults.MagicMarkEgress, ipv4, ipv6).Listen(context.Background(),
-		"tcp", bindAddr)
-	if err != nil {
-		return nil, nil, err
+	var ipFamilies []ipFamily
+	if ipv4 {
+		ipFamilies = append(ipFamilies, ipv4Family())
+	}
+	if ipv6 {
+		ipFamilies = append(ipFamilies, ipv6Family())
 	}
 
-	conn, err = listenConfig(linux_defaults.MagicMarkEgress, ipv4, ipv6).ListenPacket(context.Background(),
-		"udp", listener.Addr().String())
-	if err != nil {
-		return nil, nil, err
+	for _, ipf := range ipFamilies {
+		lc := listenConfig(linux_defaults.MagicMarkEgress, ipf.IPv4Enabled, ipf.IPv6Enabled)
+
+		tcpListener, err := lc.Listen(context.Background(), ipf.TCPAddress, evaluateAddress(address, port, bindPort, ipf))
+		if err != nil {
+			return nil, 0, fmt.Errorf("failed to listen on %s: %w", ipf.TCPAddress, err)
+		}
+		dnsServers = append(dnsServers, &dns.Server{
+			Listener: tcpListener, Handler: handler,
+			// Net & Addr are only set for logging purposes and aren't used if using ActivateAndServe.
+			Net: ipf.TCPAddress, Addr: tcpListener.Addr().String(),
+		})
+
+		bindPort = uint16(tcpListener.Addr().(*net.TCPAddr).Port)
+
+		udpConn, err := lc.ListenPacket(context.Background(), ipf.UDPAddress, evaluateAddress(address, port, bindPort, ipf))
+		if err != nil {
+			return nil, 0, fmt.Errorf("failed to listen on %s: %w", ipf.UDPAddress, err)
+		}
+		dnsServers = append(dnsServers, &dns.Server{
+			PacketConn: udpConn, Handler: handler, SessionUDPFactory: sessUdpFactory,
+			// Net & Addr are only set for logging purposes and aren't used if using ActivateAndServe.
+			Net: ipf.UDPAddress, Addr: udpConn.LocalAddr().String(),
+		})
+	}
+
+	return dnsServers, bindPort, nil
+}
+
+func evaluateAddress(address string, port uint16, bindPort uint16, ipFamily ipFamily) string {
+	addr := ipFamily.Localhost
+
+	if address != "" {
+		addr = address
 	}
 
-	return conn.(*net.UDPConn), listener.(*net.TCPListener), nil
+	if bindPort == 0 {
+		return net.JoinHostPort(addr, strconv.Itoa(int(port)))
+	} else {
+		// Already bound to a port by a previous server -> reuse same port
+		return net.JoinHostPort(addr, strconv.Itoa(int(bindPort)))
+	}
 }
 
 // shouldCompressResponse returns true when the response needs to be compressed