Skip to content

Commit

Permalink
Fix handling of IP with canonical headers
Browse files Browse the repository at this point in the history
  • Loading branch information
e-n-0 committed Oct 14, 2024
1 parent e7e418b commit 3aaea03
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 6 deletions.
10 changes: 6 additions & 4 deletions contrib/envoyproxy/envoy/envoy.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/emitter/waf/actions"
"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener/waf"
"io"
"net/http"
"net/url"
"strconv"
"strings"
Expand Down Expand Up @@ -175,7 +176,7 @@ func ProcessRequestHeaders(ctx context.Context, req *extproc.ProcessingRequest_R
currentRequest.parsedUrl = parsedUrl

// client ip set in the x-forwarded-for header (cf: https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers#x-forwarded-for)
ipTags, clientIp := httpsec2.ClientIPTags(headers, false, "")
ipTags, clientIp := httpsec2.ClientIPTags(headers, true, "")

currentRequest.requestArgs = httpsec.MakeHandlerOperationArgs(headers, method, host, clientIp, parsedUrl)
headers = currentRequest.requestArgs.Headers // Replace headers with the ones from the args because it has been modified
Expand Down Expand Up @@ -345,16 +346,17 @@ func createExternalProcessedSpan(ctx context.Context, headers map[string][]strin
}

// Separate normal headers of the initial request made by the client and the pseudo headers of HTTP/2
// Also format the headers to be used by the tracer as a map[string][]string
// - Format the headers to be used by the tracer as a map[string][]string
// - Set header keys to be canonical
func separateEnvoyHeaders(receivedHeaders []*corev3.HeaderValue) (map[string][]string, map[string][]string) {
headers := make(map[string][]string)
pseudoHeadersHttp2 := make(map[string][]string)
for _, v := range receivedHeaders {
key := strings.ToLower(v.GetKey())
key := v.GetKey()
if key[0] == ':' {
pseudoHeadersHttp2[key] = []string{string(v.GetRawValue())}
} else {
headers[key] = []string{string(v.GetRawValue())}
headers[http.CanonicalHeaderKey(key)] = []string{string(v.GetRawValue())}
}
}
return headers, pseudoHeadersHttp2
Expand Down
42 changes: 42 additions & 0 deletions contrib/envoyproxy/envoy/envoy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ func TestGeneratedSpan(t *testing.T) {
}

func TestXForwardedForHeaderClientIp(t *testing.T) {
t.Setenv("DD_APPSEC_RULES", "../../../internal/appsec/testdata/blocking.json")
appsec.Start()
defer appsec.Stop()
if !appsec.Enabled() {
Expand Down Expand Up @@ -368,6 +369,47 @@ func TestXForwardedForHeaderClientIp(t *testing.T) {
// Appsec
require.Equal(t, 1, span.Tag("_dd.appsec.enabled"))
})

t.Run("blocking-client-ip", func(t *testing.T) {
client, mt, cleanup := setup()
defer cleanup()

ctx := context.Background()
stream, err := client.Process(ctx)
require.NoError(t, err)

err = stream.Send(&extproc.ProcessingRequest{
Request: &extproc.ProcessingRequest_RequestHeaders{
RequestHeaders: &extproc.HttpHeaders{
Headers: makeRequestHeaders(map[string]string{"User-Agent": "Mistake not...", "X-Forwarded-For": "1.2.3.4"}, "GET", "/"),
},
},
})
require.NoError(t, err)

// Handle the immediate response
res, err := stream.Recv()
require.Equal(t, uint32(0), res.GetImmediateResponse().GetGrpcStatus().Status)
require.Equal(t, typev3.StatusCode(403), res.GetImmediateResponse().GetStatus().Code)
require.Equal(t, "Content-Type", res.GetImmediateResponse().GetHeaders().SetHeaders[0].GetHeader().Key)
require.Equal(t, "application/json", string(res.GetImmediateResponse().GetHeaders().SetHeaders[0].GetHeader().RawValue))
require.NoError(t, err)

err = stream.CloseSend()
require.NoError(t, err)
stream.Recv() // to flush the spans

finished := mt.FinishedSpans()
require.Len(t, finished, 1)
checkForAppsecEvent(t, finished, map[string]int{"blk-001-001": 1})

// Check for tags
span := finished[0]
require.Equal(t, "1.2.3.4", span.Tag("http.client_ip"))
require.Equal(t, 1, span.Tag("_dd.appsec.enabled"))
require.Equal(t, true, span.Tag("appsec.event"))
require.Equal(t, true, span.Tag("appsec.blocked"))
})
}

func newEnvoyAppsecRig(traceClient bool, interceptorOpts ...ddgrpc.Option) (*envoyAppsecRig, error) {
Expand Down
3 changes: 1 addition & 2 deletions internal/appsec/emitter/httpsec/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,7 @@ func MakeHandlerOperationArgs(headers map[string][]string, method string, host s
func headersRemoveCookies(headers http.Header) map[string][]string {
headersNoCookies := make(http.Header, len(headers))
for k, v := range headers {
k := strings.ToLower(k)
if k == "cookie" {
if k == "Cookie" {
continue
}
headersNoCookies[k] = v
Expand Down

0 comments on commit 3aaea03

Please sign in to comment.