Skip to content

Commit 3e0f593

Browse files
eliottnesseliottness
authored
fix(net/http): fix segfault on nil response (#4099)
Co-authored-by: eliott.bouhana <[email protected]>
1 parent f144d6a commit 3e0f593

File tree

5 files changed

+464
-231
lines changed

5 files changed

+464
-231
lines changed

‎.github/workflows/appsec.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ jobs:
108108
needs: go-mod-caching
109109
strategy:
110110
matrix:
111-
runs-on: [ macos-13, macos-15 ] # oldest and newest macos runners available - macos-15 is an ARM runner
111+
runs-on: [ macos-14, macos-latest ] # oldest and newest macos runners available
112112
go-version: [ "1.25", "1.24" ]
113113
fail-fast: true # saving some CI time - macos runners are too long to get
114114
steps:

‎contrib/net/http/appsec_test.go‎

Lines changed: 340 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,340 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed
2+
// under the Apache License Version 2.0.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/).
4+
// Copyright 2016 Datadog, Inc.
5+
6+
package http
7+
8+
import (
9+
"context"
10+
"fmt"
11+
"io"
12+
"net/http"
13+
"net/http/httptest"
14+
"strconv"
15+
"strings"
16+
"testing"
17+
18+
internal "github.com/DataDog/dd-trace-go/contrib/net/http/v2/internal/config"
19+
"github.com/DataDog/dd-trace-go/v2/appsec/events"
20+
"github.com/DataDog/dd-trace-go/v2/ddtrace/mocktracer"
21+
"github.com/DataDog/dd-trace-go/v2/instrumentation/appsec/emitter/waf/addresses"
22+
"github.com/DataDog/dd-trace-go/v2/instrumentation/testutils"
23+
"github.com/stretchr/testify/require"
24+
)
25+
26+
func TestAppsec(t *testing.T) {
27+
t.Setenv("DD_APPSEC_RULES", "../../../internal/appsec/testdata/rasp.json")
28+
29+
client := WrapRoundTripper(&emptyRoundTripper{})
30+
31+
for _, enabled := range []bool{true, false} {
32+
33+
t.Run(strconv.FormatBool(enabled), func(t *testing.T) {
34+
t.Setenv("DD_APPSEC_RASP_ENABLED", strconv.FormatBool(enabled))
35+
36+
mt := mocktracer.Start()
37+
defer mt.Stop()
38+
39+
testutils.StartAppSec(t)
40+
if !internal.Instrumentation.AppSecEnabled() {
41+
t.Skip("appsec not enabled")
42+
}
43+
44+
w := httptest.NewRecorder()
45+
r, err := http.NewRequest("GET", "?value=169.254.169.254", nil)
46+
require.NoError(t, err)
47+
48+
TraceAndServe(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
49+
req, err := http.NewRequestWithContext(r.Context(), "GET", "http://169.254.169.254", nil)
50+
require.NoError(t, err)
51+
52+
resp, err := client.RoundTrip(req)
53+
54+
if enabled {
55+
require.True(t, events.IsSecurityError(err))
56+
} else {
57+
require.NoError(t, err)
58+
}
59+
60+
if resp != nil {
61+
defer resp.Body.Close()
62+
}
63+
}), w, r, &ServeConfig{
64+
Service: "service",
65+
Resource: "resource",
66+
})
67+
68+
spans := mt.FinishedSpans()
69+
require.Len(t, spans, 2) // service entry serviceSpan & http request serviceSpan
70+
serviceSpan := spans[1]
71+
72+
if !enabled {
73+
require.NotContains(t, serviceSpan.Tags(), "_dd.appsec.json")
74+
require.NotContains(t, serviceSpan.Tags(), "_dd.stack")
75+
return
76+
}
77+
78+
require.Contains(t, serviceSpan.Tags(), "_dd.appsec.json")
79+
appsecJSON := serviceSpan.Tag("_dd.appsec.json")
80+
require.Contains(t, appsecJSON, addresses.ServerIONetURLAddr)
81+
82+
require.Contains(t, serviceSpan.Tags(), "_dd.stack")
83+
require.NotContains(t, serviceSpan.Tags(), "error.message")
84+
85+
// This is a nested event so it should contain the child span id in the service entry span
86+
// TODO(eliott.bouhana): uncomment this once we have the child span id in the service entry span
87+
// require.Contains(t, appsecJSON, `"span_id":`+strconv.FormatUint(requestSpan.SpanID(), 10))
88+
})
89+
}
90+
}
91+
92+
func TestAppsecAPI10(t *testing.T) {
93+
t.Setenv("DD_APPSEC_RULES", "../../../internal/appsec/testdata/api10.json")
94+
t.Setenv("DD_API_SECURITY_DOWNSTREAM_REQUEST_BODY_ANALYSIS_SAMPLE_RATE", "1.0")
95+
96+
var b strings.Builder
97+
b.WriteString(`{"payload_in":"%s"`)
98+
for i := 0; i < 1<<12; i++ {
99+
b.WriteString(fmt.Sprintf(`,"%d":"b"`, i))
100+
}
101+
b.WriteString(`}`)
102+
103+
for _, tc := range []struct {
104+
name string
105+
request func(ctx context.Context) *http.Request
106+
response *http.Response
107+
tagName string
108+
tagValue string
109+
}{
110+
{
111+
name: "method",
112+
request: func(ctx context.Context) *http.Request {
113+
req, _ := http.NewRequestWithContext(ctx, "TRACE", "http://localhost:8080", nil)
114+
return req
115+
},
116+
tagName: "_dd.appsec.trace.req_method",
117+
tagValue: "TAG_API10_REQ_METHOD",
118+
},
119+
{
120+
name: "headers",
121+
request: func(ctx context.Context) *http.Request {
122+
req, _ := http.NewRequestWithContext(ctx, "GET", "http://localhost:8080", nil)
123+
req.Header.Set("Witness", "pwq3ojtropiw3hjtowir")
124+
return req
125+
},
126+
tagName: "_dd.appsec.trace.req_headers",
127+
tagValue: "TAG_API10_REQ_HEADERS",
128+
},
129+
{
130+
name: "body",
131+
request: func(ctx context.Context) *http.Request {
132+
req, _ := http.NewRequestWithContext(ctx, "GET", "http://localhost:8080", io.NopCloser(strings.NewReader(`{"payload_in":"qw2jedrkjerbgol23ewpfirj2qw3or"}`)))
133+
req.Header.Set("Content-Type", "application/json")
134+
return req
135+
},
136+
tagName: "_dd.appsec.trace.req_body",
137+
tagValue: "TAG_API10_REQ_BODY",
138+
},
139+
{
140+
name: "big-body",
141+
request: func(ctx context.Context) *http.Request {
142+
t.Setenv("DD_APPSEC_WAF_TIMEOUT", "1s")
143+
body := fmt.Sprintf(b.String(), "qw2jedrkjerbgol23ewpfirj2qw3or")
144+
req, _ := http.NewRequestWithContext(ctx, "GET", "http://localhost:8080", io.NopCloser(strings.NewReader(body)))
145+
req.Header.Set("Content-Type", "application/json")
146+
return req
147+
},
148+
tagName: "_dd.appsec.trace.req_body",
149+
tagValue: "TAG_API10_REQ_BODY",
150+
},
151+
{
152+
name: "resp-status",
153+
request: func(ctx context.Context) *http.Request {
154+
req, _ := http.NewRequestWithContext(ctx, "GET", "http://localhost:8080", nil)
155+
return req
156+
},
157+
response: &http.Response{
158+
StatusCode: 201,
159+
},
160+
tagName: "_dd.appsec.trace.res_status",
161+
tagValue: "TAG_API10_RES_STATUS",
162+
},
163+
{
164+
name: "resp-headers",
165+
request: func(ctx context.Context) *http.Request {
166+
req, _ := http.NewRequestWithContext(ctx, "GET", "http://localhost:8080", nil)
167+
return req
168+
},
169+
response: &http.Response{
170+
StatusCode: 200,
171+
Header: map[string][]string{
172+
"echo-headers": {"qwoierj12l3"},
173+
},
174+
},
175+
tagName: "_dd.appsec.trace.res_headers",
176+
tagValue: "TAG_API10_RES_HEADERS",
177+
},
178+
{
179+
name: "resp-body",
180+
request: func(ctx context.Context) *http.Request {
181+
req, _ := http.NewRequestWithContext(ctx, "GET", "http://localhost:8080", nil)
182+
return req
183+
},
184+
response: &http.Response{
185+
StatusCode: 200,
186+
Header: map[string][]string{
187+
"Content-Type": {"application/json"},
188+
},
189+
Body: io.NopCloser(strings.NewReader(`{"payload_out":"kqehf09123r4lnksef"}`)),
190+
},
191+
tagName: "_dd.appsec.trace.res_body",
192+
tagValue: "TAG_API10_RES_BODY",
193+
},
194+
{
195+
name: "resp-big-body",
196+
request: func(ctx context.Context) *http.Request {
197+
t.Setenv("DD_APPSEC_WAF_TIMEOUT", "1s")
198+
req, _ := http.NewRequestWithContext(ctx, "GET", "http://localhost:8080", nil)
199+
return req
200+
},
201+
response: &http.Response{
202+
StatusCode: 200,
203+
Header: map[string][]string{
204+
"Content-Type": {"application/json"},
205+
},
206+
Body: io.NopCloser(strings.NewReader(fmt.Sprintf(b.String(), "kqehf09123r4lnksef"))),
207+
},
208+
tagName: "_dd.appsec.trace.res_body",
209+
tagValue: "TAG_API10_RES_BODY",
210+
},
211+
} {
212+
t.Run(tc.name, func(t *testing.T) {
213+
214+
client := WrapRoundTripper(&emptyRoundTripper{customResponse: tc.response})
215+
216+
mt := mocktracer.Start()
217+
defer mt.Stop()
218+
219+
testutils.StartAppSec(t)
220+
if !internal.Instrumentation.AppSecEnabled() {
221+
t.Skip("appsec not enabled")
222+
}
223+
224+
w := httptest.NewRecorder()
225+
r, err := http.NewRequest("GET", "", nil)
226+
require.NoError(t, err)
227+
228+
TraceAndServe(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
229+
resp, err := client.RoundTrip(tc.request(r.Context()))
230+
require.NoError(t, err)
231+
if resp != nil && resp.Body != nil {
232+
defer resp.Body.Close()
233+
}
234+
}), w, r, &ServeConfig{
235+
Service: "service",
236+
Resource: "resource",
237+
})
238+
239+
spans := mt.FinishedSpans()
240+
require.Len(t, spans, 2) // service entry serviceSpan & http request serviceSpan
241+
serviceSpan := spans[1]
242+
243+
require.Contains(t, serviceSpan.Tags(), tc.tagName)
244+
require.Equal(t, serviceSpan.Tags()[tc.tagName], tc.tagValue)
245+
})
246+
}
247+
}
248+
249+
func TestAppsecHTTP30X(t *testing.T) {
250+
t.Setenv("DD_APPSEC_RULES", "../../../internal/appsec/testdata/api10.json")
251+
t.Setenv("DD_API_SECURITY_DOWNSTREAM_REQUEST_BODY_ANALYSIS_SAMPLE_RATE", "1.0")
252+
253+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
254+
switch r.URL.Path {
255+
case "/redirect":
256+
http.Redirect(w, r, "/final", http.StatusTemporaryRedirect)
257+
case "/move":
258+
http.Redirect(w, r, "/final", http.StatusMovedPermanently)
259+
case "/final":
260+
w.WriteHeader(http.StatusOK)
261+
default:
262+
require.Failf(t, "unexpected request", "path: %s", r.URL.Path)
263+
}
264+
}))
265+
266+
defer srv.Close()
267+
268+
httpClient := srv.Client()
269+
httpClient.Transport = WrapRoundTripper(httpClient.Transport)
270+
271+
t.Run("move", func(t *testing.T) {
272+
mt := mocktracer.Start()
273+
defer mt.Stop()
274+
275+
testutils.StartAppSec(t)
276+
if !internal.Instrumentation.AppSecEnabled() {
277+
t.Skip("appsec not enabled")
278+
}
279+
280+
w := httptest.NewRecorder()
281+
r, err := http.NewRequest("GET", srv.URL+"/move", nil)
282+
require.NoError(t, err)
283+
284+
TraceAndServe(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
285+
resp, err := httpClient.Do(r)
286+
require.NoError(t, err)
287+
if resp != nil && resp.Body != nil {
288+
defer resp.Body.Close()
289+
}
290+
}), w, r, &ServeConfig{
291+
Service: "service",
292+
Resource: "resource",
293+
})
294+
295+
spans := mt.FinishedSpans()
296+
require.Len(t, spans, 3) // service entry serviceSpan & http request serviceSpan
297+
serviceSpan := spans[2]
298+
299+
require.Contains(t, serviceSpan.Tags(), "appsec.api.redirection.move_target")
300+
require.Equal(t, serviceSpan.Tags()["appsec.api.redirection.move_target"], "/final")
301+
302+
require.Contains(t, serviceSpan.Tags(), "appsec.api.redirection.nothing")
303+
require.Equal(t, serviceSpan.Tags()["appsec.api.redirection.nothing"], float64(1))
304+
})
305+
306+
t.Run("redirect", func(t *testing.T) {
307+
mt := mocktracer.Start()
308+
defer mt.Stop()
309+
310+
testutils.StartAppSec(t)
311+
if !internal.Instrumentation.AppSecEnabled() {
312+
t.Skip("appsec not enabled")
313+
}
314+
315+
w := httptest.NewRecorder()
316+
r, err := http.NewRequest("GET", srv.URL+"/redirect", nil)
317+
require.NoError(t, err)
318+
319+
TraceAndServe(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
320+
resp, err := httpClient.Do(r)
321+
require.NoError(t, err)
322+
if resp != nil && resp.Body != nil {
323+
defer resp.Body.Close()
324+
}
325+
}), w, r, &ServeConfig{
326+
Service: "service",
327+
Resource: "resource",
328+
})
329+
330+
spans := mt.FinishedSpans()
331+
require.Len(t, spans, 3) // service entry serviceSpan & http request serviceSpan
332+
serviceSpan := spans[2]
333+
334+
require.Contains(t, serviceSpan.Tags(), "appsec.api.redirection.redirect_target")
335+
require.Equal(t, serviceSpan.Tags()["appsec.api.redirection.redirect_target"], "/final")
336+
337+
require.Contains(t, serviceSpan.Tags(), "appsec.api.redirection.nothing")
338+
require.Equal(t, serviceSpan.Tags()["appsec.api.redirection.nothing"], float64(1))
339+
})
340+
}

0 commit comments

Comments
 (0)