Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into julio.guerra/fix-grpc…
Browse files Browse the repository at this point in the history
…-blocking
  • Loading branch information
Julio-Guerra committed Jun 4, 2024
2 parents 89ecea1 + 6126fb6 commit 3a3cc96
Show file tree
Hide file tree
Showing 67 changed files with 1,930 additions and 545 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/unit-integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -311,14 +311,16 @@ jobs:
mkdir -p $TEST_RESULTS
PACKAGE_NAMES=$(go list ./... | grep -v /contrib/)
gotestsum --junitfile ${TEST_RESULTS}/gotestsum-report.xml -- $PACKAGE_NAMES -v -race -coverprofile=coverage.txt -covermode=atomic
cd ./internal/exectracetest
gotestsum --junitfile ${TEST_RESULTS}/gotestsum-report-exectrace.xml -- -v -race -coverprofile=coverage.txt -covermode=atomic
- name: Upload the results to Datadog CI App
if: always()
continue-on-error: true
uses: ./.github/actions/dd-ci-upload
with:
dd-api-key: ${{ secrets.DD_CI_API_KEY }}
files: ${{ env.TEST_RESULTS }}/gotestsum-report.xml
files: ${{ env.TEST_RESULTS }}/gotestsum-report.xml ${{ env.TEST_RESULTS }}/gotestsum-report-exectrace.xml
tags: go:${{ inputs.go-version }}},arch:${{ runner.arch }},os:${{ runner.os }},distribution:${{ runner.distribution }}
- name: Upload Coverage
if: always()
Expand Down
33 changes: 33 additions & 0 deletions appsec/events/block.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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 2022 Datadog, Inc.

// Package events provides security event types that appsec can return in function calls it monitors when blocking them.
// It allows finer-grained integrations of appsec into your Go errors' management logic.
package events

import "errors"

var _ error = (*BlockingSecurityEvent)(nil)

var securityError = &BlockingSecurityEvent{}

// BlockingSecurityEvent is the error type returned by function calls blocked by appsec.
// Even though appsec takes care of responding automatically to the blocked requests, it
// is your duty to abort the request handlers that are calling functions blocked by appsec.
// For instance, if a gRPC handler performs a SQL query blocked by appsec, the SQL query
// function call gets blocked and aborted by returning an error of type SecurityBlockingEvent.
// This allows you to safely abort your request handlers, and to be able to leverage errors.As if
// necessary in your Go error management logic to be able to tell if the error is a blocking security
// event or not (eg. to avoid retrying an HTTP client request).
type BlockingSecurityEvent struct{}

func (*BlockingSecurityEvent) Error() string {
return "request blocked by WAF"
}

// IsSecurityError returns true if the error is a security event.
func IsSecurityError(err error) bool {
return errors.Is(err, securityError)
}
6 changes: 3 additions & 3 deletions contrib/google.golang.org/grpc/fixtures_test.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions contrib/google.golang.org/grpc/fixtures_test_grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion contrib/google.golang.org/grpc/gen_proto.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ COPYRIGHT_HEADER="// Unless explicitly stated otherwise all files in this reposi
// Copyright ${YEAR} Datadog, Inc.
"

go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.31.0
go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.33.0
go install google.golang.org/grpc/cmd/[email protected]

protoc fixtures_test.proto \
Expand Down
4 changes: 2 additions & 2 deletions contrib/labstack/echo.v4/appsec.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ package echo
import (
"net/http"

"gopkg.in/DataDog/dd-trace-go.v1/appsec/events"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/emitter/httpsec"
"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/emitter/httpsec/types"

"github.com/labstack/echo/v4"
)
Expand All @@ -27,7 +27,7 @@ func withAppSec(next echo.HandlerFunc, span tracer.Span) echo.HandlerFunc {
err = next(c)
// If the error is a monitoring one, it means appsec actions will take care of writing the response
// and handling the error. Don't call the echo error handler in this case
if _, ok := err.(*types.MonitoringError); !ok && err != nil {
if _, ok := err.(*events.BlockingSecurityEvent); !ok && err != nil {
c.Error(err)
}
})
Expand Down
17 changes: 16 additions & 1 deletion contrib/net/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package http // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http"

