Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass HTTP client IP to WAF #2316

Merged
merged 17 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -388,14 +388,13 @@ task :ci do
# AppSec contrib specs
declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ jruby' => 'bundle exec appraisal contrib rake spec:appsec:rack'
declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ jruby' => 'bundle exec appraisal contrib rake spec:appsec:sinatra'
declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ✅ jruby' => 'bundle exec appraisal rails4-mysql2 rake spec:rails'

# AppSec Rails specs
declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails32-mysql2 rake spec:rails'
declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails4-mysql2 rake spec:appsec:rails'
declare '❌ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails5-mysql2 rake spec:appsec:rails'
declare '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails6-mysql2 rake spec:appsec:rails'
declare '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails61-mysql2 rake spec:appsec:rails'
declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails32-mysql2 rake spec:rails'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to disable JRuby for Tracing tests? bundle exec appraisal rails32-mysql2 rake spec:rails

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good catch @marcotc, I think I fat-fingered that one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the fat finger was in the original PR that added it? This actually seems duplicated from the non-appsec section 😱

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, actually:

diff --git a/Rakefile b/Rakefile
index 9b854bd151..c0b07de3d3 100644
--- a/Rakefile
+++ b/Rakefile
@@ -388,7 +388,6 @@ task :ci do
   # AppSec contrib specs
   declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ jruby' => 'bundle exec appraisal contrib rake spec:appsec:rack'
   declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ jruby' => 'bundle exec appraisal contrib rake spec:appsec:sinatra'
-  declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ✅ jruby' => 'bundle exec appraisal rails4-mysql2 rake spec:rails'
 
   # AppSec Rails specs
   declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ✅ jruby' => 'bundle exec appraisal rails32-mysql2 rake spec:rails'

☝️ This diff removed an extra line that was duplicated.

diff --git a/Rakefile b/Rakefile
index c0b07de3d3..30fd9257e6 100644
--- a/Rakefile
+++ b/Rakefile
@@ -390,11 +390,11 @@ task :ci do
   declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ jruby' => 'bundle exec appraisal contrib rake spec:appsec:sinatra'
 
   # AppSec Rails specs
-  declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ✅ jruby' => 'bundle exec appraisal rails32-mysql2 rake spec:rails'
-  declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ✅ jruby' => 'bundle exec appraisal rails4-mysql2 rake spec:appsec:rails'
-  declare '❌ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ✅ jruby' => 'bundle exec appraisal rails5-mysql2 rake spec:appsec:rails'
-  declare '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ✅ jruby' => 'bundle exec appraisal rails6-mysql2 rake spec:appsec:rails'
-  declare '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ❌ 3.2 / ✅ jruby' => 'bundle exec appraisal rails61-mysql2 rake spec:appsec:rails'
+  declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ❌ jruby' => 'bundle exec appraisal rails32-mysql2 rake spec:rails'
+  declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ❌ jruby' => 'bundle exec appraisal rails4-mysql2 rake spec:appsec:rails'
+  declare '❌ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ❌ jruby' => 'bundle exec appraisal rails5-mysql2 rake spec:appsec:rails'
+  declare '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / ❌ jruby' => 'bundle exec appraisal rails6-mysql2 rake spec:appsec:rails'
+  declare '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ jruby' => 'bundle exec appraisal rails61-mysql2 rake spec:appsec:rails'
 end
 
 namespace :coverage do

☝️ This diff had the corresponding line that was duplicated. Notice though that both are spec:rails... instead of spec:appsec:rails, which is the actual mistake!

declare '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails4-mysql2 rake spec:appsec:rails'
declare '❌ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails5-mysql2 rake spec:appsec:rails'
declare '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ❌ 3.0 / ❌ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails6-mysql2 rake spec:appsec:rails'
declare '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ❌ 3.2 / jruby' => 'bundle exec appraisal rails61-mysql2 rake spec:appsec:rails'
end

namespace :coverage do
Expand Down
4 changes: 2 additions & 2 deletions lib/datadog/appsec/assets.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ def waf_rules(kind = :recommended)
read("waf_rules/#{kind}.json")
end

def blocked
@blocked ||= read('blocked.html')
def blocked(format: :html)
(@blocked ||= {})[format] ||= read("blocked.#{format}")
end

def path
Expand Down
101 changes: 98 additions & 3 deletions lib/datadog/appsec/assets/blocked.html

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions lib/datadog/appsec/assets/blocked.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"errors": [{"title": "You've been blocked", "detail": "Sorry, you cannot access this page. Please contact the customer service team. Security provided by Datadog."}]}
5 changes: 5 additions & 0 deletions lib/datadog/appsec/assets/blocked.text
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
You've been blocked
lloeki marked this conversation as resolved.
Show resolved Hide resolved

Sorry, you cannot access this page. Please contact the customer service team.

Security provided by Datadog.
4 changes: 4 additions & 0 deletions lib/datadog/appsec/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ def ruleset=(value)
options[:ruleset] = value
end

