Fixes multiple problems with http processing/tagging on Windows. (#15022

)

* Fixes multiple problems with http processing/tagging on Windows.
- There was an offset error in which the port was not properly computed
  on ipv6 connections
- There was a problem with computing whether an ipv6 address was loopback or
  not
- The fullpath indication (which is used to compute the key) was not
  properly being computed.  This led to the same tuple being used
  as a different key, so transactions were not properly combined.

* fix grammar error in release notes

pkg/network/protocols/http/etw_http_service.go

@@ -486,7 +486,7 @@ func httpCallbackOnHTTPConnectionTraceTaskConnConn(eventInfo *etw.DDEtwEventInfo
 		connOpen.conn.tup.Family = binary.LittleEndian.Uint16(userData[12:14])
 		connOpen.conn.tup.SrvPort = binary.BigEndian.Uint16(userData[14:16])
 		copy(connOpen.conn.tup.SrvAddr[:], userData[20:36])
-		connOpen.conn.tup.CliPort = binary.BigEndian.Uint16(userData[36:48])
+		connOpen.conn.tup.CliPort = binary.BigEndian.Uint16(userData[46:48])
 		copy(connOpen.conn.tup.CliAddr[:], userData[52:68])
 	}
 
@@ -682,7 +682,7 @@ func httpCallbackOnHTTPRequestTraceTaskParse(eventInfo *etw.DDEtwEventInfo) {
 		// whole thing
 		httpConnLink.http.RequestFragment = make([]byte, maxRequestFragmentBytes)
 		httpConnLink.http.Txn.MaxRequestFragment = uint16(maxRequestFragmentBytes)
-		httpConnLink.http.RequestFragment[0] = 32
+		httpConnLink.http.RequestFragment[0] = 32 // this is a leading space.
 
 		// copy rest of arguments
 		copy(httpConnLink.http.RequestFragment[1:], urlParsed.Path)
@@ -1293,6 +1293,7 @@ func SetMaxRequestBytes(maxRequestBytes uint64) {
 }
 
 func SetEnabledProtocols(http, https bool) {
+	captureHTTP = http
 	captureHTTPS = https
 }
 func (hei *httpEtwInterface) OnStart() {

pkg/network/protocols/http/model_windows.go

@@ -178,7 +178,7 @@ func (tx *WinHttpTransaction) Path(buffer []byte) ([]byte, bool) {
 	}
 	n := copy(buffer, b[:j])
 	// indicate if we knowingly captured the entire path
-	fullPath := n < len(b)
+	fullPath := n <= len(b)
 	return buffer[:n], fullPath
 
 }

pkg/network/protocols/http/monitor_windows.go

@@ -146,7 +146,11 @@ func removeDuplicates(stats map[Key]*RequestStats) {
 
 func isLocalhost(k Key) bool {
 	var sAddr util.Address
-	if k.SrcIPHigh == 0 {
+	// this little hack is because ipv6 loopback (::1) has the property of having
+	// the top 64 bits be zero (just like an ipv4).  Could just skip the call to
+	// IsLoopback() below, but leaving it to allow the underlying library to do
+	// the check as originally desired.
+	if k.SrcIPHigh == 0 && k.SrcIPLow != uint64(0x0100000000000000) {
 		sAddr = util.V4Address(uint32(k.SrcIPLow))
 	} else {
 		sAddr = util.V6Address(k.SrcIPLow, k.SrcIPHigh)

pkg/network/protocols/http/monitor_windows_test.go

@@ -0,0 +1,75 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2016-present Datadog, Inc.
+
+//go:build windows && npm
+// +build windows,npm
+
+package http
+
+import (
+	"encoding/binary"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsLocalhost(t *testing.T) {
+
+	tests := []struct {
+		key      Key
+		expected bool
+	}{
+		// the isLocalhost function checks only the srcip, but set them both
+		{
+			key: Key{
+				KeyTuple: KeyTuple{
+					SrcIPHigh: 0,
+					SrcIPLow:  uint64(binary.LittleEndian.Uint32([]uint8{127, 0, 0, 1})),
+					DstIPHigh: 0,
+					DstIPLow:  uint64(binary.LittleEndian.Uint32([]uint8{127, 0, 0, 1})),
+				},
+			},
+			expected: true,
+		},
+		{
+			key: Key{
+				KeyTuple: KeyTuple{
+					SrcIPHigh: 0,
+					SrcIPLow:  uint64(binary.LittleEndian.Uint32([]uint8{192, 168, 1, 1})),
+					DstIPHigh: 0,
+					DstIPLow:  uint64(binary.LittleEndian.Uint32([]uint8{192, 168, 1, 1})),
+				},
+			},
+			expected: false,
+		},
+		{
+			key: Key{
+				KeyTuple: KeyTuple{
+					SrcIPHigh: 0,
+					SrcIPLow:  binary.LittleEndian.Uint64([]uint8{0, 0, 0, 0, 0, 0, 0, 1}),
+					DstIPHigh: 0,
+					DstIPLow:  binary.LittleEndian.Uint64([]uint8{0, 0, 0, 0, 0, 0, 0, 1}),
+				},
+			},
+			expected: true,
+		},
+		{
+			key: Key{
+				KeyTuple: KeyTuple{
+					SrcIPHigh: binary.LittleEndian.Uint64([]uint8{0xf, 0xe, 0x8, 0, 0, 0, 0, 0}),
+					SrcIPLow:  binary.LittleEndian.Uint64([]uint8{0x1, 0x9, 0x3, 0xe, 0x4, 0xc, 0xd, 0x6, 0xf, 0xf, 0xa, 0x4}),
+					DstIPHigh: binary.LittleEndian.Uint64([]uint8{0xf, 0xe, 0x8, 0, 0, 0, 0, 0}),
+					DstIPLow:  binary.LittleEndian.Uint64([]uint8{0x1, 0x9, 0x3, 0xe, 0x4, 0xc, 0xd, 0x6, 0xf, 0xf, 0xa, 0x4}),
+				},
+			},
+			expected: false,
+		},
+	}
+	for idx, tt := range tests {
+		is := isLocalhost(tt.key)
+		assert.Equal(t, tt.expected, is, "Unexpected result %v for test %v", is, idx)
+	}
+
+}

releasenotes/notes/fixwinhttploopback-cec77f41a8feff7f.yaml

@@ -0,0 +1,12 @@
+# Each section from every release note are combined when the
+# CHANGELOG.rst is rendered. So the text needs to be worded so that
+# it does not depend on any information only available in another
+# section. This may mean repeating some details, but each section
+# must be readable independently of the other.
+#
+# Each section note must be formatted as reStructuredText.
+---
+fixes:
+  - |
+    On Windows, fixes bug in which HTTP connections were not properly accounted
+    for when the client and server were the same host (loopback).