From 03106a24821275e094316cba726b3a9b532e5bd2 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 12 Aug 2024 16:18:48 +0200 Subject: [PATCH 01/36] Add DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED env var (does nothing for now) --- lib/datadog/appsec/configuration/settings.rb | 8 ++++ .../appsec/configuration/settings_spec.rb | 40 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/lib/datadog/appsec/configuration/settings.rb b/lib/datadog/appsec/configuration/settings.rb index e5dbecfb0e1..e8ed0a8240a 100644 --- a/lib/datadog/appsec/configuration/settings.rb +++ b/lib/datadog/appsec/configuration/settings.rb @@ -197,6 +197,14 @@ def self.add_settings!(base) o.type :bool, nilable: true o.env 'DD_APPSEC_SCA_ENABLED' end + + settings :standalone do + option :enabled do |o| + o.type :bool + o.env 'DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED' + o.default false + end + end end end end diff --git a/spec/datadog/appsec/configuration/settings_spec.rb b/spec/datadog/appsec/configuration/settings_spec.rb index a19a0a94550..3d8115bc3fc 100644 --- a/spec/datadog/appsec/configuration/settings_spec.rb +++ b/spec/datadog/appsec/configuration/settings_spec.rb @@ -757,5 +757,45 @@ def patcher end end end + + describe 'standalone' do + describe '#enabled' do + subject(:enabled) { settings.appsec.standalone.enabled } + + context 'when DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED' do + around do |example| + ClimateControl.modify('DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED' => appsec_standalone_enabled) do + example.run + end + end + + context 'is not defined' do + let(:appsec_standalone_enabled) { nil } + + it { is_expected.to eq false } + end + + context 'is defined' do + let(:appsec_standalone_enabled) { 'true' } + + it { is_expected.to eq(true) } + end + end + end + + describe '#enabled=' do + subject(:set_appsec_standalone_enabled) { settings.appsec.standalone.enabled = appsec_standalone_enabled } + + [true, false].each do |value| + context "when given #{value}" do + let(:appsec_standalone_enabled) { value } + + before { set_appsec_standalone_enabled } + + it { expect(settings.appsec.standalone.enabled).to eq(value) } + end + end + end + end end end From 000bf668161f96f1e5400ec3fa7f7d61593ce7f4 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Tue, 13 Aug 2024 11:56:47 +0200 Subject: [PATCH 02/36] Add _dd.apm.enabled tag --- .../appsec/contrib/rack/request_middleware.rb | 1 + .../contrib/rack/integration_test_spec.rb | 9 ++++++++ .../contrib/rails/integration_test_spec.rb | 9 ++++++++ .../contrib/sinatra/integration_test_spec.rb | 9 ++++++++ .../support/integration/shared_examples.rb | 22 +++++++++++++++++++ 5 files changed, 50 insertions(+) diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index 55323226019..4261345ed99 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -150,6 +150,7 @@ def add_appsec_tags(processor, scope) return unless trace && span + span.set_tag('_dd.apm.enabled', 0) if Datadog.configuration.appsec.standalone.enabled span.set_tag('_dd.appsec.enabled', 1) span.set_tag('_dd.runtime_family', 'ruby') span.set_tag('_dd.appsec.waf.version', Datadog::AppSec::WAF::VERSION::BASE_STRING) diff --git a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb index 05eb3f25f2e..34426e6979f 100644 --- a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb @@ -19,6 +19,7 @@ include Rack::Test::Methods let(:appsec_enabled) { true } + let(:appsec_standalone_enabled) { false } let(:tracing_enabled) { true } let(:remote_enabled) { false } let(:appsec_ip_passlist) { [] } @@ -136,6 +137,7 @@ c.tracing.instrument :rack c.appsec.enabled = appsec_enabled + c.appsec.standalone.enabled = appsec_standalone_enabled c.appsec.waf_timeout = 10_000_000 # in us c.appsec.instrument :rack c.appsec.ip_passlist = appsec_ip_passlist @@ -813,6 +815,13 @@ it_behaves_like 'a trace with AppSec api security tags' end end + + context 'with APM disabled' do + let(:appsec_standalone_enabled) { true } + + it_behaves_like 'normal with tracing disable' + it_behaves_like 'a trace with ASM Standalone tags' + 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 211a33bba8b..7196c1f2d58 100644 --- a/spec/datadog/appsec/contrib/rails/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rails/integration_test_spec.rb @@ -29,6 +29,7 @@ let(:rack_span) { sorted_spans.reverse.find { |x| x.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST } } let(:appsec_enabled) { true } + let(:appsec_standalone_enabled) { false } let(:tracing_enabled) { true } let(:appsec_ip_denylist) { [] } let(:appsec_user_id_denylist) { [] } @@ -90,6 +91,7 @@ c.tracing.instrument :rails c.appsec.enabled = appsec_enabled + c.appsec.standalone.enabled = appsec_standalone_enabled c.appsec.waf_timeout = 10_000_000 # in us c.appsec.instrument :rails c.appsec.ip_denylist = appsec_ip_denylist @@ -306,6 +308,13 @@ def set_user it_behaves_like 'a trace with AppSec api security tags' end end + + context 'with APM disabled' do + let(:appsec_standalone_enabled) { true } + + it_behaves_like 'normal with tracing disable' + it_behaves_like 'a trace with ASM Standalone tags' + 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 fddbdc5b69a..d1d6e53d056 100644 --- a/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb @@ -41,6 +41,7 @@ let(:rack_span) { sorted_spans.reverse.find { |x| x.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST } } let(:appsec_enabled) { true } + let(:appsec_standalone_enabled) { false } let(:tracing_enabled) { true } let(:appsec_ip_denylist) { [] } let(:appsec_user_id_denylist) { [] } @@ -101,6 +102,7 @@ c.tracing.instrument :sinatra c.appsec.enabled = appsec_enabled + c.appsec.standalone.enabled = appsec_standalone_enabled c.appsec.waf_timeout = 10_000_000 # in us c.appsec.instrument :sinatra c.appsec.ip_denylist = appsec_ip_denylist @@ -312,6 +314,13 @@ it_behaves_like 'a trace with AppSec api security tags' end end + + context 'with APM disabled' do + let(:appsec_standalone_enabled) { true } + + it_behaves_like 'normal with tracing disable' + it_behaves_like 'a trace with ASM Standalone tags' + end end describe 'POST request' do diff --git a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb index 00c2a41b01a..7b428d97341 100644 --- a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb +++ b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb @@ -165,3 +165,25 @@ it_behaves_like 'a trace without AppSec events' end end + +RSpec.shared_examples 'a trace with ASM Standalone tags' do + context 'with appsec enabled' do + let(:appsec_enabled) { true } + it do + expect(service_span.send(:metrics)['_dd.apm.enabled']).to eq(0) + expect(service_span.send(:metrics)['_dd.appsec.enabled']).to eq(1.0) + expect(service_span.send(:meta)['_dd.runtime_family']).to eq('ruby') + expect(service_span.send(:meta)['_dd.appsec.waf.version']).to match(/^\d+\.\d+\.\d+/) + end + end + + context 'with appsec disabled' do + let(:appsec_enabled) { false } + it do + expect(service_span.send(:metrics)['_dd.apm.enabled']).to be_nil + expect(service_span.send(:metrics)['_dd.appsec.enabled']).to be_nil + expect(service_span.send(:meta)['_dd.runtime_family']).to be_nil + expect(service_span.send(:meta)['_dd.appsec.waf.version']).to be_nil + end + end +end From fa4540db30868c2d01d87c20592b8c313143165c Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Fri, 13 Sep 2024 15:05:46 +0200 Subject: [PATCH 03/36] Add APM rate limiting when ASM standalone is activated --- lib/datadog/tracing/sampling/rule_sampler.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/datadog/tracing/sampling/rule_sampler.rb b/lib/datadog/tracing/sampling/rule_sampler.rb index 398926233e3..73cffb295b3 100644 --- a/lib/datadog/tracing/sampling/rule_sampler.rb +++ b/lib/datadog/tracing/sampling/rule_sampler.rb @@ -30,15 +30,20 @@ def initialize( default_sampler: nil ) @rules = rules - @rate_limiter = if rate_limiter + @rate_limiter = if Datadog.configuration.appsec.standalone.enabled + # 0.0167 ~ 1 trace per minute + Core::TokenBucket.new(0.0167, 1.0) + elsif rate_limiter rate_limiter elsif rate_limit Core::TokenBucket.new(rate_limit) else Core::UnlimitedLimiter.new end - - @default_sampler = if default_sampler + @default_sampler = if Datadog.configuration.appsec.standalone.enabled + @rules = [SimpleRule.new(sample_rate: 1.0)] + nil + elsif default_sampler default_sampler elsif default_sample_rate # Add to the end of the rule list a rule always matches any trace From fbfd4afbffebc09cd30f157866e6cd1065ca5c90 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Fri, 13 Sep 2024 16:51:41 +0200 Subject: [PATCH 04/36] Add Datadog-Client-Computed-Stats header with ASM standalone --- lib/datadog/core/remote/transport/http.rb | 4 ++++ lib/datadog/core/transport/ext.rb | 1 + lib/datadog/tracing/transport/http.rb | 4 ++++ sig/datadog/core/transport/ext.rbs | 2 ++ spec/datadog/tracing/transport/http_spec.rb | 16 ++++++++++++++++ 5 files changed, 27 insertions(+) diff --git a/lib/datadog/core/remote/transport/http.rb b/lib/datadog/core/remote/transport/http.rb index ed87c0dc5cb..a49a72f11f6 100644 --- a/lib/datadog/core/remote/transport/http.rb +++ b/lib/datadog/core/remote/transport/http.rb @@ -120,6 +120,10 @@ def default_headers # Add container ID, if present. container_id = Datadog::Core::Environment::Container.container_id headers[Datadog::Core::Transport::Ext::HTTP::HEADER_CONTAINER_ID] = container_id unless container_id.nil? + # Pretend that stats computation are already done by the client + if Datadog.configuration.appsec.standalone.enabled + headers[Datadog::Core::Transport::Ext::HTTP::HEADER_CLIENT_COMPUTED_STATS] = 'yes' + end end end diff --git a/lib/datadog/core/transport/ext.rb b/lib/datadog/core/transport/ext.rb index 470d4b3ad09..e0830510b92 100644 --- a/lib/datadog/core/transport/ext.rb +++ b/lib/datadog/core/transport/ext.rb @@ -16,6 +16,7 @@ module HTTP # # Setting this header to any non-empty value enables this feature. HEADER_CLIENT_COMPUTED_TOP_LEVEL = 'Datadog-Client-Computed-Top-Level' + HEADER_CLIENT_COMPUTED_STATS = 'Datadog-Client-Computed-Stats' HEADER_META_LANG = 'Datadog-Meta-Lang' HEADER_META_LANG_VERSION = 'Datadog-Meta-Lang-Version' HEADER_META_LANG_INTERPRETER = 'Datadog-Meta-Lang-Interpreter' diff --git a/lib/datadog/tracing/transport/http.rb b/lib/datadog/tracing/transport/http.rb index 25a7e77c426..190dc31e408 100644 --- a/lib/datadog/tracing/transport/http.rb +++ b/lib/datadog/tracing/transport/http.rb @@ -74,6 +74,10 @@ def default_headers # Add container ID, if present. container_id = Datadog::Core::Environment::Container.container_id headers[Datadog::Core::Transport::Ext::HTTP::HEADER_CONTAINER_ID] = container_id unless container_id.nil? + # Pretend that stats computation are already done by the client + if Datadog.configuration.appsec.standalone.enabled + headers[Datadog::Core::Transport::Ext::HTTP::HEADER_CLIENT_COMPUTED_STATS] = 'yes' + end end end diff --git a/sig/datadog/core/transport/ext.rbs b/sig/datadog/core/transport/ext.rbs index 335e293b941..74a3d3e21b8 100644 --- a/sig/datadog/core/transport/ext.rbs +++ b/sig/datadog/core/transport/ext.rbs @@ -15,6 +15,8 @@ module Datadog HEADER_CLIENT_COMPUTED_TOP_LEVEL: ::String + HEADER_CLIENT_COMPUTED_STATS: ::String + HEADER_META_LANG: ::String HEADER_META_LANG_INTERPRETER_VENDOR: ::String diff --git a/spec/datadog/tracing/transport/http_spec.rb b/spec/datadog/tracing/transport/http_spec.rb index 830f9d372f1..f7af5db609c 100644 --- a/spec/datadog/tracing/transport/http_spec.rb +++ b/spec/datadog/tracing/transport/http_spec.rb @@ -180,6 +180,22 @@ it { is_expected.to_not include(Datadog::Core::Transport::Ext::HTTP::HEADER_CONTAINER_ID) } end end + + context 'when Datadog.configuration.appsec.standalone.enabled' do + before { expect(Datadog.configuration.appsec.standalone).to receive(:enabled).and_return(asm_standalone_enabled) } + + context 'is true' do + let(:asm_standalone_enabled) { true } + + it { is_expected.to include(Datadog::Core::Transport::Ext::HTTP::HEADER_CLIENT_COMPUTED_STATS => 'yes') } + end + + context 'is false' do + let(:asm_standalone_enabled) { false } + + it { is_expected.to_not include(Datadog::Core::Transport::Ext::HTTP::HEADER_CLIENT_COMPUTED_STATS) } + end + end end describe '.default_adapter' do From a617a2c5710a78fc5a9674295e7f0b400305ef31 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 23 Sep 2024 16:30:08 +0200 Subject: [PATCH 05/36] Moved hardcoded _dd.appsec/apm.enabled tags to Ext file --- lib/datadog/appsec/contrib/rack/request_middleware.rb | 4 ++-- lib/datadog/appsec/ext.rb | 4 ++++ sig/datadog/appsec/ext.rbs | 4 ++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index 4261345ed99..3c10561dd28 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -150,8 +150,8 @@ def add_appsec_tags(processor, scope) return unless trace && span - span.set_tag('_dd.apm.enabled', 0) if Datadog.configuration.appsec.standalone.enabled - span.set_tag('_dd.appsec.enabled', 1) + span.set_tag(Datadog::AppSec::Ext::TAG_APM_ENABLED, 0) if Datadog.configuration.appsec.standalone.enabled + span.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_ENABLED, 1) span.set_tag('_dd.runtime_family', 'ruby') span.set_tag('_dd.appsec.waf.version', Datadog::AppSec::WAF::VERSION::BASE_STRING) diff --git a/lib/datadog/appsec/ext.rb b/lib/datadog/appsec/ext.rb index 2de81657dec..7a6aa15db11 100644 --- a/lib/datadog/appsec/ext.rb +++ b/lib/datadog/appsec/ext.rb @@ -5,6 +5,10 @@ module AppSec module Ext INTERRUPT = :datadog_appsec_interrupt SCOPE_KEY = 'datadog.appsec.scope' + + TAG_APPSEC_ENABLED = '_dd.appsec.enabled' + TAG_APM_ENABLED = '_dd.apm.enabled' + TAG_APPSEC_EVENT = '_dd.p.appsec' end end end diff --git a/sig/datadog/appsec/ext.rbs b/sig/datadog/appsec/ext.rbs index 39703d21368..b72a22c944d 100644 --- a/sig/datadog/appsec/ext.rbs +++ b/sig/datadog/appsec/ext.rbs @@ -3,6 +3,10 @@ module Datadog module Ext INTERRUPT: Symbol SCOPE_KEY: String + + TAG_APPSEC_ENABLED: String + TAG_APM_ENABLED: String + TAG_APPSEC_EVENT: String end end end From d0b5f33aeb97cd576efdece79c60b9913e0aa0e7 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 23 Sep 2024 16:30:35 +0200 Subject: [PATCH 06/36] Add _dd.p.appsec tag to traces containing appsec event --- .../appsec/contrib/graphql/gateway/watcher.rb | 4 ++++ lib/datadog/appsec/contrib/rack/gateway/watcher.rb | 12 ++++++++++++ lib/datadog/appsec/contrib/rails/gateway/watcher.rb | 4 ++++ .../appsec/contrib/sinatra/gateway/watcher.rb | 8 ++++++++ lib/datadog/appsec/monitor/gateway/watcher.rb | 4 ++++ .../contrib/support/integration/shared_examples.rb | 2 ++ 6 files changed, 34 insertions(+) diff --git a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb index 1ddf89c1c98..bd0c9bcf62f 100644 --- a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb @@ -43,6 +43,10 @@ def watch_multiplex(gateway = Instrumentation.gateway) scope.service_entry_span.set_tag('appsec.event', 'true') end + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index 112d5609e08..9b97b8a1f52 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -46,6 +46,10 @@ def watch_request(gateway = Instrumentation.gateway) scope.service_entry_span.set_tag('appsec.event', 'true') end + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + scope.processor_context.events << event end end @@ -90,6 +94,10 @@ def watch_response(gateway = Instrumentation.gateway) scope.service_entry_span.set_tag('appsec.event', 'true') end + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + scope.processor_context.events << event end end @@ -134,6 +142,10 @@ def watch_request_body(gateway = Instrumentation.gateway) scope.service_entry_span.set_tag('appsec.event', 'true') end + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index ea4b34f3713..4109dcee8b6 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -43,6 +43,10 @@ def watch_request_action(gateway = Instrumentation.gateway) scope.service_entry_span.set_tag('appsec.event', 'true') end + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index 93ba40c4f92..d85558ae08f 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -45,6 +45,10 @@ def watch_request_dispatch(gateway = Instrumentation.gateway) scope.service_entry_span.set_tag('appsec.event', 'true') end + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + scope.processor_context.events << event end end @@ -89,6 +93,10 @@ def watch_request_routed(gateway = Instrumentation.gateway) scope.service_entry_span.set_tag('appsec.event', 'true') end + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index 9262cc277a6..4e938e15c78 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -40,6 +40,10 @@ def watch_user_id(gateway = Instrumentation.gateway) scope.service_entry_span.set_tag('appsec.event', 'true') end + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + scope.processor_context.events << event end end diff --git a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb index 7b428d97341..2bcb7aa8a08 100644 --- a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb +++ b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb @@ -146,6 +146,7 @@ RSpec.shared_examples 'a trace without AppSec events' do it do expect(spans.select { |s| s.get_tag('appsec.event') }).to be_empty + expect(spans.select { |s| s.get_tag('_dd.p.appsec') }).to be_empty expect(service_span.send(:meta)['_dd.appsec.triggers']).to be_nil end end @@ -155,6 +156,7 @@ it do expect(spans.select { |s| s.get_tag('appsec.event') }).to_not be_empty + expect(spans.select { |s| s.get_tag('_dd.p.appsec') }).to_not be_empty expect(service_span.send(:meta)['_dd.appsec.json']).to be_a String expect(spans.select { |s| s.get_tag('appsec.blocked') }).to_not be_empty if blocking_request end From 31d6326e706edd3a50fa42e1480644bc0f5d9a3a Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Fri, 27 Sep 2024 11:33:52 +0200 Subject: [PATCH 07/36] Skip distributed tracing if standalone appsec is enabled and trace contains no appsec event --- lib/datadog/tracing/contrib/http/circuit_breaker.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/datadog/tracing/contrib/http/circuit_breaker.rb b/lib/datadog/tracing/contrib/http/circuit_breaker.rb index ffa8e164ce8..052523c4909 100644 --- a/lib/datadog/tracing/contrib/http/circuit_breaker.rb +++ b/lib/datadog/tracing/contrib/http/circuit_breaker.rb @@ -29,6 +29,14 @@ def internal_request?(request) end def should_skip_distributed_tracing?(client_config) + if Datadog.configuration.appsec.standalone.enabled + active_trace = Tracing.active_trace + + # return true if absence of upstream ASM event (_dd.p.appsec:1) + # and no local security event (which sets _dd.p.appsec:1 locally) + return true if active_trace.nil? || active_trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1' + end + return !client_config[:distributed_tracing] if client_config && client_config.key?(:distributed_tracing) !Datadog.configuration.tracing[:http][:distributed_tracing] From a3a26d03c85375b0eeaebe1107278e67a236ebcf Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Fri, 27 Sep 2024 11:35:10 +0200 Subject: [PATCH 08/36] Reject trace if appsec event is not detected when asm standalone is activated --- lib/datadog/tracing/contrib/http/instrumentation.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/datadog/tracing/contrib/http/instrumentation.rb b/lib/datadog/tracing/contrib/http/instrumentation.rb index cd59f93fccd..89ee039571c 100644 --- a/lib/datadog/tracing/contrib/http/instrumentation.rb +++ b/lib/datadog/tracing/contrib/http/instrumentation.rb @@ -39,6 +39,11 @@ def request(req, body = nil, &block) Contrib::HTTP.inject(trace, req) end + if Datadog.configuration.appsec.standalone.enabled && + (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1') + trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT + end + # Add additional request specific tags to the span. annotate_span_with_request!(span, req, request_options) From 63ab56bec4588938d5f66a2168d007e7c8d3ea19 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 30 Sep 2024 11:09:39 +0200 Subject: [PATCH 09/36] Force keep traces with appsec event --- .../appsec/contrib/graphql/gateway/watcher.rb | 5 +++++ .../appsec/contrib/rack/gateway/watcher.rb | 15 +++++++++++++++ .../appsec/contrib/rails/gateway/watcher.rb | 5 +++++ .../appsec/contrib/sinatra/gateway/watcher.rb | 10 ++++++++++ lib/datadog/appsec/monitor/gateway/watcher.rb | 5 +++++ .../tracing/contrib/http/instrumentation.rb | 8 ++++---- 6 files changed, 44 insertions(+), 4 deletions(-) diff --git a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb index bd0c9bcf62f..08ae9bba3cb 100644 --- a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb @@ -45,6 +45,11 @@ def watch_multiplex(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') scope.processor_context.events << event diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index 9b97b8a1f52..2d5a7abcb2d 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -48,6 +48,11 @@ def watch_request(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') scope.processor_context.events << event @@ -96,6 +101,11 @@ def watch_response(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') scope.processor_context.events << event @@ -144,6 +154,11 @@ def watch_request_body(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') scope.processor_context.events << event diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index 4109dcee8b6..39716eb0ab0 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -45,6 +45,11 @@ def watch_request_action(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') scope.processor_context.events << event diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index d85558ae08f..56925946eaa 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -47,6 +47,11 @@ def watch_request_dispatch(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') scope.processor_context.events << event @@ -95,6 +100,11 @@ def watch_request_routed(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') scope.processor_context.events << event diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index 4e938e15c78..9240c9e4a59 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -42,6 +42,11 @@ def watch_user_id(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') scope.processor_context.events << event diff --git a/lib/datadog/tracing/contrib/http/instrumentation.rb b/lib/datadog/tracing/contrib/http/instrumentation.rb index 89ee039571c..63f137c18b8 100644 --- a/lib/datadog/tracing/contrib/http/instrumentation.rb +++ b/lib/datadog/tracing/contrib/http/instrumentation.rb @@ -35,15 +35,15 @@ def request(req, body = nil, &block) span.type = Tracing::Metadata::Ext::HTTP::TYPE_OUTBOUND span.resource = req.method - if Tracing.enabled? && !Contrib::HTTP.should_skip_distributed_tracing?(client_config) - Contrib::HTTP.inject(trace, req) - end - if Datadog.configuration.appsec.standalone.enabled && (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1') trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT end + if Tracing.enabled? && !Contrib::HTTP.should_skip_distributed_tracing?(client_config) + Contrib::HTTP.inject(trace, req) + end + # Add additional request specific tags to the span. annotate_span_with_request!(span, req, request_options) From 81d20f240fbfd31632d64aa7ba49c7028f14e339 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 30 Sep 2024 18:16:03 +0200 Subject: [PATCH 10/36] Add conditional access to scope.trace --- .../appsec/contrib/graphql/gateway/watcher.rb | 14 ++++--- .../appsec/contrib/rack/gateway/watcher.rb | 42 +++++++++++-------- .../appsec/contrib/rails/gateway/watcher.rb | 14 ++++--- .../appsec/contrib/sinatra/gateway/watcher.rb | 28 +++++++------ lib/datadog/appsec/monitor/gateway/watcher.rb | 14 ++++--- 5 files changed, 64 insertions(+), 48 deletions(-) diff --git a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb index 08ae9bba3cb..84d88c53516 100644 --- a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb @@ -45,12 +45,14 @@ def watch_multiplex(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + if scope.trace + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + end scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index 2d5a7abcb2d..c61495966c9 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -48,12 +48,14 @@ def watch_request(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + if scope.trace + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + end scope.processor_context.events << event end @@ -101,12 +103,14 @@ def watch_response(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + if scope.trace + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + end scope.processor_context.events << event end @@ -154,12 +158,14 @@ def watch_request_body(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + if scope.trace + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + end scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index 39716eb0ab0..c25053b92d6 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -45,12 +45,14 @@ def watch_request_action(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + if scope.trace + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + end scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index 56925946eaa..f1808d1f194 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -47,12 +47,14 @@ def watch_request_dispatch(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + if scope.trace + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + end scope.processor_context.events << event end @@ -100,12 +102,14 @@ def watch_request_routed(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + if scope.trace + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + end scope.processor_context.events << event end diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index 9240c9e4a59..959569d1250 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -42,12 +42,14 @@ def watch_user_id(gateway = Instrumentation.gateway) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + if scope.trace + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + end scope.processor_context.events << event end From 091a27fe6e1edaf6f9ba125ea95bd78697580a42 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 30 Sep 2024 18:19:33 +0200 Subject: [PATCH 11/36] Force execute asm standalone system tests --- .github/workflows/system-tests.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index baa872e118c..ffa1ae816d9 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -11,9 +11,9 @@ on: env: REGISTRY: ghcr.io REPO: ghcr.io/datadog/dd-trace-rb - ST_REF: main - FORCE_TESTS: -F tests/appsec/waf/test_addresses.py::Test_GraphQL -F tests/appsec/test_blocking_addresses.py::Test_BlockingGraphqlResolvers - FORCE_TESTS_SCENARIO: GRAPHQL_APPSEC + ST_REF: vpellan/ruby-asm-standalone + FORCE_TESTS: -F tests/appsec/test_asm_standalone.py + FORCE_TESTS_SCENARIO: APPSEC_STANDALONE jobs: build-harness: From b32f0f8c697c4348bddace8cf113ccebc33f0ac6 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Tue, 1 Oct 2024 13:12:40 +0200 Subject: [PATCH 12/36] Add APPSEC_STANDALONE scenario to system-tests GH workflow --- .github/workflows/system-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index ffa1ae816d9..5d89c63b2a6 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -199,6 +199,7 @@ jobs: - APPSEC_DISABLED - APPSEC_BLOCKING_FULL_DENYLIST - APPSEC_REQUEST_BLOCKING + - APPSEC_STANDALONE include: - library: ruby app: rack From 7549839e837e2bb16838acf27ccb0c2cdc88ab7c Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Tue, 1 Oct 2024 13:55:48 +0200 Subject: [PATCH 13/36] Refactored tagging in gateway watchers --- .../appsec/contrib/graphql/gateway/watcher.rb | 17 +------ .../appsec/contrib/rack/gateway/watcher.rb | 51 ++----------------- .../appsec/contrib/rails/gateway/watcher.rb | 17 +------ .../appsec/contrib/sinatra/gateway/watcher.rb | 34 +------------ lib/datadog/appsec/event.rb | 18 +++++++ lib/datadog/appsec/monitor/gateway/watcher.rb | 17 +------ 6 files changed, 26 insertions(+), 128 deletions(-) diff --git a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb index 84d88c53516..04d0a5899a0 100644 --- a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb @@ -38,22 +38,7 @@ def watch_multiplex(gateway = Instrumentation.gateway) actions: result.actions } - if scope.service_entry_span - scope.service_entry_span.set_tag('appsec.blocked', 'true') if result.actions.include?('block') - scope.service_entry_span.set_tag('appsec.event', 'true') - end - - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event - if scope.trace - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') - end - + Datadog::AppSec::Event.add_event_tags(scope, result) scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index c61495966c9..7a9aa000620 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -41,22 +41,7 @@ def watch_request(gateway = Instrumentation.gateway) actions: result.actions } - if scope.service_entry_span - scope.service_entry_span.set_tag('appsec.blocked', 'true') if result.actions.include?('block') - scope.service_entry_span.set_tag('appsec.event', 'true') - end - - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event - if scope.trace - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') - end - + Datadog::AppSec::Event.add_event_tags(scope, result) scope.processor_context.events << event end end @@ -96,22 +81,7 @@ def watch_response(gateway = Instrumentation.gateway) actions: result.actions } - if scope.service_entry_span - scope.service_entry_span.set_tag('appsec.blocked', 'true') if result.actions.include?('block') - scope.service_entry_span.set_tag('appsec.event', 'true') - end - - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event - if scope.trace - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') - end - + Datadog::AppSec::Event.add_event_tags(scope, result) scope.processor_context.events << event end end @@ -151,22 +121,7 @@ def watch_request_body(gateway = Instrumentation.gateway) actions: result.actions } - if scope.service_entry_span - scope.service_entry_span.set_tag('appsec.blocked', 'true') if result.actions.include?('block') - scope.service_entry_span.set_tag('appsec.event', 'true') - end - - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event - if scope.trace - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') - end - + Datadog::AppSec::Event.add_event_tags(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index c25053b92d6..b7e383f7a0b 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -38,22 +38,7 @@ def watch_request_action(gateway = Instrumentation.gateway) actions: result.actions } - if scope.service_entry_span - scope.service_entry_span.set_tag('appsec.blocked', 'true') if result.actions.include?('block') - scope.service_entry_span.set_tag('appsec.event', 'true') - end - - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event - if scope.trace - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') - end - + Datadog::AppSec::Event.add_event_tags(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index f1808d1f194..9136a753136 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -40,22 +40,7 @@ def watch_request_dispatch(gateway = Instrumentation.gateway) actions: result.actions } - if scope.service_entry_span - scope.service_entry_span.set_tag('appsec.blocked', 'true') if result.actions.include?('block') - scope.service_entry_span.set_tag('appsec.event', 'true') - end - - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event - if scope.trace - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') - end - + Datadog::AppSec::Event.add_event_tags(scope, result) scope.processor_context.events << event end end @@ -95,22 +80,7 @@ def watch_request_routed(gateway = Instrumentation.gateway) actions: result.actions } - if scope.service_entry_span - scope.service_entry_span.set_tag('appsec.blocked', 'true') if result.actions.include?('block') - scope.service_entry_span.set_tag('appsec.event', 'true') - end - - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event - if scope.trace - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') - end - + Datadog::AppSec::Event.add_event_tags(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/event.rb b/lib/datadog/appsec/event.rb index 65077de96b6..2fa2515cd04 100644 --- a/lib/datadog/appsec/event.rb +++ b/lib/datadog/appsec/event.rb @@ -137,6 +137,24 @@ def build_service_entry_tags(event_group) end # rubocop:enable Metrics/MethodLength + def add_event_tags(scope, waf_result) + if scope.service_entry_span + scope.service_entry_span.set_tag('appsec.blocked', 'true') if waf_result.actions.include?('block') + scope.service_entry_span.set_tag('appsec.event', 'true') + end + + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event + if scope.trace + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + end + end + private def compressed_and_base64_encoded(value) diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index 959569d1250..d425622cb86 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -35,22 +35,7 @@ def watch_user_id(gateway = Instrumentation.gateway) actions: result.actions } - if scope.service_entry_span - scope.service_entry_span.set_tag('appsec.blocked', 'true') if result.actions.include?('block') - scope.service_entry_span.set_tag('appsec.event', 'true') - end - - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event - if scope.trace - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') - end - + Datadog::AppSec::Event.add_event_tags(scope, result) scope.processor_context.events << event end end From c882353fc8ee6a7465cf518b8356561c1c91c66f Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Tue, 1 Oct 2024 14:49:55 +0200 Subject: [PATCH 14/36] Fix _dd.p.appsec condition in shared examples --- .../appsec/contrib/support/integration/shared_examples.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb index 2bcb7aa8a08..7501afa2bf3 100644 --- a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb +++ b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb @@ -146,7 +146,7 @@ RSpec.shared_examples 'a trace without AppSec events' do it do expect(spans.select { |s| s.get_tag('appsec.event') }).to be_empty - expect(spans.select { |s| s.get_tag('_dd.p.appsec') }).to be_empty + expect(trace.send(:meta)['_dd.p.appsec']).to be_nil expect(service_span.send(:meta)['_dd.appsec.triggers']).to be_nil end end @@ -156,7 +156,7 @@ it do expect(spans.select { |s| s.get_tag('appsec.event') }).to_not be_empty - expect(spans.select { |s| s.get_tag('_dd.p.appsec') }).to_not be_empty + expect(trace.send(:meta)['_dd.p.appsec']).to eq('1') expect(service_span.send(:meta)['_dd.appsec.json']).to be_a String expect(spans.select { |s| s.get_tag('appsec.blocked') }).to_not be_empty if blocking_request end From e86c729f54fa9b7cd3a35da5443bb88d2671b2b9 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Tue, 1 Oct 2024 16:09:12 +0200 Subject: [PATCH 15/36] Remove checks not related to ASM standalone in RSpec shared example --- .../appsec/contrib/support/integration/shared_examples.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb index 7501afa2bf3..1d8fcdafd02 100644 --- a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb +++ b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb @@ -174,8 +174,6 @@ it do expect(service_span.send(:metrics)['_dd.apm.enabled']).to eq(0) expect(service_span.send(:metrics)['_dd.appsec.enabled']).to eq(1.0) - expect(service_span.send(:meta)['_dd.runtime_family']).to eq('ruby') - expect(service_span.send(:meta)['_dd.appsec.waf.version']).to match(/^\d+\.\d+\.\d+/) end end @@ -184,8 +182,6 @@ it do expect(service_span.send(:metrics)['_dd.apm.enabled']).to be_nil expect(service_span.send(:metrics)['_dd.appsec.enabled']).to be_nil - expect(service_span.send(:meta)['_dd.runtime_family']).to be_nil - expect(service_span.send(:meta)['_dd.appsec.waf.version']).to be_nil end end end From 3c4de1af620fe39b6a4e7014c5d4079318d8732b Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 3 Oct 2024 15:23:00 +0200 Subject: [PATCH 16/36] Apply PR review suggestions --- .../appsec/contrib/rack/request_middleware.rb | 2 +- lib/datadog/appsec/event.rb | 16 ++++++++-------- .../tracing/contrib/http/circuit_breaker.rb | 4 +++- .../tracing/contrib/http/instrumentation.rb | 6 ++++-- lib/datadog/tracing/sampling/rule_sampler.rb | 4 ++-- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index 3c10561dd28..6e72035d083 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -150,8 +150,8 @@ def add_appsec_tags(processor, scope) return unless trace && span - span.set_tag(Datadog::AppSec::Ext::TAG_APM_ENABLED, 0) if Datadog.configuration.appsec.standalone.enabled span.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_ENABLED, 1) + span.set_tag(Datadog::AppSec::Ext::TAG_APM_ENABLED, 0) if Datadog.configuration.appsec.standalone.enabled span.set_tag('_dd.runtime_family', 'ruby') span.set_tag('_dd.appsec.waf.version', Datadog::AppSec::WAF::VERSION::BASE_STRING) diff --git a/lib/datadog/appsec/event.rb b/lib/datadog/appsec/event.rb index 2fa2515cd04..be6c32bf584 100644 --- a/lib/datadog/appsec/event.rb +++ b/lib/datadog/appsec/event.rb @@ -145,14 +145,14 @@ def add_event_tags(scope, waf_result) # Propagate to downstream services the information that the current distributed trace is # containing at least one ASM security event - if scope.trace - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') - end + return unless scope.trace + + scope.trace.keep! + scope.trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') end private diff --git a/lib/datadog/tracing/contrib/http/circuit_breaker.rb b/lib/datadog/tracing/contrib/http/circuit_breaker.rb index 052523c4909..fb9de7df93b 100644 --- a/lib/datadog/tracing/contrib/http/circuit_breaker.rb +++ b/lib/datadog/tracing/contrib/http/circuit_breaker.rb @@ -34,7 +34,9 @@ def should_skip_distributed_tracing?(client_config) # return true if absence of upstream ASM event (_dd.p.appsec:1) # and no local security event (which sets _dd.p.appsec:1 locally) - return true if active_trace.nil? || active_trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1' + no_upstream_or_local_event = active_trace.nil? || + active_trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1' + return true if no_upstream_or_local_event end return !client_config[:distributed_tracing] if client_config && client_config.key?(:distributed_tracing) diff --git a/lib/datadog/tracing/contrib/http/instrumentation.rb b/lib/datadog/tracing/contrib/http/instrumentation.rb index 63f137c18b8..fdbbaad8d9f 100644 --- a/lib/datadog/tracing/contrib/http/instrumentation.rb +++ b/lib/datadog/tracing/contrib/http/instrumentation.rb @@ -35,8 +35,10 @@ def request(req, body = nil, &block) span.type = Tracing::Metadata::Ext::HTTP::TYPE_OUTBOUND span.resource = req.method - if Datadog.configuration.appsec.standalone.enabled && - (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1') + asm_standalone_no_upstream_or_local_event = Datadog.configuration.appsec.standalone.enabled && + (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1') + + if asm_standalone_no_upstream_or_local_event trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT end diff --git a/lib/datadog/tracing/sampling/rule_sampler.rb b/lib/datadog/tracing/sampling/rule_sampler.rb index 73cffb295b3..9a040617e11 100644 --- a/lib/datadog/tracing/sampling/rule_sampler.rb +++ b/lib/datadog/tracing/sampling/rule_sampler.rb @@ -31,8 +31,8 @@ def initialize( ) @rules = rules @rate_limiter = if Datadog.configuration.appsec.standalone.enabled - # 0.0167 ~ 1 trace per minute - Core::TokenBucket.new(0.0167, 1.0) + # 1 trace per minute + Core::TokenBucket.new(1.0 / 60, 1.0) elsif rate_limiter rate_limiter elsif rate_limit From f362662ce734311c59cee3f495883e87466ec55d Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 3 Oct 2024 17:42:47 +0200 Subject: [PATCH 17/36] Add /requestdownstream and /returnheaders endpoints to Rack, Rails and Sinatra integration test spec --- .../contrib/rack/integration_test_spec.rb | 38 +++++++++++++++- .../contrib/rails/integration_test_spec.rb | 45 ++++++++++++++++--- .../contrib/sinatra/integration_test_spec.rb | 39 ++++++++++++++-- 3 files changed, 113 insertions(+), 9 deletions(-) diff --git a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb index 34426e6979f..d669f31c3d8 100644 --- a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb @@ -1,5 +1,6 @@ require 'datadog/tracing/contrib/support/spec_helper' require 'datadog/appsec/contrib/support/integration/shared_examples' +require 'datadog/appsec/spec_helper' require 'rack/test' require 'securerandom' @@ -131,15 +132,29 @@ end before do + WebMock.enable! + stub_request(:get, 'http://localhost:3000/returnheaders') + .to_return do |request| + { + status: 200, + body: request.headers.to_json, + headers: { 'Content-Type' => 'application/json' } + } + end + unless remote_enabled Datadog.configure do |c| c.tracing.enabled = tracing_enabled + c.tracing.instrument :rack + c.tracing.instrument :http c.appsec.enabled = appsec_enabled + + c.appsec.instrument :rack + c.appsec.standalone.enabled = appsec_standalone_enabled c.appsec.waf_timeout = 10_000_000 # in us - c.appsec.instrument :rack c.appsec.ip_passlist = appsec_ip_passlist c.appsec.ip_denylist = appsec_ip_denylist c.appsec.user_id_denylist = appsec_user_id_denylist @@ -153,6 +168,9 @@ end after do + WebMock.reset! + WebMock.disable! + Datadog.configuration.reset! Datadog.registry[:rack].reset_configuration! end @@ -225,6 +243,24 @@ map '/success/' do run(proc { |_env| [200, { 'Content-Type' => 'text/html' }, ['OK']] }) end + + map '/requestdownstream' do + run( + proc do |_env| + uri = URI('http://localhost:3000/returnheaders') + ext_request = nil + ext_response = nil + + Net::HTTP.start(uri.host, uri.port) do |http| + ext_request = Net::HTTP::Get.new(uri) + + ext_response = http.request(ext_request) + end + + [200, { 'Content-Type' => 'application/json' }, [ext_response.body]] + end + ) + end end end diff --git a/spec/datadog/appsec/contrib/rails/integration_test_spec.rb b/spec/datadog/appsec/contrib/rails/integration_test_spec.rb index 7196c1f2d58..7b039e6d8f8 100644 --- a/spec/datadog/appsec/contrib/rails/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rails/integration_test_spec.rb @@ -1,5 +1,6 @@ require 'datadog/tracing/contrib/rails/rails_helper' require 'datadog/appsec/contrib/support/integration/shared_examples' +require 'datadog/appsec/spec_helper' require 'rack/test' require 'datadog/tracing' @@ -34,7 +35,7 @@ let(:appsec_ip_denylist) { [] } let(:appsec_user_id_denylist) { [] } let(:appsec_ruleset) { :recommended } - let(:nested_app) { false } + let(:appsec_instrument_rack) { false } let(:api_security_enabled) { false } let(:api_security_sample) { 0.0 } @@ -86,25 +87,44 @@ end before do + # It may have been better to add this endpoint to the Rails app, + # but I couldn't figure out how to call the Rails app from itself using Net::HTTP. + # Creating a WebMock and stubbing it was easier. + WebMock.enable! + stub_request(:get, 'http://localhost:3000/returnheaders') + .to_return do |request| + { + status: 200, + body: request.headers.to_json, + headers: { 'Content-Type' => 'application/json' } + } + end + Datadog.configure do |c| c.tracing.enabled = tracing_enabled + c.tracing.instrument :rails + c.tracing.instrument :http c.appsec.enabled = appsec_enabled + + c.appsec.instrument :rails + c.appsec.instrument :rack if appsec_instrument_rack + c.appsec.standalone.enabled = appsec_standalone_enabled c.appsec.waf_timeout = 10_000_000 # in us - 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 c.appsec.api_security.enabled = api_security_enabled c.appsec.api_security.sample_rate = api_security_sample - - c.appsec.instrument :rack if nested_app end end after do + WebMock.reset! + WebMock.disable! + Datadog.configuration.reset! Datadog.registry[:rails].reset_configuration! end @@ -137,6 +157,20 @@ def set_user Datadog::Kit::Identity.set_user(Datadog::Tracing.active_trace, id: 'blocked-user-id') head :ok end + + def request_downstream + uri = URI('http://localhost:3000/returnheaders') + ext_request = nil + ext_response = nil + + Net::HTTP.start(uri.host, uri.port) do |http| + ext_request = Net::HTTP::Get.new('/returnheaders') + + ext_response = http.request(ext_request) + end + + render json: ext_response.body, content_type: 'application/json' + end end ) end @@ -166,6 +200,7 @@ def set_user '/success' => 'test#success', [:post, '/success'] => 'test#success', '/set_user' => 'test#set_user', + '/requestdownstream' => 'test#request_downstream', } end @@ -413,7 +448,7 @@ def set_user end describe 'Nested apps' do - let(:nested_app) { true } + let(:appsec_instrument_rack) { true } let(:middlewares) do [ Datadog::Tracing::Contrib::Rack::TraceMiddleware, diff --git a/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb b/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb index d1d6e53d056..eba67d07216 100644 --- a/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb @@ -1,5 +1,6 @@ require 'datadog/tracing/contrib/support/spec_helper' require 'datadog/appsec/contrib/support/integration/shared_examples' +require 'datadog/appsec/spec_helper' require 'rack/test' require 'securerandom' @@ -97,25 +98,41 @@ end before do + WebMock.enable! + stub_request(:get, 'http://localhost:3000/returnheaders') + .to_return do |request| + { + status: 200, + body: request.headers.to_json, + headers: { 'Content-Type' => 'application/json' } + } + end + Datadog.configure do |c| c.tracing.enabled = tracing_enabled + c.tracing.instrument :sinatra + c.tracing.instrument :http c.appsec.enabled = appsec_enabled + + c.appsec.instrument :sinatra + # TODO: test with c.appsec.instrument :rack + c.appsec.standalone.enabled = appsec_standalone_enabled c.appsec.waf_timeout = 10_000_000 # in us - 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 c.appsec.api_security.enabled = api_security_enabled c.appsec.api_security.sample_rate = api_security_sample - - # TODO: test with c.appsec.instrument :rack end end after do + WebMock.reset! + WebMock.disable! + Datadog.configuration.reset! Datadog.registry[:rack].reset_configuration! Datadog.registry[:sinatra].reset_configuration! @@ -170,6 +187,22 @@ Datadog::Kit::Identity.set_user(Datadog::Tracing.active_trace, id: 'blocked-user-id') 'ok' end + + get '/requestdownstream' do + content_type :json + + uri = URI('http://localhost:3000/returnheaders') + ext_request = nil + ext_response = nil + + Net::HTTP.start(uri.host, uri.port) do |http| + ext_request = Net::HTTP::Get.new(uri) + + ext_response = http.request(ext_request) + end + + ext_response.body + end end end From a7c3eed0f531e81375d9bc290f163726ab4d7300 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 7 Oct 2024 17:37:51 +0200 Subject: [PATCH 18/36] - Added mocked agent and force trace formatting to test Datadog-Client-Computed-Stats header and _sampling_priority_v1 tag - Reproduced system-tests ASM Standalone tests --- .../contrib/rack/integration_test_spec.rb | 97 ++++-- .../contrib/rails/integration_test_spec.rb | 62 +++- .../contrib/sinatra/integration_test_spec.rb | 59 +++- .../support/integration/shared_examples.rb | 290 +++++++++++++++++- 4 files changed, 440 insertions(+), 68 deletions(-) diff --git a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb index d669f31c3d8..325541ed0b1 100644 --- a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb @@ -19,9 +19,34 @@ RSpec.describe 'Rack integration tests' do include Rack::Test::Methods + # We send the trace to a mocked agent to verify that the trace includes the headers that we want + # In the future, it might be a good idea to use the traces that the mocked agent + # receives in the tests/shared examples + let(:agent_http_client) do + Datadog::Tracing::Transport::HTTP.default do |t| + t.adapter agent_http_adapter + end + end + + let(:agent_http_adapter) { Datadog::Core::Transport::HTTP::Adapters::Net.new(agent_settings) } + + let(:agent_settings) do + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings.new( + adapter: nil, + ssl: false, + uds_path: nil, + hostname: 'localhost', + port: 6218, + timeout_seconds: 30, + ) + end + + let(:agent_tested_headers) { {} } + + let(:tracing_enabled) { true } let(:appsec_enabled) { true } + let(:appsec_standalone_enabled) { false } - let(:tracing_enabled) { true } let(:remote_enabled) { false } let(:appsec_ip_passlist) { [] } let(:appsec_ip_denylist) { [] } @@ -142,6 +167,22 @@ } end + # Mocked agent with correct headers + stub_request(:post, 'http://localhost:6218/v0.4/traces') + .with do |request| + agent_tested_headers <= request.headers + end + .to_return(status: 200) + + # DEV: Would it be faster to do another stub for requests that don't match the headers + # rather than waiting for the TCP connection to fail? + + # TODO: Mocked agent that matches a given body, then use it in the shared examples, + # That way it would be real integration tests + + # We must format the trace to have the same result as the agent + # This is especially important for _sampling_priority_v1 metric + unless remote_enabled Datadog.configure do |c| c.tracing.enabled = tracing_enabled @@ -205,11 +246,12 @@ let(:client_ip) { remote_addr } let(:service_span) do - span = spans.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } - - expect(span.name).to eq 'rack.request' + spans.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } + end - span + let(:span) do + Datadog::Tracing::Transport::TraceFormatter.format!(trace) + spans.find { |s| s.name == 'rack.request' } end context 'with remote configuration' do @@ -243,24 +285,6 @@ map '/success/' do run(proc { |_env| [200, { 'Content-Type' => 'text/html' }, ['OK']] }) end - - map '/requestdownstream' do - run( - proc do |_env| - uri = URI('http://localhost:3000/returnheaders') - ext_request = nil - ext_response = nil - - Net::HTTP.start(uri.host, uri.port) do |http| - ext_request = Net::HTTP::Get.new(uri) - - ext_response = http.request(ext_request) - end - - [200, { 'Content-Type' => 'application/json' }, [ext_response.body]] - end - ) - end end end @@ -626,6 +650,24 @@ end ) end + + map '/requestdownstream' do + run( + proc do |_env| + uri = URI('http://localhost:3000/returnheaders') + ext_request = nil + ext_response = nil + + Net::HTTP.start(uri.host, uri.port) do |http| + ext_request = Net::HTTP::Get.new(uri) + + ext_response = http.request(ext_request) + end + + [200, { 'Content-Type' => 'application/json' }, [ext_response.body]] + end + ) + end end end @@ -851,13 +893,6 @@ it_behaves_like 'a trace with AppSec api security tags' end end - - context 'with APM disabled' do - let(:appsec_standalone_enabled) { true } - - it_behaves_like 'normal with tracing disable' - it_behaves_like 'a trace with ASM Standalone tags' - end end describe 'POST request' do @@ -987,6 +1022,8 @@ end end end + + it_behaves_like 'appsec standalone billing' end end end diff --git a/spec/datadog/appsec/contrib/rails/integration_test_spec.rb b/spec/datadog/appsec/contrib/rails/integration_test_spec.rb index 7b039e6d8f8..b564e477c9f 100644 --- a/spec/datadog/appsec/contrib/rails/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rails/integration_test_spec.rb @@ -9,7 +9,33 @@ RSpec.describe 'Rails integration tests' do include Rack::Test::Methods + # We send the trace to a mocked agent to verify that the trace includes the headers that we want + # In the future, it might be a good idea to use the traces that the mocked agent + # receives in the tests/shared examples + let(:agent_http_client) do + Datadog::Tracing::Transport::HTTP.default do |t| + t.adapter agent_http_adapter + end + end + + let(:agent_http_adapter) { Datadog::Core::Transport::HTTP::Adapters::Net.new(agent_settings) } + + let(:agent_settings) do + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings.new( + adapter: nil, + ssl: false, + uds_path: nil, + hostname: 'localhost', + port: 6218, + timeout_seconds: 30, + ) + end + let(:sorted_spans) do + # We must format the trace to have the same result as the agent + # This is especially important for _sampling_priority_v1 metric + Datadog::Tracing::Transport::TraceFormatter.format!(trace) + chain = lambda do |start| loop.with_object([start]) do |_, o| # root reached (default) @@ -27,15 +53,19 @@ sort.call(spans) end + let(:agent_tested_headers) { {} } + let(:rack_span) { sorted_spans.reverse.find { |x| x.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST } } + let(:tracing_enabled) { true } let(:appsec_enabled) { true } + + let(:appsec_instrument_rack) { false } + let(:appsec_standalone_enabled) { false } - let(:tracing_enabled) { true } let(:appsec_ip_denylist) { [] } let(:appsec_user_id_denylist) { [] } let(:appsec_ruleset) { :recommended } - let(:appsec_instrument_rack) { false } let(:api_security_enabled) { false } let(:api_security_sample) { 0.0 } @@ -100,6 +130,19 @@ } end + # Mocked agent with correct headers + stub_request(:post, 'http://localhost:6218/v0.4/traces') + .with do |request| + agent_tested_headers <= request.headers + end + .to_return(status: 200) + + # DEV: Would it be faster to do another stub for requests that don't match the headers + # rather than waiting for the TCP connection to fail? + + # TODO: Mocked agent that matches a given body, then use it in the shared examples, + # That way it would be real integration tests + Datadog.configure do |c| c.tracing.enabled = tracing_enabled @@ -185,11 +228,7 @@ def request_downstream let(:client_ip) { remote_addr } let(:service_span) do - span = sorted_spans.reverse.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } - - expect(span.name).to eq 'rack.request' - - span + sorted_spans.reverse.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } end let(:span) { rack_span } @@ -343,13 +382,6 @@ def request_downstream it_behaves_like 'a trace with AppSec api security tags' end end - - context 'with APM disabled' do - let(:appsec_standalone_enabled) { true } - - it_behaves_like 'normal with tracing disable' - it_behaves_like 'a trace with ASM Standalone tags' - end end describe 'POST request' do @@ -525,6 +557,8 @@ def request_downstream end end end + + it_behaves_like 'appsec standalone billing' end end end diff --git a/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb b/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb index eba67d07216..e9f25a12b15 100644 --- a/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb @@ -19,7 +19,33 @@ RSpec.describe 'Sinatra integration tests' do include Rack::Test::Methods + # We send the trace to a mocked agent to verify that the trace includes the headers that we want + # In the future, it might be a good idea to use the traces that the mocked agent + # receives in the tests/shared examples + let(:agent_http_client) do + Datadog::Tracing::Transport::HTTP.default do |t| + t.adapter agent_http_adapter + end + end + + let(:agent_http_adapter) { Datadog::Core::Transport::HTTP::Adapters::Net.new(agent_settings) } + + let(:agent_settings) do + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings.new( + adapter: nil, + ssl: false, + uds_path: nil, + hostname: 'localhost', + port: 6218, + timeout_seconds: 30, + ) + end + let(:sorted_spans) do + # We must format the trace to have the same result as the agent + # This is especially important for _sampling_priority_v1 metric + Datadog::Tracing::Transport::TraceFormatter.format!(trace) + chain = lambda do |start| loop.with_object([start]) do |_, o| # root reached (default) @@ -37,13 +63,16 @@ sort.call(spans) end + let(:agent_tested_headers) { {} } + let(:sinatra_span) { sorted_spans.reverse.find { |x| x.name == Datadog::Tracing::Contrib::Sinatra::Ext::SPAN_REQUEST } } let(:route_span) { sorted_spans.find { |x| x.name == Datadog::Tracing::Contrib::Sinatra::Ext::SPAN_ROUTE } } let(:rack_span) { sorted_spans.reverse.find { |x| x.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST } } + let(:tracing_enabled) { true } let(:appsec_enabled) { true } + let(:appsec_standalone_enabled) { false } - let(:tracing_enabled) { true } let(:appsec_ip_denylist) { [] } let(:appsec_user_id_denylist) { [] } let(:appsec_ruleset) { :recommended } @@ -108,6 +137,19 @@ } end + # Mocked agent with correct headers + stub_request(:post, 'http://localhost:6218/v0.4/traces') + .with do |request| + agent_tested_headers <= request.headers + end + .to_return(status: 200) + + # DEV: Would it be faster to do another stub for requests that don't match the headers + # rather than waiting for the TCP connection to fail? + + # TODO: Mocked agent that matches a given body, then use it in the shared examples, + # That way it would be real integration tests + Datadog.configure do |c| c.tracing.enabled = tracing_enabled @@ -163,11 +205,7 @@ let(:client_ip) { remote_addr } let(:service_span) do - span = sorted_spans.reverse.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } - - expect(span.name).to eq 'rack.request' - - span + sorted_spans.reverse.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } end let(:span) { rack_span } @@ -347,13 +385,6 @@ it_behaves_like 'a trace with AppSec api security tags' end end - - context 'with APM disabled' do - let(:appsec_standalone_enabled) { true } - - it_behaves_like 'normal with tracing disable' - it_behaves_like 'a trace with ASM Standalone tags' - end end describe 'POST request' do @@ -441,6 +472,8 @@ it_behaves_like 'a trace with AppSec api security tags' end end + + it_behaves_like 'appsec standalone billing' end end end diff --git a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb index 1d8fcdafd02..a2ebbe02c47 100644 --- a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb +++ b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb @@ -168,20 +168,288 @@ end end -RSpec.shared_examples 'a trace with ASM Standalone tags' do - context 'with appsec enabled' do - let(:appsec_enabled) { true } - it do - expect(service_span.send(:metrics)['_dd.apm.enabled']).to eq(0) - expect(service_span.send(:metrics)['_dd.appsec.enabled']).to eq(1.0) +RSpec.shared_examples 'a trace with ASM Standalone tags' do |params = {}| + let(:tag_apm_enabled) { params[:tag_apm_enabled] || 0 } + let(:tag_appsec_enabled) { params[:tag_appsec_enabled] || 1.0 } + let(:tag_appsec_propagation) { params[:tag_appsec_propagation] } + let(:tag_other_propagation) { params[:tag_other_propagation] || :any } + # We use a lambda as we may change the comparison type + let(:tag_sampling_priority_condition) { params[:tag_sampling_priority_condition] || ->(x) { x == 0 } } + let(:tag_trace_id) { params[:tag_trace_id] || headers_trace_id.to_i } + + it do + expect(span.send(:metrics)['_dd.apm.enabled']).to eq(tag_apm_enabled) + expect(span.send(:metrics)['_dd.appsec.enabled']).to eq(tag_appsec_enabled) + expect(span.send(:metrics)['_sampling_priority_v1']).to(satisfy { |x| tag_sampling_priority_condition.call(x) }) + + expect(span.send(:meta)['_dd.p.appsec']).to eq(tag_appsec_propagation) + expect(span.send(:meta)['_dd.p.other']).to eq(tag_other_propagation) unless tag_other_propagation == :any + + expect(span.send(:trace_id)).to eq(tag_trace_id) + expect(trace.send(:spans)[0].send(:trace_id)).to eq(tag_trace_id) + end +end + +RSpec.shared_examples 'a request with propagated headers' do |params = {}| + let(:res_origin) { params[:res_origin] } + let(:res_parent_id_not_equal) { params[:res_parent_id_not_equal] } + let(:res_tags) { params[:res_tags] } + let(:res_sampling_priority_condition) { params[:res_sampling_priority_condition] || ->(x) { x.nil? } } + let(:res_trace_id) { params[:res_trace_id] } + + let(:res_headers) { JSON.parse(response.body) } + + it do + expect(res_headers['X-Datadog-Origin']).to eq(res_origin) + expect(res_headers['X-Datadog-Parent']).to_not eq(res_parent_id_not_equal) if res_parent_id_not_equal + expect(res_headers['X-Datadog-Sampling-Priority']).to(satisfy { |x| res_sampling_priority_condition.call(x) }) + expect(res_headers['X-Datadog-Trace-Id']).to eq(res_trace_id) + expect(res_headers['X-Datadog-Tags'].split(',')).to include(*res_tags) if res_tags + end +end + +RSpec.shared_examples 'a trace sent to agent with Datadog-Client-Computed-Stats header' do + let(:agent_tested_headers) { { 'Datadog-Client-Computed-Stats' => 'yes' } } + + it do + agent_return = agent_http_client.send_traces(traces) + expect(agent_return[0].ok?).to be true + end +end + +RSpec.shared_examples 'appsec standalone billing' do + subject(:response) { get url, params, env } + + let(:appsec_standalone_enabled) { true } + + let(:url) { '/requestdownstream' } + let(:params) { {} } + let(:headers) do + { + 'HTTP_X_DATADOG_TRACE_ID' => headers_trace_id, + 'HTTP_X_DATADOG_PARENT_ID' => headers_parent_id, + 'HTTP_X_DATADOG_SAMPLING_PRIORITY' => headers_sampling_priority, + 'HTTP_X_DATADOG_ORIGIN' => headers_origin, + 'HTTP_X_DATADOG_TAGS' => headers_tags, + 'HTTP_USER_AGENT' => user_agent + } + end + let(:env) { headers } + + # Default values for headers + let(:headers_trace_id) { '1212121212121212121' } + let(:headers_parent_id) { '34343434' } + let(:headers_origin) { 'rum' } + let(:headers_sampling_priority) { '-1' } + let(:headers_tags) { '_dd.p.other=1' } + let(:user_agent) { nil } + + context 'without appsec upstream without attack and trace is kept with priority 1' do + context 'from -1 sampling priority' do + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_other_propagation: '1', + tag_sampling_priority_condition: ->(x) { x < 2 } + } + it_behaves_like 'a request with propagated headers' + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 0 sampling priority' do + let(:headers_sampling_priority) { '0' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_other_propagation: '1', + tag_sampling_priority_condition: ->(x) { x < 2 } + } + it_behaves_like 'a request with propagated headers' + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 1 sampling priority' do + let(:headers_sampling_priority) { '1' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_other_propagation: '1', + tag_sampling_priority_condition: ->(x) { x < 2 } + } + it_behaves_like 'a request with propagated headers' + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 2 sampling priority' do + let(:headers_sampling_priority) { '2' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_other_propagation: '1', + tag_sampling_priority_condition: ->(x) { x < 2 } + } + it_behaves_like 'a request with propagated headers' + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' end end - context 'with appsec disabled' do - let(:appsec_enabled) { false } - it do - expect(service_span.send(:metrics)['_dd.apm.enabled']).to be_nil - expect(service_span.send(:metrics)['_dd.appsec.enabled']).to be_nil + context 'without upstream appsec propagation with attack and trace is kept with priority 2' do + let(:user_agent) { 'Arachni/v1' } + + context 'from -1 sampling priority' do + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.other=1', '_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 0 sampling priority' do + let(:headers_sampling_priority) { '0' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.other=1', '_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + end + + context 'with upstream appsec propagation without attack and trace is propagated as is' do + let(:headers_tags) { '_dd.p.appsec=1' } + + context 'from 0 sampling priority' do + let(:headers_sampling_priority) { '0' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { [0, 2].include?(x) } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { ['0', '2'].include?(x) }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 1 sampling priority' do + let(:headers_sampling_priority) { '1' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { [1, 2].include?(x) } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { ['1', '2'].include?(x) }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 2 sampling priority' do + let(:headers_sampling_priority) { '2' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + end + + context 'with any upstream propagation with attack and raises trace priority to 2' do + let(:user_agent) { 'Arachni/v1' } + let(:headers_tags) { nil } + + context 'from -1 sampling priority' do + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 0 sampling priority' do + let(:headers_sampling_priority) { '0' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 1 sampling priority' do + let(:headers_sampling_priority) { '1' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' end end end From ee6dc539b9604aa33a891d1a046c1423d346f61c Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 7 Oct 2024 18:58:57 +0200 Subject: [PATCH 19/36] Applied code review suggestions --- .../appsec/contrib/graphql/gateway/watcher.rb | 2 +- .../appsec/contrib/rack/gateway/watcher.rb | 6 +++--- .../appsec/contrib/rails/gateway/watcher.rb | 2 +- .../appsec/contrib/sinatra/gateway/watcher.rb | 4 ++-- lib/datadog/appsec/event.rb | 6 +++--- lib/datadog/appsec/monitor/gateway/watcher.rb | 2 +- lib/datadog/core/remote/transport/http.rb | 3 ++- .../tracing/contrib/http/circuit_breaker.rb | 11 +++++------ .../tracing/contrib/http/instrumentation.rb | 5 +---- lib/datadog/tracing/sampling/rule_sampler.rb | 17 ++++++++++------- lib/datadog/tracing/trace_operation.rb | 5 +++++ .../support/integration/shared_examples.rb | 2 +- 12 files changed, 35 insertions(+), 30 deletions(-) diff --git a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb index 04d0a5899a0..97a1c4fd4ed 100644 --- a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb @@ -38,7 +38,7 @@ def watch_multiplex(gateway = Instrumentation.gateway) actions: result.actions } - Datadog::AppSec::Event.add_event_tags(scope, result) + Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index 7a9aa000620..26b7ddb8bc6 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -41,7 +41,7 @@ def watch_request(gateway = Instrumentation.gateway) actions: result.actions } - Datadog::AppSec::Event.add_event_tags(scope, result) + Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end end @@ -81,7 +81,7 @@ def watch_response(gateway = Instrumentation.gateway) actions: result.actions } - Datadog::AppSec::Event.add_event_tags(scope, result) + Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end end @@ -121,7 +121,7 @@ def watch_request_body(gateway = Instrumentation.gateway) actions: result.actions } - Datadog::AppSec::Event.add_event_tags(scope, result) + Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index b7e383f7a0b..6bc6f7956d0 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -38,7 +38,7 @@ def watch_request_action(gateway = Instrumentation.gateway) actions: result.actions } - Datadog::AppSec::Event.add_event_tags(scope, result) + Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index 9136a753136..04fb9637f59 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -40,7 +40,7 @@ def watch_request_dispatch(gateway = Instrumentation.gateway) actions: result.actions } - Datadog::AppSec::Event.add_event_tags(scope, result) + Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end end @@ -80,7 +80,7 @@ def watch_request_routed(gateway = Instrumentation.gateway) actions: result.actions } - Datadog::AppSec::Event.add_event_tags(scope, result) + Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/event.rb b/lib/datadog/appsec/event.rb index be6c32bf584..bfdfacbff0e 100644 --- a/lib/datadog/appsec/event.rb +++ b/lib/datadog/appsec/event.rb @@ -137,16 +137,16 @@ def build_service_entry_tags(event_group) end # rubocop:enable Metrics/MethodLength - def add_event_tags(scope, waf_result) + def add_tags(scope, waf_result) if scope.service_entry_span scope.service_entry_span.set_tag('appsec.blocked', 'true') if waf_result.actions.include?('block') scope.service_entry_span.set_tag('appsec.event', 'true') end - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event return unless scope.trace + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event. scope.trace.keep! scope.trace.set_tag( Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index d425622cb86..0a0d6444768 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -35,7 +35,7 @@ def watch_user_id(gateway = Instrumentation.gateway) actions: result.actions } - Datadog::AppSec::Event.add_event_tags(scope, result) + Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/core/remote/transport/http.rb b/lib/datadog/core/remote/transport/http.rb index a49a72f11f6..190a9c45646 100644 --- a/lib/datadog/core/remote/transport/http.rb +++ b/lib/datadog/core/remote/transport/http.rb @@ -120,7 +120,8 @@ def default_headers # Add container ID, if present. container_id = Datadog::Core::Environment::Container.container_id headers[Datadog::Core::Transport::Ext::HTTP::HEADER_CONTAINER_ID] = container_id unless container_id.nil? - # Pretend that stats computation are already done by the client + # Sending this header to the agent will disable metrics computation (and billing) on the agent side + # by pretending it has already been done on the library side. if Datadog.configuration.appsec.standalone.enabled headers[Datadog::Core::Transport::Ext::HTTP::HEADER_CLIENT_COMPUTED_STATS] = 'yes' end diff --git a/lib/datadog/tracing/contrib/http/circuit_breaker.rb b/lib/datadog/tracing/contrib/http/circuit_breaker.rb index fb9de7df93b..90575422065 100644 --- a/lib/datadog/tracing/contrib/http/circuit_breaker.rb +++ b/lib/datadog/tracing/contrib/http/circuit_breaker.rb @@ -30,13 +30,12 @@ def internal_request?(request) def should_skip_distributed_tracing?(client_config) if Datadog.configuration.appsec.standalone.enabled - active_trace = Tracing.active_trace + # Skip distributed tracing so that we don't bill distributed traces in case of absence of + # upstream ASM event (_dd.p.appsec:1) and no local security event (which sets _dd.p.appsec:1 locally). + # If there is an ASM event, we still have to check if distributed tracing is enabled or not + return true unless Tracing.active_trace - # return true if absence of upstream ASM event (_dd.p.appsec:1) - # and no local security event (which sets _dd.p.appsec:1 locally) - no_upstream_or_local_event = active_trace.nil? || - active_trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1' - return true if no_upstream_or_local_event + return true if Tracing.active_trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1' end return !client_config[:distributed_tracing] if client_config && client_config.key?(:distributed_tracing) diff --git a/lib/datadog/tracing/contrib/http/instrumentation.rb b/lib/datadog/tracing/contrib/http/instrumentation.rb index fdbbaad8d9f..5b60e20daa4 100644 --- a/lib/datadog/tracing/contrib/http/instrumentation.rb +++ b/lib/datadog/tracing/contrib/http/instrumentation.rb @@ -35,10 +35,7 @@ def request(req, body = nil, &block) span.type = Tracing::Metadata::Ext::HTTP::TYPE_OUTBOUND span.resource = req.method - asm_standalone_no_upstream_or_local_event = Datadog.configuration.appsec.standalone.enabled && - (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1') - - if asm_standalone_no_upstream_or_local_event + if Tracing::TraceOperation.asm_standalone_reject?(trace) trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT end diff --git a/lib/datadog/tracing/sampling/rule_sampler.rb b/lib/datadog/tracing/sampling/rule_sampler.rb index 9a040617e11..2c2fb8e0a31 100644 --- a/lib/datadog/tracing/sampling/rule_sampler.rb +++ b/lib/datadog/tracing/sampling/rule_sampler.rb @@ -29,7 +29,14 @@ def initialize( default_sample_rate: Datadog.configuration.tracing.sampling.default_rate, default_sampler: nil ) - @rules = rules + @rules = if Datadog.configuration.appsec.standalone.enabled + [SimpleRule.new(sample_rate: 1.0)] + elsif default_sample_rate && !default_sampler + # Add to the end of the rule list a rule always matches any trace + rules << SimpleRule.new(sample_rate: default_sample_rate) + else + rules + end @rate_limiter = if Datadog.configuration.appsec.standalone.enabled # 1 trace per minute Core::TokenBucket.new(1.0 / 60, 1.0) @@ -40,15 +47,11 @@ def initialize( else Core::UnlimitedLimiter.new end - @default_sampler = if Datadog.configuration.appsec.standalone.enabled - @rules = [SimpleRule.new(sample_rate: 1.0)] + @default_sampler = if Datadog.configuration.appsec.standalone.enabled || + (default_sample_rate && !default_sampler) nil elsif default_sampler default_sampler - elsif default_sample_rate - # Add to the end of the rule list a rule always matches any trace - @rules << SimpleRule.new(sample_rate: default_sample_rate) - nil else # TODO: Simplify .tags access, as `Tracer#tags` can't be arbitrarily changed anymore RateByServiceSampler.new(1.0, env: -> { Tracing.send(:tracer).tags['env'] }) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index eb85182af75..db7fb8251df 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -373,6 +373,11 @@ def fork_clone ) end + def self.asm_standalone_reject?(trace) + Datadog.configuration.appsec.standalone.enabled && + (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1') + end + # Callback behavior class Events include Tracing::Events diff --git a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb index a2ebbe02c47..bcf82aaa8aa 100644 --- a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb +++ b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb @@ -213,7 +213,7 @@ it do agent_return = agent_http_client.send_traces(traces) - expect(agent_return[0].ok?).to be true + expect(agent_return.first.ok?).to be true end end From 72a1027ded09b9329e4bab4f48feaa9f934e8abf Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Tue, 8 Oct 2024 14:23:11 +0200 Subject: [PATCH 20/36] Renamed TAG_APPSEC_EVENT to TAG_DISTRIBUTED_APPSEC_EVENT --- lib/datadog/appsec/event.rb | 2 +- lib/datadog/appsec/ext.rb | 2 +- lib/datadog/tracing/contrib/http/circuit_breaker.rb | 2 +- lib/datadog/tracing/trace_operation.rb | 2 +- sig/datadog/appsec/ext.rbs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/datadog/appsec/event.rb b/lib/datadog/appsec/event.rb index bfdfacbff0e..db22ca50930 100644 --- a/lib/datadog/appsec/event.rb +++ b/lib/datadog/appsec/event.rb @@ -152,7 +152,7 @@ def add_tags(scope, waf_result) Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, Datadog::Tracing::Sampling::Ext::Decision::ASM ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT, '1') + scope.trace.set_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT, '1') end private diff --git a/lib/datadog/appsec/ext.rb b/lib/datadog/appsec/ext.rb index 7a6aa15db11..30c21b7d240 100644 --- a/lib/datadog/appsec/ext.rb +++ b/lib/datadog/appsec/ext.rb @@ -8,7 +8,7 @@ module Ext TAG_APPSEC_ENABLED = '_dd.appsec.enabled' TAG_APM_ENABLED = '_dd.apm.enabled' - TAG_APPSEC_EVENT = '_dd.p.appsec' + TAG_DISTRIBUTED_APPSEC_EVENT = '_dd.p.appsec' end end end diff --git a/lib/datadog/tracing/contrib/http/circuit_breaker.rb b/lib/datadog/tracing/contrib/http/circuit_breaker.rb index 90575422065..d35ceda5a3e 100644 --- a/lib/datadog/tracing/contrib/http/circuit_breaker.rb +++ b/lib/datadog/tracing/contrib/http/circuit_breaker.rb @@ -35,7 +35,7 @@ def should_skip_distributed_tracing?(client_config) # If there is an ASM event, we still have to check if distributed tracing is enabled or not return true unless Tracing.active_trace - return true if Tracing.active_trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1' + return true if Tracing.active_trace.get_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT) != '1' end return !client_config[:distributed_tracing] if client_config && client_config.key?(:distributed_tracing) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index db7fb8251df..89dd4b4a62a 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -375,7 +375,7 @@ def fork_clone def self.asm_standalone_reject?(trace) Datadog.configuration.appsec.standalone.enabled && - (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_APPSEC_EVENT) != '1') + (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT) != '1') end # Callback behavior diff --git a/sig/datadog/appsec/ext.rbs b/sig/datadog/appsec/ext.rbs index b72a22c944d..2ac545ba1d3 100644 --- a/sig/datadog/appsec/ext.rbs +++ b/sig/datadog/appsec/ext.rbs @@ -6,7 +6,7 @@ module Datadog TAG_APPSEC_ENABLED: String TAG_APM_ENABLED: String - TAG_APPSEC_EVENT: String + TAG_DISTRIBUTED_APPSEC_EVENT: String end end end From efcf8ddfad16266991b48954124bcedd6d9dffab Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Tue, 8 Oct 2024 14:24:26 +0200 Subject: [PATCH 21/36] Add comment for appsec _dd.apm.enabled=0 tag --- lib/datadog/appsec/contrib/rack/request_middleware.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index 6e72035d083..895fe18b390 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -151,6 +151,7 @@ def add_appsec_tags(processor, scope) return unless trace && span span.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_ENABLED, 1) + # We add this tag when ASM standalone is enabled to make sure we don't bill APM span.set_tag(Datadog::AppSec::Ext::TAG_APM_ENABLED, 0) if Datadog.configuration.appsec.standalone.enabled span.set_tag('_dd.runtime_family', 'ruby') span.set_tag('_dd.appsec.waf.version', Datadog::AppSec::WAF::VERSION::BASE_STRING) From 9acdafbacbb60202b8fcf0c1c2c94623bd057b21 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Tue, 8 Oct 2024 14:43:48 +0200 Subject: [PATCH 22/36] Separated tagging and force_keep of traces in gateway watchers --- .../appsec/contrib/graphql/gateway/watcher.rb | 2 ++ .../appsec/contrib/rack/gateway/watcher.rb | 6 +++++ .../appsec/contrib/rails/gateway/watcher.rb | 2 ++ .../appsec/contrib/sinatra/gateway/watcher.rb | 4 ++++ lib/datadog/appsec/event.rb | 23 +++++++++++-------- lib/datadog/appsec/monitor/gateway/watcher.rb | 2 ++ sig/datadog/appsec/event.rbs | 6 ++++- 7 files changed, 34 insertions(+), 11 deletions(-) diff --git a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb index 97a1c4fd4ed..660bd743f1a 100644 --- a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb @@ -38,6 +38,8 @@ def watch_multiplex(gateway = Instrumentation.gateway) actions: result.actions } + # We want to keep the trace in case of security event + scope.trace.keep! if scope.trace Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index 26b7ddb8bc6..07b1e945184 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -41,6 +41,8 @@ def watch_request(gateway = Instrumentation.gateway) actions: result.actions } + # We want to keep the trace in case of security event + scope.trace.keep! if scope.trace Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end @@ -81,6 +83,8 @@ def watch_response(gateway = Instrumentation.gateway) actions: result.actions } + # We want to keep the trace in case of security event + scope.trace.keep! if scope.trace Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end @@ -121,6 +125,8 @@ def watch_request_body(gateway = Instrumentation.gateway) actions: result.actions } + # We want to keep the trace in case of security event + scope.trace.keep! if scope.trace Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index 6bc6f7956d0..e617b915bda 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -38,6 +38,8 @@ def watch_request_action(gateway = Instrumentation.gateway) actions: result.actions } + # We want to keep the trace in case of security event + scope.trace.keep! if scope.trace Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index 04fb9637f59..52c550dfeda 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -40,6 +40,8 @@ def watch_request_dispatch(gateway = Instrumentation.gateway) actions: result.actions } + # We want to keep the trace in case of security event + scope.trace.keep! if scope.trace Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end @@ -80,6 +82,8 @@ def watch_request_routed(gateway = Instrumentation.gateway) actions: result.actions } + # We want to keep the trace in case of security event + scope.trace.keep! if scope.trace Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end diff --git a/lib/datadog/appsec/event.rb b/lib/datadog/appsec/event.rb index db22ca50930..45ed589311a 100644 --- a/lib/datadog/appsec/event.rb +++ b/lib/datadog/appsec/event.rb @@ -143,16 +143,7 @@ def add_tags(scope, waf_result) scope.service_entry_span.set_tag('appsec.event', 'true') end - return unless scope.trace - - # Propagate to downstream services the information that the current distributed trace is - # containing at least one ASM security event. - scope.trace.keep! - scope.trace.set_tag( - Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, - Datadog::Tracing::Sampling::Ext::Decision::ASM - ) - scope.trace.set_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT, '1') + add_distributed_tags(scope.trace) end private @@ -183,6 +174,18 @@ def gzip(value) gz.close sio.string end + + # Propagate to downstream services the information that the current distributed trace is + # containing at least one ASM security event. + def add_distributed_tags(trace) + return unless trace + + trace.set_tag( + Datadog::Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, + Datadog::Tracing::Sampling::Ext::Decision::ASM + ) + trace.set_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT, '1') + end end end end diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index 0a0d6444768..efe09a0b899 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -35,6 +35,8 @@ def watch_user_id(gateway = Instrumentation.gateway) actions: result.actions } + # We want to keep the trace in case of security event + scope.trace.keep! if scope.trace Datadog::AppSec::Event.add_tags(scope, result) scope.processor_context.events << event end diff --git a/sig/datadog/appsec/event.rbs b/sig/datadog/appsec/event.rbs index 36d75ca6e55..f4d2a953219 100644 --- a/sig/datadog/appsec/event.rbs +++ b/sig/datadog/appsec/event.rbs @@ -13,7 +13,9 @@ module Datadog def self.record_via_span: (Datadog::Tracing::SpanOperation, *untyped events) -> untyped def self.build_service_entry_tags: (Array[Hash[::Symbol, untyped]] event_group) -> Hash[::String, untyped] - + + def self.add_tags: (untyped scope, untyped waf_result) -> untyped + private def self.compressed_and_base64_encoded: (untyped value) -> untyped @@ -21,6 +23,8 @@ module Datadog def self.json_parse: (untyped value) -> untyped def self.gzip: (untyped value) -> untyped + + def self.add_distributed_tags: (untyped trace) -> untyped end end end From 452e7b4a5c96bc390d638dd1439f64ae3c59d5c7 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Wed, 9 Oct 2024 10:50:30 +0200 Subject: [PATCH 23/36] Add comment to explain ASM standalone rate limiting --- lib/datadog/tracing/sampling/rule_sampler.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/datadog/tracing/sampling/rule_sampler.rb b/lib/datadog/tracing/sampling/rule_sampler.rb index 2c2fb8e0a31..2afc79d534a 100644 --- a/lib/datadog/tracing/sampling/rule_sampler.rb +++ b/lib/datadog/tracing/sampling/rule_sampler.rb @@ -29,6 +29,11 @@ def initialize( default_sample_rate: Datadog.configuration.tracing.sampling.default_rate, default_sampler: nil ) + # AppSec events are sent to the backend using traces. + # Standalone ASM billing means that we don't want to charge clients for APM traces, + # so we want to send the minimum amount of traces possible (idealy only traces that contains security events), + # but for features such as API Security, we need to send at least one trace per minute, + # to keep the service alive on the backend side. @rules = if Datadog.configuration.appsec.standalone.enabled [SimpleRule.new(sample_rate: 1.0)] elsif default_sample_rate && !default_sampler From 3ffadddbb78819ca13a9847b56fccf5a6d8ed107 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Wed, 9 Oct 2024 17:44:34 +0200 Subject: [PATCH 24/36] Add Datadog::AppSec::Event.add_tags spec and sig --- sig/datadog/appsec/event.rbs | 2 +- spec/datadog/appsec/event_spec.rb | 91 ++++++++++++++++++++ vendor/rbs/libddwaf/0/datadog/appsec/waf.rbs | 3 +- 3 files changed, 94 insertions(+), 2 deletions(-) diff --git a/sig/datadog/appsec/event.rbs b/sig/datadog/appsec/event.rbs index f4d2a953219..8ff7c9eabeb 100644 --- a/sig/datadog/appsec/event.rbs +++ b/sig/datadog/appsec/event.rbs @@ -14,7 +14,7 @@ module Datadog def self.build_service_entry_tags: (Array[Hash[::Symbol, untyped]] event_group) -> Hash[::String, untyped] - def self.add_tags: (untyped scope, untyped waf_result) -> untyped + def self.add_tags: (Datadog::AppSec::Scope scope, Datadog::AppSec::WAF::Result waf_result) -> untyped private diff --git a/spec/datadog/appsec/event_spec.rb b/spec/datadog/appsec/event_spec.rb index 1678afe3f75..21fff63b285 100644 --- a/spec/datadog/appsec/event_spec.rb +++ b/spec/datadog/appsec/event_spec.rb @@ -361,4 +361,95 @@ end end end + + describe '.add_tags' do + let(:with_trace) { true } + let(:with_span) { true } + + let(:waf_actions) { [] } + let(:waf_result) do + dbl = double + + allow(dbl).to receive(:actions).and_return(waf_actions) + + dbl + end + + let(:scope) do + scope_trace = nil + scope_span = nil + + trace_operation = Datadog::Tracing::TraceOperation.new + trace_operation.measure('root') do |span, trace| + scope_trace = trace if with_trace + scope_span = span if with_span + end + + dbl = double + + allow(dbl).to receive(:trace).and_return(scope_trace) + allow(dbl).to receive(:service_entry_span).and_return(scope_span) + + dbl + end + + before do + # prevent rate limiter to bias tests + Datadog::AppSec::RateLimiter.reset!(:traces) + + described_class.add_tags(scope, waf_result) + end + + context 'with no actions' do + it 'does not add appsec.blocked tag to span' do + expect(scope.service_entry_span.send(:meta)).to_not include('appsec.blocked') + expect(scope.service_entry_span.send(:meta)['appsec.event']).to eq('true') + expect(scope.trace.send(:meta)['_dd.p.dm']).to eq('-5') + expect(scope.trace.send(:meta)['_dd.p.appsec']).to eq('1') + end + end + + context 'with block action' do + let(:waf_actions) { ['block'] } + + it 'adds appsec.blocked tag to span' do + expect(scope.service_entry_span.send(:meta)['appsec.blocked']).to eq('true') + expect(scope.service_entry_span.send(:meta)['appsec.event']).to eq('true') + expect(scope.trace.send(:meta)['_dd.p.dm']).to eq('-5') + expect(scope.trace.send(:meta)['_dd.p.appsec']).to eq('1') + end + end + + context 'without service_entry_span' do + let(:with_span) { false } + + it 'does not add appsec span tags but still add distributed tags' do + expect(scope.service_entry_span).to be nil + expect(scope.trace.send(:meta)['_dd.p.dm']).to eq('-5') + expect(scope.trace.send(:meta)['_dd.p.appsec']).to eq('1') + end + end + + context 'without trace' do + let(:with_trace) { false } + + context 'with no actions' do + it 'does not add distributed tags but still add appsec span tags' do + expect(scope.trace).to be nil + expect(scope.service_entry_span.send(:meta)['appsec.blocked']).to be nil + expect(scope.service_entry_span.send(:meta)['appsec.event']).to eq('true') + end + end + + context 'with block action' do + let(:waf_actions) { ['block'] } + + it 'does not add distributed tags but still add appsec span tags' do + expect(scope.trace).to be nil + expect(scope.service_entry_span.send(:meta)['appsec.blocked']).to eq('true') + expect(scope.service_entry_span.send(:meta)['appsec.event']).to eq('true') + end + end + end + end end diff --git a/vendor/rbs/libddwaf/0/datadog/appsec/waf.rbs b/vendor/rbs/libddwaf/0/datadog/appsec/waf.rbs index 0418c3e1247..af8c30dd3e5 100644 --- a/vendor/rbs/libddwaf/0/datadog/appsec/waf.rbs +++ b/vendor/rbs/libddwaf/0/datadog/appsec/waf.rbs @@ -182,7 +182,8 @@ module Datadog attr_reader events: data attr_reader total_runtime: ::Float attr_reader timeout: bool - attr_reader actions: data + # Until we update libddwaf, actions is an array + attr_reader actions: ::Array[data] attr_reader derivatives: data def initialize: (::Symbol, data, ::Float, bool, data, data) -> void From 97d3ebaf439b9667116c062e7646327251ebbbcc Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 10 Oct 2024 10:58:13 +0200 Subject: [PATCH 25/36] Add Datadog-Client-Computed-Stats test in Core::Remote::Transport::http_spec --- spec/datadog/core/remote/transport/http_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/datadog/core/remote/transport/http_spec.rb b/spec/datadog/core/remote/transport/http_spec.rb index e9b1656802f..bad8a785464 100644 --- a/spec/datadog/core/remote/transport/http_spec.rb +++ b/spec/datadog/core/remote/transport/http_spec.rb @@ -70,6 +70,14 @@ it { is_expected.to have_attributes(:version => '42') } it { is_expected.to have_attributes(:endpoints => ['/info', '/v0/path']) } it { is_expected.to have_attributes(:config => { max_request_bytes: '1234' }) } + + it { expect(transport.client.api.headers).to_not include('Datadog-Client-Computed-Stats') } + + context 'with ASM standalone enabled' do + before { expect(Datadog.configuration.appsec.standalone).to receive(:enabled).and_return(true) } + + it { expect(transport.client.api.headers['Datadog-Client-Computed-Stats']).to eq('yes') } + end end end @@ -202,6 +210,14 @@ it { is_expected.to have_attributes(:targets => be_a(Hash)) } it { is_expected.to have_attributes(:target_files => be_a(Array)) } + it { expect(transport.client.api.headers).to_not include('Datadog-Client-Computed-Stats') } + + context 'with ASM standalone enabled' do + before { expect(Datadog.configuration.appsec.standalone).to receive(:enabled).and_return(true) } + + it { expect(transport.client.api.headers['Datadog-Client-Computed-Stats']).to eq('yes') } + end + context 'with a network error' do it 'raises a transport error' do expect(http_connection).to receive(:request).and_raise(IOError) From 63c1b87e94aae16208a6bec16480cb8e857fcd2b Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 10 Oct 2024 14:30:59 +0200 Subject: [PATCH 26/36] Added circuit_breaker 'should_skip_distributed_tracing' spec --- .../contrib/http/circuit_breaker_spec.rb | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/spec/datadog/tracing/contrib/http/circuit_breaker_spec.rb b/spec/datadog/tracing/contrib/http/circuit_breaker_spec.rb index 4d656c7ed87..851ccddb86d 100644 --- a/spec/datadog/tracing/contrib/http/circuit_breaker_spec.rb +++ b/spec/datadog/tracing/contrib/http/circuit_breaker_spec.rb @@ -91,4 +91,68 @@ end end end + + describe '#should_skip_distributed_tracing?' do + subject(:should_skip_distributed_tracing?) { circuit_breaker.should_skip_distributed_tracing?(client_config) } + + let(:client_config) { nil } + let(:distributed_tracing) { true } + let(:appsec_standalone) { false } + let(:active_trace) { nil } + let(:distributed_appsec_event) { nil } + + before do + allow(Datadog.configuration.tracing[:http]).to receive(:[]).with(:distributed_tracing).and_return(distributed_tracing) + allow(Datadog.configuration.appsec.standalone).to receive(:enabled).and_return(appsec_standalone) + allow(Datadog::Tracing).to receive(:active_trace).and_return(active_trace) + allow(active_trace).to receive(:get_tag).with('_dd.p.appsec').and_return(distributed_appsec_event) if active_trace + end + + context 'when distributed tracing is enabled' do + it { is_expected.to be false } + end + + context 'when distributed tracing is disabled' do + let(:distributed_tracing) { false } + + it { is_expected.to be true } + end + + context 'when appsec standalone is enabled' do + let(:appsec_standalone) { true } + + context 'when there is no active trace' do + it { is_expected.to be true } + end + + context 'when there is an active trace' do + let(:active_trace) { instance_double(Datadog::Tracing::TraceOperation) } + + context 'when the active trace has no distributed appsec event' do + it { is_expected.to be true } + end + + context 'when the active trace has a distributed appsec event' do + # This should act like standalone appsec is disabled, as it does not return in the + # `if Datadog.configuration.appsec.standalone.enabled` block + # so we're only testing the "no client config, distributed tracing enabled" case here + let(:distributed_appsec_event) { '1' } + + it { is_expected.to be false } + end + end + end + + context 'given a client config with distributed_tracing disabled' do + let(:client_config) { { distributed_tracing: false } } + + it { is_expected.to be true } + end + + context 'given a client config with distributed_tracing enabled' do + let(:client_config) { { distributed_tracing: true } } + + it { is_expected.to be false } + end + end end From 8575ed2d58e554d6bdd5331b3caa2b5cba2e666c Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 10 Oct 2024 15:39:17 +0200 Subject: [PATCH 27/36] Added rule_sampler spec tests when appsec standalone is activated --- .../tracing/sampling/rule_sampler_spec.rb | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/spec/datadog/tracing/sampling/rule_sampler_spec.rb b/spec/datadog/tracing/sampling/rule_sampler_spec.rb index e63e2e9d6ca..7ff2b8c95b7 100644 --- a/spec/datadog/tracing/sampling/rule_sampler_spec.rb +++ b/spec/datadog/tracing/sampling/rule_sampler_spec.rb @@ -28,19 +28,41 @@ end end + shared_examples 'a token bucket rate limiter' do |options = { rate: 100, max_tokens: nil }| + it do + expect(rule_sampler.rate_limiter).to be_a(Datadog::Core::TokenBucket) + expect(rule_sampler.rate_limiter.rate).to eq(options[:rate]) + expect(rule_sampler.rate_limiter.max_tokens).to eq(options[:max_tokens] || options[:rate]) + end + end + describe '#initialize' do subject(:rule_sampler) { described_class.new(rules) } - it { expect(rule_sampler.rate_limiter).to be_a(Datadog::Core::TokenBucket) } + it_behaves_like 'a token bucket rate limiter', rate: 100 it { expect(rule_sampler.default_sampler).to be_a(Datadog::Tracing::Sampling::RateByServiceSampler) } + context 'with appsec standalone enabled' do + before do + allow(Datadog.configuration.appsec.standalone).to receive(:enabled).and_return(true) + end + + it_behaves_like 'a simple rule that matches all span operations', sample_rate: 1.0 do + let(:rule) { rule_sampler.rules.last } + end + + it_behaves_like 'a token bucket rate limiter', rate: 1.0 / 60, max_tokens: 1.0 + + it { expect(rule_sampler.default_sampler).to be_nil } + end + context 'with rate_limit ENV' do before do allow(Datadog.configuration.tracing.sampling).to receive(:rate_limit) .and_return(20.0) end - it { expect(rule_sampler.rate_limiter).to be_a(Datadog::Core::TokenBucket) } + it_behaves_like 'a token bucket rate limiter', rate: 20.0 end context 'with default_sample_rate ENV' do @@ -57,7 +79,7 @@ context 'with rate_limit' do subject(:rule_sampler) { described_class.new(rules, rate_limit: 1.0) } - it { expect(rule_sampler.rate_limiter).to be_a(Datadog::Core::TokenBucket) } + it_behaves_like 'a token bucket rate limiter', rate: 1.0 end context 'with nil rate_limit' do From 154f31d2ca79c699be92fc1b8f2d6eded0d14d4d Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 10 Oct 2024 16:37:47 +0200 Subject: [PATCH 28/36] Rename asm_standalone_reject? to appsec_standalone_reject? and add sig and spec --- .../tracing/contrib/http/instrumentation.rb | 2 +- lib/datadog/tracing/trace_operation.rb | 2 +- sig/datadog/tracing/trace_operation.rbs | 1 + spec/datadog/tracing/trace_operation_spec.rb | 32 +++++++++++++++++++ 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/lib/datadog/tracing/contrib/http/instrumentation.rb b/lib/datadog/tracing/contrib/http/instrumentation.rb index 5b60e20daa4..13369dfdbb1 100644 --- a/lib/datadog/tracing/contrib/http/instrumentation.rb +++ b/lib/datadog/tracing/contrib/http/instrumentation.rb @@ -35,7 +35,7 @@ def request(req, body = nil, &block) span.type = Tracing::Metadata::Ext::HTTP::TYPE_OUTBOUND span.resource = req.method - if Tracing::TraceOperation.asm_standalone_reject?(trace) + if Tracing::TraceOperation.appsec_standalone_reject?(trace) trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT end diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 89dd4b4a62a..ba0cfe74d9b 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -373,7 +373,7 @@ def fork_clone ) end - def self.asm_standalone_reject?(trace) + def self.appsec_standalone_reject?(trace) Datadog.configuration.appsec.standalone.enabled && (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT) != '1') end diff --git a/sig/datadog/tracing/trace_operation.rbs b/sig/datadog/tracing/trace_operation.rbs index e434c9872d6..3835fa01fa4 100644 --- a/sig/datadog/tracing/trace_operation.rbs +++ b/sig/datadog/tracing/trace_operation.rbs @@ -40,6 +40,7 @@ module Datadog def flush!: () { (untyped) -> untyped } -> untyped def to_digest: () -> untyped def fork_clone: () -> untyped + def self.appsec_standalone_reject?: (TraceOperation trace) -> bool class Events include Tracing::Events diff --git a/spec/datadog/tracing/trace_operation_spec.rb b/spec/datadog/tracing/trace_operation_spec.rb index d168a99f73c..d70223831ee 100644 --- a/spec/datadog/tracing/trace_operation_spec.rb +++ b/spec/datadog/tracing/trace_operation_spec.rb @@ -2579,6 +2579,38 @@ def span end end + describe '#appsec_standalone_reject?' do + subject(:appsec_standalone_reject?) { described_class.appsec_standalone_reject?(trace_op) } + + let(:appsec_standalone) { false } + let(:distributed_appsec_event) { '0' } + + before do + allow(Datadog.configuration.appsec.standalone).to receive(:enabled).and_return(appsec_standalone) + trace_op.set_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT, distributed_appsec_event) if trace_op + end + + it { is_expected.to be false } + + context 'when AppSec standalone is enabled' do + let(:appsec_standalone) { true } + + it { is_expected.to be true } + + context 'without a trace' do + let(:trace_op) { nil } + + it { is_expected.to be true } + end + + context 'with a distributed AppSec event' do + let(:distributed_appsec_event) { '1' } + + it { is_expected.to be false } + end + end + end + describe 'integration tests' do context 'service_entry attributes' do context 'when service not given' do From 56a6475f4a46138e64a9df96c69bcce3141f143d Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 10 Oct 2024 16:57:32 +0200 Subject: [PATCH 29/36] Fix typo in AppSec::Event.add_tags spec --- spec/datadog/appsec/event_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/datadog/appsec/event_spec.rb b/spec/datadog/appsec/event_spec.rb index 21fff63b285..57ce4ce1d02 100644 --- a/spec/datadog/appsec/event_spec.rb +++ b/spec/datadog/appsec/event_spec.rb @@ -395,7 +395,7 @@ before do # prevent rate limiter to bias tests - Datadog::AppSec::RateLimiter.reset!(:traces) + Datadog::AppSec::RateLimiter.reset! described_class.add_tags(scope, waf_result) end From 5f9a239c328954be1e7383faa41256cfc550d96e Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 17 Oct 2024 13:58:51 +0200 Subject: [PATCH 30/36] Update Unreleased Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d557822d1b..d2628a80ec3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +* AppSec: Add Experimental Standalone AppSec Threats billing ([#3965][]) + ## [2.4.0] - 2024-10-11 ### Added From 22f18a8a05dae7f1e94e06550c8e8066f99396a1 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 17 Oct 2024 14:03:23 +0200 Subject: [PATCH 31/36] Add correct sig to Datadog::AppSec::Event.add_tags and add_distributed_tags --- sig/datadog/appsec/event.rbs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sig/datadog/appsec/event.rbs b/sig/datadog/appsec/event.rbs index 8ff7c9eabeb..ae4cca7ec49 100644 --- a/sig/datadog/appsec/event.rbs +++ b/sig/datadog/appsec/event.rbs @@ -14,7 +14,7 @@ module Datadog def self.build_service_entry_tags: (Array[Hash[::Symbol, untyped]] event_group) -> Hash[::String, untyped] - def self.add_tags: (Datadog::AppSec::Scope scope, Datadog::AppSec::WAF::Result waf_result) -> untyped + def self.add_tags: (Datadog::AppSec::Scope scope, Datadog::AppSec::WAF::Result waf_result) -> void private @@ -24,7 +24,7 @@ module Datadog def self.gzip: (untyped value) -> untyped - def self.add_distributed_tags: (untyped trace) -> untyped + def self.add_distributed_tags: (Datadog::Tracing::TraceOperation trace) -> void end end end From 4a2bf673504f46b2d20708b218d4988643af3084 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 17 Oct 2024 14:25:06 +0200 Subject: [PATCH 32/36] Replaced set_tag by set_metric for _dd.appsec.enabled and _dd.apm.enabled metrics --- lib/datadog/appsec/contrib/rack/request_middleware.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/datadog/appsec/contrib/rack/request_middleware.rb b/lib/datadog/appsec/contrib/rack/request_middleware.rb index 895fe18b390..cbb232c00e3 100644 --- a/lib/datadog/appsec/contrib/rack/request_middleware.rb +++ b/lib/datadog/appsec/contrib/rack/request_middleware.rb @@ -150,9 +150,9 @@ def add_appsec_tags(processor, scope) return unless trace && span - span.set_tag(Datadog::AppSec::Ext::TAG_APPSEC_ENABLED, 1) + span.set_metric(Datadog::AppSec::Ext::TAG_APPSEC_ENABLED, 1) # We add this tag when ASM standalone is enabled to make sure we don't bill APM - span.set_tag(Datadog::AppSec::Ext::TAG_APM_ENABLED, 0) if Datadog.configuration.appsec.standalone.enabled + span.set_metric(Datadog::AppSec::Ext::TAG_APM_ENABLED, 0) if Datadog.configuration.appsec.standalone.enabled span.set_tag('_dd.runtime_family', 'ruby') span.set_tag('_dd.appsec.waf.version', Datadog::AppSec::WAF::VERSION::BASE_STRING) From e24310efa3ba270c6a90be6bd8e779b7d445a178 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 17 Oct 2024 17:14:43 +0200 Subject: [PATCH 33/36] Move appsec_standalone_reject? to AppSec namespace Set sampling priority to AUTO_REJECT when ASM Standalone is active and no appsec event everywhere there is HTTP.inject --- lib/datadog/appsec.rb | 1 + lib/datadog/appsec/utils.rb | 2 + lib/datadog/appsec/utils/trace_operation.rb | 15 +++++++ .../tracing/contrib/ethon/easy_patch.rb | 4 ++ .../tracing/contrib/excon/middleware.rb | 3 ++ .../tracing/contrib/faraday/middleware.rb | 3 ++ .../tracing/contrib/http/instrumentation.rb | 2 +- .../contrib/httpclient/instrumentation.rb | 4 ++ .../tracing/contrib/httprb/instrumentation.rb | 4 ++ .../contrib/rest_client/request_patch.rb | 3 ++ lib/datadog/tracing/trace_operation.rb | 5 --- sig/datadog/appsec/utils/trace_operation.rbs | 9 +++++ sig/datadog/tracing/trace_operation.rbs | 1 - .../appsec/utils/trace_operation_spec.rb | 40 +++++++++++++++++++ spec/datadog/tracing/trace_operation_spec.rb | 32 --------------- 15 files changed, 89 insertions(+), 39 deletions(-) create mode 100644 lib/datadog/appsec/utils/trace_operation.rb create mode 100644 sig/datadog/appsec/utils/trace_operation.rbs create mode 100644 spec/datadog/appsec/utils/trace_operation_spec.rb diff --git a/lib/datadog/appsec.rb b/lib/datadog/appsec.rb index c19c54770d9..a00403bc5f2 100644 --- a/lib/datadog/appsec.rb +++ b/lib/datadog/appsec.rb @@ -4,6 +4,7 @@ require_relative 'appsec/extensions' require_relative 'appsec/scope' require_relative 'appsec/ext' +require_relative 'appsec/utils' module Datadog # Namespace for Datadog AppSec instrumentation diff --git a/lib/datadog/appsec/utils.rb b/lib/datadog/appsec/utils.rb index 8e4083533db..b38ec5b96f5 100644 --- a/lib/datadog/appsec/utils.rb +++ b/lib/datadog/appsec/utils.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative 'utils/trace_operation' + module Datadog module AppSec # Utilities for AppSec diff --git a/lib/datadog/appsec/utils/trace_operation.rb b/lib/datadog/appsec/utils/trace_operation.rb new file mode 100644 index 00000000000..19f2b0b2187 --- /dev/null +++ b/lib/datadog/appsec/utils/trace_operation.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Datadog + module AppSec + module Utils + # Utility class to to AppSec-specific trace operations + class TraceOperation + def self.appsec_standalone_reject?(trace) + Datadog.configuration.appsec.standalone.enabled && + (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT) != '1') + end + end + end + end +end diff --git a/lib/datadog/tracing/contrib/ethon/easy_patch.rb b/lib/datadog/tracing/contrib/ethon/easy_patch.rb index af661e86b0c..72e2c104f6b 100644 --- a/lib/datadog/tracing/contrib/ethon/easy_patch.rb +++ b/lib/datadog/tracing/contrib/ethon/easy_patch.rb @@ -110,6 +110,10 @@ def datadog_before_request(continue_from: nil) datadog_tag_request + if Datadog::AppSec::Utils::TraceOperation.appsec_standalone_reject?(datadog_trace) + datadog_trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT + end + if datadog_configuration[:distributed_tracing] @datadog_original_headers ||= {} Contrib::HTTP.inject(datadog_trace, @datadog_original_headers) diff --git a/lib/datadog/tracing/contrib/excon/middleware.rb b/lib/datadog/tracing/contrib/excon/middleware.rb index e25ee1941cd..9bb3f8629ef 100644 --- a/lib/datadog/tracing/contrib/excon/middleware.rb +++ b/lib/datadog/tracing/contrib/excon/middleware.rb @@ -30,6 +30,9 @@ def request_call(datum) trace = Tracing.active_trace datum[:datadog_span] = span annotate!(span, datum) + if Datadog::AppSec::Utils::TraceOperation.appsec_standalone_reject?(trace) + trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT + end propagate!(trace, span, datum) if distributed_tracing? span diff --git a/lib/datadog/tracing/contrib/faraday/middleware.rb b/lib/datadog/tracing/contrib/faraday/middleware.rb index 9bdea9c364b..0795ba29b2c 100644 --- a/lib/datadog/tracing/contrib/faraday/middleware.rb +++ b/lib/datadog/tracing/contrib/faraday/middleware.rb @@ -29,6 +29,9 @@ def call(env) Tracing.trace(Ext::SPAN_REQUEST, on_error: request_options[:on_error]) do |span, trace| annotate!(span, env, request_options) + if Datadog::AppSec::Utils::TraceOperation.appsec_standalone_reject?(trace) + trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT + end propagate!(trace, span, env) if request_options[:distributed_tracing] && Tracing.enabled? app.call(env).on_complete { |resp| handle_response(span, resp, request_options) } end diff --git a/lib/datadog/tracing/contrib/http/instrumentation.rb b/lib/datadog/tracing/contrib/http/instrumentation.rb index 13369dfdbb1..b913a7d41dd 100644 --- a/lib/datadog/tracing/contrib/http/instrumentation.rb +++ b/lib/datadog/tracing/contrib/http/instrumentation.rb @@ -35,7 +35,7 @@ def request(req, body = nil, &block) span.type = Tracing::Metadata::Ext::HTTP::TYPE_OUTBOUND span.resource = req.method - if Tracing::TraceOperation.appsec_standalone_reject?(trace) + if Datadog::AppSec::Utils::TraceOperation.appsec_standalone_reject?(trace) trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT end diff --git a/lib/datadog/tracing/contrib/httpclient/instrumentation.rb b/lib/datadog/tracing/contrib/httpclient/instrumentation.rb index 5cbc784bf59..15c218bf333 100644 --- a/lib/datadog/tracing/contrib/httpclient/instrumentation.rb +++ b/lib/datadog/tracing/contrib/httpclient/instrumentation.rb @@ -30,6 +30,10 @@ def do_get_block(req, proxy, conn, &block) span.service = service_name(host, request_options, client_config) span.type = Tracing::Metadata::Ext::HTTP::TYPE_OUTBOUND + if Datadog::AppSec::Utils::TraceOperation.appsec_standalone_reject?(trace) + trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT + end + if Tracing.enabled? && !should_skip_distributed_tracing?(client_config) Contrib::HTTP.inject(trace, req.header) end diff --git a/lib/datadog/tracing/contrib/httprb/instrumentation.rb b/lib/datadog/tracing/contrib/httprb/instrumentation.rb index c39916ebb4a..7d0a11e7ff9 100644 --- a/lib/datadog/tracing/contrib/httprb/instrumentation.rb +++ b/lib/datadog/tracing/contrib/httprb/instrumentation.rb @@ -30,6 +30,10 @@ def perform(req, options) span.service = service_name(host, request_options, client_config) span.type = Tracing::Metadata::Ext::HTTP::TYPE_OUTBOUND + if Datadog::AppSec::Utils::TraceOperation.appsec_standalone_reject?(trace) + trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT + end + Contrib::HTTP.inject(trace, req) if Tracing.enabled? && !should_skip_distributed_tracing?(client_config) # Add additional request specific tags to the span. diff --git a/lib/datadog/tracing/contrib/rest_client/request_patch.rb b/lib/datadog/tracing/contrib/rest_client/request_patch.rb index cabb1b73161..4f340c5356d 100644 --- a/lib/datadog/tracing/contrib/rest_client/request_patch.rb +++ b/lib/datadog/tracing/contrib/rest_client/request_patch.rb @@ -25,6 +25,9 @@ def execute(&block) return super(&block) unless Tracing.enabled? datadog_trace_request(uri) do |_span, trace| + if Datadog::AppSec::Utils::TraceOperation.appsec_standalone_reject?(trace) + trace.sampling_priority = Tracing::Sampling::Ext::Priority::AUTO_REJECT + end Contrib::HTTP.inject(trace, processed_headers) if datadog_configuration[:distributed_tracing] super(&block) diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index ba0cfe74d9b..eb85182af75 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -373,11 +373,6 @@ def fork_clone ) end - def self.appsec_standalone_reject?(trace) - Datadog.configuration.appsec.standalone.enabled && - (trace.nil? || trace.get_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT) != '1') - end - # Callback behavior class Events include Tracing::Events diff --git a/sig/datadog/appsec/utils/trace_operation.rbs b/sig/datadog/appsec/utils/trace_operation.rbs new file mode 100644 index 00000000000..cd7a86680a1 --- /dev/null +++ b/sig/datadog/appsec/utils/trace_operation.rbs @@ -0,0 +1,9 @@ +module Datadog + module AppSec + module Utils + class TraceOperation + def self.appsec_standalone_reject?: (Datadog::Tracing::TraceOperation trace) -> bool + end + end + end +end diff --git a/sig/datadog/tracing/trace_operation.rbs b/sig/datadog/tracing/trace_operation.rbs index 3835fa01fa4..e434c9872d6 100644 --- a/sig/datadog/tracing/trace_operation.rbs +++ b/sig/datadog/tracing/trace_operation.rbs @@ -40,7 +40,6 @@ module Datadog def flush!: () { (untyped) -> untyped } -> untyped def to_digest: () -> untyped def fork_clone: () -> untyped - def self.appsec_standalone_reject?: (TraceOperation trace) -> bool class Events include Tracing::Events diff --git a/spec/datadog/appsec/utils/trace_operation_spec.rb b/spec/datadog/appsec/utils/trace_operation_spec.rb new file mode 100644 index 00000000000..15692054506 --- /dev/null +++ b/spec/datadog/appsec/utils/trace_operation_spec.rb @@ -0,0 +1,40 @@ +require 'datadog/appsec/spec_helper' +require 'datadog/appsec/utils/trace_operation' + +RSpec.describe Datadog::AppSec::Utils::TraceOperation do + describe '#appsec_standalone_reject?' do + subject(:appsec_standalone_reject?) do + described_class.appsec_standalone_reject?(trace_op) + end + + let(:trace_op) { Datadog::Tracing::TraceOperation.new(**options) } + let(:options) { {} } + let(:appsec_standalone) { false } + let(:distributed_appsec_event) { '0' } + + before do + allow(Datadog.configuration.appsec.standalone).to receive(:enabled).and_return(appsec_standalone) + trace_op.set_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT, distributed_appsec_event) if trace_op + end + + it { is_expected.to be false } + + context 'when AppSec standalone is enabled' do + let(:appsec_standalone) { true } + + it { is_expected.to be true } + + context 'without a trace' do + let(:trace_op) { nil } + + it { is_expected.to be true } + end + + context 'with a distributed AppSec event' do + let(:distributed_appsec_event) { '1' } + + it { is_expected.to be false } + end + end + end +end diff --git a/spec/datadog/tracing/trace_operation_spec.rb b/spec/datadog/tracing/trace_operation_spec.rb index d70223831ee..d168a99f73c 100644 --- a/spec/datadog/tracing/trace_operation_spec.rb +++ b/spec/datadog/tracing/trace_operation_spec.rb @@ -2579,38 +2579,6 @@ def span end end - describe '#appsec_standalone_reject?' do - subject(:appsec_standalone_reject?) { described_class.appsec_standalone_reject?(trace_op) } - - let(:appsec_standalone) { false } - let(:distributed_appsec_event) { '0' } - - before do - allow(Datadog.configuration.appsec.standalone).to receive(:enabled).and_return(appsec_standalone) - trace_op.set_tag(Datadog::AppSec::Ext::TAG_DISTRIBUTED_APPSEC_EVENT, distributed_appsec_event) if trace_op - end - - it { is_expected.to be false } - - context 'when AppSec standalone is enabled' do - let(:appsec_standalone) { true } - - it { is_expected.to be true } - - context 'without a trace' do - let(:trace_op) { nil } - - it { is_expected.to be true } - end - - context 'with a distributed AppSec event' do - let(:distributed_appsec_event) { '1' } - - it { is_expected.to be false } - end - end - end - describe 'integration tests' do context 'service_entry attributes' do context 'when service not given' do From 73f8af2cd11e7e37a9d91a6b0cb1e72e1edfae02 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Thu, 17 Oct 2024 17:34:43 +0200 Subject: [PATCH 34/36] Rename AppSec::Event.add_tags to AppSec::Event.tag_and_keep! and move trace.keep in it --- lib/datadog/appsec/contrib/graphql/gateway/watcher.rb | 4 +--- lib/datadog/appsec/contrib/rack/gateway/watcher.rb | 6 +++--- lib/datadog/appsec/contrib/rails/gateway/watcher.rb | 2 +- lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb | 4 ++-- lib/datadog/appsec/event.rb | 5 ++++- lib/datadog/appsec/monitor/gateway/watcher.rb | 2 +- sig/datadog/appsec/event.rbs | 2 +- spec/datadog/appsec/event_spec.rb | 4 ++-- 8 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb index 660bd743f1a..68a1c5acf11 100644 --- a/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/graphql/gateway/watcher.rb @@ -38,9 +38,7 @@ def watch_multiplex(gateway = Instrumentation.gateway) actions: result.actions } - # We want to keep the trace in case of security event - scope.trace.keep! if scope.trace - Datadog::AppSec::Event.add_tags(scope, result) + Datadog::AppSec::Event.tag_and_keep!(scope, result) scope.processor_context.events << event end diff --git a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb index 07b1e945184..a66387329c3 100644 --- a/lib/datadog/appsec/contrib/rack/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rack/gateway/watcher.rb @@ -43,7 +43,7 @@ def watch_request(gateway = Instrumentation.gateway) # We want to keep the trace in case of security event scope.trace.keep! if scope.trace - Datadog::AppSec::Event.add_tags(scope, result) + Datadog::AppSec::Event.tag_and_keep!(scope, result) scope.processor_context.events << event end end @@ -85,7 +85,7 @@ def watch_response(gateway = Instrumentation.gateway) # We want to keep the trace in case of security event scope.trace.keep! if scope.trace - Datadog::AppSec::Event.add_tags(scope, result) + Datadog::AppSec::Event.tag_and_keep!(scope, result) scope.processor_context.events << event end end @@ -127,7 +127,7 @@ def watch_request_body(gateway = Instrumentation.gateway) # We want to keep the trace in case of security event scope.trace.keep! if scope.trace - Datadog::AppSec::Event.add_tags(scope, result) + Datadog::AppSec::Event.tag_and_keep!(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb index e617b915bda..ab1bfe612ec 100644 --- a/lib/datadog/appsec/contrib/rails/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/rails/gateway/watcher.rb @@ -40,7 +40,7 @@ def watch_request_action(gateway = Instrumentation.gateway) # We want to keep the trace in case of security event scope.trace.keep! if scope.trace - Datadog::AppSec::Event.add_tags(scope, result) + Datadog::AppSec::Event.tag_and_keep!(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb index 52c550dfeda..91383478c29 100644 --- a/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb +++ b/lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb @@ -42,7 +42,7 @@ def watch_request_dispatch(gateway = Instrumentation.gateway) # We want to keep the trace in case of security event scope.trace.keep! if scope.trace - Datadog::AppSec::Event.add_tags(scope, result) + Datadog::AppSec::Event.tag_and_keep!(scope, result) scope.processor_context.events << event end end @@ -84,7 +84,7 @@ def watch_request_routed(gateway = Instrumentation.gateway) # We want to keep the trace in case of security event scope.trace.keep! if scope.trace - Datadog::AppSec::Event.add_tags(scope, result) + Datadog::AppSec::Event.tag_and_keep!(scope, result) scope.processor_context.events << event end end diff --git a/lib/datadog/appsec/event.rb b/lib/datadog/appsec/event.rb index 45ed589311a..01f0de86d20 100644 --- a/lib/datadog/appsec/event.rb +++ b/lib/datadog/appsec/event.rb @@ -137,7 +137,10 @@ def build_service_entry_tags(event_group) end # rubocop:enable Metrics/MethodLength - def add_tags(scope, waf_result) + def tag_and_keep!(scope, waf_result) + # We want to keep the trace in case of security event + scope.trace.keep! if scope.trace + if scope.service_entry_span scope.service_entry_span.set_tag('appsec.blocked', 'true') if waf_result.actions.include?('block') scope.service_entry_span.set_tag('appsec.event', 'true') diff --git a/lib/datadog/appsec/monitor/gateway/watcher.rb b/lib/datadog/appsec/monitor/gateway/watcher.rb index efe09a0b899..74fc4d3fd60 100644 --- a/lib/datadog/appsec/monitor/gateway/watcher.rb +++ b/lib/datadog/appsec/monitor/gateway/watcher.rb @@ -37,7 +37,7 @@ def watch_user_id(gateway = Instrumentation.gateway) # We want to keep the trace in case of security event scope.trace.keep! if scope.trace - Datadog::AppSec::Event.add_tags(scope, result) + Datadog::AppSec::Event.tag_and_keep!(scope, result) scope.processor_context.events << event end end diff --git a/sig/datadog/appsec/event.rbs b/sig/datadog/appsec/event.rbs index ae4cca7ec49..b4b0e584513 100644 --- a/sig/datadog/appsec/event.rbs +++ b/sig/datadog/appsec/event.rbs @@ -14,7 +14,7 @@ module Datadog def self.build_service_entry_tags: (Array[Hash[::Symbol, untyped]] event_group) -> Hash[::String, untyped] - def self.add_tags: (Datadog::AppSec::Scope scope, Datadog::AppSec::WAF::Result waf_result) -> void + def self.tag_and_keep!: (Datadog::AppSec::Scope scope, Datadog::AppSec::WAF::Result waf_result) -> void private diff --git a/spec/datadog/appsec/event_spec.rb b/spec/datadog/appsec/event_spec.rb index 57ce4ce1d02..ba03bb3bda2 100644 --- a/spec/datadog/appsec/event_spec.rb +++ b/spec/datadog/appsec/event_spec.rb @@ -362,7 +362,7 @@ end end - describe '.add_tags' do + describe '.tag_and_keep!' do let(:with_trace) { true } let(:with_span) { true } @@ -397,7 +397,7 @@ # prevent rate limiter to bias tests Datadog::AppSec::RateLimiter.reset! - described_class.add_tags(scope, waf_result) + described_class.tag_and_keep!(scope, waf_result) end context 'with no actions' do From a2f973b7f5f4fff7da9c021c2a3f37d2114ff588 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 21 Oct 2024 15:46:24 +0200 Subject: [PATCH 35/36] Changed RuleSampler initialization with ASM Standalone to Tracing::Component.build_sampler --- lib/datadog/tracing/component.rb | 13 ++++++++++++ lib/datadog/tracing/sampling/rule_sampler.rb | 21 +++++-------------- .../tracing/sampling/rule_sampler_spec.rb | 14 ------------- 3 files changed, 18 insertions(+), 30 deletions(-) diff --git a/lib/datadog/tracing/component.rb b/lib/datadog/tracing/component.rb index 96c91ad48f2..34744e53ef6 100644 --- a/lib/datadog/tracing/component.rb +++ b/lib/datadog/tracing/component.rb @@ -73,6 +73,19 @@ def build_sampler(settings) return sampler end + # AppSec events are sent to the backend using traces. + # Standalone ASM billing means that we don't want to charge clients for APM traces, + # so we want to send the minimum amount of traces possible (idealy only traces that contains security events), + # but for features such as API Security, we need to send at least one trace per minute, + # to keep the service alive on the backend side. + if settings.appsec.standalone.enabled + post_sampler = Tracing::Sampling::RuleSampler.new( + [Tracing::Sampling::SimpleRule.new(sample_rate: 1.0)], + rate_limiter: Datadog::Core::TokenBucket.new(1.0 / 60, 1.0), + default_sample_rate: 1.0 / 60 + ) + end + # Sampling rules are provided if (rules = settings.tracing.sampling.rules) post_sampler = Tracing::Sampling::RuleSampler.parse( diff --git a/lib/datadog/tracing/sampling/rule_sampler.rb b/lib/datadog/tracing/sampling/rule_sampler.rb index 2afc79d534a..a6542edc9f3 100644 --- a/lib/datadog/tracing/sampling/rule_sampler.rb +++ b/lib/datadog/tracing/sampling/rule_sampler.rb @@ -29,34 +29,23 @@ def initialize( default_sample_rate: Datadog.configuration.tracing.sampling.default_rate, default_sampler: nil ) - # AppSec events are sent to the backend using traces. - # Standalone ASM billing means that we don't want to charge clients for APM traces, - # so we want to send the minimum amount of traces possible (idealy only traces that contains security events), - # but for features such as API Security, we need to send at least one trace per minute, - # to keep the service alive on the backend side. - @rules = if Datadog.configuration.appsec.standalone.enabled - [SimpleRule.new(sample_rate: 1.0)] - elsif default_sample_rate && !default_sampler + @rules = if default_sample_rate && !default_sampler # Add to the end of the rule list a rule always matches any trace rules << SimpleRule.new(sample_rate: default_sample_rate) else rules end - @rate_limiter = if Datadog.configuration.appsec.standalone.enabled - # 1 trace per minute - Core::TokenBucket.new(1.0 / 60, 1.0) - elsif rate_limiter + @rate_limiter = if rate_limiter rate_limiter elsif rate_limit Core::TokenBucket.new(rate_limit) else Core::UnlimitedLimiter.new end - @default_sampler = if Datadog.configuration.appsec.standalone.enabled || - (default_sample_rate && !default_sampler) - nil - elsif default_sampler + @default_sampler = if default_sampler default_sampler + elsif default_sample_rate + nil else # TODO: Simplify .tags access, as `Tracer#tags` can't be arbitrarily changed anymore RateByServiceSampler.new(1.0, env: -> { Tracing.send(:tracer).tags['env'] }) diff --git a/spec/datadog/tracing/sampling/rule_sampler_spec.rb b/spec/datadog/tracing/sampling/rule_sampler_spec.rb index 7ff2b8c95b7..d4eb379f82d 100644 --- a/spec/datadog/tracing/sampling/rule_sampler_spec.rb +++ b/spec/datadog/tracing/sampling/rule_sampler_spec.rb @@ -42,20 +42,6 @@ it_behaves_like 'a token bucket rate limiter', rate: 100 it { expect(rule_sampler.default_sampler).to be_a(Datadog::Tracing::Sampling::RateByServiceSampler) } - context 'with appsec standalone enabled' do - before do - allow(Datadog.configuration.appsec.standalone).to receive(:enabled).and_return(true) - end - - it_behaves_like 'a simple rule that matches all span operations', sample_rate: 1.0 do - let(:rule) { rule_sampler.rules.last } - end - - it_behaves_like 'a token bucket rate limiter', rate: 1.0 / 60, max_tokens: 1.0 - - it { expect(rule_sampler.default_sampler).to be_nil } - end - context 'with rate_limit ENV' do before do allow(Datadog.configuration.tracing.sampling).to receive(:rate_limit) From 8e54f7db38e6f49a712564f02123edf215f51c19 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 21 Oct 2024 16:05:53 +0200 Subject: [PATCH 36/36] revert system-tests branch to main --- .github/workflows/system-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index 5d89c63b2a6..3b9ea847e79 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -11,7 +11,7 @@ on: env: REGISTRY: ghcr.io REPO: ghcr.io/datadog/dd-trace-rb - ST_REF: vpellan/ruby-asm-standalone + ST_REF: main FORCE_TESTS: -F tests/appsec/test_asm_standalone.py FORCE_TESTS_SCENARIO: APPSEC_STANDALONE