Skip to content

Commit

Permalink
fix multiple bugs:
Browse files Browse the repository at this point in the history 
* After recieving the order to stop appsec via RC we did not reset the ruleset to the default one
* some code in http listeners where deduplicated for RASP SSRF making SSRF RASP span tags not working
* its 'exclusions' but its 'exclusion_data' without an 's'

Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
  • Loading branch information
@eliottness
eliottness committed Sep 18, 2024
1 parent fda0071 commit 8f88b8d
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 14 deletions.
4 changes: 4 additions & 0 deletions internal/appsec/appsec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,10 @@ func (a *appsec) stop() {
a.wafHandle.Close()
a.wafHandle = nil
}

// Reset rules edits received from the remote configuration
a.cfg.RulesManager, _ = config.NewRulesManager(nil)

// TODO: block until no more requests are using dyngo operations

a.limiter.Stop()
Expand Down
2 changes: 1 addition & 1 deletion internal/appsec/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func NewConfig() (*Config, error) {
return nil, err
}

r, err := NewRulesManeger(rules)
r, err := NewRulesManager(rules)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/appsec/config/rules_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ func (f *RulesFragment) clone() (clone RulesFragment) {
return
}

// NewRulesManeger initializes and returns a new RulesManager using the provided rules.
// NewRulesManager initializes and returns a new RulesManager using the provided rules.
// If no rules are provided (nil), the default rules are used instead.
// If the provided rules are invalid, an error is returned
func NewRulesManeger(rules []byte) (*RulesManager, error) {
func NewRulesManager(rules []byte) (*RulesManager, error) {
var f RulesFragment
if rules == nil {
f = DefaultRulesFragment()
Expand Down
4 changes: 1 addition & 3 deletions internal/appsec/listener/httpsec/http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,7 @@ func (l *wafEventListener) onEvent(op *types.Operation, args types.HandlerOperat
}

if SSRFAddressesPresent(l.addresses) {
dyngo.On(op, shared.MakeWAFRunListener(&op.SecurityEventsHolder, wafCtx, l.limiter, func(args types.RoundTripOperationArgs) waf.RunAddressData {
return waf.RunAddressData{Ephemeral: map[string]any{ServerIoNetURLAddr: args.URL}}
}))
RegisterRoundTripperListener(op, &op.SecurityEventsHolder, wafCtx, l.limiter)
}

if ossec.OSAddressesPresent(l.addresses) {
Expand Down
2 changes: 1 addition & 1 deletion internal/appsec/remoteconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func mergeRulesData(u remoteconfig.ProductUpdate) (config.RulesFragment, map[str
continue
}

if exclusionData, ok := parsedUpdate["exclusions_data"]; ok {
if exclusionData, ok := parsedUpdate["exclusion_data"]; ok {
mergeUpdateEntry(mergedExclusionData, exclusionData)
}

Expand Down
14 changes: 7 additions & 7 deletions internal/appsec/remoteconfig_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func TestMergeRulesData(t *testing.T) {
{
name: "single-exclusions-value",
update: map[string][]byte{
"some/path": []byte(`{"exclusions_data":[{"id":"test","type":"data_with_expiration","data":[{"expiration":3494138481,"value":"user1"}]}]}`),
"some/path": []byte(`{"exclusion_data":[{"id":"test","type":"data_with_expiration","data":[{"expiration":3494138481,"value":"user1"}]}]}`),
},
expected: config.RulesFragment{ExclusionData: []config.DataEntry{{ID: "test", Type: "data_with_expiration", Data: []rc.ASMDataRuleDataEntry{
{Expiration: 3494138481, Value: "user1"},
Expand All @@ -202,7 +202,7 @@ func TestMergeRulesData(t *testing.T) {
{
name: "multiple-exclusions-values",
update: map[string][]byte{
"some/path": []byte(`{"exclusions_data":[{"id":"test","type":"data_with_expiration","data":[{"expiration":3494138481,"value":"user1"},{"expiration":3494138441,"value":"user2"}]}]}`),
"some/path": []byte(`{"exclusion_data":[{"id":"test","type":"data_with_expiration","data":[{"expiration":3494138481,"value":"user1"},{"expiration":3494138441,"value":"user2"}]}]}`),
},
expected: config.RulesFragment{ExclusionData: []config.DataEntry{{ID: "test", Type: "data_with_expiration", Data: []rc.ASMDataRuleDataEntry{
{Expiration: 3494138481, Value: "user1"},
Expand All @@ -213,7 +213,7 @@ func TestMergeRulesData(t *testing.T) {
{
name: "multiple-exclusions-entries",
update: map[string][]byte{
"some/path": []byte(`{"exclusions_data":[{"id":"test1","type":"data_with_expiration","data":[{"expiration":3494138444,"value":"user3"}]},{"id":"test2","type":"data_with_expiration","data":[{"expiration":3495138481,"value":"user4"}]}]}`),
"some/path": []byte(`{"exclusion_data":[{"id":"test1","type":"data_with_expiration","data":[{"expiration":3494138444,"value":"user3"}]},{"id":"test2","type":"data_with_expiration","data":[{"expiration":3495138481,"value":"user4"}]}]}`),
},
expected: config.RulesFragment{ExclusionData: []config.DataEntry{
{ID: "test1", Type: "data_with_expiration", Data: []rc.ASMDataRuleDataEntry{
Expand All @@ -227,8 +227,8 @@ func TestMergeRulesData(t *testing.T) {
{
name: "merging-exclusions-entries",
update: map[string][]byte{
"some/path/1": []byte(`{"exclusions_data":[{"id":"test1","type":"data_with_expiration","data":[{"expiration":3494138444,"value":"user3"}]},{"id":"test2","type":"data_with_expiration","data":[{"expiration":3495138481,"value":"user4"}]}]}`),
"some/path/2": []byte(`{"exclusions_data":[{"id":"test1","type":"data_with_expiration","data":[{"expiration":3494138445,"value":"user3"}]},{"id":"test2","type":"data_with_expiration","data":[{"expiration":0,"value":"user5"}]}]}`),
"some/path/1": []byte(`{"exclusion_data":[{"id":"test1","type":"data_with_expiration","data":[{"expiration":3494138444,"value":"user3"}]},{"id":"test2","type":"data_with_expiration","data":[{"expiration":3495138481,"value":"user4"}]}]}`),
"some/path/2": []byte(`{"exclusion_data":[{"id":"test1","type":"data_with_expiration","data":[{"expiration":3494138445,"value":"user3"}]},{"id":"test2","type":"data_with_expiration","data":[{"expiration":0,"value":"user5"}]}]}`),
},
expected: config.RulesFragment{ExclusionData: []config.DataEntry{
{ID: "test1", Type: "data_with_expiration", Data: []rc.ASMDataRuleDataEntry{
Expand Down Expand Up @@ -509,7 +509,7 @@ type testRulesOverrideEntry struct {

func TestOnRCUpdate(t *testing.T) {

BaseRuleset, err := config.NewRulesManeger(nil)
BaseRuleset, err := config.NewRulesManager(nil)
require.NoError(t, err)
BaseRuleset.Compile()

Expand Down Expand Up @@ -723,7 +723,7 @@ func TestOnRCUpdate(t *testing.T) {
}

func TestOnRCUpdateStatuses(t *testing.T) {
invalidRuleset, err := config.NewRulesManeger([]byte(`{"version": "2.2", "metadata": {"rules_version": "1.4.2"}, "rules": [{"id": "id","name":"name","tags":{},"conditions":[],"transformers":[],"on_match":[]}]}`))
invalidRuleset, err := config.NewRulesManager([]byte(`{"version": "2.2", "metadata": {"rules_version": "1.4.2"}, "rules": [{"id": "id","name":"name","tags":{},"conditions":[],"transformers":[],"on_match":[]}]}`))
require.NoError(t, err)
invalidRules := invalidRuleset.Base
overrides := config.RulesFragment{
Expand Down

0 comments on commit 8f88b8d

Please sign in to comment.