From a27ebc265b7d91a0692b31ed985ff42c57f47c60 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Tue, 14 Feb 2023 17:02:52 +0100 Subject: [PATCH 01/13] create new reactive component to listen to set_user events --- .../appsec/contrib/rack/reactive/set_user.rb | 64 ++++++++ .../contrib/rack/reactive/set_user_spec.rb | 150 ++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 lib/datadog/appsec/contrib/rack/reactive/set_user.rb create mode 100644 spec/datadog/appsec/contrib/rack/reactive/set_user_spec.rb diff --git a/lib/datadog/appsec/contrib/rack/reactive/set_user.rb b/lib/datadog/appsec/contrib/rack/reactive/set_user.rb new file mode 100644 index 00000000000..efbc1329300 --- /dev/null +++ b/lib/datadog/appsec/contrib/rack/reactive/set_user.rb @@ -0,0 +1,64 @@ +# typed: ignore +# frozen_string_literal: true + +module Datadog + module AppSec + module Contrib + module Rack + module Reactive + # Dispatch data from Datadog::Kit::Identity.set_user to the WAF context + module SetUser + ADDRESSES = [ + 'usr.id', + ].freeze + private_constant :ADDRESSES + + def self.publish(op, user) + catch(:block) do + op.publish('usr.id', user.id) + + nil + end + end + + def self.subscribe(op, waf_context) + op.subscribe(*ADDRESSES) do |*values| + Datadog.logger.debug { "reacted to #{ADDRESSES.inspect}: #{values.inspect}" } + + user_id = values[0] + + waf_args = { + 'usr.id' => user_id, + } + + waf_timeout = Datadog::AppSec.settings.waf_timeout + result = waf_context.run(waf_args, waf_timeout) + + Datadog.logger.debug { "WAF TIMEOUT: #{result.inspect}" } if result.timeout + + case result.status + when :match + Datadog.logger.debug { "WAF: #{result.inspect}" } + + block = result.actions.include?('block') + + yield [result, block] + + throw(:block, [result, true]) if block + when :ok + Datadog.logger.debug { "WAF OK: #{result.inspect}" } + when :invalid_call + Datadog.logger.debug { "WAF CALL ERROR: #{result.inspect}" } + when :invalid_rule, :invalid_flow, :no_rule + Datadog.logger.debug { "WAF RULE ERROR: #{result.inspect}" } + else + Datadog.logger.debug { "WAF UNKNOWN: #{result.status.inspect} #{result.inspect}" } + end + end + end + end + end + end + end + end +end diff --git a/spec/datadog/appsec/contrib/rack/reactive/set_user_spec.rb b/spec/datadog/appsec/contrib/rack/reactive/set_user_spec.rb new file mode 100644 index 00000000000..8e4ab4e23cc --- /dev/null +++ b/spec/datadog/appsec/contrib/rack/reactive/set_user_spec.rb @@ -0,0 +1,150 @@ +# typed: ignore +# frozen_string_literal: true + +require 'datadog/appsec/spec_helper' +require 'datadog/appsec/reactive/operation' +require 'datadog/appsec/contrib/rack/reactive/set_user' +require 'rack' + +RSpec.describe Datadog::AppSec::Contrib::Rack::Reactive::SetUser do + let(:operation) { Datadog::AppSec::Reactive::Operation.new('test') } + let(:user) { double(:user, id: 1) } + + describe '.publish' do + it 'propagates request body attributes to the operation' do + expect(operation).to receive(:publish).with('usr.id', 1) + + described_class.publish(operation, user) + end + end + + describe '.subscribe' do + let(:waf_context) { double(:waf_context) } + + context 'not all addresses have been published' do + it 'does not call the waf context' do + expect(operation).to receive(:subscribe).with('usr.id').and_call_original + expect(waf_context).to_not receive(:run) + described_class.subscribe(operation, waf_context) + end + end + + context 'all addresses have been published' do + it 'does call the waf context with the right arguments' do + expect(operation).to receive(:subscribe).and_call_original + + expected_waf_arguments = { 'usr.id' => 1 } + + waf_result = double(:waf_result, status: :ok, timeout: false) + expect(waf_context).to receive(:run).with( + expected_waf_arguments, + Datadog::AppSec.settings.waf_timeout + ).and_return(waf_result) + described_class.subscribe(operation, waf_context) + result = described_class.publish(operation, user) + expect(result).to be_nil + end + end + + context 'waf result is a match' do + it 'yields result and no blocking action' do + expect(operation).to receive(:subscribe).and_call_original + + waf_result = double(:waf_result, status: :match, timeout: false, actions: ['']) + expect(waf_context).to receive(:run).and_return(waf_result) + described_class.subscribe(operation, waf_context) do |result, block| + expect(result).to eq(waf_result) + expect(block).to eq(false) + end + result = described_class.publish(operation, user) + expect(result).to be_nil + end + + it 'yields result and blocking action. The publish method catches the resul as well' do + expect(operation).to receive(:subscribe).and_call_original + + waf_result = double(:waf_result, status: :match, timeout: false, actions: ['block']) + expect(waf_context).to receive(:run).and_return(waf_result) + described_class.subscribe(operation, waf_context) do |result, block| + expect(result).to eq(waf_result) + expect(block).to eq(true) + end + publish_result, publish_block = described_class.publish(operation, user) + expect(publish_result).to eq(waf_result) + expect(publish_block).to eq(true) + end + end + + context 'waf result is ok' do + it 'does not yield' do + expect(operation).to receive(:subscribe).and_call_original + + waf_result = double(:waf_result, status: :ok, timeout: false) + expect(waf_context).to receive(:run).and_return(waf_result) + expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control + result = described_class.publish(operation, user) + expect(result).to be_nil + end + end + + context 'waf result is invalid_call' do + it 'does not yield' do + expect(operation).to receive(:subscribe).and_call_original + + waf_result = double(:waf_result, status: :invalid_call, timeout: false) + expect(waf_context).to receive(:run).and_return(waf_result) + expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control + result = described_class.publish(operation, user) + expect(result).to be_nil + end + end + + context 'waf result is invalid_rule' do + it 'does not yield' do + expect(operation).to receive(:subscribe).and_call_original + + waf_result = double(:waf_result, status: :invalid_rule, timeout: false) + expect(waf_context).to receive(:run).and_return(waf_result) + expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control + result = described_class.publish(operation, user) + expect(result).to be_nil + end + end + + context 'waf result is invalid_flow' do + it 'does not yield' do + expect(operation).to receive(:subscribe).and_call_original + + waf_result = double(:waf_result, status: :invalid_flow, timeout: false) + expect(waf_context).to receive(:run).and_return(waf_result) + expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control + result = described_class.publish(operation, user) + expect(result).to be_nil + end + end + + context 'waf result is no_rule' do + it 'does not yield' do + expect(operation).to receive(:subscribe).and_call_original + + waf_result = double(:waf_result, status: :no_rule, timeout: false) + expect(waf_context).to receive(:run).and_return(waf_result) + expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control + result = described_class.publish(operation, user) + expect(result).to be_nil + end + end + + context 'waf result is unknown' do + it 'does not yield' do + expect(operation).to receive(:subscribe).and_call_original + + waf_result = double(:waf_result, status: :foo, timeout: false) + expect(waf_context).to receive(:run).and_return(waf_result) + expect { |b| described_class.subscribe(operation, waf_context, &b) }.not_to yield_control + result = described_class.publish(operation, user) + expect(result).to be_nil + end + end + end +end From ccee5a03ca839679c12ef663fd42d4f32b52f293 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Tue, 14 Feb 2023 17:04:34 +0100 Subject: [PATCH 02/13] store waf context on the local thread. Make sure to reset it at the end of the Request lifecycle --- .../appsec/contrib/rack/request_middleware.rb | 3 +- lib/datadog/appsec/processor.rb | 21 +++++++++ spec/datadog/appsec/processor_spec.rb | 44 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index 7d200378bc4..5181ab0b10b 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -33,6 +33,7 @@ def call(env) context = processor.new_context env['datadog.waf.context'] = context + Datadog::AppSec::Processor.current_context = context request = ::Rack::Request.new(env) @@ -68,7 +69,7 @@ def call(env) ensure if context add_waf_runtime_tags(active_trace, context) - context.finalize + Datadog::AppSec::Processor.reset_current_context end end diff --git a/lib/datadog/appsec/processor.rb b/lib/datadog/appsec/processor.rb index b2051b07a86..055328eea8c 100644 --- a/lib/datadog/appsec/processor.rb +++ b/lib/datadog/appsec/processor.rb @@ -39,6 +39,27 @@ def finalize end end + class InvalidContextError < StandardError; end + + class << self + def current_context + Thread.current[:datadog_current_waf_context] + end + + def current_context=(context) + unless context.instance_of?(Context) + raise InvalidContextError, + "The context provide: #{context.inspect} is not a Datadog::AppSec::Processor::Context" + end + + Thread.current[:datadog_current_waf_context] = context + end + + def reset_current_context + Thread.current[:datadog_current_waf_context] = nil + end + end + attr_reader :ruleset_info, :addresses def initialize diff --git a/spec/datadog/appsec/processor_spec.rb b/spec/datadog/appsec/processor_spec.rb index 03991bd1fba..80ed664e48f 100644 --- a/spec/datadog/appsec/processor_spec.rb +++ b/spec/datadog/appsec/processor_spec.rb @@ -50,6 +50,50 @@ expect(described_class.require_libddwaf).to be true end + + describe '.current_context' do + it 'return nil if not set earlier' do + expect(described_class.current_context).to be_nil + end + + it 'return the previously set current context' do + processor = described_class.new + context = processor.new_context + + described_class.current_context = context + + expect(described_class.current_context).to eq(context) + + context.finalize + processor.finalize + described_class.reset_current_context + end + + describe '.current_context=' do + it 'raises InvalidContextError when trying to setup current conetxt to a non Context instance' do + expect do + described_class.current_context = 'foo' + end.to raise_error(described_class::InvalidContextError) + end + end + + describe '.reset_current_context' do + it 'sets current_context to nil' do + processor = described_class.new + context = processor.new_context + + described_class.current_context = context + + expect(described_class.current_context).to eq(context) + + described_class.reset_current_context + expect(described_class.current_context).to be_nil + + context.finalize + processor.finalize + end + end + end end describe '#load_libddwaf' do From b6130cf73448f5aefc717214941c06d22611f8e3 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Tue, 14 Feb 2023 18:11:35 +0100 Subject: [PATCH 03/13] refactor contrib gateway watch method and split each subscribition into a separate method --- .../appsec/contrib/rack/gateway/watcher.rb | 218 +++++++++--------- .../appsec/contrib/rails/gateway/watcher.rb | 76 +++--- .../appsec/contrib/sinatra/gateway/watcher.rb | 145 ++++++------ 3 files changed, 230 insertions(+), 209 deletions(-) diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index 14765e7357b..f2f1751e334 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -14,140 +14,148 @@ module Rack module Gateway # Watcher for Rack gateway events module Watcher - # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/MethodLength - # rubocop:disable Metrics/CyclomaticComplexity - # rubocop:disable Metrics/PerceivedComplexity - def self.watch - Instrumentation.gateway.watch('rack.request', :appsec) do |stack, request| - block = false - event = nil - waf_context = request.env['datadog.waf.context'] - - AppSec::Reactive::Operation.new('rack.request') do |op| - trace = active_trace - span = active_span - - Rack::Reactive::Request.subscribe(op, waf_context) do |result, _block| - if result.status == :match - # TODO: should this hash be an Event instance instead? - event = { - waf_result: result, - trace: trace, - span: span, - request: request, - actions: result.actions - } - - span.set_tag('appsec.event', 'true') if span - - waf_context.events << event + class << self + def watch + gateway = Instrumentation.gateway + + watch_request(gateway) + watch_response(gateway) + watch_request_body(gateway) + end + + def watch_request(gateway = Instrumentation.gateway) + gateway.watch('rack.request', :appsec) do |stack, request| + block = false + event = nil + waf_context = request.env['datadog.waf.context'] + + AppSec::Reactive::Operation.new('rack.request') do |op| + trace = active_trace + span = active_span + + Rack::Reactive::Request.subscribe(op, waf_context) do |result, _block| + if result.status == :match + # TODO: should this hash be an Event instance instead? + event = { + waf_result: result, + trace: trace, + span: span, + request: request, + actions: result.actions + } + + span.set_tag('appsec.event', 'true') if span + + waf_context.events << event + end end + + _result, block = Rack::Reactive::Request.publish(op, request) end - _result, block = Rack::Reactive::Request.publish(op, request) - end + next [nil, [[:block, event]]] if block - next [nil, [[:block, event]]] if block + ret, res = stack.call(request) - ret, res = stack.call(request) + if event + res ||= [] + res << [:monitor, event] + end - if event - res ||= [] - res << [:monitor, event] + [ret, res] end - - [ret, res] end - Instrumentation.gateway.watch('rack.response', :appsec) do |stack, response| - block = false - event = nil - waf_context = response.instance_eval { @waf_context } - - AppSec::Reactive::Operation.new('rack.response') do |op| - trace = active_trace - span = active_span - - Rack::Reactive::Response.subscribe(op, waf_context) do |result, _block| - if result.status == :match - # TODO: should this hash be an Event instance instead? - event = { - waf_result: result, - trace: trace, - span: span, - response: response, - actions: result.actions - } - - span.set_tag('appsec.event', 'true') if span - - waf_context.events << event + def watch_response(gateway = Instrumentation.gateway) + gateway.watch('rack.response', :appsec) do |stack, response| + block = false + event = nil + waf_context = response.instance_eval { @waf_context } + + AppSec::Reactive::Operation.new('rack.response') do |op| + trace = active_trace + span = active_span + + Rack::Reactive::Response.subscribe(op, waf_context) do |result, _block| + if result.status == :match + # TODO: should this hash be an Event instance instead? + event = { + waf_result: result, + trace: trace, + span: span, + response: response, + actions: result.actions + } + + span.set_tag('appsec.event', 'true') if span + + waf_context.events << event + end end + + _result, block = Rack::Reactive::Response.publish(op, response) end - _result, block = Rack::Reactive::Response.publish(op, response) - end + next [nil, [[:block, event]]] if block - next [nil, [[:block, event]]] if block + ret, res = stack.call(response) - ret, res = stack.call(response) + if event + res ||= [] + res << [:monitor, event] + end - if event - res ||= [] - res << [:monitor, event] + [ret, res] end - - [ret, res] end - Instrumentation.gateway.watch('rack.request.body', :appsec) do |stack, request| - block = false - event = nil - waf_context = request.env['datadog.waf.context'] - - AppSec::Reactive::Operation.new('rack.request.body') do |op| - trace = active_trace - span = active_span - - Rack::Reactive::RequestBody.subscribe(op, waf_context) do |result, _block| - if result.status == :match - # TODO: should this hash be an Event instance instead? - event = { - waf_result: result, - trace: trace, - span: span, - request: request, - actions: result.actions - } - - span.set_tag('appsec.event', 'true') if span - - waf_context.events << event + def watch_request_body(gateway = Instrumentation.gateway) + gateway.watch('rack.request.body', :appsec) do |stack, request| + block = false + event = nil + waf_context = request.env['datadog.waf.context'] + + AppSec::Reactive::Operation.new('rack.request.body') do |op| + trace = active_trace + span = active_span + + Rack::Reactive::RequestBody.subscribe(op, waf_context) do |result, _block| + if result.status == :match + # TODO: should this hash be an Event instance instead? + event = { + waf_result: result, + trace: trace, + span: span, + request: request, + actions: result.actions + } + + span.set_tag('appsec.event', 'true') if span + + waf_context.events << event + end end + + _result, block = Rack::Reactive::RequestBody.publish(op, request) end - _result, block = Rack::Reactive::RequestBody.publish(op, request) - end + next [nil, [[:block, event]]] if block - next [nil, [[:block, event]]] if block + ret, res = stack.call(request) - ret, res = stack.call(request) + if event + res ||= [] + res << [:monitor, event] + end - if event - res ||= [] - res << [:monitor, event] + [ret, res] end + end + [ret, res] end - end - # rubocop:enable Metrics/CyclomaticComplexity - # rubocop:enable Metrics/PerceivedComplexity - # rubocop:enable Metrics/MethodLength - # rubocop:enable Metrics/AbcSize - class << self private def active_trace diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index a43e8709e80..7248a70bb58 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -12,50 +12,56 @@ module Rails module Gateway # Watcher for Rails gateway events module Watcher - def self.watch - Instrumentation.gateway.watch('rails.request.action', :appsec) do |stack, request| - block = false - event = nil - waf_context = request.env['datadog.waf.context'] - - AppSec::Reactive::Operation.new('rails.request.action') do |op| - trace = active_trace - span = active_span - - Rails::Reactive::Action.subscribe(op, waf_context) do |result, _block| - if result.status == :match - # TODO: should this hash be an Event instance instead? - event = { - waf_result: result, - trace: trace, - span: span, - request: request, - actions: result.actions - } - - span.set_tag('appsec.event', 'true') if span - - waf_context.events << event + class << self + def watch + gateway = Instrumentation.gateway + + watch_request_action(gateway) + end + + def watch_request_action(gateway = Instrumentation.gateway) + gateway.watch('rails.request.action', :appsec) do |stack, request| + block = false + event = nil + waf_context = request.env['datadog.waf.context'] + + AppSec::Reactive::Operation.new('rails.request.action') do |op| + trace = active_trace + span = active_span + + Rails::Reactive::Action.subscribe(op, waf_context) do |result, _block| + if result.status == :match + # TODO: should this hash be an Event instance instead? + event = { + waf_result: result, + trace: trace, + span: span, + request: request, + actions: result.actions + } + + span.set_tag('appsec.event', 'true') if span + + waf_context.events << event + end end + + _result, block = Rails::Reactive::Action.publish(op, request) end - _result, block = Rails::Reactive::Action.publish(op, request) - end + next [nil, [[:block, event]]] if block - next [nil, [[:block, event]]] if block + ret, res = stack.call(request) - ret, res = stack.call(request) + if event + res ||= [] + res << [:monitor, event] + end - if event - res ||= [] - res << [:monitor, event] + [ret, res] end - - [ret, res] end - end - class << self private def active_trace diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index 87af97c8073..eb8266373f8 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -13,93 +13,100 @@ module Sinatra module Gateway # Watcher for Sinatra gateway events module Watcher - # rubocop:disable Metrics/MethodLength - def self.watch - Instrumentation.gateway.watch('sinatra.request.dispatch', :appsec) do |stack, request| - block = false - event = nil - waf_context = request.env['datadog.waf.context'] - - AppSec::Reactive::Operation.new('sinatra.request.dispatch') do |op| - trace = active_trace - span = active_span - - Rack::Reactive::RequestBody.subscribe(op, waf_context) do |result, _block| - if result.status == :match - # TODO: should this hash be an Event instance instead? - event = { - waf_result: result, - trace: trace, - span: span, - request: request, - actions: result.actions - } - - span.set_tag('appsec.event', 'true') if span - - waf_context.events << event + class << self + def watch + gateway = Instrumentation.gateway + + watch_request_dispatch(gateway) + watch_request_routed(gateway) + end + + def watch_request_dispatch(gateway = Instrumentation.gateway) + gateway.watch('sinatra.request.dispatch', :appsec) do |stack, request| + block = false + event = nil + waf_context = request.env['datadog.waf.context'] + + AppSec::Reactive::Operation.new('sinatra.request.dispatch') do |op| + trace = active_trace + span = active_span + + Rack::Reactive::RequestBody.subscribe(op, waf_context) do |result, _block| + if result.status == :match + # TODO: should this hash be an Event instance instead? + event = { + waf_result: result, + trace: trace, + span: span, + request: request, + actions: result.actions + } + + span.set_tag('appsec.event', 'true') if span + + waf_context.events << event + end end + + _result, block = Rack::Reactive::RequestBody.publish(op, request) end - _result, block = Rack::Reactive::RequestBody.publish(op, request) - end + next [nil, [[:block, event]]] if block - next [nil, [[:block, event]]] if block + ret, res = stack.call(request) - ret, res = stack.call(request) + if event + res ||= [] + res << [:monitor, event] + end - if event - res ||= [] - res << [:monitor, event] + [ret, res] end - - [ret, res] end - Instrumentation.gateway.watch('sinatra.request.routed', :appsec) do |stack, (request, route_params)| - block = false - event = nil - waf_context = request.env['datadog.waf.context'] - - AppSec::Reactive::Operation.new('sinatra.request.routed') do |op| - trace = active_trace - span = active_span - - Sinatra::Reactive::Routed.subscribe(op, waf_context) do |result, _block| - if result.status == :match - # TODO: should this hash be an Event instance instead? - event = { - waf_result: result, - trace: trace, - span: span, - request: request, - actions: result.actions - } - - span.set_tag('appsec.event', 'true') if span - - waf_context.events << event + def watch_request_routed(gateway = Instrumentation.gateway) + gateway.watch('sinatra.request.routed', :appsec) do |stack, (request, route_params)| + block = false + event = nil + waf_context = request.env['datadog.waf.context'] + + AppSec::Reactive::Operation.new('sinatra.request.routed') do |op| + trace = active_trace + span = active_span + + Sinatra::Reactive::Routed.subscribe(op, waf_context) do |result, _block| + if result.status == :match + # TODO: should this hash be an Event instance instead? + event = { + waf_result: result, + trace: trace, + span: span, + request: request, + actions: result.actions + } + + span.set_tag('appsec.event', 'true') if span + + waf_context.events << event + end end + + _result, block = Sinatra::Reactive::Routed.publish(op, [request, route_params]) end - _result, block = Sinatra::Reactive::Routed.publish(op, [request, route_params]) - end + next [nil, [[:block, event]]] if block - next [nil, [[:block, event]]] if block + ret, res = stack.call(request) - ret, res = stack.call(request) + if event + res ||= [] + res << [:monitor, event] + end - if event - res ||= [] - res << [:monitor, event] + [ret, res] end - - [ret, res] end - end - # rubocop:enable Metrics/MethodLength - class << self private def active_trace From fd15c5cf719ae5b388b2f397de243b9351ef7755 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Tue, 14 Feb 2023 18:12:04 +0100 Subject: [PATCH 04/13] rack watch for user_id events --- .../appsec/contrib/rack/gateway/watcher.rb | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index f2f1751e334..366d965b92d 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -21,6 +21,7 @@ def watch watch_request(gateway) watch_response(gateway) watch_request_body(gateway) + watch_user_id(gateway) end def watch_request(gateway = Instrumentation.gateway) @@ -152,8 +153,47 @@ def watch_request_body(gateway = Instrumentation.gateway) end end + def watch_user_id(gateway = Instrumentation.gateway) + gateway.watch('identity.set_user', :appsec) do |stack, user| + block = false + event = nil + waf_context = Datadog::AppSec::Processor.current_context + + AppSec::Reactive::Operation.new('identity.set_user') do |op| + trace = active_trace + span = active_span + + Rack::Reactive::SetUser.subscribe(op, waf_context) do |result, _block| + if result.status == :match + # TODO: should this hash be an Event instance instead? + event = { + waf_result: result, + trace: trace, + span: span, + user: user, + actions: result.actions + } + + span.set_tag('appsec.event', 'true') if span - [ret, res] + waf_context.events << event + end + end + + _result, block = Rack::Reactive::SetUser.publish(op, user) + end + + next [nil, [[:block, event]]] if block + + ret, res = stack.call(user) + + if event + res ||= [] + res << [:monitor, event] + end + + [ret, res] + end end private From cb552deb5983685cf4e9c05e961c3f28156517e9 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Tue, 14 Feb 2023 18:59:45 +0100 Subject: [PATCH 05/13] update rbs signature --- .../appsec/contrib/rack/gateway/watcher.rbs | 8 ++++++++ .../appsec/contrib/rack/reactive/set_user.rbs | 17 +++++++++++++++++ .../appsec/contrib/rails/gateway/watcher.rbs | 2 ++ .../appsec/contrib/sinatra/gateway/watcher.rbs | 4 ++++ sig/datadog/appsec/processor.rbs | 7 +++++++ 5 files changed, 38 insertions(+) create mode 100644 sig/datadog/appsec/contrib/rack/reactive/set_user.rbs diff --git a/sig/datadog/appsec/contrib/rack/gateway/watcher.rbs b/sig/datadog/appsec/contrib/rack/gateway/watcher.rbs index 8c7e0a9c5ac..be6702fbebe 100644 --- a/sig/datadog/appsec/contrib/rack/gateway/watcher.rbs +++ b/sig/datadog/appsec/contrib/rack/gateway/watcher.rbs @@ -6,6 +6,14 @@ module Datadog module Watcher def self.watch: () -> untyped + def self.watch_request: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped + + def self.watch_response: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped + + def self.watch_request_body: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped + + def self.watch_user_id: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped + private def self.active_trace: () -> (nil | untyped) diff --git a/sig/datadog/appsec/contrib/rack/reactive/set_user.rbs b/sig/datadog/appsec/contrib/rack/reactive/set_user.rbs new file mode 100644 index 00000000000..bc6155db19a --- /dev/null +++ b/sig/datadog/appsec/contrib/rack/reactive/set_user.rbs @@ -0,0 +1,17 @@ +module Datadog + module AppSec + module Contrib + module Rack + module Reactive + module SetUser + ADDRESSES: ::Array[::String] + + def self.publish: (untyped op, untyped user) -> untyped + + def self.subscribe: (untyped op, untyped waf_context) { (untyped) -> untyped } -> untyped + end + end + end + end + end +end diff --git a/sig/datadog/appsec/contrib/rails/gateway/watcher.rbs b/sig/datadog/appsec/contrib/rails/gateway/watcher.rbs index 38a1c6701b9..11c37538354 100644 --- a/sig/datadog/appsec/contrib/rails/gateway/watcher.rbs +++ b/sig/datadog/appsec/contrib/rails/gateway/watcher.rbs @@ -6,6 +6,8 @@ module Datadog module Watcher def self.watch: () -> untyped + def self.watch_request_action: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped + private def self.active_trace: () -> (nil | untyped) diff --git a/sig/datadog/appsec/contrib/sinatra/gateway/watcher.rbs b/sig/datadog/appsec/contrib/sinatra/gateway/watcher.rbs index 164f9de6543..9441f63c7e6 100644 --- a/sig/datadog/appsec/contrib/sinatra/gateway/watcher.rbs +++ b/sig/datadog/appsec/contrib/sinatra/gateway/watcher.rbs @@ -6,6 +6,10 @@ module Datadog module Watcher def self.watch: () -> untyped + def self.watch_request_dispatch: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped + + def self.watch_request_routed: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped + private def self.active_trace: () -> (nil | untyped) diff --git a/sig/datadog/appsec/processor.rbs b/sig/datadog/appsec/processor.rbs index d226476b322..13ee4b2a3f3 100644 --- a/sig/datadog/appsec/processor.rbs +++ b/sig/datadog/appsec/processor.rbs @@ -17,6 +17,13 @@ module Datadog def finalize: () -> void end + class InvalidContextError < StandardError + end + + def self.current_context: () -> Context? + def self.current_context=: (untyped context) -> void + def self.reset_current_context: () -> void + attr_reader ruleset_info: untyped attr_reader addresses: untyped From 96394a108d30bce9af31db77c9fc13b031a9f2a8 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Fri, 17 Feb 2023 10:30:51 +0100 Subject: [PATCH 06/13] rename current_context to active_context --- .../appsec/contrib/rack/request_middleware.rb | 5 ++-- lib/datadog/appsec/processor.rb | 10 +++---- spec/datadog/appsec/processor_spec.rb | 30 +++++++++---------- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index 5181ab0b10b..066ab03e8a4 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -33,7 +33,7 @@ def call(env) context = processor.new_context env['datadog.waf.context'] = context - Datadog::AppSec::Processor.current_context = context + Datadog::AppSec::Processor.active_context = context request = ::Rack::Request.new(env) @@ -69,7 +69,8 @@ def call(env) ensure if context add_waf_runtime_tags(active_trace, context) - Datadog::AppSec::Processor.reset_current_context + Datadog::AppSec::Processor.reset_active_context + context.finalize end end diff --git a/lib/datadog/appsec/processor.rb b/lib/datadog/appsec/processor.rb index 055328eea8c..4e0ad5e18b3 100644 --- a/lib/datadog/appsec/processor.rb +++ b/lib/datadog/appsec/processor.rb @@ -39,23 +39,21 @@ def finalize end end - class InvalidContextError < StandardError; end - class << self - def current_context + def active_context Thread.current[:datadog_current_waf_context] end - def current_context=(context) + def active_context=(context) unless context.instance_of?(Context) - raise InvalidContextError, + raise ArgumentError, "The context provide: #{context.inspect} is not a Datadog::AppSec::Processor::Context" end Thread.current[:datadog_current_waf_context] = context end - def reset_current_context + def reset_active_context Thread.current[:datadog_current_waf_context] = nil end end diff --git a/spec/datadog/appsec/processor_spec.rb b/spec/datadog/appsec/processor_spec.rb index 80ed664e48f..23f8eda25bc 100644 --- a/spec/datadog/appsec/processor_spec.rb +++ b/spec/datadog/appsec/processor_spec.rb @@ -51,43 +51,43 @@ expect(described_class.require_libddwaf).to be true end - describe '.current_context' do + describe '.active_context' do it 'return nil if not set earlier' do - expect(described_class.current_context).to be_nil + expect(described_class.active_context).to be_nil end it 'return the previously set current context' do processor = described_class.new context = processor.new_context - described_class.current_context = context + described_class.active_context = context - expect(described_class.current_context).to eq(context) + expect(described_class.active_context).to eq(context) context.finalize processor.finalize - described_class.reset_current_context + described_class.reset_active_context end - describe '.current_context=' do - it 'raises InvalidContextError when trying to setup current conetxt to a non Context instance' do + describe '.active_context=' do + it 'raises ArgumentError when trying to setup current conetxt to a non Context instance' do expect do - described_class.current_context = 'foo' - end.to raise_error(described_class::InvalidContextError) + described_class.active_context = 'foo' + end.to raise_error(ArgumentError) end end - describe '.reset_current_context' do - it 'sets current_context to nil' do + describe '.reset_active_context' do + it 'sets active_context to nil' do processor = described_class.new context = processor.new_context - described_class.current_context = context + described_class.active_context = context - expect(described_class.current_context).to eq(context) + expect(described_class.active_context).to eq(context) - described_class.reset_current_context - expect(described_class.current_context).to be_nil + described_class.reset_active_context + expect(described_class.active_context).to be_nil context.finalize processor.finalize From 0ea7be49006a2b0f1cf100acc3bade5370cbaec4 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Fri, 17 Feb 2023 16:13:07 +0100 Subject: [PATCH 07/13] introduce Processor#activate_context and Processor#deactive_context --- .../appsec/contrib/rack/request_middleware.rb | 6 +-- lib/datadog/appsec/processor.rb | 18 +++++++++ sig/datadog/appsec/processor.rbs | 15 +++++--- spec/datadog/appsec/processor_spec.rb | 38 ++++++++++++++++--- 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index 066ab03e8a4..c362aa6a19d 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -31,9 +31,8 @@ def call(env) # TODO: handle exceptions, except for @app.call - context = processor.new_context + context = processor.activate_context env['datadog.waf.context'] = context - Datadog::AppSec::Processor.active_context = context request = ::Rack::Request.new(env) @@ -69,8 +68,7 @@ def call(env) ensure if context add_waf_runtime_tags(active_trace, context) - Datadog::AppSec::Processor.reset_active_context - context.finalize + processor.deactive_context end end diff --git a/lib/datadog/appsec/processor.rb b/lib/datadog/appsec/processor.rb index 4e0ad5e18b3..8207e981756 100644 --- a/lib/datadog/appsec/processor.rb +++ b/lib/datadog/appsec/processor.rb @@ -44,6 +44,8 @@ def active_context Thread.current[:datadog_current_waf_context] end + private + def active_context=(context) unless context.instance_of?(Context) raise ArgumentError, @@ -58,6 +60,8 @@ def reset_active_context end end + class NoActiveContextError < StandardError; end + attr_reader :ruleset_info, :addresses def initialize @@ -82,6 +86,20 @@ def new_context Context.new(self) end + def activate_context + context = new_context + Processor.send(:active_context=, context) + context + end + + def deactivate_context + context = Processor.active_context + raise NoActiveContextError unless context + + Processor.send(:reset_active_context) + context.finalize + end + def update_rule_data(data) @handle.update_rule_data(data) end diff --git a/sig/datadog/appsec/processor.rbs b/sig/datadog/appsec/processor.rbs index 13ee4b2a3f3..e2348c912e8 100644 --- a/sig/datadog/appsec/processor.rbs +++ b/sig/datadog/appsec/processor.rbs @@ -17,12 +17,15 @@ module Datadog def finalize: () -> void end - class InvalidContextError < StandardError - end + def self.active_context: () -> Context + + private - def self.current_context: () -> Context? - def self.current_context=: (untyped context) -> void - def self.reset_current_context: () -> void + def self.active_context=: (untyped context) -> untyped + def self.reset_active_context: () -> untyped + + class NoActiveContextError < StandardError + end attr_reader ruleset_info: untyped attr_reader addresses: untyped @@ -34,6 +37,8 @@ module Datadog def initialize: () -> void def ready?: () -> bool def new_context: () -> Context + def activate_context: () -> Context + def deactivate_context: () -> void def update_rule_data: (untyped data) -> untyped def toggle_rules: (untyped map) -> untyped def finalize: () -> void diff --git a/spec/datadog/appsec/processor_spec.rb b/spec/datadog/appsec/processor_spec.rb index 23f8eda25bc..08beed9f9a5 100644 --- a/spec/datadog/appsec/processor_spec.rb +++ b/spec/datadog/appsec/processor_spec.rb @@ -20,6 +20,10 @@ allow(Datadog).to receive(:logger).and_return(logger) end + after do + described_class.send(:reset_active_context) + end + context 'self' do it 'detects if the WAF is unavailable' do hide_const('Datadog::AppSec::WAF') @@ -60,19 +64,19 @@ processor = described_class.new context = processor.new_context - described_class.active_context = context + described_class.send(:active_context=, context) expect(described_class.active_context).to eq(context) context.finalize processor.finalize - described_class.reset_active_context + described_class.send(:reset_active_context) end describe '.active_context=' do it 'raises ArgumentError when trying to setup current conetxt to a non Context instance' do expect do - described_class.active_context = 'foo' + described_class.send(:active_context=, 'foo') end.to raise_error(ArgumentError) end end @@ -82,11 +86,11 @@ processor = described_class.new context = processor.new_context - described_class.active_context = context + described_class.send(:active_context=, context) expect(described_class.active_context).to eq(context) - described_class.reset_active_context + described_class.send(:reset_active_context) expect(described_class.active_context).to be_nil context.finalize @@ -520,4 +524,28 @@ end end end + + describe '#active_context' do + it 'creates a new context and store in the class .active_context variable' do + context = described_class.new.activate_context + expect(context).to eq(described_class.active_context) + end + end + + describe '#deactivate_context' do + it 'finalize the active context and reset the class .active_context variable' do + handler = described_class.new + context = handler.activate_context + + expect(context).to receive(:finalize) + handler.deactivate_context + expect(described_class.active_context).to be_nil + end + + context 'without an active_context' do + it 'raises NoActiveContextError' do + expect { described_class.new.deactivate_context }.to raise_error(described_class::NoActiveContextError) + end + end + end end From 00e620540568e343891aeea1462f19f84b4739b0 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Fri, 17 Feb 2023 16:55:23 +0100 Subject: [PATCH 08/13] create Appsec::Monitor to handle internal appsec events agnostic to integrations. Move Rack::Reactive::SetUser to Monitor::Reactive::SetUser --- Steepfile | 1 + lib/datadog/appsec.rb | 6 ++ .../appsec/contrib/rack/gateway/watcher.rb | 44 ---------- .../appsec/contrib/rack/reactive/set_user.rb | 64 -------------- .../appsec/contrib/rack/request_middleware.rb | 2 +- lib/datadog/appsec/monitor.rb | 12 +++ lib/datadog/appsec/monitor/gateway/watcher.rb | 86 +++++++++++++++++++ .../appsec/monitor/reactive/set_user.rb | 62 +++++++++++++ .../appsec/contrib/rack/gateway/watcher.rbs | 2 - .../appsec/contrib/rack/reactive/set_user.rbs | 17 ---- .../appsec/monitor/gateway/watcher.rbs | 19 ++++ .../appsec/monitor/reactive/set_user.rbs | 15 ++++ .../reactive/set_user_spec.rb | 5 +- 13 files changed, 204 insertions(+), 131 deletions(-) delete mode 100644 lib/datadog/appsec/contrib/rack/reactive/set_user.rb create mode 100644 lib/datadog/appsec/monitor.rb create mode 100644 lib/datadog/appsec/monitor/gateway/watcher.rb create mode 100644 lib/datadog/appsec/monitor/reactive/set_user.rb delete mode 100644 sig/datadog/appsec/contrib/rack/reactive/set_user.rbs create mode 100644 sig/datadog/appsec/monitor/gateway/watcher.rbs create mode 100644 sig/datadog/appsec/monitor/reactive/set_user.rbs rename spec/datadog/appsec/{contrib/rack => monitor}/reactive/set_user_spec.rb (97%) diff --git a/Steepfile b/Steepfile index e6613ec5db6..ee8895b179b 100644 --- a/Steepfile +++ b/Steepfile @@ -7,6 +7,7 @@ target :appsec do # check 'lib/datadog/kit' ignore 'lib/datadog/appsec/contrib' + ignore 'lib/datadog/appsec/monitor' library 'pathname', 'set' library 'cgi' diff --git a/lib/datadog/appsec.rb b/lib/datadog/appsec.rb index e9c75d96298..8c7299856ad 100644 --- a/lib/datadog/appsec.rb +++ b/lib/datadog/appsec.rb @@ -40,4 +40,10 @@ def self.writer require_relative 'appsec/contrib/sinatra/integration' require_relative 'appsec/contrib/rails/integration' + require_relative 'appsec/autoload' + +# Internal appsec events monitor +require_relative 'appsec/monitor' +Datadog::AppSec::Monitor::Gateway::Watcher.watch + diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index 366d965b92d..0c7ecab7c60 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -21,7 +21,6 @@ def watch watch_request(gateway) watch_response(gateway) watch_request_body(gateway) - watch_user_id(gateway) end def watch_request(gateway = Instrumentation.gateway) @@ -153,49 +152,6 @@ def watch_request_body(gateway = Instrumentation.gateway) end end - def watch_user_id(gateway = Instrumentation.gateway) - gateway.watch('identity.set_user', :appsec) do |stack, user| - block = false - event = nil - waf_context = Datadog::AppSec::Processor.current_context - - AppSec::Reactive::Operation.new('identity.set_user') do |op| - trace = active_trace - span = active_span - - Rack::Reactive::SetUser.subscribe(op, waf_context) do |result, _block| - if result.status == :match - # TODO: should this hash be an Event instance instead? - event = { - waf_result: result, - trace: trace, - span: span, - user: user, - actions: result.actions - } - - span.set_tag('appsec.event', 'true') if span - - waf_context.events << event - end - end - - _result, block = Rack::Reactive::SetUser.publish(op, user) - end - - next [nil, [[:block, event]]] if block - - ret, res = stack.call(user) - - if event - res ||= [] - res << [:monitor, event] - end - - [ret, res] - end - end - private def active_trace diff --git a/lib/datadog/appsec/contrib/rack/reactive/set_user.rb b/lib/datadog/appsec/contrib/rack/reactive/set_user.rb deleted file mode 100644 index efbc1329300..00000000000 --- a/lib/datadog/appsec/contrib/rack/reactive/set_user.rb +++ /dev/null @@ -1,64 +0,0 @@ -# typed: ignore -# frozen_string_literal: true - -module Datadog - module AppSec - module Contrib - module Rack - module Reactive - # Dispatch data from Datadog::Kit::Identity.set_user to the WAF context - module SetUser - ADDRESSES = [ - 'usr.id', - ].freeze - private_constant :ADDRESSES - - def self.publish(op, user) - catch(:block) do - op.publish('usr.id', user.id) - - nil - end - end - - def self.subscribe(op, waf_context) - op.subscribe(*ADDRESSES) do |*values| - Datadog.logger.debug { "reacted to #{ADDRESSES.inspect}: #{values.inspect}" } - - user_id = values[0] - - waf_args = { - 'usr.id' => user_id, - } - - waf_timeout = Datadog::AppSec.settings.waf_timeout - result = waf_context.run(waf_args, waf_timeout) - - Datadog.logger.debug { "WAF TIMEOUT: #{result.inspect}" } if result.timeout - - case result.status - when :match - Datadog.logger.debug { "WAF: #{result.inspect}" } - - block = result.actions.include?('block') - - yield [result, block] - - throw(:block, [result, true]) if block - when :ok - Datadog.logger.debug { "WAF OK: #{result.inspect}" } - when :invalid_call - Datadog.logger.debug { "WAF CALL ERROR: #{result.inspect}" } - when :invalid_rule, :invalid_flow, :no_rule - Datadog.logger.debug { "WAF RULE ERROR: #{result.inspect}" } - else - Datadog.logger.debug { "WAF UNKNOWN: #{result.status.inspect} #{result.inspect}" } - end - end - end - end - end - end - end - end -end diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index c362aa6a19d..e49c9cd48b6 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -68,7 +68,7 @@ def call(env) ensure if context add_waf_runtime_tags(active_trace, context) - processor.deactive_context + processor.deactivate_context end end diff --git a/lib/datadog/appsec/monitor.rb b/lib/datadog/appsec/monitor.rb new file mode 100644 index 00000000000..b944ab20e5c --- /dev/null +++ b/lib/datadog/appsec/monitor.rb @@ -0,0 +1,12 @@ +# typed: ignore +# frozen_string_literal: true + +require_relative 'monitor/gateway/watcher' + +module Datadog + module AppSec + # Monitor for internal AppSec Events + module Monitor + end + end +end diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb new file mode 100644 index 00000000000..917dee693c4 --- /dev/null +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -0,0 +1,86 @@ +# typed: ignore +# frozen_string_literal: true + +require_relative '../../instrumentation/gateway' +require_relative '../../reactive/operation' +require_relative '../reactive/set_user' + +module Datadog + module AppSec + module Monitor + module Gateway + # Watcher for Apssec internal events + module Watcher + class << self + def watch + gateway = Instrumentation.gateway + + watch_user_id(gateway) + end + + def watch_user_id(gateway = Instrumentation.gateway) + gateway.watch('identity.set_user', :appsec) do |stack, user| + block = false + event = nil + waf_context = Datadog::AppSec::Processor.active_context + + AppSec::Reactive::Operation.new('identity.set_user') do |op| + trace = active_trace + span = active_span + + Monitor::Reactive::SetUser.subscribe(op, waf_context) do |result, _block| + if result.status == :match + # TODO: should this hash be an Event instance instead? + event = { + waf_result: result, + trace: trace, + span: span, + user: user, + actions: result.actions + } + + span.set_tag('appsec.event', 'true') if span + + waf_context.events << event + end + end + + _result, block = Monitor::Reactive::SetUser.publish(op, user) + end + + next [nil, [[:block, event]]] if block + + ret, res = stack.call(user) + + if event + res ||= [] + res << [:monitor, event] + end + + [ret, res] + end + end + + private + + def active_trace + # TODO: factor out tracing availability detection + + return unless defined?(Datadog::Tracing) + + Datadog::Tracing.active_trace + end + + def active_span + # TODO: factor out tracing availability detection + + return unless defined?(Datadog::Tracing) + + Datadog::Tracing.active_span + end + end + end + end + end + end +end diff --git a/lib/datadog/appsec/monitor/reactive/set_user.rb b/lib/datadog/appsec/monitor/reactive/set_user.rb new file mode 100644 index 00000000000..3d65cf1e79c --- /dev/null +++ b/lib/datadog/appsec/monitor/reactive/set_user.rb @@ -0,0 +1,62 @@ +# typed: ignore +# frozen_string_literal: true + +module Datadog + module AppSec + module Monitor + module Reactive + # Dispatch data from Datadog::Kit::Identity.set_user to the WAF context + module SetUser + ADDRESSES = [ + 'usr.id', + ].freeze + private_constant :ADDRESSES + + def self.publish(op, user) + catch(:block) do + op.publish('usr.id', user.id) + + nil + end + end + + def self.subscribe(op, waf_context) + op.subscribe(*ADDRESSES) do |*values| + Datadog.logger.debug { "reacted to #{ADDRESSES.inspect}: #{values.inspect}" } + + user_id = values[0] + + waf_args = { + 'usr.id' => user_id, + } + + waf_timeout = Datadog::AppSec.settings.waf_timeout + result = waf_context.run(waf_args, waf_timeout) + + Datadog.logger.debug { "WAF TIMEOUT: #{result.inspect}" } if result.timeout + + case result.status + when :match + Datadog.logger.debug { "WAF: #{result.inspect}" } + + block = result.actions.include?('block') + + yield [result, block] + + throw(:block, [result, true]) if block + when :ok + Datadog.logger.debug { "WAF OK: #{result.inspect}" } + when :invalid_call + Datadog.logger.debug { "WAF CALL ERROR: #{result.inspect}" } + when :invalid_rule, :invalid_flow, :no_rule + Datadog.logger.debug { "WAF RULE ERROR: #{result.inspect}" } + else + Datadog.logger.debug { "WAF UNKNOWN: #{result.status.inspect} #{result.inspect}" } + end + end + end + end + end + end + end +end diff --git a/sig/datadog/appsec/contrib/rack/gateway/watcher.rbs b/sig/datadog/appsec/contrib/rack/gateway/watcher.rbs index be6702fbebe..5a830812e3e 100644 --- a/sig/datadog/appsec/contrib/rack/gateway/watcher.rbs +++ b/sig/datadog/appsec/contrib/rack/gateway/watcher.rbs @@ -12,8 +12,6 @@ module Datadog def self.watch_request_body: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped - def self.watch_user_id: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped - private def self.active_trace: () -> (nil | untyped) diff --git a/sig/datadog/appsec/contrib/rack/reactive/set_user.rbs b/sig/datadog/appsec/contrib/rack/reactive/set_user.rbs deleted file mode 100644 index bc6155db19a..00000000000 --- a/sig/datadog/appsec/contrib/rack/reactive/set_user.rbs +++ /dev/null @@ -1,17 +0,0 @@ -module Datadog - module AppSec - module Contrib - module Rack - module Reactive - module SetUser - ADDRESSES: ::Array[::String] - - def self.publish: (untyped op, untyped user) -> untyped - - def self.subscribe: (untyped op, untyped waf_context) { (untyped) -> untyped } -> untyped - end - end - end - end - end -end diff --git a/sig/datadog/appsec/monitor/gateway/watcher.rbs b/sig/datadog/appsec/monitor/gateway/watcher.rbs new file mode 100644 index 00000000000..8a3f0937960 --- /dev/null +++ b/sig/datadog/appsec/monitor/gateway/watcher.rbs @@ -0,0 +1,19 @@ +module Datadog + module AppSec + module Monitor + module Gateway + module Watcher + def self.watch: () -> untyped + + def self.watch_user_id: (?Datadog::AppSec::Instrumentation::Gateway gateway) -> untyped + + private + + def self.active_trace: () -> (nil | untyped) + + def self.active_span: () -> (nil | untyped) + end + end + end + end +end diff --git a/sig/datadog/appsec/monitor/reactive/set_user.rbs b/sig/datadog/appsec/monitor/reactive/set_user.rbs new file mode 100644 index 00000000000..529d31fe2d2 --- /dev/null +++ b/sig/datadog/appsec/monitor/reactive/set_user.rbs @@ -0,0 +1,15 @@ +module Datadog + module AppSec + module Monitor + module Reactive + module SetUser + ADDRESSES: ::Array[::String] + + def self.publish: (untyped op, untyped user) -> untyped + + def self.subscribe: (untyped op, untyped waf_context) { (untyped) -> untyped } -> untyped + end + end + end + end +end diff --git a/spec/datadog/appsec/contrib/rack/reactive/set_user_spec.rb b/spec/datadog/appsec/monitor/reactive/set_user_spec.rb similarity index 97% rename from spec/datadog/appsec/contrib/rack/reactive/set_user_spec.rb rename to spec/datadog/appsec/monitor/reactive/set_user_spec.rb index 8e4ab4e23cc..11e5381fcb2 100644 --- a/spec/datadog/appsec/contrib/rack/reactive/set_user_spec.rb +++ b/spec/datadog/appsec/monitor/reactive/set_user_spec.rb @@ -3,10 +3,9 @@ require 'datadog/appsec/spec_helper' require 'datadog/appsec/reactive/operation' -require 'datadog/appsec/contrib/rack/reactive/set_user' -require 'rack' +require 'datadog/appsec/monitor/reactive/set_user' -RSpec.describe Datadog::AppSec::Contrib::Rack::Reactive::SetUser do +RSpec.describe Datadog::AppSec::Monitor::Reactive::SetUser do let(:operation) { Datadog::AppSec::Reactive::Operation.new('test') } let(:user) { double(:user, id: 1) } From a9fa5a24aa686fb98bf7c695ead4712ec8234654 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Wed, 22 Feb 2023 14:24:33 +0100 Subject: [PATCH 09/13] move monitor initialization to AppSec::Component --- lib/datadog/appsec.rb | 5 ----- lib/datadog/appsec/component.rb | 9 +++++++++ sig/datadog/appsec/component.rbs | 4 ++++ spec/datadog/appsec/component_spec.rb | 10 ++++++++++ 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/lib/datadog/appsec.rb b/lib/datadog/appsec.rb index 8c7299856ad..cca99dfb945 100644 --- a/lib/datadog/appsec.rb +++ b/lib/datadog/appsec.rb @@ -40,10 +40,5 @@ def self.writer require_relative 'appsec/contrib/sinatra/integration' require_relative 'appsec/contrib/rails/integration' - require_relative 'appsec/autoload' -# Internal appsec events monitor -require_relative 'appsec/monitor' -Datadog::AppSec::Monitor::Gateway::Watcher.watch - diff --git a/lib/datadog/appsec/component.rb b/lib/datadog/appsec/component.rb index 887285c552b..2576c7cfca1 100644 --- a/lib/datadog/appsec/component.rb +++ b/lib/datadog/appsec/component.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require_relative 'processor' +require_relative 'monitor' module Datadog module AppSec @@ -29,6 +30,8 @@ def create_processor def initialize(processor:) @processor = processor + + start_monitor end def shutdown! @@ -37,6 +40,12 @@ def shutdown! @processor = nil end end + + private + + def start_monitor + Datadog::AppSec::Monitor::Gateway::Watcher.watch + end end end end diff --git a/sig/datadog/appsec/component.rbs b/sig/datadog/appsec/component.rbs index 6ceacdaaab9..eced9ba6c12 100644 --- a/sig/datadog/appsec/component.rbs +++ b/sig/datadog/appsec/component.rbs @@ -12,6 +12,10 @@ module Datadog def initialize: (processor: Datadog::AppSec::Processor?) -> void def shutdown!: () -> untyped + + private + + def start_monitor: () -> untyped end end end diff --git a/spec/datadog/appsec/component_spec.rb b/spec/datadog/appsec/component_spec.rb index f10f244c382..2828a301104 100644 --- a/spec/datadog/appsec/component_spec.rb +++ b/spec/datadog/appsec/component_spec.rb @@ -48,6 +48,16 @@ end end + describe '#new' do + context 'Watch monitor' do + it 'calls watch on the monitor gateway' do + expect(Datadog::AppSec::Monitor::Gateway::Watcher).to receive(:watch) + + described_class.new + end + end + end + describe '#shutdown!' do context 'when processor is not nil and ready' do it 'finalizes the processor' do From d280590ae430727eec78af696213c0fd36d699a1 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Wed, 22 Feb 2023 15:47:35 +0100 Subject: [PATCH 10/13] skip steep check for lib/datadog/appsec/component.rb --- Steepfile | 1 + lib/datadog/appsec.rb | 1 - lib/datadog/appsec/monitor.rb | 2 +- lib/datadog/appsec/monitor/gateway/watcher.rb | 2 +- lib/datadog/appsec/monitor/reactive/set_user.rb | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Steepfile b/Steepfile index ee8895b179b..98981b55e77 100644 --- a/Steepfile +++ b/Steepfile @@ -8,6 +8,7 @@ target :appsec do ignore 'lib/datadog/appsec/contrib' ignore 'lib/datadog/appsec/monitor' + ignore 'lib/datadog/appsec/component.rb' library 'pathname', 'set' library 'cgi' diff --git a/lib/datadog/appsec.rb b/lib/datadog/appsec.rb index cca99dfb945..e9c75d96298 100644 --- a/lib/datadog/appsec.rb +++ b/lib/datadog/appsec.rb @@ -41,4 +41,3 @@ def self.writer require_relative 'appsec/contrib/rails/integration' require_relative 'appsec/autoload' - diff --git a/lib/datadog/appsec/monitor.rb b/lib/datadog/appsec/monitor.rb index b944ab20e5c..f02c188a77d 100644 --- a/lib/datadog/appsec/monitor.rb +++ b/lib/datadog/appsec/monitor.rb @@ -1,4 +1,4 @@ -# typed: ignore +# typed: false # frozen_string_literal: true require_relative 'monitor/gateway/watcher' diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index 917dee693c4..9282ef21d27 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -1,4 +1,4 @@ -# typed: ignore +# typed: false # frozen_string_literal: true require_relative '../../instrumentation/gateway' diff --git a/lib/datadog/appsec/monitor/reactive/set_user.rb b/lib/datadog/appsec/monitor/reactive/set_user.rb index 3d65cf1e79c..50daa496c50 100644 --- a/lib/datadog/appsec/monitor/reactive/set_user.rb +++ b/lib/datadog/appsec/monitor/reactive/set_user.rb @@ -1,4 +1,4 @@ -# typed: ignore +# typed: false # frozen_string_literal: true module Datadog From e6258071515a735f147cd3ed7a2f74099c1541c6 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Thu, 23 Feb 2023 10:04:46 +0100 Subject: [PATCH 11/13] raise AlreadyActiveContextError if trying to activate a new context without having deactivate the previous one --- lib/datadog/appsec/processor.rb | 4 ++++ sig/datadog/appsec/processor.rbs | 3 +++ spec/datadog/appsec/processor_spec.rb | 9 ++++++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/datadog/appsec/processor.rb b/lib/datadog/appsec/processor.rb index 8207e981756..1d868ca312c 100644 --- a/lib/datadog/appsec/processor.rb +++ b/lib/datadog/appsec/processor.rb @@ -61,6 +61,7 @@ def reset_active_context end class NoActiveContextError < StandardError; end + class AlreadyActiveContextError < StandardError; end attr_reader :ruleset_info, :addresses @@ -87,6 +88,9 @@ def new_context end def activate_context + existing_active_context = Processor.active_context + raise AlreadyActiveContextError if existing_active_context + context = new_context Processor.send(:active_context=, context) context diff --git a/sig/datadog/appsec/processor.rbs b/sig/datadog/appsec/processor.rbs index e2348c912e8..9b5a4537a4b 100644 --- a/sig/datadog/appsec/processor.rbs +++ b/sig/datadog/appsec/processor.rbs @@ -27,6 +27,9 @@ module Datadog class NoActiveContextError < StandardError end + class AlreadyActiveContextError < StandardError + end + attr_reader ruleset_info: untyped attr_reader addresses: untyped diff --git a/spec/datadog/appsec/processor_spec.rb b/spec/datadog/appsec/processor_spec.rb index 08beed9f9a5..083f8f7aee4 100644 --- a/spec/datadog/appsec/processor_spec.rb +++ b/spec/datadog/appsec/processor_spec.rb @@ -74,7 +74,7 @@ end describe '.active_context=' do - it 'raises ArgumentError when trying to setup current conetxt to a non Context instance' do + it 'raises ArgumentError when trying to setup current context to a non Context instance' do expect do described_class.send(:active_context=, 'foo') end.to raise_error(ArgumentError) @@ -530,6 +530,13 @@ context = described_class.new.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) + end + end end describe '#deactivate_context' do From b174cfbfb7747250d8bea68fb2c94ccec7a4f4dc Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Thu, 23 Feb 2023 11:06:28 +0100 Subject: [PATCH 12/13] move appsec monitor watch to rack patcher --- lib/datadog/appsec/component.rb | 9 --------- lib/datadog/appsec/contrib/rack/patcher.rb | 2 ++ sig/datadog/appsec/component.rbs | 4 ---- spec/datadog/appsec/component_spec.rb | 10 ---------- 4 files changed, 2 insertions(+), 23 deletions(-) diff --git a/lib/datadog/appsec/component.rb b/lib/datadog/appsec/component.rb index 2576c7cfca1..887285c552b 100644 --- a/lib/datadog/appsec/component.rb +++ b/lib/datadog/appsec/component.rb @@ -2,7 +2,6 @@ # frozen_string_literal: true require_relative 'processor' -require_relative 'monitor' module Datadog module AppSec @@ -30,8 +29,6 @@ def create_processor def initialize(processor:) @processor = processor - - start_monitor end def shutdown! @@ -40,12 +37,6 @@ def shutdown! @processor = nil end end - - private - - def start_monitor - Datadog::AppSec::Monitor::Gateway::Watcher.watch - end end end end diff --git a/lib/datadog/appsec/contrib/rack/patcher.rb b/lib/datadog/appsec/contrib/rack/patcher.rb index 6797385d27c..d439d53973b 100644 --- a/lib/datadog/appsec/contrib/rack/patcher.rb +++ b/lib/datadog/appsec/contrib/rack/patcher.rb @@ -1,6 +1,7 @@ # typed: ignore require_relative '../patcher' +require_relative '../../monitor' require_relative 'gateway/watcher' module Datadog @@ -22,6 +23,7 @@ def target_version end def patch + Monitor::Gateway::Watcher.watch Gateway::Watcher.watch Patcher.instance_variable_set(:@patched, true) end diff --git a/sig/datadog/appsec/component.rbs b/sig/datadog/appsec/component.rbs index eced9ba6c12..6ceacdaaab9 100644 --- a/sig/datadog/appsec/component.rbs +++ b/sig/datadog/appsec/component.rbs @@ -12,10 +12,6 @@ module Datadog def initialize: (processor: Datadog::AppSec::Processor?) -> void def shutdown!: () -> untyped - - private - - def start_monitor: () -> untyped end end end diff --git a/spec/datadog/appsec/component_spec.rb b/spec/datadog/appsec/component_spec.rb index 2828a301104..f10f244c382 100644 --- a/spec/datadog/appsec/component_spec.rb +++ b/spec/datadog/appsec/component_spec.rb @@ -48,16 +48,6 @@ end end - describe '#new' do - context 'Watch monitor' do - it 'calls watch on the monitor gateway' do - expect(Datadog::AppSec::Monitor::Gateway::Watcher).to receive(:watch) - - described_class.new - end - end - end - describe '#shutdown!' do context 'when processor is not nil and ready' do it 'finalizes the processor' do From 4abbd896a01075c3cbb28a0c623705306acdd293 Mon Sep 17 00:00:00 2001 From: Gustavo Caso Date: Thu, 23 Feb 2023 12:37:47 +0100 Subject: [PATCH 13/13] remove sorbet type checking job from CircleCI --- .circleci/config.yml | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index efa1999ac47..d92d81d456d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -138,10 +138,6 @@ step_rubocop: &step_rubocop # The workaround is to use `cpu.shares / 1024`: # https://discuss.circleci.com/t/environment-variable-set-to-the-number-of-available-cpus/32670/4 command: PARALLEL_PROCESSOR_COUNT=$((`cat /sys/fs/cgroup/cpu/cpu.shares` / 1024)) bundle exec rake rubocop -step_sorbet_type_checker: &step_sorbet_type_checker - run: - name: Run sorbet type checker - command: bundle exec rake typecheck step_appraisal_install: &step_appraisal_install run: name: Install Appraisal gems @@ -348,16 +344,6 @@ orbs: keys: - bundle-{{ .Environment.CIRCLE_CACHE_VERSION }}-{{ checksum ".circleci/images/primary/binary_version" }}-<>-{{ checksum "lib/ddtrace/version.rb" }}-{{ .Branch }}-{{ checksum ".circleci/bundle_checksum" }} - *step_rubocop - sorbet_type_checker: - <<: *test_job_default - steps: - - restore_cache: - keys: - - '{{ .Environment.CIRCLE_CACHE_VERSION }}-bundled-repo-<>-{{ .Environment.CIRCLE_SHA1 }}' - - restore_cache: - keys: - - bundle-{{ .Environment.CIRCLE_CACHE_VERSION }}-{{ checksum ".circleci/images/primary/binary_version" }}-<>-{{ checksum "lib/ddtrace/version.rb" }}-{{ .Branch }}-{{ checksum ".circleci/bundle_checksum" }} - - *step_sorbet_type_checker coverage: <<: *test_job_default steps: @@ -564,11 +550,6 @@ workflows: name: lint requires: - build-2.7 - - orb/sorbet_type_checker: - <<: *config-2_7-small - name: sorbet_type_checker - requires: - - build-2.7 - orb/coverage: <<: *config-2_7-small name: coverage @@ -766,7 +747,6 @@ workflows: <<: *filters_all_branches_and_tags requires: - lint - - sorbet_type_checker - test-2.1 - test-2.2 - test-2.3 @@ -786,7 +766,6 @@ workflows: <<: *filters_only_release_tags requires: - lint - - sorbet_type_checker - test-2.1 - test-2.2 - test-2.3