From 7daa730d0ba6c2aae0e110949b1b76aaa1777981 Mon Sep 17 00:00:00 2001 From: graysonwu Date: Fri, 27 Jan 2023 16:12:48 -0800 Subject: [PATCH] Add FQDN TCP DNS support Signed-off-by: graysonwu --- build/images/ovs/apply-patches.sh | 7 ++ pkg/agent/controller/networkpolicy/fqdn.go | 89 +++++++++++++++++--- pkg/agent/controller/networkpolicy/reject.go | 6 +- pkg/agent/openflow/client.go | 21 ++++- pkg/agent/openflow/client_test.go | 20 +++++ pkg/agent/openflow/network_policy.go | 39 ++++++++- pkg/agent/openflow/network_policy_test.go | 1 - pkg/agent/openflow/pipeline.go | 6 ++ pkg/agent/openflow/testing/mock_openflow.go | 8 +- pkg/agent/types/networkpolicy.go | 1 + pkg/ovs/openflow/interfaces.go | 4 + pkg/ovs/openflow/ofctrl_builder.go | 7 ++ pkg/ovs/openflow/ofctrl_packetin.go | 38 +++++++-- pkg/ovs/openflow/ofctrl_packetin_test.go | 2 +- pkg/ovs/openflow/ofctrl_packetout.go | 32 ++++++- pkg/ovs/openflow/testing/mock_openflow.go | 56 ++++++++++++ test/e2e/antreapolicy_test.go | 45 +++++++++- test/e2e/k8s_util.go | 49 +++++++++++ 18 files changed, 392 insertions(+), 39 deletions(-) diff --git a/build/images/ovs/apply-patches.sh b/build/images/ovs/apply-patches.sh index aa62b5a53ed..f17f0b26479 100755 --- a/build/images/ovs/apply-patches.sh +++ b/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_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. diff --git a/pkg/agent/controller/networkpolicy/fqdn.go b/pkg/agent/controller/networkpolicy/fqdn.go index 48a7eb3b70e..d6bb7bd1130 100644 --- a/pkg/agent/controller/networkpolicy/fqdn.go +++ b/pkg/agent/controller/networkpolicy/fqdn.go @@ -746,8 +746,7 @@ 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 @@ -762,14 +761,34 @@ func (f *fqdnController) handlePacketIn(pktIn *ofctrl.PacketIn) error { } 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 { + return + } + dnsData, err := binding.GetTCPDataWithoutOptions(tcpPkt) + if err != 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 { + return + } + handleDNSData(tcpPkt.Data) } } }() @@ -803,18 +822,24 @@ func (f *fqdnController) sendDNSPacketout(pktIn *ofctrl.PacketIn) error { dstIP = ipPkt.NWDst.String() prot = ipPkt.Protocol isIPv6 = false - switch dnsPkt := ipPkt.Data.(type) { - case *protocol.UDP: - packetData = dnsPkt.Data + switch prot { + case protocol.Type_UDP: + packetData = ipPkt.Data.(*protocol.UDP).Data + case protocol.Type_TCP: + tcpPkt, _ := binding.GetTCPPacketFromIPMessage(ipPkt) + packetData = tcpPkt.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 + switch prot { + case protocol.Type_UDP: + packetData = ipPkt.Data.(*protocol.UDP).Data + case protocol.Type_TCP: + tcpPkt, _ := binding.GetTCPPacketFromIPMessage(ipPkt) + packetData = tcpPkt.Data } } if prot == protocol.Type_UDP { @@ -848,6 +873,42 @@ func (f *fqdnController) sendDNSPacketout(pktIn *ofctrl.PacketIn) error { udpDstPort, packetData, mutatePacketOut) + } else if prot == protocol.Type_TCP { + 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 purpose. + matches := pktIn.GetMatches() + inPortField := matches.GetMatchByName("OXM_OF_IN_PORT") + if inPortField != nil { + inPort = inPortField.GetValue().(uint32) + } + } + tcpSrcPort, tcpDstPort, tcpSeqNum, tcpAckNum, tcpHdrLen, tcpFlag, tcpWinSize, err := binding.GetTCPHeaderData(ethernetPkt.Data) + if err != nil { + klog.ErrorS(err, "Failed to get TCP header data") + return err + } + mutatePacketOut := func(packetOutBuilder binding.PacketOutBuilder) binding.PacketOutBuilder { + return packetOutBuilder.AddLoadRegMark(openflow.CustomReasonDNSRegMark) + } + return f.ofClient.SendTCPPacketOut( + ethernetPkt.HWSrc.String(), + ethernetPkt.HWDst.String(), + srcIP, + dstIP, + inPort, + 0, + isIPv6, + tcpSrcPort, + tcpDstPort, + tcpSeqNum, + tcpAckNum, + tcpHdrLen, + tcpFlag, + tcpWinSize, + packetData, + mutatePacketOut) } return nil } diff --git a/pkg/agent/controller/networkpolicy/reject.go b/pkg/agent/controller/networkpolicy/reject.go index c468f11a6d3..b35643bff73 100644 --- a/pkg/agent/controller/networkpolicy/reject.go +++ b/pkg/agent/controller/networkpolicy/reject.go @@ -170,7 +170,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 } @@ -186,8 +186,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. diff --git a/pkg/agent/openflow/client.go b/pkg/agent/openflow/client.go index a425dee9757..4c655750fae 100644 --- a/pkg/agent/openflow/client.go +++ b/pkg/agent/openflow/client.go @@ -242,8 +242,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( @@ -1122,8 +1126,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) @@ -1137,10 +1145,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) diff --git a/pkg/agent/openflow/client_test.go b/pkg/agent/openflow/client_test.go index c4bd7bf0b30..55f5ec6e8e9 100644 --- a/pkg/agent/openflow/client_test.go +++ b/pkg/agent/openflow/client_test.go @@ -1561,8 +1561,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 @@ -1572,8 +1576,12 @@ 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", @@ -1581,8 +1589,12 @@ func Test_client_SendPacketOut(t *testing.T) { 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", @@ -1681,8 +1693,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(), @@ -1692,8 +1708,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) diff --git a/pkg/agent/openflow/network_policy.go b/pkg/agent/openflow/network_policy.go index 4a72cc9fb00..624a8e091f7 100644 --- a/pkg/agent/openflow/network_policy.go +++ b/pkg/agent/openflow/network_policy.go @@ -71,6 +71,7 @@ var ( MatchServiceGroupID = types.NewMatchKey(binding.ProtocolIP, types.ServiceGroupIDAddr, "reg7[0..31]") MatchIGMPProtocol = types.NewMatchKey(binding.ProtocolIGMP, types.IGMPAddr, "igmp") MatchLabelID = types.NewMatchKey(binding.ProtocolIP, types.LabelIDAddr, "tun_id") + MatchTCPFlag = types.NewMatchKey(binding.ProtocolTCP, types.TCPFlagAddr, "tcp_flags") Unsupported = types.NewMatchKey(binding.ProtocolIP, types.UnSupported, "unknown") // metricFlowIdentifier is used to identify metric flows in metric table. @@ -79,9 +80,15 @@ var ( metricFlowIdentifier = fmt.Sprintf("priority=%d,", priorityNormal) protocolUDP = v1beta2.ProtocolUDP + protocolTCP = v1beta2.ProtocolTCP dnsPort = intstr.FromInt(53) ) +type TCPFlag struct { + Flag uint16 + Mask uint16 +} + // IP address calculated from Pod's address. type IPAddress net.IP @@ -699,17 +706,43 @@ func (c *client) NewDNSpacketInConjunction(id uint32) error { if err := c.ofEntryOperations.AddAll(conj.actionFlows); err != nil { return fmt.Errorf("error when adding action flows for the DNS conjunction: %w", err) } + dnsPriority := priorityDNSIntercept + conj.serviceClause = conj.newClause(1, 2, getTableByID(conj.ruleTableID), nil) + conj.toClause = conj.newClause(2, 2, getTableByID(conj.ruleTableID), nil) udpService := v1beta2.Service{ Protocol: &protocolUDP, Port: &dnsPort, } - dnsPriority := priorityDNSIntercept - conj.serviceClause = conj.newClause(1, 2, getTableByID(conj.ruleTableID), nil) - conj.toClause = conj.newClause(2, 2, getTableByID(conj.ruleTableID), nil) + tcpService := v1beta2.Service{ + Protocol: &protocolTCP, + Port: &dnsPort, + } + tcpServiceMatch := &conjunctiveMatch{ + tableID: conj.serviceClause.ruleTable.GetID(), + matchPairs: []matchPair{ + getServiceMatchPairs(tcpService, c.featureNetworkPolicy.ipProtocols, true)[0][0], + { + matchKey: MatchTCPFlag, + matchValue: TCPFlag{ + // URG|ACK|PSH|RST|SYN|FIN| + Flag: 0b011000, + Mask: 0b011000, + }, + }, + }, + priority: &dnsPriority, + } c.featureNetworkPolicy.conjMatchFlowLock.Lock() defer c.featureNetworkPolicy.conjMatchFlowLock.Unlock() ctxChanges := conj.serviceClause.addServiceFlows(c.featureNetworkPolicy, []v1beta2.Service{udpService}, &dnsPriority, true, false) + ctxChange := conj.serviceClause.addConjunctiveMatchFlow(c.featureNetworkPolicy, tcpServiceMatch, false, false) + ctxChanges = append(ctxChanges, ctxChange) + for _, change := range ctxChanges { + for _, pa := range change.context.matchPairs { + klog.Infof("%s:%s", pa.matchKey.GetKeyString(), pa.matchValue) + } + } if err := c.featureNetworkPolicy.applyConjunctiveMatchFlows(ctxChanges); err != nil { return err } diff --git a/pkg/agent/openflow/network_policy_test.go b/pkg/agent/openflow/network_policy_test.go index 284e7e416ed..59e26f99d38 100644 --- a/pkg/agent/openflow/network_policy_test.go +++ b/pkg/agent/openflow/network_policy_test.go @@ -65,7 +65,6 @@ var ( actionAllow = crdv1alpha1.RuleActionAllow actionDrop = crdv1alpha1.RuleActionDrop port8080 = intstr.FromInt(8080) - protocolTCP = v1beta2.ProtocolTCP protocolICMP = v1beta2.ProtocolICMP priority100 = uint16(100) priority200 = uint16(200) diff --git a/pkg/agent/openflow/pipeline.go b/pkg/agent/openflow/pipeline.go index 50620b28bfb..932a12a60c7 100644 --- a/pkg/agent/openflow/pipeline.go +++ b/pkg/agent/openflow/pipeline.go @@ -2010,6 +2010,12 @@ func (f *featureNetworkPolicy) addFlowMatch(fb binding.FlowBuilder, matchKey *ty fb = fb.MatchProtocol(matchKey.GetOFProtocol()) case MatchLabelID: fb = fb.MatchTunnelID(uint64(matchValue.(uint32))) + case MatchTCPFlag: + fb = fb.MatchProtocol(matchKey.GetOFProtocol()) + if matchValue != nil { + tcpFlag := matchValue.(TCPFlag) + fb = fb.MatchTCPFlag(tcpFlag.Flag, tcpFlag.Mask) + } } return fb } diff --git a/pkg/agent/openflow/testing/mock_openflow.go b/pkg/agent/openflow/testing/mock_openflow.go index 06356eb9391..089ef60e42b 100644 --- a/pkg/agent/openflow/testing/mock_openflow.go +++ b/pkg/agent/openflow/testing/mock_openflow.go @@ -714,17 +714,17 @@ func (mr *MockClientMockRecorder) SendIGMPRemoteReportPacketOut(arg0, arg1, arg2 } // SendTCPPacketOut mocks base method -func (m *MockClient) SendTCPPacketOut(arg0, arg1, arg2, arg3 string, arg4, arg5 uint32, arg6 bool, arg7, arg8 uint16, arg9 uint32, arg10 byte, arg11 func(openflow.PacketOutBuilder) openflow.PacketOutBuilder) error { +func (m *MockClient) SendTCPPacketOut(arg0, arg1, arg2, arg3 string, arg4, arg5 uint32, arg6 bool, arg7, arg8 uint16, arg9, arg10 uint32, arg11, arg12 byte, arg13 uint16, arg14 []byte, arg15 func(openflow.PacketOutBuilder) openflow.PacketOutBuilder) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SendTCPPacketOut", arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11) + ret := m.ctrl.Call(m, "SendTCPPacketOut", arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15) ret0, _ := ret[0].(error) return ret0 } // SendTCPPacketOut indicates an expected call of SendTCPPacketOut -func (mr *MockClientMockRecorder) SendTCPPacketOut(arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) SendTCPPacketOut(arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendTCPPacketOut", reflect.TypeOf((*MockClient)(nil).SendTCPPacketOut), arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendTCPPacketOut", reflect.TypeOf((*MockClient)(nil).SendTCPPacketOut), arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15) } // SendTraceflowPacket mocks base method diff --git a/pkg/agent/types/networkpolicy.go b/pkg/agent/types/networkpolicy.go index 90bcc6a275f..574d02150e5 100644 --- a/pkg/agent/types/networkpolicy.go +++ b/pkg/agent/types/networkpolicy.go @@ -57,6 +57,7 @@ const ( ServiceGroupIDAddr IGMPAddr LabelIDAddr + TCPFlagAddr UnSupported ) diff --git a/pkg/ovs/openflow/interfaces.go b/pkg/ovs/openflow/interfaces.go index 6fbc0d735b2..522437341a7 100644 --- a/pkg/ovs/openflow/interfaces.go +++ b/pkg/ovs/openflow/interfaces.go @@ -280,6 +280,7 @@ type FlowBuilder interface { MatchConjID(value uint32) FlowBuilder MatchDstPort(port uint16, portMask *uint16) FlowBuilder MatchSrcPort(port uint16, portMask *uint16) FlowBuilder + MatchTCPFlag(flag, mask uint16) FlowBuilder MatchICMPType(icmpType byte) FlowBuilder MatchICMPCode(icmpCode byte) FlowBuilder MatchICMPv6Type(icmp6Type byte) FlowBuilder @@ -401,6 +402,9 @@ type PacketOutBuilder interface { SetTCPFlags(flags uint8) PacketOutBuilder SetTCPSeqNum(seqNum uint32) PacketOutBuilder SetTCPAckNum(ackNum uint32) PacketOutBuilder + SetTCPHdrLen(hdrLen uint8) PacketOutBuilder + SetTCPWinSize(winSize uint16) PacketOutBuilder + SetTCPData(data []byte) PacketOutBuilder SetUDPSrcPort(port uint16) PacketOutBuilder SetUDPDstPort(port uint16) PacketOutBuilder SetUDPData(data []byte) PacketOutBuilder diff --git a/pkg/ovs/openflow/ofctrl_builder.go b/pkg/ovs/openflow/ofctrl_builder.go index 456873d17b1..578f40b5315 100644 --- a/pkg/ovs/openflow/ofctrl_builder.go +++ b/pkg/ovs/openflow/ofctrl_builder.go @@ -598,6 +598,13 @@ func (b *ofFlowBuilder) MatchSrcPort(port uint16, portMask *uint16) FlowBuilder return b } +func (b *ofFlowBuilder) MatchTCPFlag(flag, mask uint16) FlowBuilder { + b.matchers = append(b.matchers, fmt.Sprintf("tcp_flags=%b/%b", uint8(flag), uint8(mask))) + b.Match.TcpFlags = &flag + b.Match.TcpFlagsMask = &mask + return b +} + // MatchCTSrcIP matches the source IPv4 address of the connection tracker original direction tuple. This match requires // a match to valid connection tracking state as a prerequisite, and valid connection tracking state matches include // "+new", "+est", "+rel" and "+trk-inv". diff --git a/pkg/ovs/openflow/ofctrl_packetin.go b/pkg/ovs/openflow/ofctrl_packetin.go index 61ae8199b0a..4d20450a044 100644 --- a/pkg/ovs/openflow/ofctrl_packetin.go +++ b/pkg/ovs/openflow/ofctrl_packetin.go @@ -26,10 +26,21 @@ import ( const ( icmpEchoRequestType uint8 = 8 icmp6EchoRequestType uint8 = 128 + // tcpStandardHdrLen is the TCP header length without options. + tcpStandardHdrLen uint8 = 5 + tcpOptionEndKind uint8 = 0 ) -// GetTCPHeaderData gets TCP header data from IP packet. -func GetTCPHeaderData(ipPkt util.Message) (tcpSrcPort, tcpDstPort uint16, tcpSeqNum, tcpAckNum uint32, tcpFlags uint8, err error) { +func GetTCPHeaderData(ipPkt util.Message) (tcpSrcPort, tcpDstPort uint16, tcpSeqNum, tcpAckNum uint32, tcpHdrLen uint8, tcpFlags uint8, tcpWinSize uint16, err error) { + tcpIn, err := GetTCPPacketFromIPMessage(ipPkt) + if err != nil { + return 0, 0, 0, 0, 0, 0, 0, err + } + return tcpIn.PortSrc, tcpIn.PortDst, tcpIn.SeqNum, tcpIn.AckNum, tcpIn.HdrLen, tcpIn.Code, tcpIn.WinSize, nil +} + +// GetTCPPacketFromIPMessage gets a TCP struct from an IP message. +func GetTCPPacketFromIPMessage(ipPkt util.Message) (tcpPkt *protocol.TCP, err error) { var tcpBytes []byte // Transfer Buffer to TCP @@ -40,15 +51,26 @@ func GetTCPHeaderData(ipPkt util.Message) (tcpSrcPort, tcpDstPort uint16, tcpSeq tcpBytes, err = typedIPPkt.Data.(*util.Buffer).MarshalBinary() } if err != nil { - return 0, 0, 0, 0, 0, err + return nil, err } - tcpIn := new(protocol.TCP) - err = tcpIn.UnmarshalBinary(tcpBytes) + tcpPkt = new(protocol.TCP) + err = tcpPkt.UnmarshalBinary(tcpBytes) if err != nil { - return 0, 0, 0, 0, 0, err + return nil, err } - return tcpIn.PortSrc, tcpIn.PortDst, tcpIn.SeqNum, tcpIn.AckNum, tcpIn.Code, nil + return tcpPkt, nil +} + +func GetTCPDataWithoutOptions(tcpPkt *protocol.TCP) (data []byte, err error) { + tcpOptionsLen := (tcpPkt.HdrLen - tcpStandardHdrLen) * 4 + if tcpPkt.Data[tcpOptionsLen] == tcpOptionEndKind { + tcpOptionsLen += 2 + } + if err != nil { + return nil, err + } + return tcpPkt.Data[tcpOptionsLen:], nil } func GetUDPHeaderData(ipPkt util.Message) (udpSrcPort, udpDstPort uint16, err error) { @@ -122,7 +144,7 @@ func ParsePacketIn(pktIn *ofctrl.PacketIn) (*Packet, error) { var err error if packet.IPProto == protocol.Type_TCP { - packet.SourcePort, packet.DestinationPort, _, _, packet.TCPFlags, err = GetTCPHeaderData(ethernetData.Data) + packet.SourcePort, packet.DestinationPort, _, _, _, packet.TCPFlags, _, err = GetTCPHeaderData(ethernetData.Data) } else if packet.IPProto == protocol.Type_UDP { packet.SourcePort, packet.DestinationPort, err = GetUDPHeaderData(ethernetData.Data) } else if packet.IPProto == protocol.Type_ICMP || packet.IPProto == protocol.Type_IPv6ICMP { diff --git a/pkg/ovs/openflow/ofctrl_packetin_test.go b/pkg/ovs/openflow/ofctrl_packetin_test.go index 536f0fb8174..71d9986a2a6 100644 --- a/pkg/ovs/openflow/ofctrl_packetin_test.go +++ b/pkg/ovs/openflow/ofctrl_packetin_test.go @@ -66,7 +66,7 @@ func TestGetTCPHeaderData(t *testing.T) { bf.UnmarshalBinary(bytes) pktIn.Data = bf - tcpSrcPort, tcpDstPort, tcpSeqNum, tcpAckNum, tcpCode, err := GetTCPHeaderData(pktIn) + tcpSrcPort, tcpDstPort, tcpSeqNum, tcpAckNum, _, tcpCode, _, err := GetTCPHeaderData(pktIn) require.NoError(t, err, "GetTCPHeaderData() returned an error") assert.Equal(t, tt.args.expectTCPSrcPort, tcpSrcPort) assert.Equal(t, tt.args.expectTCPDstPort, tcpDstPort) diff --git a/pkg/ovs/openflow/ofctrl_packetout.go b/pkg/ovs/openflow/ofctrl_packetout.go index d119667261e..4c69125fe8f 100644 --- a/pkg/ovs/openflow/ofctrl_packetout.go +++ b/pkg/ovs/openflow/ofctrl_packetout.go @@ -211,6 +211,30 @@ func (b *ofPacketOutBuilder) SetTCPAckNum(ackNum uint32) PacketOutBuilder { return b } +func (b *ofPacketOutBuilder) SetTCPHdrLen(hdrLen uint8) PacketOutBuilder { + if b.pktOut.TCPHeader == nil { + b.pktOut.TCPHeader = new(protocol.TCP) + } + b.pktOut.TCPHeader.HdrLen = hdrLen + return b +} + +func (b *ofPacketOutBuilder) SetTCPWinSize(winSize uint16) PacketOutBuilder { + if b.pktOut.TCPHeader == nil { + b.pktOut.TCPHeader = new(protocol.TCP) + } + b.pktOut.TCPHeader.WinSize = winSize + return b +} + +func (b *ofPacketOutBuilder) SetTCPData(data []byte) PacketOutBuilder { + if b.pktOut.TCPHeader == nil { + b.pktOut.TCPHeader = new(protocol.TCP) + } + b.pktOut.TCPHeader.Data = data + return b +} + // SetUDPSrcPort sets the source port in the packet's UDP header. func (b *ofPacketOutBuilder) SetUDPSrcPort(port uint16) PacketOutBuilder { if b.pktOut.UDPHeader == nil { @@ -344,7 +368,9 @@ func (b *ofPacketOutBuilder) Done() *ofctrl.PacketOut { b.pktOut.ICMPHeader.Checksum = b.icmpHeaderChecksum() b.pktOut.IPHeader.Length = 20 + b.pktOut.ICMPHeader.Len() } else if b.pktOut.TCPHeader != nil { - b.pktOut.TCPHeader.HdrLen = 5 + if b.pktOut.TCPHeader.HdrLen == 0 { + b.pktOut.TCPHeader.HdrLen = 5 + } if b.pktOut.TCPHeader.SeqNum == 0 { // #nosec G404: random number generator not used for security purposes b.pktOut.TCPHeader.SeqNum = pktRand.Uint32() @@ -387,7 +413,9 @@ func (b *ofPacketOutBuilder) Done() *ofctrl.PacketOut { b.pktOut.ICMPHeader.Checksum = b.icmpHeaderChecksum() b.pktOut.IPv6Header.Length = b.pktOut.ICMPHeader.Len() } else if b.pktOut.TCPHeader != nil { - b.pktOut.TCPHeader.HdrLen = 5 + if b.pktOut.TCPHeader.HdrLen == 0 { + b.pktOut.TCPHeader.HdrLen = 5 + } if b.pktOut.TCPHeader.SeqNum == 0 { // #nosec G404: random number generator not used for security purposes b.pktOut.TCPHeader.SeqNum = pktRand.Uint32() diff --git a/pkg/ovs/openflow/testing/mock_openflow.go b/pkg/ovs/openflow/testing/mock_openflow.go index ce28d5953de..8aec4ffa741 100644 --- a/pkg/ovs/openflow/testing/mock_openflow.go +++ b/pkg/ovs/openflow/testing/mock_openflow.go @@ -2058,6 +2058,20 @@ func (mr *MockFlowBuilderMockRecorder) MatchSrcPort(arg0, arg1 interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MatchSrcPort", reflect.TypeOf((*MockFlowBuilder)(nil).MatchSrcPort), arg0, arg1) } +// MatchTCPFlag mocks base method +func (m *MockFlowBuilder) MatchTCPFlag(arg0, arg1 uint16) openflow.FlowBuilder { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "MatchTCPFlag", arg0, arg1) + ret0, _ := ret[0].(openflow.FlowBuilder) + return ret0 +} + +// MatchTCPFlag indicates an expected call of MatchTCPFlag +func (mr *MockFlowBuilderMockRecorder) MatchTCPFlag(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MatchTCPFlag", reflect.TypeOf((*MockFlowBuilder)(nil).MatchTCPFlag), arg0, arg1) +} + // MatchTunMetadata mocks base method func (m *MockFlowBuilder) MatchTunMetadata(arg0 int, arg1 uint32) openflow.FlowBuilder { m.ctrl.T.Helper() @@ -2756,6 +2770,20 @@ func (mr *MockPacketOutBuilderMockRecorder) SetTCPAckNum(arg0 interface{}) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTCPAckNum", reflect.TypeOf((*MockPacketOutBuilder)(nil).SetTCPAckNum), arg0) } +// SetTCPData mocks base method +func (m *MockPacketOutBuilder) SetTCPData(arg0 []byte) openflow.PacketOutBuilder { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetTCPData", arg0) + ret0, _ := ret[0].(openflow.PacketOutBuilder) + return ret0 +} + +// SetTCPData indicates an expected call of SetTCPData +func (mr *MockPacketOutBuilderMockRecorder) SetTCPData(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTCPData", reflect.TypeOf((*MockPacketOutBuilder)(nil).SetTCPData), arg0) +} + // SetTCPDstPort mocks base method func (m *MockPacketOutBuilder) SetTCPDstPort(arg0 uint16) openflow.PacketOutBuilder { m.ctrl.T.Helper() @@ -2784,6 +2812,20 @@ func (mr *MockPacketOutBuilderMockRecorder) SetTCPFlags(arg0 interface{}) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTCPFlags", reflect.TypeOf((*MockPacketOutBuilder)(nil).SetTCPFlags), arg0) } +// SetTCPHdrLen mocks base method +func (m *MockPacketOutBuilder) SetTCPHdrLen(arg0 byte) openflow.PacketOutBuilder { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetTCPHdrLen", arg0) + ret0, _ := ret[0].(openflow.PacketOutBuilder) + return ret0 +} + +// SetTCPHdrLen indicates an expected call of SetTCPHdrLen +func (mr *MockPacketOutBuilderMockRecorder) SetTCPHdrLen(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTCPHdrLen", reflect.TypeOf((*MockPacketOutBuilder)(nil).SetTCPHdrLen), arg0) +} + // SetTCPSeqNum mocks base method func (m *MockPacketOutBuilder) SetTCPSeqNum(arg0 uint32) openflow.PacketOutBuilder { m.ctrl.T.Helper() @@ -2812,6 +2854,20 @@ func (mr *MockPacketOutBuilderMockRecorder) SetTCPSrcPort(arg0 interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTCPSrcPort", reflect.TypeOf((*MockPacketOutBuilder)(nil).SetTCPSrcPort), arg0) } +// SetTCPWinSize mocks base method +func (m *MockPacketOutBuilder) SetTCPWinSize(arg0 uint16) openflow.PacketOutBuilder { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetTCPWinSize", arg0) + ret0, _ := ret[0].(openflow.PacketOutBuilder) + return ret0 +} + +// SetTCPWinSize indicates an expected call of SetTCPWinSize +func (mr *MockPacketOutBuilderMockRecorder) SetTCPWinSize(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTCPWinSize", reflect.TypeOf((*MockPacketOutBuilder)(nil).SetTCPWinSize), arg0) +} + // SetTTL mocks base method func (m *MockPacketOutBuilder) SetTTL(arg0 byte) openflow.PacketOutBuilder { m.ctrl.T.Helper() diff --git a/test/e2e/antreapolicy_test.go b/test/e2e/antreapolicy_test.go index 28855576d3b..7fee77fd0f8 100644 --- a/test/e2e/antreapolicy_test.go +++ b/test/e2e/antreapolicy_test.go @@ -3320,6 +3320,48 @@ func testFQDNPolicyInClusterService(t *testing.T) { failOnError(k8sUtils.DeleteACNP(builder.Name), t) } +// testFQDNPolicyTCP +func testFQDNPolicyTCP(t *testing.T) { + // The ipv6-only test env doesn't have IPv6 access to the web. + skipIfNotIPv4Cluster(t) + // It is convenient to have higher log verbosity for FQDNtests for troubleshooting failures. + logLevel := log.GetLevel() + log.SetLevel(log.TraceLevel) + defer log.SetLevel(logLevel) + builder := &ClusterNetworkPolicySpecBuilder{} + builder = builder.SetName("test-acnp-fqdn-tcp"). + SetTier("application"). + SetPriority(1.0). + SetAppliedToGroup([]ACNPAppliedToSpec{{NSSelector: map[string]string{}}}) + builder.AddFQDNRule("github.com", ProtocolTCP, nil, nil, nil, "", nil, crdv1alpha1.RuleActionDrop) + + testcases := []podToAddrTestStep{ + { + Pod(namespaces["y"] + "/a"), + "github.com", + 80, + Dropped, + }, + } + acnp, err := k8sUtils.CreateOrUpdateACNP(builder.Get()) + failOnError(err, t) + failOnError(waitForResourceReady(t, timeout, acnp), t) + for _, tc := range testcases { + destIP := k8sUtils.digDNS(tc.clientPod.PodName(), tc.clientPod.Namespace(), tc.destAddr, true) + log.Tracef("Probing: %s -> %s(%s)", tc.clientPod.PodName(), tc.destAddr, destIP) + connectivity, err := k8sUtils.ProbeAddr(tc.clientPod.Namespace(), "pod", tc.clientPod.PodName(), destIP, tc.destPort, ProtocolTCP) + if err != nil { + t.Errorf("failure -- could not complete probe: %v", err) + } + if connectivity != tc.expectedConnectivity { + t.Errorf("failure -- wrong results for probe: Source %s/%s --> Dest %s:%d connectivity: %v, expected: %v", + tc.clientPod.Namespace(), tc.clientPod.PodName(), tc.destAddr, tc.destPort, connectivity, tc.expectedConnectivity) + } + } + // cleanup test resources + failOnError(k8sUtils.DeleteACNP(builder.Name), t) +} + func testToServices(t *testing.T) { skipIfProxyDisabled(t) var services []*v1.Service @@ -4286,7 +4328,8 @@ func TestAntreaPolicy(t *testing.T) { t.Run("Case=ANPGroupRefRuleIPBlocks", func(t *testing.T) { testANPGroupRefRuleIPBlocks(t) }) t.Run("Case=ANPNestedGroup", func(t *testing.T) { testANPNestedGroupCreateAndUpdate(t, data) }) t.Run("Case=ACNPFQDNPolicy", func(t *testing.T) { testFQDNPolicy(t) }) - t.Run("Case=FQDNPolicyInCluster", func(t *testing.T) { testFQDNPolicyInClusterService(t) }) + t.Run("Case=ACNPFQDNPolicyInCluster", func(t *testing.T) { testFQDNPolicyInClusterService(t) }) + t.Run("Case=ACNPFQDNPolicyTCP", func(t *testing.T) { testFQDNPolicyTCP(t) }) t.Run("Case=ACNPToServices", func(t *testing.T) { testToServices(t) }) t.Run("Case=ACNPServiceAccountSelector", func(t *testing.T) { testServiceAccountSelector(t, data) }) t.Run("Case=ACNPNodeSelectorEgress", func(t *testing.T) { testACNPNodeSelectorEgress(t) }) diff --git a/test/e2e/k8s_util.go b/test/e2e/k8s_util.go index edfb4ad32bf..915dc846ec4 100644 --- a/test/e2e/k8s_util.go +++ b/test/e2e/k8s_util.go @@ -210,6 +210,55 @@ func (k *KubernetesUtils) pingProbe( return DecidePingProbeResult(stdout, 3) } +func (k *KubernetesUtils) digDNS( + podName string, + podNamespace string, + dstAddr string, + useTCP bool, +) string { + pod, err := k.GetPodByLabel(podNamespace, podName) + if err != nil { + return "" + } + digCmd := fmt.Sprintf("dig %s", dstAddr) + if useTCP { + digCmd += " +tcp" + } + cmd := []string{ + "/bin/sh", + "-c", + digCmd, + } + log.Tracef("Running: kubectl exec %s -c %s -n %s -- %s", pod.Name, pod.Spec.Containers[0].Name, pod.Namespace, strings.Join(cmd, " ")) + stdout, stderr, err := k.RunCommandFromPod(pod.Namespace, pod.Name, pod.Spec.Containers[0].Name, cmd) + log.Tracef("%s -> %s: error when running command: err - %v /// stdout - %s /// stderr - %s", podName, dstAddr, err, stdout, stderr) + //========DiG command stdout example======== + //; <<>> DiG 9.16.6 <<>> github.com +tcp + //;; global options: +cmd + //;; Got answer: + //;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 21816 + //;; flags: qr aa rd ra; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1 + // + //;; OPT PSEUDOSECTION: + //; EDNS: version: 0, flags:; udp: 4096 + //; COOKIE: 2d7fe493ea37c430 (echoed) + //;; QUESTION SECTION: + //;github.com. IN A + // + //;; ANSWER SECTION: + //github.com. 6 IN A 140.82.113.3 + // + //;; Query time: 0 msec + //;; SERVER: 10.96.0.10#53(10.96.0.10) + //;; WHEN: Tue Feb 14 22:34:23 UTC 2023 + //;; MSG SIZE rcvd: 77 + //========================================== + answerMarkIdx := strings.Index(stdout, ";; ANSWER SECTION:") + ipLine := strings.Split(stdout[answerMarkIdx:], "\n")[1] + lastTab := strings.LastIndex(ipLine, "\t") + return ipLine[lastTab:] +} + // DecidePingProbeResult uses the pingProbe stdout to decide the connectivity. func DecidePingProbeResult(stdout string, probeNum int) PodConnectivityMark { // Provide stdout example for different connectivity: