Skip to content

Commit

Permalink
Change the method signature for loading rule data.
Browse files Browse the repository at this point in the history
Unfify building rule data information into a single method:
`denylist_data(id, denylist)`

Pass the settings information as an argument to apply_denylist_data,
load_ruleset, and create_waf_handle. That way we couold more easily
mock the settings value in tests.
  • Loading branch information
GustavoCaso committed Feb 13, 2023
1 parent ecf17ad commit 36aaf27
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 41 deletions.
35 changes: 14 additions & 21 deletions lib/datadog/appsec/processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,15 @@ def finalize
def initialize
@ruleset_info = nil
@addresses = []
settings = Datadog::AppSec.settings

unless load_libddwaf && load_ruleset && create_waf_handle
unless load_libddwaf && load_ruleset(settings) && create_waf_handle(settings)
Datadog.logger.warn { 'AppSec is disabled, see logged errors above' }

return
end

update_rules_data_with_static_configured_values
apply_denylist_data(settings)
end

def ready?
Expand Down Expand Up @@ -80,36 +81,28 @@ def finalize

private

def update_rules_data_with_static_configured_values
def apply_denylist_data(settings)
ruledata_setting = []
ruledata_setting << ip_denylist_data(Datadog::AppSec.settings.ip_denylist)
ruledata_setting << user_id_denylist_data(Datadog::AppSec.settings.user_id_denylist)
ruledata_setting << denylist_data('blocked_ips', settings.ip_denylist)
ruledata_setting << denylist_data('blocked_users', settings.user_id_denylist)

update_rule_data(ruledata_setting)
end

def ip_denylist_data(denylist, id: 'blocked_ips')
def denylist_data(id, denylist)
{
'id' => id,
'type' => 'data_with_expiration',
'data' => denylist.map { |ip| { 'value' => ip.to_s, 'expiration' => 2**63 } }
}
end

def user_id_denylist_data(denylist, id: 'blocked_users')
{
'id' => id,
'type' => 'data_with_expiration',
'data' => denylist.map { |ip| { 'value' => ip.to_s, 'expiration' => 2**63 } }
'data' => denylist.map { |v| { 'value' => v.to_s, 'expiration' => 2**63 } }
}
end

def load_libddwaf
Processor.require_libddwaf && Processor.libddwaf_provides_waf?
end

def load_ruleset
ruleset_setting = Datadog::AppSec.settings.ruleset
def load_ruleset(settings)
ruleset_setting = settings.ruleset

begin
@ruleset = case ruleset_setting
Expand Down Expand Up @@ -142,13 +135,13 @@ def load_ruleset
end
end

def create_waf_handle
def create_waf_handle(settings)
# TODO: this may need to be reset if the main Datadog logging level changes after initialization
Datadog::AppSec::WAF.logger = Datadog.logger if Datadog.logger.debug? && Datadog::AppSec.settings.waf_debug
Datadog::AppSec::WAF.logger = Datadog.logger if Datadog.logger.debug? && settings.waf_debug

obfuscator_config = {
key_regex: Datadog::AppSec.settings.obfuscator_key_regex,
value_regex: Datadog::AppSec.settings.obfuscator_value_regex,
key_regex: settings.obfuscator_key_regex,
value_regex: settings.obfuscator_value_regex,
}
@handle = Datadog::AppSec::WAF::Handle.new(@ruleset, obfuscator: obfuscator_config)
@ruleset_info = @handle.ruleset_info
Expand Down
9 changes: 4 additions & 5 deletions sig/datadog/appsec/processor.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,17 @@ module Datadog
def new_context: () -> Context
def update_rule_data: (untyped data) -> untyped
def toggle_rules: (untyped map) -> untyped
def update_rules_data_with_static_configured_values: () -> untyped
def ip_denylist_data: (::Array[String?] denylist, ?id: ::String) -> ::Hash[::String, untyped]
def user_id_denylist_data: (::Array[String?] denylist, ?id: ::String) -> ::Hash[::String, untyped]
def finalize: () -> void

attr_reader handle: untyped

private

