diff --git a/Gemfile b/Gemfile index ccf97b60..22c6d8be 100644 --- a/Gemfile +++ b/Gemfile @@ -22,7 +22,7 @@ gem 'sentry-raven' gem 'yard-sinatra', github: 'rkh/yard-sinatra' gem 'rack-contrib' gem 'rack-cache', github: 'rtomayko/rack-cache' -gem 'rack-attack' +gem 'rack-attack', '5.0.0.beta1' gem 'gh' gem 'bunny', '~> 0.8.0' gem 'dalli' @@ -33,6 +33,7 @@ gem 'micro_migrations' gem 'simplecov' gem 'skylight', '~> 0.6.0.beta.1' gem 'stackprof' +gem 'netaddr' gem 'jemalloc' gem 'customerio' diff --git a/Gemfile.lock b/Gemfile.lock index 96104e61..0238e071 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -260,6 +260,7 @@ GEM multipart-post (2.0.0) net-http-persistent (2.9.4) net-http-pipeline (1.0.1) + netaddr (1.5.1) os (0.9.6) pg (0.18.4) proxies (0.2.1) @@ -273,7 +274,7 @@ GEM pusher-signature (~> 0.1.8) pusher-signature (0.1.8) rack (1.4.7) - rack-attack (4.4.1) + rack-attack (5.0.0.beta1) rack rack-contrib (1.4.0) git-version-bump (~> 0.15) @@ -397,8 +398,9 @@ DEPENDENCIES micro_migrations mocha (~> 0.12) mustermann! + netaddr pry - rack-attack + rack-attack (= 5.0.0.beta1) rack-cache! rack-contrib rake (~> 0.9.2) diff --git a/lib/travis/api/app.rb b/lib/travis/api/app.rb index 1e1ec814..d61a498b 100644 --- a/lib/travis/api/app.rb +++ b/lib/travis/api/app.rb @@ -132,7 +132,7 @@ module Travis::Api use Travis::Api::App::Middleware::UserAgentTracker # make sure this is below ScopeCheck so we have the token - use Rack::Attack if Endpoint.production? + use Rack::Attack if Endpoint.production? and not Travis.config.enterprise # if this is a v3 API request, ignore everything after use Travis::API::V3::OptIn diff --git a/lib/travis/api/app/endpoint/authorization.rb b/lib/travis/api/app/endpoint/authorization.rb index 3cf72469..30d85546 100644 --- a/lib/travis/api/app/endpoint/authorization.rb +++ b/lib/travis/api/app/endpoint/authorization.rb @@ -114,7 +114,7 @@ class Travis::Api::App # access token and user payload to the parent window via postMessage. # # However, the endpoint to send the payload to has to be explicitely - # whitelisted in production, as this is endpoint is only meant to be used + # safelisted in production, as this is endpoint is only meant to be used # with the official Travis CI client at the moment. # # Example usage: @@ -408,7 +408,7 @@ __END__ @@ invalid_target @@ common diff --git a/lib/travis/api/attack.rb b/lib/travis/api/attack.rb index 4bf4fd5d..63dbc405 100644 --- a/lib/travis/api/attack.rb +++ b/lib/travis/api/attack.rb @@ -1,4 +1,5 @@ require 'rack/attack' +require 'netaddr' class Rack::Attack class Request @@ -31,29 +32,38 @@ class Rack::Attack "/auth/post_message/iframe" ] - whitelist('safelist build status images') do |request| + GITHUB_CIDR = NetAddr::CIDR.create('192.30.252.0/22') + + safelist('safelist build status images') do |request| /\.(png|svg)$/.match(request.path) end + # https://help.github.com/articles/what-ip-addresses-does-github-use-that-i-should-safelist/ + safelist('safelist anything coming from github') do |request| + request.ip && GITHUB_CIDR.contains?(request.ip) + end + #### # Whitelisted IP addresses - whitelist('whitelist client requesting from redis') do |request| - Travis.redis.sismember(:api_whitelisted_ips, request.ip) + safelist('safelist client requesting from redis') do |request| + # TODO: deprecate :api_whitelisted_ips in favour of api_safelisted_ips + Travis.redis.sismember(:api_whitelisted_ips, request.ip) || Travis.redis.sismember(:api_safelisted_ips, request.ip) end #### # Ban based on: IP address # Ban time: indefinite # Ban after: manually banned - blacklist('block client requesting from redis') do |request| - Travis.redis.sismember(:api_blacklisted_ips, request.ip) + blocklist('block client requesting from redis') do |request| + # TODO: deprecate :api_blacklisted_ips in favour of api_blocklisted_ips + Travis.redis.sismember(:api_blacklisted_ips, request.ip) || Travis.redis.sismember(:api_blocklisted_ips, request.ip) end #### # Ban based on: IP address or access token # Ban time: 5 hours # Ban after: 10 POST requests within five minutes to /auth/github - blacklist('hammering /auth/github') do |request| + blocklist('hammering /auth/github') do |request| Rack::Attack::Allow2Ban.filter(request.identifier, maxretry: 2, findtime: 5.minutes, bantime: bantime(5.hours)) do request.post? and request.path == '/auth/github' end @@ -63,7 +73,7 @@ class Rack::Attack # Ban based on: IP address or access token # Ban time: 1 hour # Ban after: 10 POST requests within 30 seconds - blacklist('spamming with POST requests') do |request| + blocklist('spamming with POST requests') do |request| Rack::Attack::Allow2Ban.filter(request.identifier, maxretry: 10, findtime: 30.seconds, bantime: bantime(1.hour)) do request.post? and not POST_SAFELIST.include? request.path end diff --git a/lib/travis/api/v3/router.rb b/lib/travis/api/v3/router.rb index 91c5fd4a..2d29b9f1 100644 --- a/lib/travis/api/v3/router.rb +++ b/lib/travis/api/v3/router.rb @@ -29,7 +29,7 @@ module Travis::API::V3 result = service.run metrics.tick(:service) - env_params.each_key { |key| result.ignored_param(key, reason: "not whitelisted".freeze) unless filtered.include?(key) } + env_params.each_key { |key| result.ignored_param(key, reason: "not safelisted".freeze) unless filtered.include?(key) } response = render(result, env_params, env) metrics.tick(:renderer) diff --git a/spec/unit/attack_spec.rb b/spec/unit/attack_spec.rb index 7bae3546..0f5dec39 100644 --- a/spec/unit/attack_spec.rb +++ b/spec/unit/attack_spec.rb @@ -6,18 +6,31 @@ describe Rack::Attack do } it 'should be safelisted' do - expect(Rack::Attack.whitelisted?(request)).to be_truthy + expect(Rack::Attack.safelisted?(request)).to be_truthy end end - describe 'non-image API request' do + describe 'request from GitHub ip' do + let(:request) { + env = Rack::MockRequest.env_for("https://api-test.travis-ci.org/repos/rails/rails/branches", { + 'REMOTE_ADDR' => '192.30.252.42', + }) + Rack::Attack::Request.new(env) + } + + it 'should be safelisted' do + expect(Rack::Attack.safelisted?(request)).to be_truthy + end + end + + describe 'non-safelisted request' do let(:request) { env = Rack::MockRequest.env_for("https://api-test.travis-ci.org/repos/rails/rails/branches") Rack::Attack::Request.new(env) } it 'should not be safelisted' do - expect(Rack::Attack.whitelisted?(request)).to be_falsy + expect(Rack::Attack.safelisted?(request)).to be_falsy end end end diff --git a/spec/v3/services/owner/find_spec.rb b/spec/v3/services/owner/find_spec.rb index 5bb8e591..a15e82f1 100644 --- a/spec/v3/services/owner/find_spec.rb +++ b/spec/v3/services/owner/find_spec.rb @@ -167,7 +167,7 @@ describe Travis::API::V3::Services::Owner::Find, set_app: true do "avatar_url" => nil, "@warnings" => [{ "@type" => "warning", - "message" => "query parameter organization.id not whitelisted, ignored", + "message" => "query parameter organization.id not safelisted, ignored", "warning_type" => "ignored_parameter", "parameter" => "organization.id"}] }} @@ -254,7 +254,7 @@ describe Travis::API::V3::Services::Owner::Find, set_app: true do "synced_at" => nil, "@warnings" => [{ "@type" => "warning", - "message" => "query parameter user.id not whitelisted, ignored", + "message" => "query parameter user.id not safelisted, ignored", "warning_type" => "ignored_parameter", "parameter" => "user.id"}] }} diff --git a/spec_core/model/job_spec.rb b/spec_core/model/job_spec.rb index 14c02f46..f5706e02 100644 --- a/spec_core/model/job_spec.rb +++ b/spec_core/model/job_spec.rb @@ -129,7 +129,7 @@ describe Job do } end - it 'removes addons items which are not whitelisted' do + it 'removes addons items which are not safelisted' do job = Job.new(repository: repo) config = { rvm: '1.8.7', addons: { sauce_connect: true, firefox: '22.0' }, @@ -328,7 +328,7 @@ describe Job do } end - it 'removes addons items which are not whitelisted' do + it 'removes addons items which are not safelisted' do config = { rvm: '1.8.7', addons: { sauce_connect: { diff --git a/vendor/travis-core/lib/travis/model/job.rb b/vendor/travis-core/lib/travis/model/job.rb index 1d86e278..1aac654e 100644 --- a/vendor/travis-core/lib/travis/model/job.rb +++ b/vendor/travis-core/lib/travis/model/job.rb @@ -14,7 +14,7 @@ class Job < Travis::Model require 'travis/model/job/test' require 'travis/model/env_helpers' - WHITELISTED_ADDONS = %w( + SAFELISTED_ADDONS = %w( apt apt_packages apt_sources @@ -167,7 +167,7 @@ class Job < Travis::Model def delete_addons(config) if config[:addons].is_a?(Hash) - config[:addons].keep_if { |key, _| WHITELISTED_ADDONS.include? key.to_s } + config[:addons].keep_if { |key, _| SAFELISTED_ADDONS.include? key.to_s } else config.delete(:addons) end