import (
"net/http"
"strings"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
Expand Down Expand Up @@ -49,7 +50,8 @@ func (mux *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}
// get the resource associated to this request
_, route := mux.Handler(r)
_, pattern := mux.Handler(r)
route := patternRoute(pattern)
resource := mux.cfg.resourceNamer(r)
if resource == "" {
resource = r.Method + " " + route
Expand All @@ -65,6 +67,19 @@ func (mux *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
})
}

// patternRoute returns the route part of a go1.22 style ServeMux pattern. I.e.
// it returns "/foo" for the pattern "/foo" as well as the pattern "GET /foo".
func patternRoute(s string) string {
// Support go1.22 serve mux patterns: [METHOD ][HOST]/[PATH]
// Consider any text before a space or tab to be the method of the pattern.
// See net/http.parsePattern and the link below for more information.
// https://pkg.go.dev/net/http#hdr-Patterns
if i := strings.IndexAny(s, " \t"); i > 0 && len(s) >= i+1 {
return strings.TrimLeft(s[i+1:], " \t")
}
return s
}

// WrapHandler wraps an http.Handler with tracing using the given service and resource.
// If the WithResourceNamer option is provided as part of opts, it will take precedence over the resource argument.
func WrapHandler(h http.Handler, service, resource string, opts ...Option) http.Handler {
Expand Down
46 changes: 46 additions & 0 deletions contrib/net/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,52 @@ func TestServeMuxUsesResourceNamer(t *testing.T) {
assert.Equal("net/http", s.Tag(ext.Component))
}

func TestServeMuxGo122Patterns(t *testing.T) {
mt := mocktracer.Start()
defer mt.Stop()

// A mux with go1.21 patterns ("/bar") and go1.22 patterns ("GET /foo")
mux := NewServeMux()
mux.HandleFunc("/bar", func(w http.ResponseWriter, r *http.Request) {})
mux.HandleFunc("GET /foo", func(w http.ResponseWriter, r *http.Request) {})

// Try to hit both routes
barW := httptest.NewRecorder()
mux.ServeHTTP(barW, httptest.NewRequest("GET", "/bar", nil))
fooW := httptest.NewRecorder()
mux.ServeHTTP(fooW, httptest.NewRequest("GET", "/foo", nil))

// Assert the number of spans
assert := assert.New(t)
spans := mt.FinishedSpans()
assert.Equal(2, len(spans))

// Check the /bar span
barSpan := spans[0]
assert.Equal(http.StatusOK, barW.Code)
assert.Equal("/bar", barSpan.Tag(ext.HTTPRoute))
assert.Equal("GET /bar", barSpan.Tag(ext.ResourceName))

// Check the /foo span
fooSpan := spans[1]
if fooW.Code == http.StatusOK {
assert.Equal("/foo", fooSpan.Tag(ext.HTTPRoute))
assert.Equal("GET /foo", fooSpan.Tag(ext.ResourceName))
} else {
// Until our go.mod version is go1.22 or greater, the mux will not
// understand the "GET /foo" pattern, causing the request to be handled
// by the 404 handler. Let's assert what we can, and mark the test as
// skipped to highlight the issue.
assert.Equal(http.StatusNotFound, fooW.Code)
assert.Equal(nil, fooSpan.Tag(ext.HTTPRoute))
// Using "GET " as a resource name doesn't seem ideal, but that's how
// the mux instrumentation deals with 404s right now.
assert.Equal("GET ", fooSpan.Tag(ext.ResourceName))
t.Skip("run `go mod edit -go=1.22` to run the full test")
}

}

func TestWrapHandlerWithResourceNameNoRace(_ *testing.T) {
mt := mocktracer.Start()
defer mt.Stop()
Expand Down
13 changes: 12 additions & 1 deletion contrib/net/http/roundtripper.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package http

import (
"fmt"
"gopkg.in/DataDog/dd-trace-go.v1/appsec/events"
"math"
"net/http"
"os"
Expand All @@ -15,6 +16,8 @@ import (
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec"
"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/emitter/httpsec"
)

type roundTripper struct {
Expand Down Expand Up @@ -57,7 +60,7 @@ func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err er
if rt.cfg.after != nil {
rt.cfg.after(res, span)
}
if rt.cfg.errCheck == nil || rt.cfg.errCheck(err) {
if !events.IsSecurityError(err) && (rt.cfg.errCheck == nil || rt.cfg.errCheck(err)) {
span.Finish(tracer.WithError(err))
} else {
span.Finish()
Expand All @@ -75,7 +78,15 @@ func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err er
fmt.Fprintf(os.Stderr, "contrib/net/http.Roundtrip: failed to inject http headers: %v\n", err)
}
}

if appsec.RASPEnabled() {
if err := httpsec.ProtectRoundTrip(ctx, r2.URL.String()); err != nil {
return nil, err
}
}

res, err = rt.base.RoundTrip(r2)

if err != nil {
span.SetTag("http.errors", err.Error())
if rt.cfg.errCheck == nil || rt.cfg.errCheck(err) {
Expand Down
78 changes: 78 additions & 0 deletions contrib/net/http/roundtripper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@ import (
"testing"
"time"

"gopkg.in/DataDog/dd-trace-go.v1/appsec/events"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/namingschematest"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec"
"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener/httpsec"
"gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -619,3 +622,78 @@ func TestClientNamingSchema(t *testing.T) {
t.Run("ServiceName", namingschematest.NewServiceNameTest(genSpans, wantServiceNameV0))
t.Run("SpanName", namingschematest.NewSpanNameTest(genSpans, assertOpV0, assertOpV1))
}

type emptyRoundTripper struct{}

func (rt *emptyRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) {
recorder := httptest.NewRecorder()
recorder.WriteHeader(200)
return recorder.Result(), nil
}

func TestAppsec(t *testing.T) {
t.Setenv("DD_APPSEC_RULES", "../../../internal/appsec/testdata/rasp.json")

client := WrapRoundTripper(&emptyRoundTripper{})

for _, enabled := range []bool{true, false} {

t.Run(strconv.FormatBool(enabled), func(t *testing.T) {
t.Setenv("DD_APPSEC_RASP_ENABLED", strconv.FormatBool(enabled))

mt := mocktracer.Start()
defer mt.Stop()

appsec.Start()
if !appsec.Enabled() {
t.Skip("appsec not enabled")
}

defer appsec.Stop()

w := httptest.NewRecorder()
r, err := http.NewRequest("GET", "?value=169.254.169.254", nil)
require.NoError(t, err)

TraceAndServe(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
req, err := http.NewRequest("GET", "http://169.254.169.254", nil)
require.NoError(t, err)

resp, err := client.RoundTrip(req.WithContext(r.Context()))

if enabled {
require.ErrorIs(t, err, &events.BlockingSecurityEvent{})
} else {
require.NoError(t, err)
}

if resp != nil {
defer resp.Body.Close()
}
}), w, r, &ServeConfig{
Service: "service",
Resource: "resource",
})

spans := mt.FinishedSpans()
require.Len(t, spans, 2) // service entry serviceSpan & http request serviceSpan
serviceSpan := spans[1]

if !enabled {
require.NotContains(t, serviceSpan.Tags(), "_dd.appsec.json")
require.NotContains(t, serviceSpan.Tags(), "_dd.stack")
return
}

require.Contains(t, serviceSpan.Tags(), "_dd.appsec.json")
appsecJSON := serviceSpan.Tag("_dd.appsec.json")
require.Contains(t, appsecJSON, httpsec.ServerIoNetURLAddr)

require.Contains(t, serviceSpan.Tags(), "_dd.stack")

// This is a nested event so it should contain the child span id in the service entry span
// TODO(eliott.bouhana): uncomment this once we have the child span id in the service entry span
// require.Contains(t, appsecJSON, `"span_id":`+strconv.FormatUint(requestSpan.SpanID(), 10))
})
}
}
9 changes: 5 additions & 4 deletions ddtrace/tracer/dynamic_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type dynamicConfig[T any] struct {
current T // holds the current configuration value
startup T // holds the startup configuration value
cfgName string // holds the name of the configuration, has to be compatible with telemetry.Configuration.Name
cfgOrigin string // holds the origin of the current configuration value (currently only supports remote_config, empty otherwise)
cfgOrigin telemetry.Origin // holds the origin of the current configuration value (currently only supports remote_config, empty otherwise)
apply func(T) bool // executes any config-specific operations to propagate the update properly, returns whether the update was applied
equal func(x, y T) bool // compares two configuration values, this is used to avoid unnecessary config and telemetry updates
}
Expand All @@ -42,7 +42,7 @@ func (dc *dynamicConfig[T]) get() T {
}

// update applies a new configuration value
func (dc *dynamicConfig[T]) update(val T, origin string) bool {
func (dc *dynamicConfig[T]) update(val T, origin telemetry.Origin) bool {
dc.Lock()
defer dc.Unlock()
if dc.equal(dc.current, val) {
Expand All @@ -61,15 +61,16 @@ func (dc *dynamicConfig[T]) reset() bool {
return false
}
dc.current = dc.startup
dc.cfgOrigin = ""
// TODO: set the origin to the startup value's origin
dc.cfgOrigin = telemetry.OriginDefault
return dc.apply(dc.startup)
}

// handleRC processes a new configuration value from remote config
// Returns whether the configuration value has been updated or not
func (dc *dynamicConfig[T]) handleRC(val *T) bool {
if val != nil {
return dc.update(*val, "remote_config")
return dc.update(*val, telemetry.OriginRemoteConfig)
}
return dc.reset()
}
Expand Down
6 changes: 4 additions & 2 deletions ddtrace/tracer/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ type startupInfo struct {
AnalyticsEnabled bool `json:"analytics_enabled"` // True if there is a global analytics rate set
SampleRate string `json:"sample_rate"` // The default sampling rate for the rules sampler
SampleRateLimit string `json:"sample_rate_limit"` // The rate limit configured with the rules sampler
SamplingRules []SamplingRule `json:"sampling_rules"` // Rules used by the rules sampler
TraceSamplingRules []SamplingRule `json:"trace_sampling_rules"` // Trace rules used by the rules sampler
SpanSamplingRules []SamplingRule `json:"span_sampling_rules"` // Span rules used by the rules sampler
SamplingRulesError string `json:"sampling_rules_error"` // Any errors that occurred while parsing sampling rules
ServiceMappings map[string]string `json:"service_mappings"` // Service Mappings
Tags map[string]string `json:"tags"` // Global tags
Expand Down Expand Up @@ -103,7 +104,8 @@ func logStartup(t *tracer) {
AnalyticsEnabled: !math.IsNaN(globalconfig.AnalyticsRate()),
SampleRate: fmt.Sprintf("%f", t.rulesSampling.traces.globalRate),
SampleRateLimit: "disabled",
SamplingRules: append(t.config.traceRules, t.config.spanRules...),
TraceSamplingRules: t.config.traceRules,
SpanSamplingRules: t.config.spanRules,
ServiceMappings: t.config.serviceMappings,
Tags: tags,
RuntimeMetricsEnabled: t.config.runtimeMetrics,
Expand Down
Loading

0 comments on commit 3a3cc96

Please sign in to comment.