From 335b8faa6b6ad182b9a7237b509cb087fbf1ec1f Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Mon, 17 Jul 2023 18:14:38 +0200 Subject: [PATCH 1/2] Allow blocking response configuration --- lib/datadog/appsec/configuration/settings.rb | 56 ++++++++++++++++++++ lib/datadog/appsec/response.rb | 44 +++++++++++++-- sig/datadog/appsec/response.rbs | 14 +++-- sig/datadog/core/configuration/settings.rbs | 17 ++++++ 4 files changed, 125 insertions(+), 6 deletions(-) diff --git a/lib/datadog/appsec/configuration/settings.rb b/lib/datadog/appsec/configuration/settings.rb index 867ef19c102..05528e9752d 100644 --- a/lib/datadog/appsec/configuration/settings.rb +++ b/lib/datadog/appsec/configuration/settings.rb @@ -95,6 +95,62 @@ def self.add_settings!(base) o.default DEFAULT_OBFUSCATOR_VALUE_REGEX end + settings :block do + # HTTP status code to block with + option :status do |o| + o.default { 403 } + end + + # only applies to redirect status codes + option :location do |o| + o.setter { |v| URI(v) unless v.nil? } + end + + # only applies to non-redirect status codes with bodies + option :templates do |o| + o.default do + json = ENV.fetch( + 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_JSON', + :json + ) + + html = ENV.fetch( + 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_HTML', + :html + ) + + text = ENV.fetch( + 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_TEXT', + :text + ) + + { + 'application/json' => json, + 'text/html' => html, + 'text/plain' => text, + } + end + o.setter do |v| + next if v.nil? + + # TODO: should merge with o.default to allow overriding only one mime type + + v.each do |k, w| + case w + when :json, :html, :text + next + when String, Pathname + next if File.exist?(w.to_s) + + raise(ArgumentError, "appsec.templates.#{k}: file not found: #{w}") + else + raise ArgumentError, "appsec.templates.#{k}: unexpected value: #{w.inspect}" + end + end + end + end + end + settings :track_user_events do option :enabled do |o| o.default true diff --git a/lib/datadog/appsec/response.rb b/lib/datadog/appsec/response.rb index dae3e5d30d3..88ead7de220 100644 --- a/lib/datadog/appsec/response.rb +++ b/lib/datadog/appsec/response.rb @@ -33,10 +33,16 @@ def negotiate(env) Datadog.logger.debug { "negotiated response content type: #{content_type}" } + headers = { 'Content-Type' => content_type } + headers['Location'] = location.to_s if redirect? + + body = [] + body << content(content_type) unless redirect? + Response.new( - status: 403, - headers: { 'Content-Type' => content_type }, - body: [Datadog::AppSec::Assets.blocked(format: CONTENT_TYPE_TO_FORMAT[content_type])] + status: status, + headers: headers, + body: body, ) end @@ -49,6 +55,7 @@ def negotiate(env) }.freeze DEFAULT_CONTENT_TYPE = 'application/json' + REDIRECT_STATUS = [301, 302, 303, 307, 308].freeze def content_type(env) return DEFAULT_CONTENT_TYPE unless env.key?('HTTP_ACCEPT') @@ -67,6 +74,37 @@ def content_type(env) rescue Datadog::AppSec::Utils::HTTP::MediaRange::ParseError DEFAULT_CONTENT_TYPE end + + def status + Datadog.configuration.appsec.block.status + end + + def redirect? + REDIRECT_STATUS.include?(status) + end + + def location + Datadog.configuration.appsec.block.location + end + + def content(content_type) + setting = Datadog.configuration.appsec.block.templates[content_type] + + case setting + when :html, :json, :text + Datadog::AppSec::Assets.blocked(format: setting) + when String, Pathname + path = setting.to_s + + cache[path] ||= (File.open(path, 'rb', &:read) || '') + else + raise ArgumentError, "unexpected type: #{content_type.inspect}" + end + end + + def cache + @cache ||= {} + end end end end diff --git a/sig/datadog/appsec/response.rbs b/sig/datadog/appsec/response.rbs index 66569537d42..102030602dc 100644 --- a/sig/datadog/appsec/response.rbs +++ b/sig/datadog/appsec/response.rbs @@ -3,9 +3,10 @@ module Datadog class Response attr_reader status: ::Integer attr_reader headers: ::Hash[::String, ::String] - attr_reader body: ::Array[untyped] + attr_reader body: ::Array[::String] - def initialize: (status: ::Integer, ?headers: ::Hash[::String, ::String], ?body: ::Array[untyped]) -> void + + def initialize: (status: ::Integer, ?headers: ::Hash[::String, ::String], ?body: ::Array[::String]) -> void def to_rack: () -> ::Array[untyped] def to_sinatra_response: () -> ::Sinatra::Response def to_action_dispatch_response: () -> ::ActionDispatch::Response @@ -16,9 +17,16 @@ module Datadog CONTENT_TYPE_TO_FORMAT: ::Hash[::String, ::Symbol] DEFAULT_CONTENT_TYPE: ::String + REDIRECT_STATUS: ::Array[::Integer] - def self.format: (::Hash[untyped, untyped] env) -> ::Symbol def self.content_type: (::Hash[untyped, untyped] env) -> ::String + def self.content: (::String) -> ::String + def self.status: () -> ::Integer + def self.location: () -> ::URI? + def self.redirect?: () -> bool + + self.@cache: ::Hash[::String, ::String] + def self.cache: () -> ::Hash[::String, ::String] end end end diff --git a/sig/datadog/core/configuration/settings.rbs b/sig/datadog/core/configuration/settings.rbs index 44c8807ae1f..8817656058a 100644 --- a/sig/datadog/core/configuration/settings.rbs +++ b/sig/datadog/core/configuration/settings.rbs @@ -44,6 +44,23 @@ module Datadog def ruleset=: (String | Symbol | File | StringIO | ::Hash[untyped, untyped]) -> void def using_default?: (Symbol option) -> bool + + def block: () -> _AppSecBlock + + end + + interface _AppSecBlock + def templates=: (::Hash[::String, ::Symbol | ::String | ::Pathname]) -> void + + def templates: () -> ::Hash[::String, ::Symbol | ::String | ::Pathname] + + def status=: (::Integer) -> void + + def status: () -> ::Integer + + def location=: (::URI | ::String | nil) -> void + + def location: () -> ::URI? end def initialize: (*untyped _) -> untyped From cf2b9682c90471b6288ed0a3445d5497cd76fc87 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Thu, 24 Aug 2023 17:37:07 +0200 Subject: [PATCH 2/2] allow only to configure response template via configuration or ENV variables --- lib/datadog/appsec/configuration/settings.rb | 76 ++++++++----------- lib/datadog/appsec/response.rb | 39 ++-------- sig/datadog/appsec/response.rbs | 7 -- sig/datadog/core/configuration/settings.rbs | 19 ++--- .../appsec/configuration/settings_spec.rb | 74 ++++++++++++++++++ spec/datadog/appsec/response_spec.rb | 28 ++++++- 6 files changed, 149 insertions(+), 94 deletions(-) diff --git a/lib/datadog/appsec/configuration/settings.rb b/lib/datadog/appsec/configuration/settings.rb index 05528e9752d..9ad3f9ac85a 100644 --- a/lib/datadog/appsec/configuration/settings.rb +++ b/lib/datadog/appsec/configuration/settings.rb @@ -25,7 +25,7 @@ def self.extended(base) add_settings!(base) end - # rubocop:disable Metrics/AbcSize,Metrics/MethodLength,Metrics/BlockLength + # rubocop:disable Metrics/AbcSize,Metrics/MethodLength,Metrics/BlockLength,Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity def self.add_settings!(base) base.class_eval do settings :appsec do @@ -96,55 +96,39 @@ def self.add_settings!(base) end settings :block do - # HTTP status code to block with - option :status do |o| - o.default { 403 } - end + settings :templates do + option :html do |o| + o.env 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_HTML' + o.type :string, nilable: true + o.setter do |value| + if value + raise(ArgumentError, "appsec.templates.html: file not found: #{value}") unless File.exist?(value) + + File.open(value, 'rb', &:read) || '' + end + end + end - # only applies to redirect status codes - option :location do |o| - o.setter { |v| URI(v) unless v.nil? } - end + option :json do |o| + o.env 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_JSON' + o.type :string, nilable: true + o.setter do |value| + if value + raise(ArgumentError, "appsec.templates.json: file not found: #{value}") unless File.exist?(value) - # only applies to non-redirect status codes with bodies - option :templates do |o| - o.default do - json = ENV.fetch( - 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_JSON', - :json - ) - - html = ENV.fetch( - 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_HTML', - :html - ) - - text = ENV.fetch( - 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_TEXT', - :text - ) - - { - 'application/json' => json, - 'text/html' => html, - 'text/plain' => text, - } + File.open(value, 'rb', &:read) || '' + end + end end - o.setter do |v| - next if v.nil? - - # TODO: should merge with o.default to allow overriding only one mime type - v.each do |k, w| - case w - when :json, :html, :text - next - when String, Pathname - next if File.exist?(w.to_s) + option :text do |o| + o.env 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_TEXT' + o.type :string, nilable: true + o.setter do |value| + if value + raise(ArgumentError, "appsec.templates.text: file not found: #{value}") unless File.exist?(value) - raise(ArgumentError, "appsec.templates.#{k}: file not found: #{w}") - else - raise ArgumentError, "appsec.templates.#{k}: unexpected value: #{w.inspect}" + File.open(value, 'rb', &:read) || '' end end end @@ -188,7 +172,7 @@ def self.add_settings!(base) end end end - # rubocop:enable Metrics/AbcSize,Metrics/MethodLength,Metrics/BlockLength + # rubocop:enable Metrics/AbcSize,Metrics/MethodLength,Metrics/BlockLength,Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity end end end diff --git a/lib/datadog/appsec/response.rb b/lib/datadog/appsec/response.rb index 88ead7de220..8347d213249 100644 --- a/lib/datadog/appsec/response.rb +++ b/lib/datadog/appsec/response.rb @@ -33,15 +33,12 @@ def negotiate(env) Datadog.logger.debug { "negotiated response content type: #{content_type}" } - headers = { 'Content-Type' => content_type } - headers['Location'] = location.to_s if redirect? - body = [] - body << content(content_type) unless redirect? + body << content(content_type) Response.new( - status: status, - headers: headers, + status: 403, + headers: { 'Content-Type' => content_type }, body: body, ) end @@ -55,7 +52,6 @@ def negotiate(env) }.freeze DEFAULT_CONTENT_TYPE = 'application/json' - REDIRECT_STATUS = [301, 302, 303, 307, 308].freeze def content_type(env) return DEFAULT_CONTENT_TYPE unless env.key?('HTTP_ACCEPT') @@ -75,36 +71,17 @@ def content_type(env) DEFAULT_CONTENT_TYPE end - def status - Datadog.configuration.appsec.block.status - end - - def redirect? - REDIRECT_STATUS.include?(status) - end - - def location - Datadog.configuration.appsec.block.location - end - def content(content_type) - setting = Datadog.configuration.appsec.block.templates[content_type] + content_format = CONTENT_TYPE_TO_FORMAT[content_type] - case setting - when :html, :json, :text - Datadog::AppSec::Assets.blocked(format: setting) - when String, Pathname - path = setting.to_s + using_default = Datadog.configuration.appsec.block.templates.using_default?(content_format) - cache[path] ||= (File.open(path, 'rb', &:read) || '') + if using_default + Datadog::AppSec::Assets.blocked(format: content_format) else - raise ArgumentError, "unexpected type: #{content_type.inspect}" + Datadog.configuration.appsec.block.templates.send(content_format) end end - - def cache - @cache ||= {} - end end end end diff --git a/sig/datadog/appsec/response.rbs b/sig/datadog/appsec/response.rbs index 102030602dc..98d5b0764b5 100644 --- a/sig/datadog/appsec/response.rbs +++ b/sig/datadog/appsec/response.rbs @@ -17,16 +17,9 @@ module Datadog CONTENT_TYPE_TO_FORMAT: ::Hash[::String, ::Symbol] DEFAULT_CONTENT_TYPE: ::String - REDIRECT_STATUS: ::Array[::Integer] def self.content_type: (::Hash[untyped, untyped] env) -> ::String def self.content: (::String) -> ::String - def self.status: () -> ::Integer - def self.location: () -> ::URI? - def self.redirect?: () -> bool - - self.@cache: ::Hash[::String, ::String] - def self.cache: () -> ::Hash[::String, ::String] end end end diff --git a/sig/datadog/core/configuration/settings.rbs b/sig/datadog/core/configuration/settings.rbs index 8817656058a..1bd02ac0c9b 100644 --- a/sig/datadog/core/configuration/settings.rbs +++ b/sig/datadog/core/configuration/settings.rbs @@ -43,24 +43,25 @@ module Datadog def ruleset=: (String | Symbol | File | StringIO | ::Hash[untyped, untyped]) -> void - def using_default?: (Symbol option) -> bool - def block: () -> _AppSecBlock - end interface _AppSecBlock - def templates=: (::Hash[::String, ::Symbol | ::String | ::Pathname]) -> void + def templates: () -> _TemplatesBlock + end + + interface _TemplatesBlock + def html=: (::String) -> void - def templates: () -> ::Hash[::String, ::Symbol | ::String | ::Pathname] + def html: () -> ::String - def status=: (::Integer) -> void + def json=: (::String) -> void - def status: () -> ::Integer + def json: () -> ::String - def location=: (::URI | ::String | nil) -> void + def text=: (::String) -> void - def location: () -> ::URI? + def text: () -> ::String end def initialize: (*untyped _) -> untyped diff --git a/spec/datadog/appsec/configuration/settings_spec.rb b/spec/datadog/appsec/configuration/settings_spec.rb index b3e7e872394..9427b44a23b 100644 --- a/spec/datadog/appsec/configuration/settings_spec.rb +++ b/spec/datadog/appsec/configuration/settings_spec.rb @@ -559,5 +559,79 @@ def patcher end end end + + describe 'block' do + describe 'templates' do + [ + { method_name: :html, env_var: 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_HTML' }, + { method_name: :json, env_var: 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_JSON' }, + { method_name: :text, env_var: 'DD_APPSEC_HTTP_BLOCKED_TEMPLATE_TEXT' } + ].each do |test_info| + describe "##{test_info[:method_name]}" do + context "when #{test_info[:env_var]}" do + subject(:template) { settings.appsec.block.templates.send(test_info[:method_name]) } + + around do |example| + ClimateControl.modify(test_info[:env_var] => template_path) do + example.run + end + end + + context 'is defined and the file exists' do + before do + File.write(template_path, 'testing') + end + + after do + File.delete(template_path) + end + + let(:template_path) do + "hello.#{test_info[:method_name]}" + end + + it { is_expected.to eq 'testing' } + end + + context 'is defined and the file do not exists' do + let(:template_path) do + "hello.#{test_info[:method_name]}" + end + + it { expect { is_expected }.to raise_error(ArgumentError) } + end + end + end + + describe "##{test_info[:method_name]}=" do + subject(:template) { settings.appsec.block.templates.send("#{test_info[:method_name]}=", template_path) } + + context 'is defined and the file exists' do + before do + File.write(template_path, 'testing') + end + + after do + File.delete(template_path) + end + + let(:template_path) do + "hello.#{test_info[:method_name]}" + end + + it { is_expected.to eq 'testing' } + end + + context 'is defined and the file do not exists' do + let(:template_path) do + "hello.#{test_info[:method_name]}" + end + + it { expect { is_expected }.to raise_error(ArgumentError) } + end + end + end + end + end end end diff --git a/spec/datadog/appsec/response_spec.rb b/spec/datadog/appsec/response_spec.rb index 5bc89c875c3..e8d742486d4 100644 --- a/spec/datadog/appsec/response_spec.rb +++ b/spec/datadog/appsec/response_spec.rb @@ -10,7 +10,7 @@ end describe '.status' do - subject(:content_type) { described_class.negotiate(env).status } + subject(:status) { described_class.negotiate(env).status } it { is_expected.to eq 403 } end @@ -23,22 +23,48 @@ expect(env).to receive(:[]).with('HTTP_ACCEPT').and_return(accept) end + shared_examples_for 'with custom response body' do |type| + before do + File.write("test.#{type}", 'testing') + Datadog.configuration.appsec.block.templates.send("#{type}=", "test.#{type}") + end + + after do + File.delete("test.#{type}") + Datadog.configuration.appsec.reset! + end + + it { is_expected.to eq ['testing'] } + end + + context 'with unsupported Accept headers' do + let(:accept) { 'application/xml' } + + it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :json)] } + end + context('with Accept: text/html') do let(:accept) { 'text/html' } it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :html)] } + + it_behaves_like 'with custom response body', :html end context('with Accept: application/json') do let(:accept) { 'application/json' } it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :json)] } + + it_behaves_like 'with custom response body', :json end context('with Accept: text/plain') do let(:accept) { 'text/plain' } it { is_expected.to eq [Datadog::AppSec::Assets.blocked(format: :text)] } + + it_behaves_like 'with custom response body', :text end end