From cdb9e8320b7c8f3698a9dc2429153982bfc83daa Mon Sep 17 00:00:00 2001 From: Hang Yan Date: Wed, 20 Nov 2024 17:23:39 +0800 Subject: [PATCH] Fix captured packets file unrecognized issue on osx (#6795) By default, gopacket will write snap length=0 in the pcapng file header, means unlimited snaplen. tcpdump on osx(libpcap version 1.10.1) cannot recognize this and will report error. This patch will set a default value(65535) for it. Signed-off-by: Hang Yan --- pkg/agent/packetcapture/capture/pcap_linux.go | 9 +- .../packetcapture/capture/pcap_unsupported.go | 2 +- pkg/agent/packetcapture/capture_interface.go | 2 +- .../packetcapture/packetcapture_controller.go | 14 ++- .../packetcapture_controller_test.go | 2 +- test/e2e/packetcapture_test.go | 99 +++++++++++++++++++ 6 files changed, 116 insertions(+), 12 deletions(-) diff --git a/pkg/agent/packetcapture/capture/pcap_linux.go b/pkg/agent/packetcapture/capture/pcap_linux.go index 58cb7559ec8..8bbd4845ab8 100644 --- a/pkg/agent/packetcapture/capture/pcap_linux.go +++ b/pkg/agent/packetcapture/capture/pcap_linux.go @@ -28,11 +28,6 @@ import ( crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" ) -const ( - // Max packet size for pcap capture. - maxSnapshotBytes = 65536 -) - type pcapCapture struct { } @@ -46,7 +41,7 @@ func zeroFilter() []bpf.Instruction { return []bpf.Instruction{returnDrop} } -func (p *pcapCapture) Capture(ctx context.Context, device string, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) { +func (p *pcapCapture) Capture(ctx context.Context, device string, snapLen int, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) { // Compile the BPF filter in advance to reduce the time window between starting the capture and applying the filter. inst := compilePacketFilter(packet, srcIP, dstIP) klog.V(5).InfoS("Generated bpf instructions for PacketCapture", "device", device, "srcIP", srcIP, "dstIP", dstIP, "packetSpec", packet, "bpf", inst) @@ -80,7 +75,7 @@ func (p *pcapCapture) Capture(ctx context.Context, device string, srcIP, dstIP n if err = eth.SetBPF(zeroRawInst); err != nil { return nil, err } - if err = eth.SetCaptureLength(maxSnapshotBytes); err != nil { + if err = eth.SetCaptureLength(snapLen); err != nil { return nil, err } diff --git a/pkg/agent/packetcapture/capture/pcap_unsupported.go b/pkg/agent/packetcapture/capture/pcap_unsupported.go index 9ccb908bca4..ef2cbfbcd01 100644 --- a/pkg/agent/packetcapture/capture/pcap_unsupported.go +++ b/pkg/agent/packetcapture/capture/pcap_unsupported.go @@ -34,6 +34,6 @@ func NewPcapCapture() (*pcapCapture, error) { return nil, errors.New("PacketCapture is not implemented") } -func (p *pcapCapture) Capture(ctx context.Context, device string, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) { +func (p *pcapCapture) Capture(ctx context.Context, device string, snapLen int, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) { return nil, errors.New("PacketCapture is not implemented") } diff --git a/pkg/agent/packetcapture/capture_interface.go b/pkg/agent/packetcapture/capture_interface.go index b7b77d9fcd8..c607cb32c30 100644 --- a/pkg/agent/packetcapture/capture_interface.go +++ b/pkg/agent/packetcapture/capture_interface.go @@ -24,5 +24,5 @@ import ( ) type PacketCapturer interface { - Capture(ctx context.Context, device string, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) + Capture(ctx context.Context, device string, snapLen int, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) } diff --git a/pkg/agent/packetcapture/packetcapture_controller.go b/pkg/agent/packetcapture/packetcapture_controller.go index 3691f0cd895..5282ee904af 100644 --- a/pkg/agent/packetcapture/packetcapture_controller.go +++ b/pkg/agent/packetcapture/packetcapture_controller.go @@ -80,6 +80,9 @@ const ( // PacketCapture uses a dedicated Secret object to store authentication information for a file server. // #nosec G101 fileServerAuthSecretName = "antrea-packetcapture-fileserver-auth" + + // max packet size we can capture. + snapLen = 65536 ) type packetCapturePhase string @@ -449,13 +452,20 @@ func (c *Controller) performCapture( if err != nil { return false, err } - pcapngWriter, err := pcapgo.NewNgWriter(file, layers.LinkTypeEthernet) + + // set SnapLength here to make tcpdump on Mac OSX works. By default, its value is + // 0 and means unlimited, but tcpdump on Mac OSX will complain: + // 'tcpdump: pcap_loop: invalid packet capture length , bigger than snaplen of 524288' + ngInterface := pcapgo.DefaultNgInterface + ngInterface.SnapLength = snapLen + ngInterface.LinkType = layers.LinkTypeEthernet + pcapngWriter, err := pcapgo.NewNgWriterInterface(file, ngInterface, pcapgo.DefaultNgWriterOptions) if err != nil { return false, fmt.Errorf("couldn't initialize a pcap writer: %w", err) } defer pcapngWriter.Flush() updateRateLimiter := rate.NewLimiter(rate.Every(captureStatusUpdatePeriod), 1) - packets, err := c.captureInterface.Capture(ctx, device, srcIP, dstIP, pc.Spec.Packet) + packets, err := c.captureInterface.Capture(ctx, device, snapLen, srcIP, dstIP, pc.Spec.Packet) if err != nil { return false, err } diff --git a/pkg/agent/packetcapture/packetcapture_controller_test.go b/pkg/agent/packetcapture/packetcapture_controller_test.go index 1c8c2506ad1..29f0f6fe62b 100644 --- a/pkg/agent/packetcapture/packetcapture_controller_test.go +++ b/pkg/agent/packetcapture/packetcapture_controller_test.go @@ -186,7 +186,7 @@ func craftTestPacket() gopacket.Packet { type testCapture struct { } -func (p *testCapture) Capture(ctx context.Context, device string, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) { +func (p *testCapture) Capture(ctx context.Context, device string, snapLen int, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) { ch := make(chan gopacket.Packet, testCaptureNum) for i := 0; i < 15; i++ { ch <- craftTestPacket() diff --git a/test/e2e/packetcapture_test.go b/test/e2e/packetcapture_test.go index c6b8c77cc4a..aa728e1c5f2 100644 --- a/test/e2e/packetcapture_test.go +++ b/test/e2e/packetcapture_test.go @@ -17,12 +17,19 @@ package e2e import ( "context" "fmt" + "io" "net" + "os" + "path/filepath" "sort" "strings" "testing" "time" + "github.com/gopacket/gopacket" + "github.com/gopacket/gopacket/layers" + "github.com/gopacket/gopacket/pcapgo" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" @@ -524,6 +531,19 @@ func runPacketCaptureTest(t *testing.T, data *TestData, tc pcTestCase) { require.NoError(t, err) } } + var srcPodIPs *PodIPs + if tc.pc.Spec.Source.IP != nil { + ip := net.ParseIP(*tc.pc.Spec.Source.IP) + srcPodIPs = &PodIPs{IPv4: &ip} + } else if tc.pc.Spec.Source.Pod != nil { + pod, err := data.clientset.CoreV1().Pods(tc.pc.Spec.Source.Pod.Namespace).Get(context.TODO(), tc.pc.Spec.Source.Pod.Name, metav1.GetOptions{}) + if err != nil { + require.True(t, errors.IsNotFound(err)) + } else { + srcPodIPs, err = parsePodIPs(pod) + require.NoError(t, err) + } + } if _, err := data.crdClient.CrdV1alpha1().PacketCaptures().Create(context.TODO(), tc.pc, metav1.CreateOptions{}); err != nil { t.Fatalf("Error when creating PacketCapture: %v", err) @@ -586,6 +606,23 @@ func runPacketCaptureTest(t *testing.T, data *TestData, tc pcTestCase) { if !packetCaptureStatusEqual(pc.Status, tc.expectedStatus) { t.Errorf("CR status not match, actual: %+v, expected: %+v", pc.Status, tc.expectedStatus) } + + if tc.expectedStatus.NumberCaptured == 0 { + return + } + // verify packets. + antreaPodName, err := data.getAntreaPodOnNode(nodeName(0)) + require.NoError(t, err) + fileName := fmt.Sprintf("%s.pcapng", tc.pc.Name) + tmpDir := t.TempDir() + dstFileName := filepath.Join(tmpDir, fileName) + packetFile := filepath.Join("/tmp", "antrea", "packetcapture", "packets", fileName) + require.NoError(t, data.copyPodFiles(antreaPodName, "antrea-agent", "kube-system", packetFile, tmpDir)) + defer os.Remove(dstFileName) + file, err := os.Open(dstFileName) + require.NoError(t, err) + defer file.Close() + require.NoError(t, verifyPacketFile(t, tc.pc, file, tc.expectedStatus.NumberCaptured, *srcPodIPs.IPv4, *dstPodIPs.IPv4)) } func (data *TestData) waitForPacketCapture(t *testing.T, name string, specTimeout int, fn func(*crdv1alpha1.PacketCapture) bool) (*crdv1alpha1.PacketCapture, error) { @@ -670,3 +707,65 @@ func conditionSliceEqualsIgnoreLastTransitionTime(as, bs []crdv1alpha1.PacketCap } return true } + +// verifyPacketFile will read the packets file and check if packet count and packet data match with CR. +func verifyPacketFile(t *testing.T, pc *crdv1alpha1.PacketCapture, reader io.Reader, targetNum int32, srcIP net.IP, dstIP net.IP) (err error) { + ngReader, err := pcapgo.NewNgReader(reader, pcapgo.DefaultNgReaderOptions) + if err != nil { + return err + } + + for i := int32(0); i < targetNum; i++ { + data, _, err := ngReader.ReadPacketData() + if err != nil { + return err + } + packet := gopacket.NewPacket(data, layers.LayerTypeEthernet, gopacket.Default) + ipLayer := packet.Layer(layers.LayerTypeIPv4) + require.NotNil(t, ipLayer) + ip, _ := ipLayer.(*layers.IPv4) + assert.Equal(t, srcIP.String(), ip.SrcIP.String()) + assert.Equal(t, dstIP.String(), ip.DstIP.String()) + + if pc.Spec.Packet == nil { + continue + } + + packetSpec := pc.Spec.Packet + proto := packetSpec.Protocol + if proto == nil { + continue + } + if strings.ToUpper(proto.StrVal) == "TCP" || proto.IntVal == 6 { + tcpLayer := packet.Layer(layers.LayerTypeTCP) + require.NotNil(t, tcpLayer) + tcp, _ := tcpLayer.(*layers.TCP) + if packetSpec.TransportHeader.TCP != nil { + ports := packetSpec.TransportHeader.TCP + if ports.DstPort != nil { + assert.Equal(t, *ports.DstPort, int32(tcp.DstPort)) + } + if ports.SrcPort != nil { + assert.Equal(t, *ports.SrcPort, int32(tcp.SrcPort)) + } + } + } else if strings.ToUpper(proto.StrVal) == "UDP" || proto.IntVal == 17 { + udpLayer := packet.Layer(layers.LayerTypeUDP) + require.NotNil(t, udpLayer) + udp, _ := udpLayer.(*layers.UDP) + if packetSpec.TransportHeader.UDP != nil { + ports := packetSpec.TransportHeader.UDP + if ports.DstPort != nil { + assert.Equal(t, *ports.DstPort, int32(udp.DstPort)) + } + if ports.SrcPort != nil { + assert.Equal(t, *ports.SrcPort, int32(udp.SrcPort)) + } + } + } else if strings.ToUpper(proto.StrVal) == "ICMP" || proto.IntVal == 1 { + icmpLayer := packet.Layer(layers.LayerTypeICMPv4) + require.NotNil(t, icmpLayer) + } + } + return nil +}