def ip_denylist=(value)
options[:ip_denylist] = value
end

# in microseconds
def waf_timeout=(value)
options[:waf_timeout] = value
Expand Down
6 changes: 6 additions & 0 deletions lib/datadog/appsec/configuration/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ def ruleset
@options[:ruleset]
end

# EXPERIMENTAL: This configurable is not meant to be publicly used, but
# is very useful for testing. It may change at any point in time.
def ip_denylist
lloeki marked this conversation as resolved.
Show resolved Hide resolved
@options[:ip_denylist]
end

def waf_timeout
@options[:waf_timeout]
end
Expand Down
12 changes: 4 additions & 8 deletions lib/datadog/appsec/contrib/rack/reactive/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ def self.publish(op, request)
op.publish('request.headers', Rack::Request.headers(request))
op.publish('request.uri.raw', Rack::Request.url(request))
op.publish('request.cookies', Rack::Request.cookies(request))
# op.publish('request.body.raw', Rack::Request.body(request))
# TODO: op.publish('request.path_params', { k: v }) # route params only?
# TODO: op.publish('request.path', request.script_name + request.path) # unused for now
op.publish('request.client_ip', Rack::Request.client_ip(request))

nil
end
Expand All @@ -30,8 +28,7 @@ def self.subscribe(op, waf_context)
'request.uri.raw',
'request.query',
'request.cookies',
# 'request.body.raw',
# TODO: 'request.path_params',
'request.client_ip',
]

op.subscribe(*addresses) do |*values|
Expand All @@ -41,16 +38,15 @@ def self.subscribe(op, waf_context)
uri_raw = values[1]
query = values[2]
cookies = values[3]
# body = values[4]
client_ip = values[4]

waf_args = {
'server.request.cookies' => cookies,
# 'server.request.body.raw' => body,
'server.request.query' => query,
'server.request.uri.raw' => uri_raw,
'server.request.headers' => headers,
'server.request.headers.no_cookies' => headers_no_cookies,
# TODO: 'server.request.path_params' => path_params,
'http.client_ip' => client_ip,
}

waf_timeout = Datadog::AppSec.settings.waf_timeout
Expand Down
17 changes: 17 additions & 0 deletions lib/datadog/appsec/contrib/rack/request.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# typed: true

require_relative '../../../tracing/client_ip'
require_relative '../../../tracing/contrib/rack/header_collection'

module Datadog
module AppSec
module Contrib
Expand Down Expand Up @@ -54,6 +57,20 @@ def self.form_hash(request)
# Hash<String,String||Array||Hash> when e.g coming from JSON
request.env['rack.request.form_hash']
end

def self.client_ip(request)
remote_ip = request.env['REMOTE_ADDR']
headers = Datadog::Tracing::Contrib::Rack::Header::RequestHeaderCollection.new(request.env)

result = Datadog::Tracing::ClientIp.raw_ip_from_request(headers, remote_ip)

if result.raw_ip
ip = Datadog::Tracing::ClientIp.strip_decorations(result.raw_ip)
return unless Datadog::Tracing::ClientIp.valid_ip?(ip)

ip
end
end
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/datadog/appsec/contrib/rack/request_body_middleware.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# typed: ignore

require_relative '../../instrumentation/gateway'
require_relative '../../assets'
require_relative '../../response'

module Datadog
module AppSec
Expand Down Expand Up @@ -29,7 +29,7 @@ def call(env)
end

if request_response && request_response.any? { |action, _event| action == :block }
request_return = [403, { 'Content-Type' => 'text/html' }, [Datadog::AppSec::Assets.blocked]]
request_return = AppSec::Response.negotiate(env).to_rack
end

request_return
Expand Down
4 changes: 2 additions & 2 deletions lib/datadog/appsec/contrib/rack/request_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

require_relative '../../instrumentation/gateway'
require_relative '../../processor'
require_relative '../../assets'
require_relative '../../response'

require_relative '../../../tracing/client_ip'
require_relative '../../../tracing/contrib/rack/header_collection'
Expand Down Expand Up @@ -40,7 +40,7 @@ def call(env)
end

if request_response && request_response.any? { |action, _event| action == :block }
request_return = [403, { 'Content-Type' => 'text/html' }, [Datadog::AppSec::Assets.blocked]]
request_return = AppSec::Response.negotiate(env).to_rack
end

response = ::Rack::Response.new(request_return[2], request_return[0], request_return[1])
Expand Down
7 changes: 2 additions & 5 deletions lib/datadog/appsec/contrib/rails/patcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

require_relative '../patcher'
require_relative 'framework'
require_relative '../../response'
require_relative '../rack/request_middleware'
require_relative '../rack/request_body_middleware'
require_relative 'gateway/watcher'
Expand Down Expand Up @@ -83,11 +84,7 @@ def process_action(*args)
end

if request_response && request_response.any? { |action, _event| action == :block }
@_response = ::ActionDispatch::Response.new(
403,
{ 'Content-Type' => 'text/html' },
[Datadog::AppSec::Assets.blocked]
)
@_response = AppSec::Response.negotiate(env).to_action_dispatch_response
request_return = @_response.body
end

Expand Down
1 change: 1 addition & 0 deletions lib/datadog/appsec/contrib/sinatra/ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module Sinatra
module Ext
APP = 'sinatra'.freeze
ENV_ENABLED = 'DD_TRACE_SINATRA_ENABLED'.freeze
ROUTE_INTERRUPT = :datadog_appsec_contrib_sinatra_route_interrupt
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/datadog/appsec/contrib/sinatra/gateway/watcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module AppSec
module Contrib
module Sinatra
module Gateway
# Watcher for Rails gateway events
# Watcher for Sinatra gateway events
module Watcher
# rubocop:disable Metrics/MethodLength
def self.watch
Expand Down
19 changes: 11 additions & 8 deletions lib/datadog/appsec/contrib/sinatra/patcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require_relative '../../../tracing/contrib/rack/middlewares'

require_relative '../patcher'
require_relative '../../response'
require_relative '../rack/request_middleware'
require_relative 'framework'
require_relative 'gateway/watcher'
Expand Down Expand Up @@ -57,15 +58,12 @@ def dispatch!
# TODO: handle exceptions, except for super

request_return, request_response = Instrumentation.gateway.push('sinatra.request.dispatch', request) do
super
# handle process_route interruption
catch(Ext::ROUTE_INTERRUPT) { super }
end

if request_response && request_response.any? { |action, _event| action == :block }
self.response = ::Sinatra::Response.new(
[Datadog::AppSec::Assets.blocked],
403,
{ 'Content-Type' => 'text/html' }
)
self.response = AppSec::Response.negotiate(env).to_sinatra_response
request_return = nil
end

Expand Down Expand Up @@ -94,9 +92,14 @@ def process_route(*)
# At this point params has both route params and normal params.
route_params = params.each.with_object({}) { |(k, v), h| h[k] = v unless base_params.key?(k) }

Instrumentation.gateway.push('sinatra.request.routed', [request, route_params])
_, request_response = Instrumentation.gateway.push('sinatra.request.routed', [request, route_params])

# TODO: handle block
if request_response && request_response.any? { |action, _event| action == :block }
self.response = AppSec::Response.negotiate(env).to_sinatra_response

# interrupt request and return response to dispatch! for consistency
throw(Ext::ROUTE_INTERRUPT, response)
end

yield(*args)
end
Expand Down
10 changes: 10 additions & 0 deletions lib/datadog/appsec/extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ def ruleset=(arg)
@settings.merge(dsl)
end

def ip_denylist=(arg)
dsl = AppSec::Configuration::DSL.new
dsl.ip_denylist = arg
@settings.merge(dsl)
end

def instrument(*args)
dsl = AppSec::Configuration::DSL.new
dsl.instrument(*args)
Expand Down Expand Up @@ -86,6 +92,10 @@ def ruleset
@settings.ruleset
end

def ruledata
@settings.ruledata
end

def waf_timeout
@settings.waf_timeout
end
Expand Down
18 changes: 18 additions & 0 deletions lib/datadog/appsec/processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ def initialize

unless load_libddwaf && load_ruleset && create_waf_handle
Datadog.logger.warn { 'AppSec is disabled, see logged errors above' }

return
end

update_ip_denylist
end

def ready?
Expand All @@ -76,6 +80,20 @@ def toggle_rules(map)
@handle.toggle_rules(map)
end

def update_ip_denylist(denylist = Datadog::AppSec.settings.ip_denylist, id: 'blocked_ips')
denylist ||= []

ruledata_setting = [
{
'id' => id,
'type' => 'data_with_expiration',
'data' => denylist.map { |ip| { 'value' => ip.to_s, 'expiration' => 2**63 } }
}
]

update_rule_data(ruledata_setting)
end

def finalize
@handle.finalize
end
Expand Down
54 changes: 54 additions & 0 deletions lib/datadog/appsec/response.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# typed: false

require_relative 'assets'

module Datadog
module AppSec
# AppSec response
class Response
attr_reader :status, :headers, :body

def initialize(status:, headers: {}, body: [])
@status = status
@headers = headers
@body = body
end

def to_rack
[status, headers, body]
end

def to_sinatra_response
::Sinatra::Response.new(body, status, headers)
end

def to_action_dispatch_response
::ActionDispatch::Response.new(status, headers, body)
end

class << self
def negotiate(env)
Response.new(
status: 403,
headers: { 'Content-Type' => 'text/html' },
body: [Datadog::AppSec::Assets.blocked(format: format(env))]
)
end

private

def format(env)
format = env['HTTP_ACCEPT'] && env['HTTP_ACCEPT'].split(',').any? do |accept|
if accept.start_with?('text/html')
break :html
elsif accept.start_with?('application/json')
break :json
end
end

format || :text
end
end
end
end
end
Loading