Skip to content

Commit

Permalink
Add FQDN TCP DNS support
Browse files Browse the repository at this point in the history 
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.

Signed-off-by: graysonwu <wgrayson@vmware.com>
  • Loading branch information
@GraysonWu
GraysonWu committed Mar 7, 2023
1 parent 8b08b45 commit 4e7b113
Show file tree
Hide file tree
Showing 20 changed files with 572 additions and 51 deletions.
7 changes: 7 additions & 0 deletions build/images/ovs/apply-patches.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
95 changes: 80 additions & 15 deletions pkg/agent/controller/networkpolicy/fqdn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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
Expand All @@ -762,14 +761,38 @@ 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
}
dnsData, err := binding.GetTCPDataWithoutOptions(tcpPkt)
if err != nil {
return
}
handleDNSData(dnsData)
}
}
}()
Expand Down Expand Up @@ -803,18 +826,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 {
Expand Down Expand Up @@ -848,6 +877,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
}
2 changes: 1 addition & 1 deletion pkg/agent/controller/networkpolicy/fqdn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 5 additions & 1 deletion pkg/agent/controller/networkpolicy/reject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,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
}
Expand All @@ -189,8 +189,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.
Expand Down
25 changes: 19 additions & 6 deletions pkg/agent/openflow/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -271,8 +275,8 @@ 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
// 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.
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
20 changes: 20 additions & 0 deletions pkg/agent/openflow/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -1572,17 +1576,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",
Expand Down Expand Up @@ -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(),
Expand All @@ -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)
Expand Down
36 changes: 32 additions & 4 deletions pkg/agent/openflow/network_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down Expand Up @@ -682,7 +689,7 @@ type clause struct {
dropTable binding.Table
}

func (c *client) NewDNSpacketInConjunction(id uint32) error {
func (c *client) NewDNSPacketInConjunction(id uint32) error {
existingConj := c.featureNetworkPolicy.getPolicyRuleConjunction(id)
if existingConj != nil {
klog.InfoS("DNS Conjunction has already been added to cache", "id", id)
Expand All @@ -699,17 +706,38 @@ 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)
if err := c.featureNetworkPolicy.applyConjunctiveMatchFlows(ctxChanges); err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/agent/openflow/network_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -174,7 +173,7 @@ func TestInstallPolicyRuleFlows(t *testing.T) {
// Create a policyRuleConjunction for the dns response interception flows
// to ensure nil NetworkPolicyReference is handled correctly by GetNetworkPolicyFlowKeys.
dnsID := uint32(1)
require.NoError(t, c.NewDNSpacketInConjunction(dnsID))
require.NoError(t, c.NewDNSPacketInConjunction(dnsID))

ruleID1 := uint32(101)
rule1 := &types.PolicyRule{
Expand Down
Loading

0 comments on commit 4e7b113

Please sign in to comment.