From ca6ed3b75a8c1bbcf798c346726d1bee664c5342 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Thu, 23 Feb 2023 13:36:08 +0100 Subject: [PATCH] block request when user ID matches --- .../appsec/contrib/rack/request_middleware.rb | 7 +++-- lib/datadog/appsec/ext.rb | 10 ++++++ lib/datadog/appsec/monitor/gateway/watcher.rb | 3 +- lib/datadog/kit/identity.rb | 5 +++ sig/datadog/appsec/ext.rbs | 7 +++++ .../contrib/rack/integration_test_spec.rb | 31 +++++++++++++++++++ .../contrib/rails/integration_test_spec.rb | 28 +++++++++++++++++ .../contrib/sinatra/integration_test_spec.rb | 27 ++++++++++++++++ spec/datadog/kit/identity_spec.rb | 21 +++++++++++++ 9 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 lib/datadog/appsec/ext.rb create mode 100644 sig/datadog/appsec/ext.rbs diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index e49c9cd48b..96f4a8d64f 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -2,6 +2,7 @@ require 'json' +require_relative '../../ext' require_relative '../../instrumentation/gateway' require_relative '../../processor' require_relative '../../response' @@ -38,8 +39,10 @@ def call(env) add_appsec_tags(processor, active_trace, active_span, env) - request_return, request_response = Instrumentation.gateway.push('rack.request', request) do - @app.call(env) + request_return, request_response = catch(::Datadog::AppSec::Ext::INTERRUPT) do + Instrumentation.gateway.push('rack.request', request) do + @app.call(env) + end end if request_response && request_response.any? { |action, _event| action == :block } diff --git a/lib/datadog/appsec/ext.rb b/lib/datadog/appsec/ext.rb new file mode 100644 index 0000000000..4fd1467b6a --- /dev/null +++ b/lib/datadog/appsec/ext.rb @@ -0,0 +1,10 @@ +# typed: false +# frozen_string_literal: true + +module Datadog + module AppSec + module Ext + INTERRUPT = :datadog_appsec_interrupt + end + end +end diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index 9282ef21d2..99f1bdf9a7 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -1,6 +1,7 @@ # typed: false # frozen_string_literal: true +require_relative '../../ext' require_relative '../../instrumentation/gateway' require_relative '../../reactive/operation' require_relative '../reactive/set_user' @@ -48,7 +49,7 @@ def watch_user_id(gateway = Instrumentation.gateway) _result, block = Monitor::Reactive::SetUser.publish(op, user) end - next [nil, [[:block, event]]] if block + throw(Datadog::AppSec::Ext::INTERRUPT, [nil, [:block, event]]) if block ret, res = stack.call(user) diff --git a/lib/datadog/kit/identity.rb b/lib/datadog/kit/identity.rb index eb3e27a9fd..e6de30fd20 100644 --- a/lib/datadog/kit/identity.rb +++ b/lib/datadog/kit/identity.rb @@ -56,6 +56,11 @@ def self.set_user(trace, id:, email: nil, name: nil, session_id: nil, role: nil, others.each do |k, v| trace.set_tag("usr.#{k}", v) unless v.nil? end + + if Datadog.configuration.appsec.enabled + user = OpenStruct.new(id: id) + ::Datadog::AppSec::Instrumentation.gateway.push('identity.set_user', user) + end end # rubocop:enable Metrics/PerceivedComplexity # rubocop:enable Metrics/CyclomaticComplexity diff --git a/sig/datadog/appsec/ext.rbs b/sig/datadog/appsec/ext.rbs new file mode 100644 index 0000000000..a339d406fa --- /dev/null +++ b/sig/datadog/appsec/ext.rbs @@ -0,0 +1,7 @@ +module Datadog + module AppSec + module Ext + INTERRUPT: Symbol + end + end +end diff --git a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb index 3c202d06c6..59f3c9aa21 100644 --- a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb @@ -22,6 +22,7 @@ let(:appsec_enabled) { true } let(:tracing_enabled) { true } let(:appsec_ip_denylist) { nil } + let(:appsec_user_id_denylist) { nil } let(:appsec_ruleset) { :recommended } let(:crs_942_100) do @@ -133,6 +134,7 @@ c.appsec.enabled = appsec_enabled c.appsec.instrument :rack c.appsec.ip_denylist = appsec_ip_denylist + c.appsec.user_id_denylist = appsec_user_id_denylist c.appsec.ruleset = appsec_ruleset end end @@ -293,6 +295,15 @@ end ) end + + map '/set_user' do + run( + proc do |_env| + Datadog::Kit::Identity.set_user(Datadog::Tracing.active_trace, id: 'blocked-user-id') + [200, { 'Content-Type' => 'text/html' }, ['OK']] + end + ) + end end end @@ -382,6 +393,26 @@ it_behaves_like 'a trace with AppSec events' end end + + context 'with user blocking ID' do + let(:url) { '/set_user' } + + it { is_expected.to be_ok } + + it_behaves_like 'a GET 200 span' + it_behaves_like 'a trace with AppSec tags' + it_behaves_like 'a trace without AppSec events' + + context 'with an event-triggering user ID' do + let(:appsec_user_id_denylist) { ['blocked-user-id'] } + + it { is_expected.to be_forbidden } + + it_behaves_like 'a GET 403 span' + it_behaves_like 'a trace with AppSec tags' + it_behaves_like 'a trace with AppSec events' + end + end end describe 'POST request' do diff --git a/spec/datadog/appsec/contrib/rails/integration_test_spec.rb b/spec/datadog/appsec/contrib/rails/integration_test_spec.rb index f9f51350f4..c4bd43c8ee 100644 --- a/spec/datadog/appsec/contrib/rails/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rails/integration_test_spec.rb @@ -32,6 +32,7 @@ let(:appsec_enabled) { true } let(:tracing_enabled) { true } let(:appsec_ip_denylist) { nil } + let(:appsec_user_id_denylist) { nil } let(:appsec_ruleset) { :recommended } let(:crs_942_100) do @@ -89,6 +90,7 @@ c.appsec.enabled = appsec_enabled c.appsec.instrument :rails c.appsec.ip_denylist = appsec_ip_denylist + c.appsec.user_id_denylist = appsec_user_id_denylist c.appsec.ruleset = appsec_ruleset # TODO: test with c.appsec.instrument :rack @@ -118,6 +120,11 @@ def success head :ok end + + def set_user + Datadog::Kit::Identity.set_user(Datadog::Tracing.active_trace, id: 'blocked-user-id') + head :ok + end end ) end @@ -244,6 +251,7 @@ def success { '/success' => 'test#success', [:post, '/success'] => 'test#success', + '/set_user' => 'test#set_user', } end @@ -347,6 +355,26 @@ def success it_behaves_like 'a trace with AppSec tags' it_behaves_like 'a trace with AppSec events' end + + context 'with user blocking ID' do + let(:url) { '/set_user' } + + it { is_expected.to be_ok } + + it_behaves_like 'a GET 200 span' + it_behaves_like 'a trace with AppSec tags' + it_behaves_like 'a trace without AppSec events' + + context 'with an event-triggering user ID' do + let(:appsec_user_id_denylist) { ['blocked-user-id'] } + + it { is_expected.to be_forbidden } + + it_behaves_like 'a GET 403 span' + it_behaves_like 'a trace with AppSec tags' + it_behaves_like 'a trace with AppSec events' + end + end end describe 'POST request' do diff --git a/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb b/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb index ba614ee7fb..0bd6106f32 100644 --- a/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb @@ -44,6 +44,7 @@ let(:appsec_enabled) { true } let(:tracing_enabled) { true } let(:appsec_ip_denylist) { nil } + let(:appsec_user_id_denylist) { nil } let(:appsec_ruleset) { :recommended } let(:crs_942_100) do @@ -101,6 +102,7 @@ c.appsec.enabled = appsec_enabled c.appsec.instrument :sinatra c.appsec.ip_denylist = appsec_ip_denylist + c.appsec.user_id_denylist = appsec_user_id_denylist c.appsec.ruleset = appsec_ruleset # TODO: test with c.appsec.instrument :rack @@ -254,6 +256,11 @@ post '/success' do 'ok' end + + get '/set_user' do + Datadog::Kit::Identity.set_user(Datadog::Tracing.active_trace, id: 'blocked-user-id') + 'ok' + end end end @@ -359,6 +366,26 @@ it_behaves_like 'a trace with AppSec tags' it_behaves_like 'a trace with AppSec events' end + + context 'with user blocking ID' do + let(:url) { '/set_user' } + + it { is_expected.to be_ok } + + it_behaves_like 'a GET 200 span' + it_behaves_like 'a trace with AppSec tags' + it_behaves_like 'a trace without AppSec events' + + context 'with an event-triggering user ID' do + let(:appsec_user_id_denylist) { ['blocked-user-id'] } + + it { is_expected.to be_forbidden } + + it_behaves_like 'a GET 403 span' + it_behaves_like 'a trace with AppSec tags' + it_behaves_like 'a trace with AppSec events' + end + end end describe 'POST request' do diff --git a/spec/datadog/kit/identity_spec.rb b/spec/datadog/kit/identity_spec.rb index ea27d35f0e..8d5d11a3be 100644 --- a/spec/datadog/kit/identity_spec.rb +++ b/spec/datadog/kit/identity_spec.rb @@ -230,5 +230,26 @@ expect { described_class.set_user(trace_op, id: 42, foo: 42) }.to raise_error(TypeError) end end + + context 'appsec' do + after { Datadog.configuration.appsec.send(:reset!) } + + context 'when is enabled' do + it 'instruments the user information to appsec' do + Datadog.configuration.appsec.enabled = true + user = OpenStruct.new(id: '42') + expect_any_instance_of(Datadog::AppSec::Instrumentation::Gateway).to receive(:push).with('identity.set_user', user) + described_class.set_user(trace_op, id: '42') + end + end + + context 'when is disabled' do + it 'does not instrument the user information to appsec' do + Datadog.configuration.appsec.enabled = false + expect_any_instance_of(Datadog::AppSec::Instrumentation::Gateway).to_not receive(:push).with('identity.set_user') + described_class.set_user(trace_op, id: '42') + end + end + end end end