From cda6cb73c4c17972c2d4ffd9b30556e8ea12d336 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Tue, 28 Mar 2023 17:56:17 +0200 Subject: [PATCH 1/5] avoid iterating over repository content multiple times --- lib/datadog/core/remote/client.rb | 56 +++++++++++++++---------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/lib/datadog/core/remote/client.rb b/lib/datadog/core/remote/client.rb index c79304fc612..62bbd5c84a9 100644 --- a/lib/datadog/core/remote/client.rb +++ b/lib/datadog/core/remote/client.rb @@ -101,6 +101,7 @@ def sync # rubocop:enable Metrics/AbcSize,Metrics/PerceivedComplexity class SyncError < StandardError; end + class ReadError < StandardError; end private @@ -189,24 +190,6 @@ def capabilities_binary cap_to_hexs.each_with_object([]) { |hex, acc| acc << hex }.map { |e| e.to_i(16) }.pack('C*') end - def select_content(repository, product, config_ids = []) - repository.contents.select do |content| - content.path.product == product && (config_ids.empty? || config_ids.include?(content.path.config_id)) - end - end - - def parse_content(content) - data = content.data.read - - content.data.rewind - - raise ReadError, 'EOF reached' if data.nil? - - JSON.parse(data) - end - - class ReadError < StandardError; end - def register_receivers matcher = Dispatcher::Matcher::Product.new(products) @@ -215,17 +198,22 @@ def register_receivers Datadog.logger.debug { "remote config change: '#{change.path}'" } end - rules_contents = select_content(repository, 'ASM_DD') - rules = rules_contents.map { |content| parse_content(content) } - - data_contents = select_content(repository, 'ASM_DATA', ['blocked_ips', 'blocked_users']) - data = data_contents.map { |content| parse_content(content) } - - overrides_contents = select_content(repository, 'ASM', ['blocking', 'disabled_rules']) - overrides = overrides_contents.map { |content| parse_content(content) } - - exclusions_contents = select_content(repository, 'ASM', ['exclusion_filters']) - exclusions = exclusions_contents.map { |content| parse_content(content) } + rules = [] + data = [] + overrides = [] + exclusions = [] + + repository.contents.each do |content| + case content.path.product + when 'ASM_DD' + rules << parse_content(content) + when 'ASM_DATA' + data << parse_content(content) if ['blocked_ips', 'blocked_users' ].include?(content.path.config_id) + when 'ASM' + overrides << parse_content(content) if ['blocking', 'disabled_rules'].include?(content.path.config_id) + exclusions << parse_content(content) if content.path.config_id == 'exclusion_filters' + end + end ruleset = AppSec::Processor::RuleMerger.merge( rules: rules, @@ -236,6 +224,16 @@ def register_receivers Datadog::AppSec.reconfigure(ruleset: ruleset) end + + def parse_content(content) + data = content.data.read + + content.data.rewind + + raise ReadError, 'EOF reached' if data.nil? + + JSON.parse(data) + end end end end From fba7de4758830a5ed1377630e6601260ac7f8d84 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Tue, 28 Mar 2023 17:57:59 +0200 Subject: [PATCH 2/5] extract rule loader logic outside of AppSec::Processor --- lib/datadog/appsec/component.rb | 17 ++++- lib/datadog/appsec/processor.rb | 69 +++------------------ lib/datadog/appsec/processor/rule_loader.rb | 67 ++++++++++++++++++++ 3 files changed, 88 insertions(+), 65 deletions(-) create mode 100644 lib/datadog/appsec/processor/rule_loader.rb diff --git a/lib/datadog/appsec/component.rb b/lib/datadog/appsec/component.rb index dcb81b28e3d..9500ede52b3 100644 --- a/lib/datadog/appsec/component.rb +++ b/lib/datadog/appsec/component.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require_relative 'processor' +require_relative 'processor/rule_merger' +require_relative 'processor/rule_loader' module Datadog module AppSec @@ -10,14 +12,23 @@ class << self def build_appsec_component(settings) return unless settings.respond_to?(:appsec) && settings.appsec.enabled - processor = create_processor + processor = create_processor(settings) new(processor: processor) end private - def create_processor - processor = Processor.new + def create_processor(settings) + rules = AppSec::Processor::RuleLoader.load_rules(settings) + return nil unless rules + data = AppSec::Processor::RuleLoader.load_data(settings) + + ruleset = AppSec::Processor::RuleMerger.merge( + rules: [rules], + data: data, + ) + + processor = Processor.new(ruleset: ruleset) return nil unless processor.ready? processor diff --git a/lib/datadog/appsec/processor.rb b/lib/datadog/appsec/processor.rb index 49b281c1c77..ceebbfa8d81 100644 --- a/lib/datadog/appsec/processor.rb +++ b/lib/datadog/appsec/processor.rb @@ -1,5 +1,4 @@ -require_relative 'assets' -require_relative 'processor/rule_merger' +# frozen_string_literal: true module Datadog module AppSec @@ -64,18 +63,18 @@ class AlreadyActiveContextError < StandardError; end attr_reader :ruleset_info, :addresses - def initialize(ruleset: nil) + def initialize(ruleset:) @ruleset_info = nil @addresses = [] settings = Datadog::AppSec.settings - unless load_libddwaf && load_ruleset(settings, ruleset: ruleset) && create_waf_handle(settings) + unless load_libddwaf && create_waf_handle(settings, ruleset) Datadog.logger.warn { 'AppSec is disabled, see logged errors above' } end end def ready? - !@ruleset.nil? && !@handle.nil? + !@handle.nil? end def new_context @@ -109,66 +108,11 @@ def finalize private - def combine_denylist_data(settings) - data = [] - data << { 'rules_data' => [denylist_data('blocked_ips', settings.ip_denylist)] } - data << { 'rules_data' => [denylist_data('blocked_users', settings.user_id_denylist)] } - end - - def denylist_data(id, denylist) - { - 'id' => id, - 'type' => 'data_with_expiration', - '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(settings, ruleset: nil) - ruleset_setting = ruleset || settings.ruleset - - ruleset = begin - case ruleset_setting - when :recommended, :strict - JSON.parse(Datadog::AppSec::Assets.waf_rules(ruleset_setting)) - when :risky - Datadog.logger.warn( - 'The :risky Application Security Management ruleset has been deprecated and no longer available.'\ - 'The `:recommended` ruleset will be used instead.'\ - 'Please remove the `appsec.ruleset = :risky` setting from your Datadog.configure block.' - ) - JSON.parse(Datadog::AppSec::Assets.waf_rules(:recommended)) - when String - JSON.parse(File.read(ruleset_setting)) - when File, StringIO - JSON.parse(ruleset_setting.read || '').tap { ruleset_setting.rewind } - when Hash - ruleset_setting - else - raise ArgumentError, "unsupported value for ruleset setting: #{ruleset_setting.inspect}" - end - rescue StandardError => e - Datadog.logger.error do - "libddwaf ruleset failed to load, ruleset: #{ruleset_setting.inspect} error: #{e.inspect}" - end - - nil - end - - return false if ruleset.nil? - - @ruleset = RuleMerger.merge( - rules: [ruleset], - data: combine_denylist_data(settings) - ) - - true - end - - def create_waf_handle(settings) + def create_waf_handle(settings, ruleset) # 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? && settings.waf_debug @@ -176,7 +120,8 @@ def create_waf_handle(settings) key_regex: settings.obfuscator_key_regex, value_regex: settings.obfuscator_value_regex, } - @handle = Datadog::AppSec::WAF::Handle.new(@ruleset, obfuscator: obfuscator_config) + + @handle = Datadog::AppSec::WAF::Handle.new(ruleset, obfuscator: obfuscator_config) @ruleset_info = @handle.ruleset_info @addresses = @handle.required_addresses diff --git a/lib/datadog/appsec/processor/rule_loader.rb b/lib/datadog/appsec/processor/rule_loader.rb new file mode 100644 index 00000000000..8e3c008c77b --- /dev/null +++ b/lib/datadog/appsec/processor/rule_loader.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require_relative '../assets' + +module Datadog + module AppSec + class Processor + # RuleLoader utility modules + # that load appsec rules and data from settings + module RuleLoader + class << self + def load_rules(settings) + ruleset_setting = settings.appsec.ruleset + + begin + case ruleset_setting + when :recommended, :strict + JSON.parse(Datadog::AppSec::Assets.waf_rules(ruleset_setting)) + when :risky + Datadog.logger.warn( + 'The :risky Application Security Management ruleset has been deprecated and no longer available.'\ + 'The `:recommended` ruleset will be used instead.'\ + 'Please remove the `appsec.ruleset = :risky` setting from your Datadog.configure block.' + ) + JSON.parse(Datadog::AppSec::Assets.waf_rules(:recommended)) + when String + JSON.parse(File.read(ruleset_setting)) + when File, StringIO + JSON.parse(ruleset_setting.read || '').tap { ruleset_setting.rewind } + when Hash + ruleset_setting + else + raise ArgumentError, "unsupported value for ruleset setting: #{ruleset_setting.inspect}" + end + rescue StandardError => e + Datadog.logger.error do + "libddwaf ruleset failed to load, ruleset: #{ruleset_setting.inspect} error: #{e.inspect}" + end + + nil + end + end + + def load_data(settings) + appsec_settings = settings.appsec + + data = [] + data << { 'rules_data' => [denylist_data('blocked_ips', appsec_settings.ip_denylist)] } if appsec_settings.ip_denylist.any? + data << { 'rules_data' => [denylist_data('blocked_users', appsec_settings.user_id_denylist)] } if appsec_settings.user_id_denylist.any? + + data.any? ? data : nil + end + + private + + def denylist_data(id, denylist) + { + 'id' => id, + 'type' => 'data_with_expiration', + 'data' => denylist.map { |v| { 'value' => v.to_s, 'expiration' => 2**63 } } + } + end + end + end + end + end +end From 935674498a08f787967f243c3e451f8be7a282a6 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Tue, 28 Mar 2023 18:00:27 +0200 Subject: [PATCH 3/5] use the appsec setting ruleset if ASM_DD data is empty --- lib/datadog/core/remote/client.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/datadog/core/remote/client.rb b/lib/datadog/core/remote/client.rb index 62bbd5c84a9..dd750ab7526 100644 --- a/lib/datadog/core/remote/client.rb +++ b/lib/datadog/core/remote/client.rb @@ -5,6 +5,7 @@ require_relative 'configuration' require_relative 'dispatcher' require_relative '../../appsec/processor/rule_merger' +require_relative '../../appsec/processor/rule_loader' module Datadog module Core @@ -215,6 +216,14 @@ def register_receivers end end + if rules.empty? + settings_rules = AppSec::Processor::RuleLoader.load_rules(ruleset: Datadog.configuration.appsec.ruleset) + + raise SyncError, 'no default rules available' unless settings_rules + + rules = [settings_rules] + end + ruleset = AppSec::Processor::RuleMerger.merge( rules: rules, data: data, From 45dd5d05d62b84bca29648ce05043e49e8c8e8ba Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Wed, 29 Mar 2023 17:54:37 +0200 Subject: [PATCH 4/5] add specs and rbs signatures --- lib/datadog/appsec/component.rb | 8 +- lib/datadog/appsec/processor/rule_loader.rb | 26 +-- lib/datadog/core/remote/client.rb | 21 +- sig/datadog/appsec/processor.rbs | 8 +- sig/datadog/appsec/processor/rule_loader.rbs | 17 ++ spec/datadog/appsec/component_spec.rb | 10 + .../appsec/processor/rule_loader_spec.rb | 158 +++++++++++++ spec/datadog/appsec/processor_spec.rb | 218 +++--------------- spec/datadog/core/remote/client_spec.rb | 135 ++++++++--- 9 files changed, 351 insertions(+), 250 deletions(-) create mode 100644 sig/datadog/appsec/processor/rule_loader.rbs create mode 100644 spec/datadog/appsec/processor/rule_loader_spec.rb diff --git a/lib/datadog/appsec/component.rb b/lib/datadog/appsec/component.rb index 9500ede52b3..e6366da0514 100644 --- a/lib/datadog/appsec/component.rb +++ b/lib/datadog/appsec/component.rb @@ -19,9 +19,13 @@ def build_appsec_component(settings) private def create_processor(settings) - rules = AppSec::Processor::RuleLoader.load_rules(settings) + rules = AppSec::Processor::RuleLoader.load_rules(ruleset: settings.appsec.ruleset) return nil unless rules - data = AppSec::Processor::RuleLoader.load_data(settings) + + data = AppSec::Processor::RuleLoader.load_data( + ip_denylist: settings.appsec.ip_denylist, + user_id_denylist: settings.appsec.user_id_denylist + ) ruleset = AppSec::Processor::RuleMerger.merge( rules: [rules], diff --git a/lib/datadog/appsec/processor/rule_loader.rb b/lib/datadog/appsec/processor/rule_loader.rb index 8e3c008c77b..7e339fc31d3 100644 --- a/lib/datadog/appsec/processor/rule_loader.rb +++ b/lib/datadog/appsec/processor/rule_loader.rb @@ -9,13 +9,11 @@ class Processor # that load appsec rules and data from settings module RuleLoader class << self - def load_rules(settings) - ruleset_setting = settings.appsec.ruleset - + def load_rules(ruleset:) begin - case ruleset_setting + case ruleset when :recommended, :strict - JSON.parse(Datadog::AppSec::Assets.waf_rules(ruleset_setting)) + JSON.parse(Datadog::AppSec::Assets.waf_rules(ruleset)) when :risky Datadog.logger.warn( 'The :risky Application Security Management ruleset has been deprecated and no longer available.'\ @@ -24,29 +22,27 @@ def load_rules(settings) ) JSON.parse(Datadog::AppSec::Assets.waf_rules(:recommended)) when String - JSON.parse(File.read(ruleset_setting)) + JSON.parse(File.read(File.expand_path(ruleset))) when File, StringIO - JSON.parse(ruleset_setting.read || '').tap { ruleset_setting.rewind } + JSON.parse(ruleset.read || '').tap { ruleset.rewind } when Hash - ruleset_setting + ruleset else - raise ArgumentError, "unsupported value for ruleset setting: #{ruleset_setting.inspect}" + raise ArgumentError, "unsupported value for ruleset setting: #{ruleset.inspect}" end rescue StandardError => e Datadog.logger.error do - "libddwaf ruleset failed to load, ruleset: #{ruleset_setting.inspect} error: #{e.inspect}" + "libddwaf ruleset failed to load, ruleset: #{ruleset.inspect} error: #{e.inspect}" end nil end end - def load_data(settings) - appsec_settings = settings.appsec - + def load_data(ip_denylist: [], user_id_denylist: []) data = [] - data << { 'rules_data' => [denylist_data('blocked_ips', appsec_settings.ip_denylist)] } if appsec_settings.ip_denylist.any? - data << { 'rules_data' => [denylist_data('blocked_users', appsec_settings.user_id_denylist)] } if appsec_settings.user_id_denylist.any? + data << { 'rules_data' => [denylist_data('blocked_ips', ip_denylist)] } if ip_denylist.any? + data << { 'rules_data' => [denylist_data('blocked_users', user_id_denylist)] } if user_id_denylist.any? data.any? ? data : nil end diff --git a/lib/datadog/core/remote/client.rb b/lib/datadog/core/remote/client.rb index dd750ab7526..cea4b229dff 100644 --- a/lib/datadog/core/remote/client.rb +++ b/lib/datadog/core/remote/client.rb @@ -204,14 +204,17 @@ def register_receivers overrides = [] exclusions = [] - repository.contents.each do |content| + asm_data_config_types = ['blocked_ips', 'blocked_users'] + asm_overrides_config_types = ['blocking', 'disabled_rules'] + + repository.contents.each do |content| case content.path.product when 'ASM_DD' rules << parse_content(content) when 'ASM_DATA' - data << parse_content(content) if ['blocked_ips', 'blocked_users' ].include?(content.path.config_id) + data << parse_content(content) if asm_data_config_types.include?(content.path.config_id) when 'ASM' - overrides << parse_content(content) if ['blocking', 'disabled_rules'].include?(content.path.config_id) + overrides << parse_content(content) if asm_overrides_config_types.include?(content.path.config_id) exclusions << parse_content(content) if content.path.config_id == 'exclusion_filters' end end @@ -233,16 +236,16 @@ def register_receivers Datadog::AppSec.reconfigure(ruleset: ruleset) end + end - def parse_content(content) - data = content.data.read + def parse_content(content) + data = content.data.read - content.data.rewind + content.data.rewind - raise ReadError, 'EOF reached' if data.nil? + raise ReadError, 'EOF reached' if data.nil? - JSON.parse(data) - end + JSON.parse(data) end end end diff --git a/sig/datadog/appsec/processor.rbs b/sig/datadog/appsec/processor.rbs index e33180ac832..94cecee5733 100644 --- a/sig/datadog/appsec/processor.rbs +++ b/sig/datadog/appsec/processor.rbs @@ -37,7 +37,7 @@ module Datadog @ruleset: ::Hash[::String, untyped] @addresses: ::Array[::String] - def initialize: (?ruleset: ::Hash[untyped, untyped]?) -> void + def initialize: (ruleset: ::Hash[untyped, untyped]) -> void def ready?: () -> bool def new_context: () -> Context def activate_context: () -> Context @@ -48,12 +48,8 @@ module Datadog private - def apply_denylist_data: (Configuration::Settings settings) -> untyped - def combine_denylist_data: (Configuration::Settings settings) -> ::Array[::Hash[::String, untyped]] - def denylist_data: (String id, ::Array[untyped] denylist) -> ::Hash[::String, untyped | "data_with_expiration"] def load_libddwaf: () -> bool - def load_ruleset: (Configuration::Settings settings, ?ruleset: ::Hash[untyped, untyped]?) -> bool - def create_waf_handle: (Configuration::Settings settings) -> bool + def create_waf_handle: (Configuration::Settings settings, ::Hash[String, untyped] ruleset) -> bool def self.libddwaf_provides_waf?: () -> bool def self.require_libddwaf: () -> bool diff --git a/sig/datadog/appsec/processor/rule_loader.rbs b/sig/datadog/appsec/processor/rule_loader.rbs new file mode 100644 index 00000000000..49509c0f723 --- /dev/null +++ b/sig/datadog/appsec/processor/rule_loader.rbs @@ -0,0 +1,17 @@ +module Datadog + module AppSec + class Processor + module RuleLoader + type ruleset = Symbol | String | File | StringIO | Hash[String, untyped] + def self.load_rules: (ruleset: ruleset) -> ::Hash[untyped, untyped]? + + def self.load_data: (?ip_denylist: Array[String], ?user_id_denylist: Array[String]) -> Array[Hash[String, untyped]]? + + private + + def self.denylist_data: (String id, Array[String] denylist) -> ::Hash[::String, untyped] + end + end + end +end + diff --git a/spec/datadog/appsec/component_spec.rb b/spec/datadog/appsec/component_spec.rb index b0936655568..3575ff9548e 100644 --- a/spec/datadog/appsec/component_spec.rb +++ b/spec/datadog/appsec/component_spec.rb @@ -40,6 +40,16 @@ expect(component.processor).to be_nil end end + + context 'when loading ruleset from settings fails' do + it 'returns a Datadog::AppSec::Component with a nil processor' do + expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).and_return(nil) + + component = described_class.build_appsec_component(settings_with_appsec) + + expect(component.processor).to be_nil + end + end end context 'when appsec is not enabled' do diff --git a/spec/datadog/appsec/processor/rule_loader_spec.rb b/spec/datadog/appsec/processor/rule_loader_spec.rb new file mode 100644 index 00000000000..553bfd8f91e --- /dev/null +++ b/spec/datadog/appsec/processor/rule_loader_spec.rb @@ -0,0 +1,158 @@ +require 'datadog/appsec/spec_helper' +require 'datadog/appsec/processor/rule_loader' + +RSpec.describe Datadog::AppSec::Processor::RuleLoader do + describe '#load_ruleset' do + let(:basic_ruleset) do + { + 'version' => '1.0', + 'events' => [ + { + 'id' => 1, + 'name' => 'Rule 1', + 'tags' => { 'type' => 'flow1' }, + 'conditions' => [ + { 'operation' => 'match_regex', 'parameters' => { 'inputs' => ['value2'], 'regex' => 'rule1' } }, + ], + 'action' => 'record', + } + ] + } + end + let(:recommended) { JSON.parse(Datadog::AppSec::Assets.waf_rules(:recommended)) } + let(:strict) { JSON.parse(Datadog::AppSec::Assets.waf_rules(:strict)) } + + subject(:rules) { described_class.load_rules(ruleset: ruleset) } + + context 'when ruleset is :recommended' do + let(:ruleset) { :recommended } + + it do + expect(rules).to_not be_nil + expect(rules).to eq(recommended) + end + end + + context 'when ruleset is :strict' do + let(:ruleset) { :strict } + + it do + expect(rules).to_not be_nil + expect(rules).to eq(strict) + end + end + + context 'when ruleset is :risky, defaults to recommended' do + let(:ruleset) { :risky } + + it do + expect(rules).to_not be_nil + expect(rules).to eq(recommended) + end + end + + context 'when ruleset is an existing path' do + let(:ruleset) { "#{__dir__}../../../../../lib/datadog/appsec/assets/waf_rules/recommended.json" } + + it { expect(rules).to_not be_nil } + end + + context 'when ruleset is a non existing path' do + let(:ruleset) { '/does/not/exist' } + + it { expect(rules).to be_nil } + end + + context 'when ruleset is IO-like' do + let(:ruleset) { StringIO.new(JSON.dump(basic_ruleset)) } + + it do + expect(rules).to_not be_nil + expect(rules).to eq(basic_ruleset) + end + end + + context 'when ruleset is Ruby' do + let(:ruleset) { basic_ruleset } + + it do + expect(rules).to_not be_nil + expect(rules).to eq(basic_ruleset) + end + end + + context 'when ruleset is not parseable' do + let(:ruleset) { StringIO.new('this is not json') } + + it { expect(rules).to be_nil } + end + end + + describe '#load_data' do + let(:ip_denylist) { [] } + let(:user_id_denylist) { [] } + subject(:data) { described_class.load_data(ip_denylist: ip_denylist, user_id_denylist: user_id_denylist) } + + context 'empty data' do + it 'returns nil' do + expect(data).to be_nil + end + end + + context 'non empty data' do + context 'with ip_denylist' do + let(:ip_denylist) { ['1.1.1.1', '1.1.1.2'] } + + it 'returns data information' do + expect(data).to_not be_nil + expect(data.size).to eq 1 + rules_data = data[0]['rules_data'][0] + + expect(rules_data).to_not be_nil + expect(rules_data['id']).to eq 'blocked_ips' + expect(rules_data['type']).to eq 'data_with_expiration' + + ip_values_data = rules_data['data'] + ip_values = ip_values_data.each.with_object([]) do |ip, acc| + acc << ip['value'] + end + + expect(ip_values_data.size).to eq 2 + expect(ip_values).to eq(ip_denylist) + end + end + + context 'with user_id_denylist' do + let(:user_id_denylist) { ['1', '2'] } + + it 'returns data information' do + expect(data).to_not be_nil + expect(data.size).to eq 1 + rules_data = data[0]['rules_data'][0] + + expect(rules_data).to_not be_nil + expect(rules_data['id']).to eq 'blocked_users' + expect(rules_data['type']).to eq 'data_with_expiration' + + user_id_values_data = rules_data['data'] + user_id_values = user_id_values_data.each.with_object([]) do |user, acc| + acc << user['value'] + end + + expect(user_id_values_data.size).to eq 2 + expect(user_id_values).to eq(user_id_denylist) + end + end + + context 'with ip_denylist and user_id_denylist' do + let(:ip_denylist) { ['1.1.1.1', '1.1.1.2'] } + let(:user_id_denylist) { ['1', '2'] } + + it 'returns data information' do + expect(data).to_not be_nil + expect(data.size).to eq 2 + end + end + end + end +end diff --git a/spec/datadog/appsec/processor_spec.rb b/spec/datadog/appsec/processor_spec.rb index 96aec69c777..67c8fab453f 100644 --- a/spec/datadog/appsec/processor_spec.rb +++ b/spec/datadog/appsec/processor_spec.rb @@ -1,5 +1,7 @@ require 'datadog/appsec/spec_helper' require 'datadog/appsec/processor' +require 'datadog/appsec/processor/rule_loader' +require 'datadog/appsec/processor/rule_merger' RSpec.describe Datadog::AppSec::Processor do before do @@ -18,6 +20,8 @@ allow(Datadog).to receive(:logger).and_return(logger) end + let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended) } + after do described_class.send(:reset_active_context) end @@ -59,7 +63,7 @@ end it 'return the previously set current context' do - processor = described_class.new + processor = described_class.new(ruleset: ruleset) context = processor.new_context described_class.send(:active_context=, context) @@ -81,7 +85,7 @@ describe '.reset_active_context' do it 'sets active_context to nil' do - processor = described_class.new + processor = described_class.new(ruleset: ruleset) context = processor.new_context described_class.send(:active_context=, context) @@ -104,7 +108,7 @@ allow(Object).to receive(:require).with('libddwaf').and_raise(LoadError) end - it { expect(described_class.new.send(:load_libddwaf)).to be false } + it { expect(described_class.new(ruleset: ruleset).send(:load_libddwaf)).to be false } end context 'when loaded but missing mandatory const' do @@ -113,7 +117,7 @@ hide_const('Datadog::AppSec::WAF') end - it { expect(described_class.new.send(:load_libddwaf)).to be false } + it { expect(described_class.new(ruleset: ruleset).send(:load_libddwaf)).to be false } end context 'when loaded successfully' do @@ -124,126 +128,15 @@ stub_const('Datadog::AppSec::WAF::LibDDWAF::Error', Class.new(StandardError)) end - it { expect(described_class.new.send(:load_libddwaf)).to be true } - end - end - - describe '#load_ruleset' do - let(:settings) { Datadog::AppSec.settings } - let(:basic_ruleset) do - { - 'version' => '1.0', - 'events' => [ - { - 'id' => 1, - 'name' => 'Rule 1', - 'tags' => { 'type' => 'flow1' }, - 'conditions' => [ - { 'operation' => 'match_regex', 'parameters' => { 'inputs' => ['value2'], 'regex' => 'rule1' } }, - ], - 'action' => 'record', - } - ] - } - 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, settings)).to be true } - end - - context 'when ruleset is :strict' do - let(:ruleset) { :strict } - - before do - expect(Datadog::AppSec::Assets).to receive(:waf_rules).with(:strict).and_call_original.twice - end - - it { expect(described_class.new.send(:load_ruleset, settings)).to be true } - end - - context 'when ruleset is :risky' do - let(:ruleset) { :risky } - - 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, 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, 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, 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, settings)).to be true } - end - - context 'when ruleset is Ruby' do - let(:ruleset) { basic_ruleset } - - 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, settings)).to be false } - end - end - - describe '#create_waf_handle' do - let(:ruleset) { :recommended } - let(:settings) { Datadog::AppSec.settings } - - before do - allow(settings).to receive(:ruleset).and_return(ruleset) - end - - context 'when ruleset is default' do - let(:ruleset) { :recommended } - - before do - expect(Datadog::AppSec::Assets).to receive(:waf_rules).with(:recommended).and_call_original - end - - 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, settings)).to be false } + it { expect(described_class.new(ruleset: ruleset).send(:load_libddwaf)).to be true } end end describe '#initialize' do - let(:ruleset) { :recommended } + subject(:processor) { described_class.new(ruleset: ruleset) } - subject(:processor) { described_class.new } - - before do - allow(Datadog::AppSec.settings).to receive(:ruleset).and_return(ruleset) + context 'when valid ruleset' do + it { is_expected.to be_ready } end context 'when libddwaf fails to load' do @@ -267,26 +160,6 @@ it { is_expected.to_not be_ready } end - context 'when ruleset is a non existing path' do - let(:ruleset) { '/does/not/exist' } - - before do - expect(Datadog.logger).to receive(:warn) - end - - it { is_expected.to_not be_ready } - end - - context 'when ruleset is not parseable' do - let(:ruleset) { StringIO.new('this is not json') } - - before do - expect(Datadog.logger).to receive(:warn) - end - - it { is_expected.to_not be_ready } - end - context 'when ruleset is invalid' do let(:ruleset) { { 'not' => 'valid' } } @@ -296,47 +169,9 @@ it { is_expected.to_not be_ready } end - - context 'when loading static data rule configuration' do - before do - allow(Datadog::AppSec.settings).to receive(:ip_denylist).and_return(['192.192.1.1']) - allow(Datadog::AppSec.settings).to receive(:user_id_denylist).and_return(['user3']) - end - - it 'stores the data as part of the @ruleset' do - processor = described_class.new - ruleset = processor.instance_variable_get(:@ruleset) - - blocked_ips = ruleset['rules_data'].find { |hash| hash['id'] == 'blocked_ips' } - blocked_users = ruleset['rules_data'].find { |hash| hash['id'] == 'blocked_users' } - - expect(blocked_ips).to_not be_nil - expect(blocked_users).to_not be_nil - expect(blocked_ips['type']).to eq('data_with_expiration') - expect(blocked_users['type']).to eq('data_with_expiration') - - blocked_ips_data = blocked_ips['data'] - blocked_user_data = blocked_users['data'] - expect(blocked_ips_data.size).to eq(1) - expect(blocked_user_data.size).to eq(1) - expect(blocked_ips_data[0]['value']).to eq('192.192.1.1') - expect(blocked_user_data[0]['value']).to eq('user3') - end - end - - context 'when things are OK' do - before do - expect(Datadog::AppSec::Assets).to receive(:waf_rules).with(:recommended).and_call_original - expect(Datadog.logger).to_not receive(:warn) - end - - it { is_expected.to be_ready } - end end describe '#new_context' do - let(:ruleset) { :recommended } - let(:input_safe) { { 'server.request.headers.no_cookies' => { 'user-agent' => 'Ruby' } } } let(:input_sqli) { { 'server.request.query' => { 'q' => '1 OR 1;' } } } let(:input_scanner) { { 'server.request.headers.no_cookies' => { 'user-agent' => 'Nessus SOAP' } } } @@ -346,7 +181,7 @@ let(:input) { input_scanner } - let(:processor) { described_class.new } + let(:processor) { described_class.new(ruleset: ruleset) } subject(:context) { processor.new_context } after do @@ -376,10 +211,7 @@ results.first end - let(:set_ip_denylist) { nil } - before do - set_ip_denylist runs end @@ -489,8 +321,14 @@ context 'one blockable attack' do let(:input) { input_client_ip } - let(:set_ip_denylist) do - allow(Datadog::AppSec.settings).to receive(:ip_denylist).and_return([client_ip]) + let(:ruleset) do + rules = Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended) + data = Datadog::AppSec::Processor::RuleLoader.load_data(ip_denylist: [client_ip]) + + Datadog::AppSec::Processor::RuleMerger.merge( + rules: [rules], + data: data, + ) end it { expect(matches).to have_attributes(count: 1) } @@ -503,21 +341,23 @@ describe '#active_context' do it 'creates a new context and store in the class .active_context variable' do - context = described_class.new.activate_context + context = described_class.new(ruleset: ruleset).activate_context expect(context).to eq(described_class.active_context) end context 'when an active context already exists' do it 'raises AlreadyActiveContextError' do - described_class.new.activate_context - expect { described_class.new.activate_context }.to raise_error(described_class::AlreadyActiveContextError) + described_class.new(ruleset: ruleset).activate_context + expect do + described_class.new(ruleset: ruleset).activate_context + end.to raise_error(described_class::AlreadyActiveContextError) end end end describe '#deactivate_context' do it 'finalize the active context and reset the class .active_context variable' do - handler = described_class.new + handler = described_class.new(ruleset: ruleset) context = handler.activate_context expect(context).to receive(:finalize) @@ -527,7 +367,9 @@ context 'without an active_context' do it 'raises NoActiveContextError' do - expect { described_class.new.deactivate_context }.to raise_error(described_class::NoActiveContextError) + expect do + described_class.new(ruleset: ruleset).deactivate_context + end.to raise_error(described_class::NoActiveContextError) end end end diff --git a/spec/datadog/core/remote/client_spec.rb b/spec/datadog/core/remote/client_spec.rb index 40b1029e006..a3ac80403b0 100644 --- a/spec/datadog/core/remote/client_spec.rb +++ b/spec/datadog/core/remote/client_spec.rb @@ -74,7 +74,8 @@ }, ] end - let(:target_content) do + + let(:exclusions_filter_content) do { 'datadog/603646/ASM/exclusion_filters/config' => { 'custom' => { @@ -84,18 +85,28 @@ }, 'v' => 21 }, - 'hashes' => { 'sha256' => Digest::SHA256.hexdigest(exclusion_content) }, + 'hashes' => { 'sha256' => Digest::SHA256.hexdigest(exclusions) }, 'length' => 645 - }, + } + } + end + + let(:blocked_ips_content) do + { 'datadog/603646/ASM_DATA/blocked_ips/config' => { 'custom' => { 'c' => ['client_id'], 'tracer-predicates' => { 'tracer_predicates_v1' => [{ 'clientID' => 'client_id' }] }, 'v' => 51 }, - 'hashes' => { 'sha256' => Digest::SHA256.hexdigest(blocked_ips_content) }, + 'hashes' => { 'sha256' => Digest::SHA256.hexdigest(blocked_ips) }, 'length' => 1834 }, + } + end + + let(:rules_content) do + { 'datadog/603646/ASM_DD/latest/config' => { 'custom' => { 'c' => ['client_id'], @@ -109,6 +120,9 @@ }, } end + + let(:target_content) { {} } + let(:targets) do { 'signatures' => [ @@ -130,12 +144,15 @@ } } end - let(:exclusion_content) do + + let(:exclusions) do '{"exclusions":[{"conditions":[{"operator":"ip_match","parameters":{"inputs":[{"address":"http.client_ip"}]}}]}]}' end - let(:blocked_ips_content) do + + let(:blocked_ips) do '{"rules_data":[{"data":[{"expiration":1678972458,"value":"42.42.42.1"}]}]}' end + let(:rules_data) do { version: '2.2', @@ -172,37 +189,63 @@ }.to_json end + let(:rules_data_response) do + { + 'path' => 'datadog/603646/ASM_DD/latest/config', + 'raw' => Base64.strict_encode64(rules_data).chomp + } + end + + let(:blocked_ips_data_response) do + { + 'path' => 'datadog/603646/ASM_DATA/blocked_ips/config', + 'raw' => Base64.strict_encode64(blocked_ips).chomp + } + end + + let(:exclusion_data_response) do + { + 'path' => 'datadog/603646/ASM/exclusion_filters/config', + 'raw' => Base64.strict_encode64(exclusions).chomp + } + end + + let(:target_files) { [] } + + let(:client_configs) { [] } + let(:response_body) do { 'roots' => roots.map { |r| Base64.strict_encode64(r.to_json).chomp }, 'targets' => Base64.strict_encode64(targets.to_json).chomp, - 'target_files' => [ - { - 'path' => 'datadog/603646/ASM_DD/latest/config', - 'raw' => Base64.strict_encode64(rules_data).chomp - }, - { - 'path' => 'datadog/603646/ASM_DATA/blocked_ips/config', - 'raw' => Base64.strict_encode64(blocked_ips_content).chomp - }, - { - 'path' => 'datadog/603646/ASM/exclusion_filters/config', - 'raw' => Base64.strict_encode64(exclusion_content).chomp - } - ], - 'client_configs' => [ - 'datadog/603646/ASM_DATA/blocked_ips/config', - 'datadog/603646/ASM_DD/latest/config', - 'datadog/603646/ASM/exclusion_filters/config' - ] + 'target_files' => target_files, + 'client_configs' => client_configs, }.to_json end + let(:repository) { Datadog::Core::Remote::Configuration::Repository.new } + subject(:client) { described_class.new(transport, repository: repository) } describe '#sync' do include_context 'HTTP connection stub' + let(:client_configs) do + [ + 'datadog/603646/ASM_DATA/blocked_ips/config', + 'datadog/603646/ASM_DD/latest/config', + 'datadog/603646/ASM/exclusion_filters/config' + ] + end + + let(:target_files) do + [rules_data_response, blocked_ips_data_response, exclusion_data_response] + end + + let(:target_content) do + {}.merge(exclusions_filter_content).merge(blocked_ips_content).merge(rules_content) + end + context 'valid response' do let(:response_code) { 200 } @@ -260,10 +303,42 @@ 'version' => '2.2' } - expect(Datadog::AppSec).to receive(:reconfigure).with(ruleset: expected_ruleset) + expect(Datadog::AppSec).to receive(:reconfigure).with(ruleset: expected_ruleset).and_call_original client.sync end + context 'when there is no ASM_DD information' do + let(:client_configs) do + [ + 'datadog/603646/ASM_DATA/blocked_ips/config', + 'datadog/603646/ASM/exclusion_filters/config' + ] + end + + let(:target_files) do + [blocked_ips_data_response, exclusion_data_response] + end + + let(:target_content) do + {}.merge(exclusions_filter_content).merge(blocked_ips_content) + end + + it 'uses the rules from the appsec settings' do + expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).with( + ruleset: Datadog.configuration.appsec.ruleset + ).and_call_original + client.sync + end + + it 'raises SyncError if no default rules available' do + expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).with( + ruleset: Datadog.configuration.appsec.ruleset + ).and_return(nil) + + expect { client.sync }.to raise_error(described_class::SyncError) + end + end + context 'when the data is the same' do it 'does not commit the information to the transaction' do expect_any_instance_of(Datadog::Core::Remote::Configuration::Repository::Transaction).to receive(:insert) @@ -288,7 +363,7 @@ }, { :path => 'datadog/603646/ASM/exclusion_filters/config', - :content => StringIO.new(exclusion_content) + :content => StringIO.new(exclusions) }, { :path => 'datadog/603646/ASM_DD/latest/config', @@ -313,7 +388,7 @@ 'tracer-predicates' => { 'tracer_predicates_v1' => [{ 'clientID' => 'client_id' }] }, 'v' => 21 }, - 'hashes' => { 'sha256' => Digest::SHA256.hexdigest(exclusion_content) }, + 'hashes' => { 'sha256' => Digest::SHA256.hexdigest(exclusions) }, 'length' => 645 }, 'datadog/603646/ASM_DATA/blocked_ips/config' => { @@ -367,7 +442,7 @@ 'target_files' => [ { 'path' => 'datadog/603646/ASM/exclusion_filters/config', - 'raw' => Base64.strict_encode64(exclusion_content).chomp + 'raw' => Base64.strict_encode64(exclusions).chomp } ], 'client_configs' => [ @@ -403,7 +478,7 @@ }, 'v' => 21 }, - 'hashes' => { 'sha256' => Digest::SHA256.hexdigest(exclusion_content) }, + 'hashes' => { 'sha256' => Digest::SHA256.hexdigest(exclusions) }, 'length' => 645 }, } From 894e8a228a4e92f43cf97ea0539192f4eedb10c5 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Thu, 30 Mar 2023 14:28:40 +0200 Subject: [PATCH 5/5] fix weird order spec issue --- spec/datadog/core/remote/client_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/datadog/core/remote/client_spec.rb b/spec/datadog/core/remote/client_spec.rb index a3ac80403b0..70fa340d297 100644 --- a/spec/datadog/core/remote/client_spec.rb +++ b/spec/datadog/core/remote/client_spec.rb @@ -326,7 +326,8 @@ it 'uses the rules from the appsec settings' do expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).with( ruleset: Datadog.configuration.appsec.ruleset - ).and_call_original + ).at_least(:once).and_call_original + client.sync end