From fd670e0afd61a4e15341a9a3f3e01156d3752541 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Sep 2024 11:40:45 +0200 Subject: [PATCH] fix(internal/appsec): move httptracemock into instrumentation and TestRASPSQLi into contrib/database/sql --- .github/workflows/appsec.yml | 1 + contrib/database/sql/appsec_test.go | 164 ++++++++++++++++++ .../httptracemock/httptracemock.go | 0 internal/appsec/waf_test.go | 146 +--------------- 4 files changed, 167 insertions(+), 144 deletions(-) create mode 100644 contrib/database/sql/appsec_test.go rename {internal/contrib => instrumentation}/httptracemock/httptracemock.go (100%) diff --git a/.github/workflows/appsec.yml b/.github/workflows/appsec.yml index f3db05a0e8..94ff807e60 100644 --- a/.github/workflows/appsec.yml +++ b/.github/workflows/appsec.yml @@ -26,6 +26,7 @@ env: ./appsec/... ./internal/appsec/... SUBMODULES: >- + ./contrib/database/sql ./contrib/gin-gonic/gin ./contrib/google.golang.org/grpc ./contrib/net/http diff --git a/contrib/database/sql/appsec_test.go b/contrib/database/sql/appsec_test.go new file mode 100644 index 0000000000..f6aa627107 --- /dev/null +++ b/contrib/database/sql/appsec_test.go @@ -0,0 +1,164 @@ +// 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 2024 Datadog, Inc. + +package sql + +import ( + "database/sql" + "fmt" + "log" + "math/rand" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/DataDog/dd-trace-go/v2/appsec/events" + "github.com/DataDog/dd-trace-go/v2/ddtrace/mocktracer" + "github.com/DataDog/dd-trace-go/v2/instrumentation/httptracemock" + "github.com/DataDog/dd-trace-go/v2/instrumentation/testutils" + "github.com/stretchr/testify/require" + + _ "github.com/glebarez/go-sqlite" +) + +func prepareSQLDB(nbEntries int) (*sql.DB, error) { + const tables = ` +CREATE TABLE user ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + name text NOT NULL, + email text NOT NULL, + password text NOT NULL +); +CREATE TABLE product ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + name text NOT NULL, + category text NOT NULL, + price int NOT NULL +); +` + db, err := Open("sqlite", ":memory:") + if err != nil { + log.Fatalln("unexpected sqltrace.Open error:", err) + } + + if _, err := db.Exec(tables); err != nil { + return nil, err + } + + for i := 0; i < nbEntries; i++ { + _, err := db.Exec( + "INSERT INTO user (name, email, password) VALUES (?, ?, ?)", + fmt.Sprintf("User#%d", i), + fmt.Sprintf("user%d@mail.com", i), + fmt.Sprintf("secret-password#%d", i)) + if err != nil { + return nil, err + } + + _, err = db.Exec( + "INSERT INTO product (name, category, price) VALUES (?, ?, ?)", + fmt.Sprintf("Product %d", i), + "sneaker", + rand.Intn(500)) + if err != nil { + return nil, err + } + } + + return db, nil +} + +func TestRASPSQLi(t *testing.T) { + t.Setenv("DD_APPSEC_RULES", "../../../internal/appsec/testdata/rasp.json") + testutils.StartAppSec(t) + + if !instr.AppSecRASPEnabled() { + t.Skip("RASP needs to be enabled for this test") + } + db, err := prepareSQLDB(10) + require.NoError(t, err) + + // Setup the http server + mux := httptracemock.NewServeMux() + mux.HandleFunc("/query", func(w http.ResponseWriter, r *http.Request) { + // Subsequent spans inherit their parent from context. + q := r.URL.Query().Get("query") + rows, err := db.QueryContext(r.Context(), q) + if events.IsSecurityError(err) { + return + } + if err == nil { + rows.Close() + } + w.Write([]byte("Hello World!\n")) + }) + mux.HandleFunc("/exec", func(w http.ResponseWriter, r *http.Request) { + // Subsequent spans inherit their parent from context. + q := r.URL.Query().Get("query") + _, err := db.ExecContext(r.Context(), q) + if events.IsSecurityError(err) { + return + } + w.Write([]byte("Hello World!\n")) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + for name, tc := range map[string]struct { + query string + err error + }{ + "no-error": { + query: url.QueryEscape("SELECT 1"), + }, + "injection/SELECT": { + query: url.QueryEscape("SELECT * FROM users WHERE user=\"\" UNION ALL SELECT NULL;version()--"), + err: &events.BlockingSecurityEvent{}, + }, + "injection/UPDATE": { + query: url.QueryEscape("UPDATE users SET pwd = \"root\" WHERE id = \"\" OR 1 = 1--"), + err: &events.BlockingSecurityEvent{}, + }, + "injection/EXEC": { + query: url.QueryEscape("EXEC version(); DROP TABLE users--"), + err: &events.BlockingSecurityEvent{}, + }, + } { + for _, endpoint := range []string{"/query", "/exec"} { + t.Run(name+endpoint, func(t *testing.T) { + // Start tracer and appsec + mt := mocktracer.Start() + defer mt.Stop() + + req, err := http.NewRequest("POST", srv.URL+endpoint+"?query="+tc.query, nil) + require.NoError(t, err) + res, err := srv.Client().Do(req) + require.NoError(t, err) + defer res.Body.Close() + + spans := mt.FinishedSpans() + + require.Len(t, spans, 2) + + if tc.err != nil { + require.Equal(t, 403, res.StatusCode) + + for _, sp := range spans { + switch sp.OperationName() { + case "http.request": + require.Contains(t, sp.Tag("_dd.appsec.json"), "rasp-942-100") + case "sqlite.query": + require.NotContains(t, sp.Tags(), "error") + } + } + } else { + require.Equal(t, 200, res.StatusCode) + } + + }) + } + } +} diff --git a/internal/contrib/httptracemock/httptracemock.go b/instrumentation/httptracemock/httptracemock.go similarity index 100% rename from internal/contrib/httptracemock/httptracemock.go rename to instrumentation/httptracemock/httptracemock.go diff --git a/internal/appsec/waf_test.go b/internal/appsec/waf_test.go index da9b7caac4..798aca030e 100644 --- a/internal/appsec/waf_test.go +++ b/internal/appsec/waf_test.go @@ -7,13 +7,9 @@ package appsec_test import ( "context" - "database/sql" "encoding/json" - "fmt" "io" "io/fs" - "log" - "math/rand" "net/http" "net/http/httptest" "net/url" @@ -30,12 +26,10 @@ import ( "github.com/DataDog/dd-trace-go/v2/ddtrace/mocktracer" "github.com/DataDog/dd-trace-go/v2/instrumentation/appsec/dyngo" "github.com/DataDog/dd-trace-go/v2/instrumentation/appsec/emitter/ossec" + httptrace "github.com/DataDog/dd-trace-go/v2/instrumentation/httptracemock" "github.com/DataDog/dd-trace-go/v2/internal/appsec" "github.com/DataDog/dd-trace-go/v2/internal/appsec/config" "github.com/DataDog/dd-trace-go/v2/internal/appsec/listener/httpsec" - httptrace "github.com/DataDog/dd-trace-go/v2/internal/contrib/httptracemock" - - _ "github.com/glebarez/go-sqlite" "github.com/stretchr/testify/require" ) @@ -506,144 +500,8 @@ func TestAPISecurity(t *testing.T) { }) } -func prepareSQLDB(nbEntries int) (*sql.DB, error) { - const tables = ` -CREATE TABLE user ( - id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, - name text NOT NULL, - email text NOT NULL, - password text NOT NULL -); -CREATE TABLE product ( - id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, - name text NOT NULL, - category text NOT NULL, - price int NOT NULL -); -` - db, err := sql.Open("sqlite", ":memory:") - if err != nil { - log.Fatalln("unexpected sql.Open error:", err) - } - - if _, err := db.Exec(tables); err != nil { - return nil, err - } - - for i := 0; i < nbEntries; i++ { - _, err := db.Exec( - "INSERT INTO user (name, email, password) VALUES (?, ?, ?)", - fmt.Sprintf("User#%d", i), - fmt.Sprintf("user%d@mail.com", i), - fmt.Sprintf("secret-password#%d", i)) - if err != nil { - return nil, err - } - - _, err = db.Exec( - "INSERT INTO product (name, category, price) VALUES (?, ?, ?)", - fmt.Sprintf("Product %d", i), - "sneaker", - rand.Intn(500)) - if err != nil { - return nil, err - } - } - - return db, nil -} - func TestRASPSQLi(t *testing.T) { - t.Setenv("DD_APPSEC_RULES", "testdata/rasp.json") - appsec.Start() - defer appsec.Stop() - - if !appsec.RASPEnabled() { - t.Skip("RASP needs to be enabled for this test") - } - db, err := prepareSQLDB(10) - require.NoError(t, err) - - // Setup the http server - mux := httptrace.NewServeMux() - mux.HandleFunc("/query", func(w http.ResponseWriter, r *http.Request) { - // Subsequent spans inherit their parent from context. - q := r.URL.Query().Get("query") - rows, err := db.QueryContext(r.Context(), q) - if events.IsSecurityError(err) { - return - } - if err == nil { - rows.Close() - } - w.Write([]byte("Hello World!\n")) - }) - mux.HandleFunc("/exec", func(w http.ResponseWriter, r *http.Request) { - // Subsequent spans inherit their parent from context. - q := r.URL.Query().Get("query") - _, err := db.ExecContext(r.Context(), q) - if events.IsSecurityError(err) { - return - } - w.Write([]byte("Hello World!\n")) - }) - srv := httptest.NewServer(mux) - defer srv.Close() - - for name, tc := range map[string]struct { - query string - err error - }{ - "no-error": { - query: url.QueryEscape("SELECT 1"), - }, - "injection/SELECT": { - query: url.QueryEscape("SELECT * FROM users WHERE user=\"\" UNION ALL SELECT NULL;version()--"), - err: &events.BlockingSecurityEvent{}, - }, - "injection/UPDATE": { - query: url.QueryEscape("UPDATE users SET pwd = \"root\" WHERE id = \"\" OR 1 = 1--"), - err: &events.BlockingSecurityEvent{}, - }, - "injection/EXEC": { - query: url.QueryEscape("EXEC version(); DROP TABLE users--"), - err: &events.BlockingSecurityEvent{}, - }, - } { - for _, endpoint := range []string{"/query", "/exec"} { - t.Run(name+endpoint, func(t *testing.T) { - // Start tracer and appsec - mt := mocktracer.Start() - defer mt.Stop() - - req, err := http.NewRequest("POST", srv.URL+endpoint+"?query="+tc.query, nil) - require.NoError(t, err) - res, err := srv.Client().Do(req) - require.NoError(t, err) - defer res.Body.Close() - - spans := mt.FinishedSpans() - - require.Len(t, spans, 2) - - if tc.err != nil { - require.Equal(t, 403, res.StatusCode) - - for _, sp := range spans { - switch sp.OperationName() { - case "http.request": - require.Contains(t, sp.Tag("_dd.appsec.json"), "rasp-942-100") - case "sqlite.query": - require.NotContains(t, sp.Tags(), "error") - } - } - } else { - require.Equal(t, 200, res.StatusCode) - } - - }) - } - } + t.Skip("TestRASPSQLi can be found in /contrib/database/sql/appsec_test.go as importing the contrib for database/sql would cause a circular dependency") } func TestRASPLFI(t *testing.T) {