Add FQDN TCP DNS support (#4612)

1. Use tp_src=53,tcp_flags=+psh+ack to match the TCP DNS response,
which can skip the handshake packets and match the packet containing
the actual data.

2. To achieve 1., additional OVS fix patch should be applied.

3. While paresing TCP DNS response, we need to trim the TCP option
part to retrieve the data. And while sending it out via packetOut we
should construct the exact packet received by the Antrea agent.

4. Using SendEthPacketOut to send the eth packet via packetOut save us
from retrieving all L2/3/4 info and make sure that we send out the
packet which is exactly the same as what we received.

Signed-off-by: graysonwu <wgrayson@vmware.com>

build/images/ovs/apply-patches.sh

@@ -99,6 +99,13 @@ if version_lt "$OVS_VERSION" "2.18.0" ; then
     apply_patch "78ff3961ca9fb012eaaca3d3af1e8186fe1827e7"
 fi
 
+# This patch fixes the issue that TCP port matching and TCP flags matching can't
+# take effect when using together.
+# See https://github.com/openvswitch/ovs-issues/issues/272
+if version_get "$OVS_VERSION" "2.13.0" && version_let "$OVS_VERSION" "2.17.3" ; then
+    apply_patch "489553b1c21692063931a9f50b6849b23128443c"
+fi
+
 # OVS hardcodes the installation path to /usr/lib/python3.7/dist-packages/ but this location
 # does not seem to be in the Python path in Ubuntu 22.04. There may be a better way to do this,
 # but this seems like an acceptable workaround.

go.mod

@@ -3,8 +3,8 @@ module antrea.io/antrea
 go 1.19
 
 require (
-	antrea.io/libOpenflow v0.9.1
-	antrea.io/ofnet v0.6.5
+	antrea.io/libOpenflow v0.9.2
+	antrea.io/ofnet v0.6.9
 	github.com/ClickHouse/clickhouse-go v1.5.4
 	github.com/DATA-DOG/go-sqlmock v1.5.0
 	github.com/Mellanox/sriovnet v1.1.0
@@ -210,5 +210,3 @@ require (
 	sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect
 )
-
-replace antrea.io/ofnet v0.6.0 => github.com/wenyingd/ofnet v0.0.0-20220817031400-cb451467adc1

go.sum

@@ -1,8 +1,7 @@
-antrea.io/libOpenflow v0.8.1/go.mod h1:CzEJZxDNAupiGxeL5VOw92PsxfyvehEAvE3PiC6gr8o=
-antrea.io/libOpenflow v0.9.1 h1:nrw7EpGuSgi932xriAHdMYGYdLnnjrj91qXGt/bzUUw=
-antrea.io/libOpenflow v0.9.1/go.mod h1:IM9mUfHh5hUNciRRcWYIaWZTlv1TI6QBEHlml7ALdS4=
-antrea.io/ofnet v0.6.5 h1:jMnrU2Iva+jn/j2tyHJ1bSmC7HXtMDYVCJb7pq8L37I=
-antrea.io/ofnet v0.6.5/go.mod h1:/gjpTqhUpyn8uZnef+ytdCCAeY5oGG1jCr/szPUqVXU=
+antrea.io/libOpenflow v0.9.2 h1:9W++nzaxxwY4NxyHHow/4bfum2UPIBJKmEOVTAG+x3o=
+antrea.io/libOpenflow v0.9.2/go.mod h1:IM9mUfHh5hUNciRRcWYIaWZTlv1TI6QBEHlml7ALdS4=
+antrea.io/ofnet v0.6.9 h1:ACoDhFhSHfNtuBKffvptspZDwKe+EQ5i35PuDUZ8svk=
+antrea.io/ofnet v0.6.9/go.mod h1:CB/Pkt+U0Yi1sM7DZ7iS215xGL+dhRRAM0EV0LTDLnY=
 bazil.org/fuse v0.0.0-20160811212531-371fbbdaa898/go.mod h1:Xbm+BRKSBEpa4q4hTSxohYNQpsxXPbPry4JJWOB3LB8=
 cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=

pkg/agent/controller/networkpolicy/fqdn.go

@@ -169,7 +169,7 @@ func newFQDNController(client openflow.Client, allocator *idAllocator, dnsServer
 		gwPort:                 gwPort,
 	}
 	if controller.ofClient != nil {
-		if err := controller.ofClient.NewDNSpacketInConjunction(dnsInterceptRuleID); err != nil {
+		if err := controller.ofClient.NewDNSPacketInConjunction(dnsInterceptRuleID); err != nil {
 			return nil, fmt.Errorf("failed to install flow for DNS response interception: %w", err)
 		}
 	}
@@ -726,7 +726,7 @@ func (f *fqdnController) makeDNSRequest(ctx context.Context, fqdn string) error
 	return nil
 }
 
-// implements openflow.PacketInHandler
+// HandlePacketIn implements openflow.PacketInHandler
 func (f *fqdnController) HandlePacketIn(pktIn *ofctrl.PacketIn) error {
 	matches := pktIn.GetMatches()
 	// Get custom reasons in this packet-in.
@@ -746,30 +746,64 @@ func (f *fqdnController) HandlePacketIn(pktIn *ofctrl.PacketIn) error {
 func (f *fqdnController) handlePacketIn(pktIn *ofctrl.PacketIn) error {
 	klog.V(4).InfoS("Received a packetIn for DNS response")
 	waitCh := make(chan error, 1)
-	handleUDPData := func(dnsPkt *protocol.UDP) {
-		dnsData := dnsPkt.Data
+	handleDNSData := func(dnsData []byte) {
 		dnsMsg := dns.Msg{}
 		if err := dnsMsg.Unpack(dnsData); err != nil {
-			waitCh <- err
+			// A non-DNS response packet or a fragmented DNS response is received. Forward it to the Pod.
+			waitCh <- nil
 			return
 		}
 		f.onDNSResponseMsg(&dnsMsg, time.Now(), waitCh)
 	}
 	go func() {
 		ethernetPkt, err := getEthernetPacket(pktIn)
 		if err != nil {
+			// Can't parse the packet. Forward it to the Pod.
+			waitCh <- nil
 			return
 		}
 		switch ipPkt := ethernetPkt.Data.(type) {
 		case *protocol.IPv4:
-			switch dnsPkt := ipPkt.Data.(type) {
-			case *protocol.UDP:
-				handleUDPData(dnsPkt)
+			proto := ipPkt.Protocol
+			switch proto {
+			case protocol.Type_UDP:
+				udpPkt := ipPkt.Data.(*protocol.UDP)
+				handleDNSData(udpPkt.Data)
+			case protocol.Type_TCP:
+				tcpPkt, err := binding.GetTCPPacketFromIPMessage(ipPkt)
+				if err != nil {
+					// Can't parse the packet. Forward it to the Pod.
+					waitCh <- nil
+					return
+				}
+				dnsData, err := binding.GetTCPDNSData(tcpPkt)
+				if err != nil {
+					// A non-DNS response packet is received or a fragmented DNS response is received. Forward it to the Pod.
+					waitCh <- nil
+					return
+				}
+				handleDNSData(dnsData)
 			}
 		case *protocol.IPv6:
-			switch dnsPkt := ipPkt.Data.(type) {
-			case *protocol.UDP:
-				handleUDPData(dnsPkt)
+			proto := ipPkt.NextHeader
+			switch proto {
+			case protocol.Type_UDP:
+				udpPkt := ipPkt.Data.(*protocol.UDP)
+				handleDNSData(udpPkt.Data)
+			case protocol.Type_TCP:
+				tcpPkt, err := binding.GetTCPPacketFromIPMessage(ipPkt)
+				if err != nil {
+					// Can't parse the packet. Forward it to the Pod.
+					waitCh <- nil
+					return
+				}
+				dnsData, err := binding.GetTCPDNSData(tcpPkt)
+				if err != nil {
+					// A non-DNS response packet is received or a fragmented DNS response is received. Forward it to the Pod.
+					waitCh <- nil
+					return
+				}
+				handleDNSData(dnsData)
 			}
 		}
 	}()
@@ -780,74 +814,29 @@ func (f *fqdnController) handlePacketIn(pktIn *ofctrl.PacketIn) error {
 		if err != nil {
 			return fmt.Errorf("error when syncing up rules for DNS reply, dropping packet: %v", err)
 		}
-		klog.V(2).InfoS("Rule sync is successful or not needed, forwarding DNS response to Pod")
+		klog.V(2).InfoS("Rule sync is successful or not needed or a non-DNS response packet or a fragmented DNS response was received, forwarding the packet to Pod")
 		return f.sendDNSPacketout(pktIn)
 	}
 }
 
 // sendDNSPacketout forwards the DNS response packet to the original requesting client.
 func (f *fqdnController) sendDNSPacketout(pktIn *ofctrl.PacketIn) error {
-	var (
-		packetData   []byte
-		srcIP, dstIP string
-		prot         uint8
-		isIPv6       bool
-	)
 	ethernetPkt, err := getEthernetPacket(pktIn)
 	if err != nil {
 		return err
 	}
-	switch ipPkt := ethernetPkt.Data.(type) {
-	case *protocol.IPv4:
-		srcIP = ipPkt.NWSrc.String()
-		dstIP = ipPkt.NWDst.String()
-		prot = ipPkt.Protocol
-		isIPv6 = false
-		switch dnsPkt := ipPkt.Data.(type) {
-		case *protocol.UDP:
-			packetData = dnsPkt.Data
-		}
-	case *protocol.IPv6:
-		srcIP = ipPkt.NWSrc.String()
-		dstIP = ipPkt.NWDst.String()
-		prot = ipPkt.NextHeader
-		isIPv6 = true
-		switch dnsPkt := ipPkt.Data.(type) {
-		case *protocol.UDP:
-			packetData = dnsPkt.Data
-		}
-	}
-	if prot == protocol.Type_UDP {
-		inPort := f.gwPort
-		if inPort == 0 {
-			// Use the original in_port number in the packetIn message to avoid an invalid input port number. Note that,
-			// this should not happen in container case as antrea-gw0 always exists. This check is for security.
-			matches := pktIn.GetMatches()
-			inPortField := matches.GetMatchByName("OXM_OF_IN_PORT")
-			if inPortField != nil {
-				inPort = inPortField.GetValue().(uint32)
-			}
-		}
-		udpSrcPort, udpDstPort, err := binding.GetUDPHeaderData(ethernetPkt.Data)
-		if err != nil {
-			klog.ErrorS(err, "Failed to get UDP header data")
-			return err
+	inPort := f.gwPort
+	if inPort == 0 {
+		// Use the original in_port number in the packetIn message to avoid an invalid input port number. Note that,
+		// this should not happen in container case as antrea-gw0 always exists. This check is for security.
+		matches := pktIn.GetMatches()
+		inPortField := matches.GetMatchByName("OXM_OF_IN_PORT")
+		if inPortField != nil {
+			inPort = inPortField.GetValue().(uint32)
 		}
-		mutatePacketOut := func(packetOutBuilder binding.PacketOutBuilder) binding.PacketOutBuilder {
-			return packetOutBuilder.AddLoadRegMark(openflow.CustomReasonDNSRegMark)
-		}
-		return f.ofClient.SendUDPPacketOut(
-			ethernetPkt.HWSrc.String(),
-			ethernetPkt.HWDst.String(),
-			srcIP,
-			dstIP,
-			inPort,
-			0,
-			isIPv6,
-			udpSrcPort,
-			udpDstPort,
-			packetData,
-			mutatePacketOut)
 	}
-	return nil
+	mutatePacketOut := func(packetOutBuilder binding.PacketOutBuilder) binding.PacketOutBuilder {
+		return packetOutBuilder.AddLoadRegMark(openflow.CustomReasonDNSRegMark)
+	}
+	return f.ofClient.SendEthPacketOut(inPort, 0, ethernetPkt, mutatePacketOut)
 }

pkg/agent/controller/networkpolicy/fqdn_test.go

@@ -31,7 +31,7 @@ import (
 
 func newMockFQDNController(t *testing.T, controller *gomock.Controller, dnsServer *string) (*fqdnController, *openflowtest.MockClient) {
 	mockOFClient := openflowtest.NewMockClient(controller)
-	mockOFClient.EXPECT().NewDNSpacketInConjunction(gomock.Any()).Return(nil).AnyTimes()
+	mockOFClient.EXPECT().NewDNSPacketInConjunction(gomock.Any()).Return(nil).AnyTimes()
 	dirtyRuleHandler := func(rule string) {}
 	dnsServerAddr := "8.8.8.8:53" // dummy DNS server, will not be used since we don't send any request in these tests
 	if dnsServer != nil {

pkg/agent/controller/networkpolicy/reject.go

@@ -193,7 +193,7 @@ func (c *Controller) rejectRequest(pktIn *ofctrl.PacketIn) error {
 
 	if proto == protocol.Type_TCP {
 		// Get TCP data.
-		oriTCPSrcPort, oriTCPDstPort, oriTCPSeqNum, _, _, err := binding.GetTCPHeaderData(ethernetPkt.Data)
+		oriTCPSrcPort, oriTCPDstPort, oriTCPSeqNum, _, _, _, _, err := binding.GetTCPHeaderData(ethernetPkt.Data)
 		if err != nil {
 			return err
 		}
@@ -209,8 +209,12 @@ func (c *Controller) rejectRequest(pktIn *ofctrl.PacketIn) error {
 			isIPv6,
 			oriTCPDstPort,
 			oriTCPSrcPort,
+			0,
 			oriTCPSeqNum+1,
+			0,
 			TCPAck|TCPRst,
+			0,
+			nil,
 			mutateFunc)
 	}
 	// Use ICMP host administratively prohibited for ICMP, UDP, SCTP reject.

pkg/agent/openflow/client.go

@@ -243,8 +243,12 @@ type Client interface {
 		isIPv6 bool,
 		tcpSrcPort uint16,
 		tcpDstPort uint16,
+		tcpSeqNum uint32,
 		tcpAckNum uint32,
+		tcpHdrLen uint8,
 		tcpFlag uint8,
+		tcpWinSize uint16,
+		tcpData []byte,
 		mutatePacketOut func(builder binding.PacketOutBuilder) binding.PacketOutBuilder) error
 	// SendICMPPacketOut sends ICMP packet as a packet-out to OVS.
 	SendICMPPacketOut(
@@ -272,8 +276,10 @@ type Client interface {
 		udpDstPort uint16,
 		udpData []byte,
 		mutatePacketOut func(builder binding.PacketOutBuilder) binding.PacketOutBuilder) error
-	// NewDNSpacketInConjunction creates a policyRuleConjunction for the dns response interception flows.
-	NewDNSpacketInConjunction(id uint32) error
+	// SendEthPacketOut sends ethernet packet as a packet-out to OVS.
+	SendEthPacketOut(inPort, outPort uint32, ethPkt *protocol.Ethernet, mutatePacketOut func(builder binding.PacketOutBuilder) binding.PacketOutBuilder) error
+	// NewDNSPacketInConjunction creates a policyRuleConjunction for the dns response interception flows.
+	NewDNSPacketInConjunction(id uint32) error
 	// AddAddressToDNSConjunction adds addresses to the toAddresses of the dns packetIn conjunction,
 	// so that dns response packets sent towards these addresses will be intercepted and parsed by
 	// the fqdnController.
@@ -1150,8 +1156,12 @@ func (c *client) SendTCPPacketOut(
 	isIPv6 bool,
 	tcpSrcPort uint16,
 	tcpDstPort uint16,
+	tcpSeqNum uint32,
 	tcpAckNum uint32,
+	tcpHdrLen uint8,
 	tcpFlag uint8,
+	tcpWinSize uint16,
+	tcpData []byte,
 	mutatePacketOut func(builder binding.PacketOutBuilder) binding.PacketOutBuilder) error {
 	// Generate a base IP PacketOutBuilder.
 	packetOutBuilder, err := setBasePacketOutBuilder(c.bridge.BuildPacketOut(), srcMAC, dstMAC, srcIP, dstIP, inPort, outPort)
@@ -1165,10 +1175,15 @@ func (c *client) SendTCPPacketOut(
 		packetOutBuilder = packetOutBuilder.SetIPProtocol(binding.ProtocolTCP)
 	}
 	// Set TCP header data.
-	packetOutBuilder = packetOutBuilder.SetTCPSrcPort(tcpSrcPort)
-	packetOutBuilder = packetOutBuilder.SetTCPDstPort(tcpDstPort)
-	packetOutBuilder = packetOutBuilder.SetTCPAckNum(tcpAckNum)
-	packetOutBuilder = packetOutBuilder.SetTCPFlags(tcpFlag)
+	packetOutBuilder = packetOutBuilder.
+		SetTCPSrcPort(tcpSrcPort).
+		SetTCPDstPort(tcpDstPort).
+		SetTCPSeqNum(tcpSeqNum).
+		SetTCPAckNum(tcpAckNum).
+		SetTCPHdrLen(tcpHdrLen).
+		SetTCPFlags(tcpFlag).
+		SetTCPWinSize(tcpWinSize).
+		SetTCPData(tcpData)
 
 	if mutatePacketOut != nil {
 		packetOutBuilder = mutatePacketOut(packetOutBuilder)
@@ -1252,6 +1267,20 @@ func (c *client) SendUDPPacketOut(
 	return c.bridge.SendPacketOut(packetOutObj)
 }
 
+func (c *client) SendEthPacketOut(inPort, outPort uint32, ethPkt *protocol.Ethernet, mutatePacketOut func(builder binding.PacketOutBuilder) binding.PacketOutBuilder) error {
+	packetOutBuilder := c.bridge.BuildPacketOut()
+	packetOutBuilder = packetOutBuilder.SetInport(inPort)
+	if outPort != 0 {
+		packetOutBuilder = packetOutBuilder.SetOutport(outPort)
+	}
+	if mutatePacketOut != nil {
+		packetOutBuilder = mutatePacketOut(packetOutBuilder)
+	}
+	packetOutBuilder.SetEthPacket(ethPkt)
+	packetOutObj := packetOutBuilder.Done()
+	return c.bridge.SendPacketOut(packetOutObj)
+}
+
 func (c *client) InstallMulticastFlows(multicastIP net.IP, groupID binding.GroupIDType) error {
 	flows := c.featureMulticast.localMulticastForwardFlows(multicastIP, groupID)
 	cacheKey := fmt.Sprintf("multicast_%s", multicastIP.String())

pkg/agent/openflow/client_test.go

@@ -1592,8 +1592,12 @@ func Test_client_SendPacketOut(t *testing.T) {
 		igmp       util.Message
 		tcpSrcPort uint16
 		tcpDstPort uint16
+		tcpSeqNum  uint32
 		tcpAckNum  uint32
+		tcpHdrLen  uint8
 		tcpFlag    uint8
+		tcpWinSize uint16
+		tcpData    []byte
 		udpSrcPort uint16
 		udpDstPort uint16
 		udpData    []byte
@@ -1603,17 +1607,25 @@ func Test_client_SendPacketOut(t *testing.T) {
 			protocol:   binding.ProtocolTCP,
 			tcpSrcPort: uint16(50000),
 			tcpDstPort: uint16(80),
+			tcpSeqNum:  uint32(7654321),
 			tcpAckNum:  uint32(1234567),
+			tcpHdrLen:  uint8(5),
 			tcpFlag:    uint8(0b000100),
+			tcpWinSize: uint16(123),
+			tcpData:    []byte{1, 2, 3},
 		},
 		{
 			name:       "SendTCPPacketOut IPv6",
 			protocol:   binding.ProtocolTCPv6,
 			isIPv6:     true,
 			tcpSrcPort: uint16(50000),
 			tcpDstPort: uint16(443),
+			tcpSeqNum:  uint32(7654321),
 			tcpAckNum:  uint32(1234567),
+			tcpHdrLen:  uint8(8),
 			tcpFlag:    uint8(0b000100),
+			tcpWinSize: uint16(123),
+			tcpData:    []byte{1, 2, 3},
 		},
 		{
 			name:       "SendUDPPacketOut IPv4",
@@ -1712,8 +1724,12 @@ func Test_client_SendPacketOut(t *testing.T) {
 			case binding.ProtocolTCP, binding.ProtocolTCPv6:
 				mockPacketOutBuilder.EXPECT().SetTCPSrcPort(tc.tcpSrcPort).Return(mockPacketOutBuilder)
 				mockPacketOutBuilder.EXPECT().SetTCPDstPort(tc.tcpDstPort).Return(mockPacketOutBuilder)
+				mockPacketOutBuilder.EXPECT().SetTCPSeqNum(tc.tcpSeqNum).Return(mockPacketOutBuilder)
 				mockPacketOutBuilder.EXPECT().SetTCPAckNum(tc.tcpAckNum).Return(mockPacketOutBuilder)
+				mockPacketOutBuilder.EXPECT().SetTCPHdrLen(tc.tcpHdrLen).Return(mockPacketOutBuilder)
 				mockPacketOutBuilder.EXPECT().SetTCPFlags(tc.tcpFlag).Return(mockPacketOutBuilder)
+				mockPacketOutBuilder.EXPECT().SetTCPWinSize(tc.tcpWinSize).Return(mockPacketOutBuilder)
+				mockPacketOutBuilder.EXPECT().SetTCPData(tc.tcpData).Return(mockPacketOutBuilder)
 				assert.NoError(t, fc.SendTCPPacketOut(srcMAC.String(),
 					dstMAC.String(),
 					srcIP.String(),
@@ -1723,8 +1739,12 @@ func Test_client_SendPacketOut(t *testing.T) {
 					tc.isIPv6,
 					tc.tcpSrcPort,
 					tc.tcpDstPort,
+					tc.tcpSeqNum,
 					tc.tcpAckNum,
+					tc.tcpHdrLen,
 					tc.tcpFlag,
+					tc.tcpWinSize,
+					tc.tcpData,
 					nil))
 			case binding.ProtocolUDP, binding.ProtocolUDPv6:
 				mockPacketOutBuilder.EXPECT().SetUDPSrcPort(tc.udpSrcPort).Return(mockPacketOutBuilder)