def apply_denylist_data: (Configuration::Settings settings) -> untyped
def denylist_data: (String id, ::Array[untyped] denylist) -> ::Hash[::String, untyped | "data_with_expiration"]
def load_libddwaf: () -> bool
def load_ruleset: () -> bool
def create_waf_handle: () -> bool
def load_ruleset: (Configuration::Settings settings) -> bool
def create_waf_handle: (Configuration::Settings settings) -> bool

def self.libddwaf_provides_waf?: () -> bool
def self.require_libddwaf: () -> bool
Expand Down
32 changes: 17 additions & 15 deletions spec/datadog/appsec/processor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,7 @@
end

describe '#load_ruleset' do
before do
allow(Datadog::AppSec.settings).to receive(:ruleset).and_return(ruleset)
end

let(:settings) { Datadog::AppSec.settings }
let(:basic_ruleset) do
{
'version' => '1.0',
Expand All @@ -106,14 +103,18 @@
}
end

before do
allow(settings).to receive(:ruleset).and_return(ruleset)
end

context 'when ruleset is :recommended' do
let(:ruleset) { :recommended }

before do
expect(Datadog::AppSec::Assets).to receive(:waf_rules).with(:recommended).and_call_original.twice
end

it { expect(described_class.new.send(:load_ruleset)).to be true }
it { expect(described_class.new.send(:load_ruleset, settings)).to be true }
end

context 'when ruleset is :strict' do
Expand All @@ -123,7 +124,7 @@
expect(Datadog::AppSec::Assets).to receive(:waf_rules).with(:strict).and_call_original.twice
end

it { expect(described_class.new.send(:load_ruleset)).to be true }
it { expect(described_class.new.send(:load_ruleset, settings)).to be true }
end

context 'when ruleset is :risky' do
Expand All @@ -133,45 +134,46 @@
expect(Datadog::AppSec::Assets).to receive(:waf_rules).with(:recommended).and_call_original.twice
end

it { expect(described_class.new.send(:load_ruleset)).to be true }
it { expect(described_class.new.send(:load_ruleset, settings)).to be true }
end

context 'when ruleset is an existing path' do
let(:ruleset) { "#{__dir__}/../../../lib/datadog/appsec/assets/waf_rules/recommended.json" }

it { expect(described_class.new.send(:load_ruleset)).to be true }
it { expect(described_class.new.send(:load_ruleset, settings)).to be true }
end

context 'when ruleset is a non existing path' do
let(:ruleset) { '/does/not/exist' }

it { expect(described_class.new.send(:load_ruleset)).to be false }
it { expect(described_class.new.send(:load_ruleset, settings)).to be false }
end

context 'when ruleset is IO-like' do
let(:ruleset) { StringIO.new(JSON.dump(basic_ruleset)) }

it { expect(described_class.new.send(:load_ruleset)).to be true }
it { expect(described_class.new.send(:load_ruleset, settings)).to be true }
end

context 'when ruleset is Ruby' do
let(:ruleset) { basic_ruleset }

it { expect(described_class.new.send(:load_ruleset)).to be true }
it { expect(described_class.new.send(:load_ruleset, settings)).to be true }
end

context 'when ruleset is not parseable' do
let(:ruleset) { StringIO.new('this is not json') }

it { expect(described_class.new.send(:load_ruleset)).to be false }
it { expect(described_class.new.send(:load_ruleset, settings)).to be false }
end
end

describe '#create_waf_handle' do
let(:ruleset) { :recommended }
let(:settings) { Datadog::AppSec.settings }

before do
allow(Datadog::AppSec.settings).to receive(:ruleset).and_return(ruleset)
allow(settings).to receive(:ruleset).and_return(ruleset)
end

context 'when ruleset is default' do
Expand All @@ -181,13 +183,13 @@
expect(Datadog::AppSec::Assets).to receive(:waf_rules).with(:recommended).and_call_original
end

it { expect(described_class.new.send(:create_waf_handle)).to be true }
it { expect(described_class.new.send(:create_waf_handle, settings)).to be true }
end

context 'when ruleset is invalid' do
let(:ruleset) { { 'not' => 'valid' } }

it { expect(described_class.new.send(:create_waf_handle)).to be false }
it { expect(described_class.new.send(:create_waf_handle, settings)).to be false }
end
end

Expand Down

0 comments on commit 36aaf27

Please sign in to comment.