fqdn proxy: fix data race by using separate sessionUDPFactories

PR cilium#25309 introduced a data race by sharing the sessionUDPFactory between the
DNS server instances for the different IP families (IPv4 & IPv6). This has been detected
by cilium#27979.

This commit fixes the data race by using dedicated instances of the sessionUDPFactory.

Fixes: cilium#28156

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>

pkg/fqdn/dnsproxy/proxy.go

@@ -242,7 +242,7 @@ func (p *DNSProxy) GetRules(endpointID uint16) (restore.DNSRules, error) {
 
 	portToSelRegex := make(map[uint16][]selRegex)
 	for port, entries := range p.allowed[uint64(endpointID)] {
-		var nidRules = make([]selRegex, 0, len(entries))
+		nidRules := make([]selRegex, 0, len(entries))
 		// Copy the entries to avoid racy map accesses after we release the lock. We don't need
 		// constant time access, hence a preallocated slice instead of another map.
 		for cs, regex := range entries {
@@ -529,6 +529,7 @@ func (e ErrFailedAcquireSemaphore) Timeout() bool { return true }
 
 // Temporary is deprecated. Return false.
 func (e ErrFailedAcquireSemaphore) Temporary() bool { return false }
+
 func (e ErrFailedAcquireSemaphore) Error() string {
 	return fmt.Sprintf(
 		"failed to acquire DNS proxy semaphore, %d parallel requests already in-flight",
@@ -922,7 +923,8 @@ func (p *DNSProxy) ServeDNS(w dns.ResponseWriter, request *dns.Msg) {
 				return err
 			}
 			return soerr
-		}}
+		},
+	}
 	client.Dialer = &dialer
 
 	conn, err := client.Dial(targetServerAddrStr)
@@ -1099,9 +1101,6 @@ func bindToAddr(address string, port uint16, handler dns.Handler, ipv4, ipv6 boo
 		}
 	}()
 
-	// Global singleton sessionUDPFactory which is used for IPv4 & IPv6
-	sessUdpFactory := &sessionUDPFactory{ipv4Enabled: ipv4, ipv6Enabled: ipv6}
-
 	var ipFamilies []ipfamily.IPFamily
 	if ipv4 {
 		ipFamilies = append(ipFamilies, ipfamily.IPv4())
@@ -1130,7 +1129,7 @@ func bindToAddr(address string, port uint16, handler dns.Handler, ipv4, ipv6 boo
 			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,
+			PacketConn: udpConn, Handler: handler, SessionUDPFactory: &sessionUDPFactory{ipv4Enabled: ipf.IPv4Enabled, ipv6Enabled: ipf.IPv6Enabled},
 			// Net & Addr are only set for logging purposes and aren't used if using ActivateAndServe.
 			Net: ipf.UDPAddress, Addr: udpConn.LocalAddr().String(),
 		})

pkg/fqdn/dnsproxy/udp.go

@@ -51,8 +51,10 @@ type sessionUDP struct {
 	oob   []byte
 }
 
-var rawconn4 *net.IPConn // raw socket for sending IPv4
-var rawconn6 *net.IPConn // raw socket for sending IPv6
+var (
+	rawconn4 *net.IPConn // raw socket for sending IPv4
+	rawconn6 *net.IPConn // raw socket for sending IPv6
+)
 
 // Set the socket options needed for tranparent proxying for the listening socket
 // IP(V6)_TRANSPARENT allows socket to receive packets with any destination address/port
@@ -111,7 +113,8 @@ func listenConfig(mark int, ipv4, ipv6 bool) *net.ListenConfig {
 			}
 
 			return opErr
-		}}
+		},
+	}
 }
 
 func bindUDP(addr string, ipv4, ipv6 bool) (*net.IPConn, error) {
@@ -123,10 +126,13 @@ func bindUDP(addr string, ipv4, ipv6 bool) (*net.IPConn, error) {
 	return conn.(*net.IPConn), nil
 }
 
-// NOTE: udpOnce is used in SetSocketOptions below, but assumes we have a
-// global singleton sessionUDPFactory. This is created in StartDNSProxy in
+// NOTE: udpIPv4Once and udpIPv6Once are used in SetSocketOptions below, but assumes we have
+// one global singleton sessionUDPFactory per IP family. These are created in StartDNSProxy in
 // order to have option.Config.EnableIPv{4,6} parsed correctly.
-var udpOnce sync.Once
+var (
+	udpIPv4Once sync.Once
+	udpIPv6Once sync.Once
+)
 
 // SetSocketOptions set's up 'conn' to be used with a SessionUDP.
 func (f *sessionUDPFactory) SetSocketOptions(conn *net.UDPConn) error {
@@ -141,13 +147,19 @@ func (f *sessionUDPFactory) SetSocketOptions(conn *net.UDPConn) error {
 	//   checking that a route exists from the source address before
 	//   the source address is replaced with the (transparently) changed one
 	var err error
-	udpOnce.Do(func() {
+	udpIPv4Once.Do(func() {
 		if f.ipv4Enabled {
 			rawconn4, err = bindUDP("127.0.0.1", true, false) // raw socket for sending IPv4
 			if err != nil {
 				return
 			}
 		}
+	})
+	if err != nil {
+		return fmt.Errorf("failed to open raw UDP IPv4 socket for DNS Proxy: %w", err)
+	}
+
+	udpIPv6Once.Do(func() {
 		if f.ipv6Enabled {
 			rawconn6, err = bindUDP("::1", false, true) // raw socket for sending IPv6
 			if err != nil {
@@ -156,8 +168,9 @@ func (f *sessionUDPFactory) SetSocketOptions(conn *net.UDPConn) error {
 		}
 	})
 	if err != nil {
-		return fmt.Errorf("failed to open raw UDP sockets for DNS Proxy: %w", err)
+		return fmt.Errorf("failed to open raw UDP IPv6 socket for DNS Proxy: %w", err)
 	}
+
 	return nil
 }