Skip to content

Commit

Permalink
Fix AppSec libddwaf actions handling after update to 1.17.0.0.0
Browse files Browse the repository at this point in the history
In version 1.17.0.0.0 libddwaf changes the format of the actions that
are returned from an array of actions IDs to an array of actions
objects. This means that we no longer have to look up actions in the
ruleset and can just use actions that libddwaf returns.
  • Loading branch information
y9v committed Nov 28, 2024
1 parent 646d17a commit c167bd0
Show file tree
Hide file tree
Showing 12 changed files with 37 additions and 396 deletions.
4 changes: 2 additions & 2 deletions lib/datadog/appsec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ def processor
appsec_component.processor if appsec_component
end

def reconfigure(ruleset:, actions:, telemetry:)
def reconfigure(ruleset:, telemetry:)
appsec_component = components.appsec

return unless appsec_component

appsec_component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
appsec_component.reconfigure(ruleset: ruleset, telemetry: telemetry)
end

def reconfigure_lock(&block)
Expand Down
9 changes: 1 addition & 8 deletions lib/datadog/appsec/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
require_relative 'processor'
require_relative 'processor/rule_merger'
require_relative 'processor/rule_loader'
require_relative 'processor/actions'

module Datadog
module AppSec
Expand Down Expand Up @@ -52,10 +51,6 @@ def create_processor(settings, telemetry)
)
return nil unless rules

actions = rules['actions']

AppSec::Processor::Actions.merge(actions) if actions

data = AppSec::Processor::RuleLoader.load_data(
ip_denylist: settings.appsec.ip_denylist,
user_id_denylist: settings.appsec.user_id_denylist,
Expand Down Expand Up @@ -84,10 +79,8 @@ def initialize(processor:)
@mutex = Mutex.new
end

def reconfigure(ruleset:, actions:, telemetry:)
def reconfigure(ruleset:, telemetry:)
@mutex.synchronize do
AppSec::Processor::Actions.merge(actions)

new = Processor.new(ruleset: ruleset, telemetry: telemetry)

if new && new.ready?
Expand Down
49 changes: 0 additions & 49 deletions lib/datadog/appsec/processor/actions.rb

This file was deleted.

4 changes: 1 addition & 3 deletions lib/datadog/appsec/remote.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ def receivers(telemetry)
data = []
overrides = []
exclusions = []
actions = []

repository.contents.each do |content|
parsed_content = parse_content(content)
Expand All @@ -81,7 +80,6 @@ def receivers(telemetry)
overrides << parsed_content['rules_override'] if parsed_content['rules_override']
exclusions << parsed_content['exclusions'] if parsed_content['exclusions']
custom_rules << parsed_content['custom_rules'] if parsed_content['custom_rules']
actions.concat(parsed_content['actions']) if parsed_content['actions']
end
end

Expand All @@ -105,7 +103,7 @@ def receivers(telemetry)
telemetry: telemetry
)

Datadog::AppSec.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
Datadog::AppSec.reconfigure(ruleset: ruleset, telemetry: telemetry)
end

[receiver]
Expand Down
11 changes: 4 additions & 7 deletions lib/datadog/appsec/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,16 @@ class << self
def negotiate(env, actions)
# @type var configured_response: Response?
configured_response = nil
actions.each do |action|
actions.each do |type, parameters|
# Need to use next to make steep happy :(
# I rather use break to stop the execution
next if configured_response

action_configuration = AppSec::Processor::Actions.fetch_configuration(action)
next unless action_configuration

configured_response = case action_configuration['type']
configured_response = case type
when 'block_request'
block_response(env, action_configuration['parameters'])
block_response(env, parameters)
when 'redirect_request'
redirect_response(env, action_configuration['parameters'])
redirect_response(env, parameters)
end
end

Expand Down
1 change: 0 additions & 1 deletion sig/datadog/appsec.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ module Datadog

def self.reconfigure: (
ruleset: ::Hash[untyped, untyped],
actions: Array[Hash[String, untyped]],
telemetry: Datadog::Core::Telemetry::Component
) -> void

Expand Down
18 changes: 0 additions & 18 deletions sig/datadog/appsec/processor/actions.rbs

This file was deleted.

2 changes: 1 addition & 1 deletion sig/datadog/appsec/response.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module Datadog
def to_sinatra_response: () -> ::Sinatra::Response
def to_action_dispatch_response: () -> ::ActionDispatch::Response

def self.negotiate: (::Hash[untyped, untyped] env, Array[String] actions) -> Response
def self.negotiate: (::Hash[untyped, untyped] env, ::Hash[String, untyped] actions) -> Response
def self.graphql_response: (Datadog::AppSec::Contrib::GraphQL::Gateway::Multiplex gateway_multiplex) -> Array[::GraphQL::Query::Result]

private
Expand Down
101 changes: 4 additions & 97 deletions spec/datadog/appsec/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,86 +73,6 @@
expect(component.processor).to be_nil
end
end

context 'when static rules have actions defined' do
it 'calls Datadog::AppSec::Processor::Actions.merge' do
actions = [
{
'id' => 'block',
'type' => 'block_request',
'parameters' => {
'type' => 'auto',
'status_code' => 403,

}
}
]
ruleset =
{
'version' => '2.2',
'rules' => [{
'conditions' => [{
'operator' => 'ip_match',
'parameters' => {
'data' => 'blocked_ips',
'inputs' => [{
'address' => 'http.client_ip'
}]
}
}],
'id' => 'blk-001-001',
'name' => 'Block IP Addresses',
'on_match' => ['block'],
'tags' => {
'category' => 'security_response', 'type' => 'block_ip'
},
'transformers' => []
}],
'actions' => actions
}

expect(Datadog::AppSec::Processor::Actions).to receive(:merge).with(actions)
expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).and_return(ruleset)

component = described_class.build_appsec_component(settings, telemetry: telemetry)

expect(component.processor).to be_a(Datadog::AppSec::Processor)
end
end

context 'when static rules do not have actions defined' do
it 'calls Datadog::AppSec::Processor::Actions.merge' do
ruleset =
{
'version' => '2.2',
'rules' => [{
'conditions' => [{
'operator' => 'ip_match',
'parameters' => {
'data' => 'blocked_ips',
'inputs' => [{
'address' => 'http.client_ip'
}]
}
}],
'id' => 'blk-001-001',
'name' => 'Block IP Addresses',
'on_match' => ['block'],
'tags' => {
'category' => 'security_response', 'type' => 'block_ip'
},
'transformers' => []
}],
}

expect(Datadog::AppSec::Processor::Actions).to_not receive(:merge)
expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).and_return(ruleset)

component = described_class.build_appsec_component(settings, telemetry: telemetry)

expect(component.processor).to be_a(Datadog::AppSec::Processor)
end
end
end

context 'when appsec is not enabled' do
Expand Down Expand Up @@ -220,27 +140,14 @@
}
end

let(:actions) { [] }

context 'lock' do
it 'makes sure to synchronize' do
mutex = Mutex.new
processor = instance_double(Datadog::AppSec::Processor)
component = described_class.new(processor: processor)
component.instance_variable_set(:@mutex, mutex)
expect(mutex).to receive(:synchronize)
component.reconfigure(ruleset: {}, actions: actions, telemetry: telemetry)
end
end

context 'actions' do
it 'merges the actions' do
processor = instance_double(Datadog::AppSec::Processor)
expect(processor).to receive(:finalize)
component = described_class.new(processor: processor)

expect(Datadog::AppSec::Processor::Actions).to receive(:merge).with(actions)
component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
component.reconfigure(ruleset: {}, telemetry: telemetry)
end
end

Expand All @@ -252,7 +159,7 @@
old_processor = component.processor

expect(old_processor).to receive(:finalize)
component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
component.reconfigure(ruleset: ruleset, telemetry: telemetry)
new_processor = component.processor
expect(new_processor).to_not eq(old_processor)
new_processor.finalize
Expand All @@ -267,7 +174,7 @@
old_processor = component.processor

expect(old_processor).to_not receive(:finalize)
component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
component.reconfigure(ruleset: ruleset, telemetry: telemetry)
new_processor = component.processor
expect(new_processor).to_not eq(old_processor)
new_processor.finalize
Expand All @@ -284,7 +191,7 @@
ruleset = { 'invalid_one' => true }

expect(old_processor).to_not receive(:finalize)
component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
component.reconfigure(ruleset: ruleset, telemetry: telemetry)
expect(component.processor).to eq(old_processor)
end
end
Expand Down
Loading

0 comments on commit c167bd0

Please sign in to comment.