diff options
Diffstat (limited to 'gems')
59 files changed, 4167 insertions, 38 deletions
diff --git a/gems/activerecord-gitlab/lib/active_record/gitlab_patches/rescue_from.rb b/gems/activerecord-gitlab/lib/active_record/gitlab_patches/rescue_from.rb index eaa42d1523d..faabddd2686 100644 --- a/gems/activerecord-gitlab/lib/active_record/gitlab_patches/rescue_from.rb +++ b/gems/activerecord-gitlab/lib/active_record/gitlab_patches/rescue_from.rb @@ -3,8 +3,10 @@ module ActiveRecord module GitlabPatches # This adds `rescue_from` to ActiveRecord::Base. - # Currently, only errors called from `ActiveRecord::Relation#exec_queries` - # will be handled by `rescue_from`. + # Currently only the following places will be handled by `rescue_from`: + # + # - `ActiveRecord::Relation#load`, and other methods that call + # `ActiveRecord::Relation#exec_queries`. # # class ApplicationRecord < ActiveRecord::Base # rescue_from MyException, with: :my_handler diff --git a/gems/activerecord-gitlab/spec/active_record/gitlab_patches/rescue_from_spec.rb b/gems/activerecord-gitlab/spec/active_record/gitlab_patches/rescue_from_spec.rb index c1537c3bd90..22729edb014 100644 --- a/gems/activerecord-gitlab/spec/active_record/gitlab_patches/rescue_from_spec.rb +++ b/gems/activerecord-gitlab/spec/active_record/gitlab_patches/rescue_from_spec.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true -RSpec.describe ActiveRecord::GitlabPatches::RescueFrom, :without_sqlite3 do +RSpec.describe ActiveRecord::GitlabPatches::RescueFrom do let(:model_with_rescue_from) do - Class.new(ActiveRecord::Base) do - rescue_from ActiveRecord::ConnectionNotEstablished, with: :handle_exception + Class.new(Project) do + rescue_from ActiveRecord::StatementInvalid, with: :handle_exception class << self def handle_exception(exception); end @@ -12,20 +12,18 @@ RSpec.describe ActiveRecord::GitlabPatches::RescueFrom, :without_sqlite3 do end let(:model_without_rescue_from) do - Class.new(ActiveRecord::Base) + Class.new(Project) end - it 'triggers rescue_from' do - stub_const('ModelWithRescueFrom', model_with_rescue_from) + context 'for errors from ActiveRelation.load' do + it 'triggers rescue_from' do + expect(model_with_rescue_from).to receive(:handle_exception) - expect(model_with_rescue_from).to receive(:handle_exception) - - expect { model_with_rescue_from.all.load }.not_to raise_error - end - - it 'does not trigger rescue_from' do - stub_const('ModelWithoutRescueFrom', model_without_rescue_from) + expect { model_with_rescue_from.where('BADQUERY').load }.not_to raise_error + end - expect { model_without_rescue_from.all.load }.to raise_error(ActiveRecord::ConnectionNotEstablished) + it 'does not trigger rescue_from' do + expect { model_without_rescue_from.where('BADQUERY').load }.to raise_error(ActiveRecord::StatementInvalid) + end end end diff --git a/gems/click_house-client/lib/click_house/client.rb b/gems/click_house-client/lib/click_house/client.rb index abc54f2bce0..1ca3653c45f 100644 --- a/gems/click_house-client/lib/click_house/client.rb +++ b/gems/click_house-client/lib/click_house/client.rb @@ -6,6 +6,9 @@ require 'active_support/time' require 'active_support/notifications' require_relative "client/database" require_relative "client/configuration" +require_relative "client/bind_index_manager" +require_relative "client/query_like" +require_relative "client/query" require_relative "client/formatter" require_relative "client/response" @@ -25,6 +28,7 @@ module ClickHouse Error = Class.new(StandardError) ConfigurationError = Class.new(Error) DatabaseError = Class.new(Error) + QueryError = Class.new(Error) # Executes a SELECT database query def self.select(query, database, configuration = self.configuration) @@ -40,15 +44,52 @@ module ClickHouse # Executes any kinds of database query without returning any data (INSERT, DELETE) def self.execute(query, database, configuration = self.configuration) instrumented_execute(query, database, configuration) do |response, instrument| - if response.headers['x-clickhouse-summary'] - instrument[:statistics] = - Gitlab::Json.parse(response.headers['x-clickhouse-summary']).symbolize_keys - end + expose_summary(response.headers, instrument) end true end + # Inserts a gzip-compressed CSV to ClickHouse + # + # Usage: + # + # Create a compressed CSV file: + # > File.binwrite("my_csv.csv", ActiveSupport::Gzip.compress("id\n10\n20")) + # + # Invoke the INSERT query: + # > ClickHouse::Client.insert_csv('INSERT INTO events (id) FORMAT CSV', File.open("my_csv.csv"), :main) + def self.insert_csv(query, io, database, configuration = self.configuration) + db = lookup_database(configuration, database) + + headers = db.headers.merge( + 'Transfer-Encoding' => 'chunked', + 'Content-Length' => File.size(io).to_s, + 'Content-Encoding' => 'gzip' + ) + + query = ClickHouse::Client::Query.build(query) + ActiveSupport::Notifications.instrument('sql.click_house', { query: query, database: database }) do |instrument| + response = configuration.http_post_proc.call( + db.build_custom_uri(extra_variables: { query: query.to_sql }).to_s, + headers, + io + ) + raise DatabaseError, response.body unless response.success? + + expose_summary(response.headers, instrument) + end + + true + end + + private_class_method def self.expose_summary(headers, instrument) + return unless headers['x-clickhouse-summary'] + + instrument[:statistics] = + Gitlab::Json.parse(headers['x-clickhouse-summary']).symbolize_keys + end + private_class_method def self.lookup_database(configuration, database) configuration.databases[database].tap do |db| raise ConfigurationError, "The database '#{database}' is not configured" unless db @@ -58,11 +99,21 @@ module ClickHouse private_class_method def self.instrumented_execute(query, database, configuration) db = lookup_database(configuration, database) + query = ClickHouse::Client::Query.build(query) + + log_contents = configuration.log_proc.call(query) + configuration.logger.info(log_contents) + ActiveSupport::Notifications.instrument('sql.click_house', { query: query, database: database }) do |instrument| + # Use a multipart POST request where the placeholders are sent with the param_ prefix + # See: https://github.com/ClickHouse/ClickHouse/issues/8842 + query_with_params = query.placeholders.transform_keys { |key| "param_#{key}" } + query_with_params['query'] = query.to_sql + response = configuration.http_post_proc.call( db.uri.to_s, db.headers, - query + query_with_params ) raise DatabaseError, response.body unless response.success? diff --git a/gems/click_house-client/lib/click_house/client/bind_index_manager.rb b/gems/click_house-client/lib/click_house/client/bind_index_manager.rb new file mode 100644 index 00000000000..618b13f2fd7 --- /dev/null +++ b/gems/click_house-client/lib/click_house/client/bind_index_manager.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module ClickHouse + module Client + class BindIndexManager + def initialize(start_index = 1) + @current_index = start_index + end + + def next_bind_str + bind_str = "$#{@current_index}" + @current_index += 1 + bind_str + end + end + end +end diff --git a/gems/click_house-client/lib/click_house/client/configuration.rb b/gems/click_house-client/lib/click_house/client/configuration.rb index 882b37993dc..4a71934466f 100644 --- a/gems/click_house-client/lib/click_house/client/configuration.rb +++ b/gems/click_house-client/lib/click_house/client/configuration.rb @@ -18,6 +18,9 @@ module ClickHouse # # *json_parser*: object for parsing JSON strings, it should respond to the "parse" method # + # *logger*: object for receiving logger commands. Default `$stdout` + # *log_proc*: any output (e.g. structure) to wrap around the query for every statement + # # Example: # # Gitlab::ClickHouse::Client.configure do |c| @@ -31,6 +34,11 @@ module ClickHouse # } # ) # + # c.logger = MyLogger.new + # c.log_proc = ->(query) do + # { query_body: query.to_redacted_sql } + # end + # # c.http_post_proc = lambda do |url, headers, body| # options = { # headers: headers, @@ -44,13 +52,15 @@ module ClickHouse # # c.json_parser = JSON # end - attr_accessor :http_post_proc, :json_parser + attr_accessor :http_post_proc, :json_parser, :logger, :log_proc attr_reader :databases def initialize @databases = {} @http_post_proc = nil @json_parser = JSON + @logger = ::Logger.new($stdout) + @log_proc = ->(query) { query.to_sql } end def register_database(name, **args) diff --git a/gems/click_house-client/lib/click_house/client/database.rb b/gems/click_house-client/lib/click_house/client/database.rb index faf5a953a12..9fac0df8d87 100644 --- a/gems/click_house-client/lib/click_house/client/database.rb +++ b/gems/click_house-client/lib/click_house/client/database.rb @@ -17,19 +17,20 @@ module ClickHouse end def uri - @uri ||= begin - parsed = Addressable::URI.parse(@url) - parsed.query_values = @variables - parsed - end + @uri ||= build_custom_uri + end + + def build_custom_uri(extra_variables: {}) + parsed = Addressable::URI.parse(@url) + parsed.query_values = @variables.merge(extra_variables) + parsed end def headers @headers ||= { 'X-ClickHouse-User' => @username, 'X-ClickHouse-Key' => @password, - 'X-ClickHouse-Format' => 'JSON', # always return JSON data - 'Content-Encoding' => 'gzip' # tell the server that we send compressed data + 'X-ClickHouse-Format' => 'JSON' # always return JSON data }.freeze end end diff --git a/gems/click_house-client/lib/click_house/client/formatter.rb b/gems/click_house-client/lib/click_house/client/formatter.rb index bb60d8db7f7..de7ae72bdf8 100644 --- a/gems/click_house-client/lib/click_house/client/formatter.rb +++ b/gems/click_house-client/lib/click_house/client/formatter.rb @@ -7,7 +7,8 @@ module ClickHouse TYPE_CASTERS = { 'UInt64' => ->(value) { Integer(value) }, - "DateTime64(6, 'UTC')" => ->(value) { ActiveSupport::TimeZone["UTC"].parse(value) } + "DateTime64(6, 'UTC')" => ->(value) { ActiveSupport::TimeZone['UTC'].parse(value) }, + "IntervalSecond" => ->(value) { ActiveSupport::Duration.build(value.to_i) } }.freeze def self.format(result) diff --git a/gems/click_house-client/lib/click_house/client/query.rb b/gems/click_house-client/lib/click_house/client/query.rb new file mode 100644 index 00000000000..41435d239cf --- /dev/null +++ b/gems/click_house-client/lib/click_house/client/query.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module ClickHouse + module Client + class Query < QueryLike + SUBQUERY_PLACEHOLDER_REGEX = /{\w+:Subquery}/ # exmaple: {var:Subquery}, special "internal" type for subqueries + PLACEHOLDER_REGEX = /{\w+:\w+}/ # exmaple: {var:UInt8} + PLACEHOLDER_NAME_REGEX = /{(\w+):/ # exmaple: {var:UInt8} => var + + def initialize(raw_query:, placeholders: {}) + raise QueryError, 'Empty query string given' if raw_query.blank? + + @raw_query = raw_query + @placeholders = placeholders || {} + end + + # List of placeholders to be sent to ClickHouse for replacement. + # If there are subqueries, merge their placeholders as well. + def placeholders + all_placeholders = @placeholders.select { |_, v| !v.is_a?(QueryLike) } + @placeholders.each do |_name, value| + next unless value.is_a?(QueryLike) + + all_placeholders.merge!(value.placeholders) do |key, a, b| + raise QueryError, "mismatching values for the '#{key}' placeholder: #{a} vs #{b}" + end + end + + all_placeholders + end + + # Placeholder replacement is handled by ClickHouse, only subquery placeholders + # will be replaced. + def to_sql + raw_query.gsub(SUBQUERY_PLACEHOLDER_REGEX) do |placeholder_in_query| + value = placeholder_value(placeholder_in_query) + + if value.is_a?(QueryLike) + value.to_sql + else + placeholder_in_query + end + end + end + + def to_redacted_sql(bind_index_manager = BindIndexManager.new) + raw_query.gsub(PLACEHOLDER_REGEX) do |placeholder_in_query| + value = placeholder_value(placeholder_in_query) + + if value.is_a?(QueryLike) + value.to_redacted_sql(bind_index_manager) + else + bind_index_manager.next_bind_str + end + end + end + + def self.build(query) + return query if query.is_a?(ClickHouse::Client::QueryLike) + + new(raw_query: query) + end + + private + + attr_reader :raw_query + + def placeholder_value(placeholder_in_query) + placeholder = placeholder_in_query[PLACEHOLDER_NAME_REGEX, 1] + @placeholders.fetch(placeholder.to_sym) + end + end + end +end diff --git a/gems/click_house-client/lib/click_house/client/query_like.rb b/gems/click_house-client/lib/click_house/client/query_like.rb new file mode 100644 index 00000000000..9e9ee46a338 --- /dev/null +++ b/gems/click_house-client/lib/click_house/client/query_like.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module ClickHouse + module Client + class QueryLike + # Build a SQL string that can be executed on a ClickHouse database. + def to_sql + raise NotImplementedError + end + + # Redacted version of the SQL query generated by the to_sql method where the + # placeholders are stripped. These queries are meant to be exported to external + # log aggregation systems. + def to_redacted_sql(bind_index_manager = BindIndexManager.new) + raise NotImplementedError + end + end + end +end diff --git a/gems/click_house-client/spec/click_house/client/bind_index_manager_spec.rb b/gems/click_house-client/spec/click_house/client/bind_index_manager_spec.rb new file mode 100644 index 00000000000..38e0865676a --- /dev/null +++ b/gems/click_house-client/spec/click_house/client/bind_index_manager_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ClickHouse::Client::BindIndexManager do + describe '#next_bind_str' do + context 'when initialized without a start index' do + let(:bind_manager) { described_class.new } + + it 'starts from index 1 by default' do + expect(bind_manager.next_bind_str).to eq('$1') + end + + it 'increments the bind string on subsequent calls' do + bind_manager.next_bind_str + expect(bind_manager.next_bind_str).to eq('$2') + end + end + + context 'when initialized with a start index' do + let(:bind_manager) { described_class.new(2) } + + it 'starts from the given index' do + expect(bind_manager.next_bind_str).to eq('$2') + end + + it 'increments the bind string on subsequent calls' do + bind_manager.next_bind_str + expect(bind_manager.next_bind_str).to eq('$3') + end + end + end +end diff --git a/gems/click_house-client/spec/click_house/client/database_spec.rb b/gems/click_house-client/spec/click_house/client/database_spec.rb index a74d4a119a4..fdb4c72c0cb 100644 --- a/gems/click_house-client/spec/click_house/client/database_spec.rb +++ b/gems/click_house-client/spec/click_house/client/database_spec.rb @@ -24,7 +24,6 @@ RSpec.describe ClickHouse::Client::Database do describe '#headers' do it 'returns the correct headers' do expect(database.headers).to eq({ - "Content-Encoding" => "gzip", "X-ClickHouse-Format" => "JSON", 'X-ClickHouse-User' => 'user', 'X-ClickHouse-Key' => 'pass' diff --git a/gems/click_house-client/spec/click_house/client/formatter_spec.rb b/gems/click_house-client/spec/click_house/client/formatter_spec.rb new file mode 100644 index 00000000000..0af3aa0bdbc --- /dev/null +++ b/gems/click_house-client/spec/click_house/client/formatter_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ClickHouse::Client::Formatter do + it 'formats values according to types in metadata' do + # this query here is just for documentation purposes, it generates the response below + _query = <<~SQL.squish + SELECT toUInt64(1) as uint64, + toDateTime64('2016-06-15 23:00:00', 6, 'UTC') as datetime64_6, + INTERVAL 1 second as interval_second + SQL + + response_json = <<~JSON +{ + "meta": + [ + { + "name": "uint64", + "type": "UInt64" + }, + { + "name": "datetime64_6", + "type": "DateTime64(6, 'UTC')" + }, + { + "name": "interval_second", + "type": "IntervalSecond" + } + ], + + "data": + [ + { + "uint64": "1", + "datetime64_6": "2016-06-15 23:00:00.000000", + "interval_second": "1" + } + ], + + "rows": 1, + + "statistics": + { + "elapsed": 0.002101, + "rows_read": 1, + "bytes_read": 1 + } +} + JSON + + parsed_response = JSON.parse(response_json) + formatted_response = described_class.format(parsed_response) + + expect(formatted_response).to( + eq( + [{ "uint64" => 1, + "datetime64_6" => ActiveSupport::TimeZone["UTC"].parse("2016-06-15 23:00:00"), + "interval_second" => 1.second }] + ) + ) + end +end diff --git a/gems/click_house-client/spec/click_house/client/query_like_spec.rb b/gems/click_house-client/spec/click_house/client/query_like_spec.rb new file mode 100644 index 00000000000..8b8426bd5fd --- /dev/null +++ b/gems/click_house-client/spec/click_house/client/query_like_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ClickHouse::Client::QueryLike do + subject(:query) { described_class.new } + + describe '#to_sql' do + it { expect { query.to_sql }.to raise_error(NotImplementedError) } + end + + describe '#to_redacted_sql' do + it { expect { query.to_redacted_sql }.to raise_error(NotImplementedError) } + end +end diff --git a/gems/click_house-client/spec/click_house/client/query_spec.rb b/gems/click_house-client/spec/click_house/client/query_spec.rb new file mode 100644 index 00000000000..82733e523b1 --- /dev/null +++ b/gems/click_house-client/spec/click_house/client/query_spec.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ClickHouse::Client::Query do + subject(:query) { described_class.new(raw_query: raw_query, placeholders: placeholders) } + + let(:sql) { query.to_sql } + let(:redacted_sql) { query.to_redacted_sql } + + context 'when using no placeholders' do + let(:raw_query) { 'SELECT * FROM events' } + let(:placeholders) { nil } + + it { expect(sql).to eq(raw_query) } + it { expect(redacted_sql).to eq(raw_query) } + + context 'when placeholders is an empty hash' do + let(:placeholders) { {} } + + it { expect(sql).to eq(raw_query) } + it { expect(redacted_sql).to eq(raw_query) } + end + end + + context 'when placeholders are given' do + let(:raw_query) { 'SELECT * FROM events WHERE id = {id:UInt64}' } + let(:placeholders) { { id: 1 } } + + it { expect(sql).to eq(raw_query) } + it { expect(redacted_sql).to eq('SELECT * FROM events WHERE id = $1') } + end + + context 'when multiple placeholders are given' do + let(:raw_query) do + <<~SQL.squish + SELECT * + FROM events + WHERE + id = {id:UInt64} AND + title = {some_title:String} AND + another_id = {id:UInt64} + SQL + end + + let(:placeholders) { { id: 1, some_title: "'title'" } } + + it do + expect(sql).to eq(raw_query) + end + + it do + expect(redacted_sql).to eq( + <<~SQL.squish + SELECT * + FROM events + WHERE + id = $1 AND + title = $2 AND + another_id = $3 + SQL + ) + end + end + + context 'when dealing with subqueries' do + let(:raw_query) { 'SELECT * FROM events WHERE id < {min_id:UInt64} AND id IN ({q:Subquery})' } + + let(:subquery) do + described_class.new(raw_query: 'SELECT id FROM events WHERE id > {max_id:UInt64}', placeholders: { max_id: 11 }) + end + + let(:placeholders) { { min_id: 100, q: subquery } } + + it 'replaces the subquery but preserves the other placeholders' do + q = 'SELECT * FROM events WHERE id < {min_id:UInt64} AND id IN (SELECT id FROM events WHERE id > {max_id:UInt64})' + expect(sql).to eq(q) + end + + it 'replaces the subquery and replaces the placeholders with indexed values' do + expect(redacted_sql).to eq('SELECT * FROM events WHERE id < $1 AND id IN (SELECT id FROM events WHERE id > $2)') + end + + it 'merges the placeholders' do + expect(query.placeholders).to eq({ min_id: 100, max_id: 11 }) + end + end + + describe 'validation' do + context 'when SQL string is empty' do + let(:raw_query) { '' } + let(:placeholders) { {} } + + it 'raises error' do + expect { query }.to raise_error(ClickHouse::Client::QueryError, /Empty query string given/) + end + end + + context 'when SQL string is nil' do + let(:raw_query) { nil } + let(:placeholders) { {} } + + it 'raises error' do + expect { query }.to raise_error(ClickHouse::Client::QueryError, /Empty query string given/) + end + end + + context 'when same placeholder value does not match' do + let(:raw_query) { 'SELECT id FROM events WHERE id = {id:UInt64} AND id IN ({q:Subquery})' } + + let(:subquery) do + subquery_string = 'SELECT id FROM events WHERE id = {id:UInt64}' + described_class.new(raw_query: subquery_string, placeholders: { id: 10 }) + end + + let(:placeholders) { { id: 5, q: subquery } } + + it 'raises error' do + expect do + query.placeholders + end.to raise_error(ClickHouse::Client::QueryError, /mismatching values for the 'id' placeholder/) + end + end + end +end diff --git a/gems/click_house-client/spec/click_house/client_spec.rb b/gems/click_house-client/spec/click_house/client_spec.rb index 883199198ba..ab2407a83d7 100644 --- a/gems/click_house-client/spec/click_house/client_spec.rb +++ b/gems/click_house-client/spec/click_house/client_spec.rb @@ -30,6 +30,9 @@ RSpec.describe ClickHouse::Client do let(:configuration) do ClickHouse::Client::Configuration.new.tap do |config| + config.log_proc = ->(query) do + { query_string: query.to_sql } + end config.register_database(:test_db, **database_config) config.http_post_proc = ->(_url, _headers, _query) { body = File.read(query_result_fixture) @@ -71,7 +74,7 @@ RSpec.describe ClickHouse::Client do end context 'when the DB is not configured' do - it 'raises erro' do + it 'raises error' do expect do described_class.select('SELECT * FROM issues', :different_db, configuration) end.to raise_error(ClickHouse::Client::ConfigurationError, /not configured/) @@ -94,5 +97,34 @@ RSpec.describe ClickHouse::Client do end.to raise_error(ClickHouse::Client::DatabaseError, 'some error') end end + + describe 'default logging' do + let(:fake_logger) { instance_double("Logger", info: 'logged!') } + let(:query_string) { 'SELECT * FROM issues' } + + before do + configuration.logger = fake_logger + end + + shared_examples 'proper logging' do + it 'calls the custom logger and log_proc' do + expect(fake_logger).to receive(:info).at_least(:once).with({ query_string: query_string }) + + described_class.select(query_object, :test_db, configuration) + end + end + + context 'when query is a string' do # rubocop:disable RSpec/MultipleMemoizedHelpers + let(:query_object) { query_string } + + it_behaves_like 'proper logging' + end + + context 'when query is a Query object' do # rubocop:disable RSpec/MultipleMemoizedHelpers + let(:query_object) { ClickHouse::Client::Query.new(raw_query: query_string) } + + it_behaves_like 'proper logging' + end + end end end diff --git a/gems/config/rubocop.yml b/gems/config/rubocop.yml index 58746de53b0..72b37aa60b5 100644 --- a/gems/config/rubocop.yml +++ b/gems/config/rubocop.yml @@ -2,7 +2,7 @@ # (AllCops/Exclude: 'gems/**/*') if RuboCop cop is run within `gems/...`. # See https://gitlab.com/gitlab-org/gitlab/-/issues/417281 <% - relative_path = ENV['PWD'].delete_prefix(File.expand_path('../..')) + relative_path = ENV['PWD'].delete_prefix(File.expand_path('../..')) RuboCop::ConfigLoader.ignore_parent_exclusion = relative_path.start_with?('/gems/') %> --- @@ -61,6 +61,9 @@ Gitlab/RSpec/AvoidSetup: Graphql/AuthorizeTypes: Enabled: false +Gitlab/Json: + Enabled: false + # This cop doesn't make sense in the context of gems Graphql/Descriptions: Enabled: false diff --git a/gems/csv_builder/lib/csv_builder/gzip.rb b/gems/csv_builder/lib/csv_builder/gzip.rb index 60875006a35..f97c066705a 100644 --- a/gems/csv_builder/lib/csv_builder/gzip.rb +++ b/gems/csv_builder/lib/csv_builder/gzip.rb @@ -2,12 +2,14 @@ module CsvBuilder class Gzip < CsvBuilder::Builder - # Writes the CSV file compressed and yields the written tempfile. + # Writes the CSV file compressed and yields the written tempfile and rows written. + # # # Example: - # > CsvBuilder::Gzip.new(Issue, { title: -> (row) { row.title.upcase }, id: :id }).render do |tempfile| + # > CsvBuilder::Gzip.new(Issue, { title: -> (row) { row.title.upcase }, id: :id }).render do |tempfile, rows| # > puts tempfile.path # > puts `zcat #{tempfile.path}` + # > puts rows # > end def render Tempfile.open(['csv_builder_gzip', '.csv.gz']) do |tempfile| @@ -16,7 +18,7 @@ module CsvBuilder write_csv csv, until_condition: -> {} # truncation must be handled outside of the CsvBuilder csv.close - yield tempfile + yield tempfile, @rows_written end end end diff --git a/gems/csv_builder/spec/csv_builder/gzip_spec.rb b/gems/csv_builder/spec/csv_builder/gzip_spec.rb index 9d24d351247..22462d8dd0a 100644 --- a/gems/csv_builder/spec/csv_builder/gzip_spec.rb +++ b/gems/csv_builder/spec/csv_builder/gzip_spec.rb @@ -26,6 +26,13 @@ RSpec.describe CsvBuilder::Gzip do ]) end + it 'yields the number of written rows as the second argument' do + row_count = 0 + builder.render { |_, rows| row_count = rows } + + expect(row_count).to eq(2) + end + it 'requires a block' do expect { builder.render }.to raise_error(LocalJumpError) end diff --git a/gems/gem.gitlab-ci.yml b/gems/gem.gitlab-ci.yml index 4e91f0cbe44..a379a887bdd 100644 --- a/gems/gem.gitlab-ci.yml +++ b/gems/gem.gitlab-ci.yml @@ -55,7 +55,7 @@ rubocop: rspec: extends: .ruby_matrix script: - - bundle exec rspec + - RAILS_ENV=test bundle exec rspec coverage: '/LOC \((\d+\.\d+%)\) covered.$/' artifacts: expire_in: 31d diff --git a/gems/gitlab-http/.gitignore b/gems/gitlab-http/.gitignore new file mode 100644 index 00000000000..b04a8c840df --- /dev/null +++ b/gems/gitlab-http/.gitignore @@ -0,0 +1,11 @@ +/.bundle/ +/.yardoc +/_yardoc/ +/coverage/ +/doc/ +/pkg/ +/spec/reports/ +/tmp/ + +# rspec failure tracking +.rspec_status diff --git a/gems/gitlab-http/.gitlab-ci.yml b/gems/gitlab-http/.gitlab-ci.yml new file mode 100644 index 00000000000..cf85b7fcc2e --- /dev/null +++ b/gems/gitlab-http/.gitlab-ci.yml @@ -0,0 +1,4 @@ +include: + - local: gems/gem.gitlab-ci.yml + inputs: + gem_name: "gitlab-http" diff --git a/gems/gitlab-http/.rspec b/gems/gitlab-http/.rspec new file mode 100644 index 00000000000..34c5164d9b5 --- /dev/null +++ b/gems/gitlab-http/.rspec @@ -0,0 +1,3 @@ +--format documentation +--color +--require spec_helper diff --git a/gems/gitlab-http/.rubocop.yml b/gems/gitlab-http/.rubocop.yml new file mode 100644 index 00000000000..73ea5f610b3 --- /dev/null +++ b/gems/gitlab-http/.rubocop.yml @@ -0,0 +1,22 @@ +inherit_from: + - ../config/rubocop.yml + +Naming/ClassAndModuleCamelCase: + AllowedNames: + - HTTP_V2 + +Performance/RegexpMatch: + Enabled: false + +Style/SpecialGlobalVars: + Enabled: false + +Lint/DuplicateBranch: + Enabled: false + +RSpec/MultipleMemoizedHelpers: + Enabled: false + +RSpec/FilePath: + CustomTransform: + HTTP_V2: http_v2 diff --git a/gems/gitlab-http/Gemfile b/gems/gitlab-http/Gemfile new file mode 100644 index 00000000000..a6a5c2a4bc1 --- /dev/null +++ b/gems/gitlab-http/Gemfile @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +source "https://rubygems.org" + +# Specify your gem's dependencies in gitlab-http.gemspec +gemspec + +group :development, :test do + gem 'gitlab-rspec', path: '../gitlab-rspec' +end + +gem 'gitlab-utils', path: '../gitlab-utils' diff --git a/gems/gitlab-http/Gemfile.lock b/gems/gitlab-http/Gemfile.lock new file mode 100644 index 00000000000..4afa39ef750 --- /dev/null +++ b/gems/gitlab-http/Gemfile.lock @@ -0,0 +1,185 @@ +PATH + remote: ../gitlab-rspec + specs: + gitlab-rspec (0.1.0) + activesupport (>= 6.1, < 7.1) + rspec (~> 3.0) + +PATH + remote: ../gitlab-utils + specs: + gitlab-utils (0.1.0) + actionview (>= 6.1.7.2) + activesupport (>= 6.1.7.2) + addressable (~> 2.8) + nokogiri (~> 1.15.2) + rake (~> 13.0) + +PATH + remote: . + specs: + gitlab-http (0.1.0) + activesupport (~> 7.0.6) + httparty (~> 0.21.0) + ipaddress (~> 0.8.3) + nokogiri (~> 1.15.4) + railties (~> 7.0.6) + +GEM + remote: https://rubygems.org/ + specs: + actionpack (7.0.7) + actionview (= 7.0.7) + activesupport (= 7.0.7) + rack (~> 2.0, >= 2.2.4) + rack-test (>= 0.6.3) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.2.0) + actionview (7.0.7) + activesupport (= 7.0.7) + builder (~> 3.1) + erubi (~> 1.4) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.1, >= 1.2.0) + activesupport (7.0.7) + concurrent-ruby (~> 1.0, >= 1.0.2) + i18n (>= 1.6, < 2) + minitest (>= 5.1) + tzinfo (~> 2.0) + addressable (2.8.4) + public_suffix (>= 2.0.2, < 6.0) + ast (2.4.2) + builder (3.2.4) + concurrent-ruby (1.2.2) + crack (0.4.5) + rexml + crass (1.0.6) + diff-lcs (1.5.0) + erubi (1.12.0) + gitlab-styles (10.1.0) + rubocop (~> 1.50.2) + rubocop-graphql (~> 0.18) + rubocop-performance (~> 1.15) + rubocop-rails (~> 2.17) + rubocop-rspec (~> 2.22) + hashdiff (1.0.1) + httparty (0.21.0) + mini_mime (>= 1.0.0) + multi_xml (>= 0.5.2) + i18n (1.14.1) + concurrent-ruby (~> 1.0) + ipaddress (0.8.3) + json (2.6.3) + loofah (2.21.3) + crass (~> 1.0.2) + nokogiri (>= 1.12.0) + method_source (1.0.0) + mini_mime (1.1.2) + mini_portile2 (2.8.4) + minitest (5.18.1) + multi_xml (0.6.0) + nokogiri (1.15.4) + mini_portile2 (~> 2.8.2) + racc (~> 1.4) + parallel (1.23.0) + parser (3.2.2.3) + ast (~> 2.4.1) + racc + public_suffix (5.0.1) + racc (1.7.1) + rack (2.2.7) + rack-test (2.1.0) + rack (>= 1.3) + rails-dom-testing (2.0.3) + activesupport (>= 4.2.0) + nokogiri (>= 1.6) + rails-html-sanitizer (1.6.0) + loofah (~> 2.21) + nokogiri (~> 1.14) + railties (7.0.7) + actionpack (= 7.0.7) + activesupport (= 7.0.7) + method_source + rake (>= 12.2) + thor (~> 1.0) + zeitwerk (~> 2.5) + rainbow (3.1.1) + rake (13.0.6) + regexp_parser (2.8.1) + rexml (3.2.5) + rspec (3.12.0) + rspec-core (~> 3.12.0) + rspec-expectations (~> 3.12.0) + rspec-mocks (~> 3.12.0) + rspec-core (3.12.2) + rspec-support (~> 3.12.0) + rspec-expectations (3.12.3) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.12.0) + rspec-mocks (3.12.5) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.12.0) + rspec-rails (6.0.3) + actionpack (>= 6.1) + activesupport (>= 6.1) + railties (>= 6.1) + rspec-core (~> 3.12) + rspec-expectations (~> 3.12) + rspec-mocks (~> 3.12) + rspec-support (~> 3.12) + rspec-support (3.12.1) + rubocop (1.50.2) + json (~> 2.3) + parallel (~> 1.10) + parser (>= 3.2.0.0) + rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 1.8, < 3.0) + rexml (>= 3.2.5, < 4.0) + rubocop-ast (>= 1.28.0, < 2.0) + ruby-progressbar (~> 1.7) + unicode-display_width (>= 2.4.0, < 3.0) + rubocop-ast (1.29.0) + parser (>= 3.2.1.0) + rubocop-capybara (2.18.0) + rubocop (~> 1.41) + rubocop-factory_bot (2.23.1) + rubocop (~> 1.33) + rubocop-graphql (0.19.0) + rubocop (>= 0.87, < 2) + rubocop-performance (1.18.0) + rubocop (>= 1.7.0, < 2.0) + rubocop-ast (>= 0.4.0) + rubocop-rails (2.20.2) + activesupport (>= 4.2.0) + rack (>= 1.1) + rubocop (>= 1.33.0, < 2.0) + rubocop-rspec (2.23.0) + rubocop (~> 1.33) + rubocop-capybara (~> 2.17) + rubocop-factory_bot (~> 2.22) + ruby-progressbar (1.13.0) + thor (1.2.2) + tzinfo (2.0.6) + concurrent-ruby (~> 1.0) + unicode-display_width (2.4.2) + webmock (3.18.1) + addressable (>= 2.8.0) + crack (>= 0.3.2) + hashdiff (>= 0.4.0, < 2.0.0) + zeitwerk (2.6.8) + +PLATFORMS + ruby + +DEPENDENCIES + gitlab-http! + gitlab-rspec! + gitlab-styles (~> 10.1.0) + gitlab-utils! + rspec-rails (~> 6.0.3) + rubocop (~> 1.50.2) + rubocop-rspec (~> 2.22) + webmock (~> 3.18.1) + +BUNDLED WITH + 2.4.14 diff --git a/gems/gitlab-http/README.md b/gems/gitlab-http/README.md new file mode 100644 index 00000000000..13ff330bb19 --- /dev/null +++ b/gems/gitlab-http/README.md @@ -0,0 +1,42 @@ +# Gitlab::HTTP_V2 + +This gem is used as a proxy for all outbounding http connection +coming from callbacks, services and hooks. The direct use of the HTTParty +is discouraged because it can lead to several security problems, like SSRF +calling internal IP or services. + +## Usage + +### Configuration + +```ruby +Gitlab::HTTP_V2.configure do |config| + config.allowed_internal_uris = [] + + config.log_exception_proc = ->(exception, extra_info) do + # operation + end + config.silent_mode_log_info_proc = ->(message, http_method) do + # operation + end +end +``` + +### Actions + +Basic examples; + +```ruby +Gitlab::HTTP_V2.post(uri, body: body) + +Gitlab::HTTP_V2.try_get(uri, params) + +response = Gitlab::HTTP_V2.head(project_url, verify: true) + +Gitlab::HTTP_V2.post(path, base_uri: base_uri, **params) +``` + +## Development + +After checking out the repo, run `bundle` to install dependencies. +Then, run `RACK_ENV=test bundle exec rspec spec` to run the tests. diff --git a/gems/gitlab-http/gitlab-http.gemspec b/gems/gitlab-http/gitlab-http.gemspec new file mode 100644 index 00000000000..2653d4d4fb7 --- /dev/null +++ b/gems/gitlab-http/gitlab-http.gemspec @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require_relative "lib/gitlab/http_v2/version" + +Gem::Specification.new do |spec| + spec.name = "gitlab-http" + spec.version = Gitlab::HTTP_V2::Version::VERSION + spec.authors = ["GitLab Engineers"] + spec.email = ["engineering@gitlab.com"] + + spec.summary = "GitLab HTTP client" + spec.description = "GitLab HTTP client" + spec.homepage = "https://gitlab.com/gitlab-org/gitlab/-/tree/master/gems/gitlab-http" + spec.license = 'MIT' + spec.required_ruby_version = ">= 3.0" + spec.metadata["rubygems_mfa_required"] = "true" + + spec.files = Dir['lib/**/*.rb'] + spec.test_files = Dir['spec/**/*'] + spec.require_paths = ["lib"] + + spec.add_runtime_dependency 'activesupport', '~> 7.0.6' + spec.add_runtime_dependency 'httparty', '~> 0.21.0' + spec.add_runtime_dependency 'ipaddress', '~> 0.8.3' + spec.add_runtime_dependency 'nokogiri', '~> 1.15.4' + spec.add_runtime_dependency "railties", "~> 7.0.6" + + spec.add_development_dependency 'gitlab-styles', '~> 10.1.0' + spec.add_development_dependency 'rspec-rails', '~> 6.0.3' + spec.add_development_dependency "rubocop", "~> 1.50.2" + spec.add_development_dependency "rubocop-rspec", "~> 2.22" + spec.add_development_dependency 'webmock', '~> 3.18.1' +end diff --git a/gems/gitlab-http/lib/gitlab-http.rb b/gems/gitlab-http/lib/gitlab-http.rb new file mode 100644 index 00000000000..1fc0e16ec9f --- /dev/null +++ b/gems/gitlab-http/lib/gitlab-http.rb @@ -0,0 +1,11 @@ +# rubocop:disable Naming/FileName + +# frozen_string_literal: true + +# When we say gem 'gitlab-http' in Gemfile, bundler will also run require gitlab-http for us and it'd +# resolve the conflict when we call `Gitlab::HTTP_V2.configure` first time. +# See more: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125024#note_1502698924 + +require_relative 'gitlab/http_v2' + +# rubocop:enable Naming/FileName diff --git a/gems/gitlab-http/lib/gitlab/http_v2.rb b/gems/gitlab-http/lib/gitlab/http_v2.rb new file mode 100644 index 00000000000..8f3ede95530 --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require_relative "http_v2/configuration" +require_relative "http_v2/patches" +require_relative "http_v2/client" + +module Gitlab + module HTTP_V2 + SUPPORTED_HTTP_METHODS = [:get, :try_get, :post, :patch, :put, :delete, :head, :options].freeze + + class << self + delegate(*SUPPORTED_HTTP_METHODS, to: ::Gitlab::HTTP_V2::Client) + + def configuration + @configuration ||= Configuration.new + end + + def configure + yield(configuration) + end + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/buffered_io.rb b/gems/gitlab-http/lib/gitlab/http_v2/buffered_io.rb new file mode 100644 index 00000000000..478b9102dec --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/buffered_io.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'net/http' +require 'webmock' if Rails.env.test? + +# The Ruby 3.2 does change Net protocol. Please see; +# https://github.com/ruby/ruby/blob/ruby_3_2/lib/net/protocol.rb#L194-L206 +# vs https://github.com/ruby/ruby/blob/ruby_3_1/lib/net/protocol.rb#L190-L200 +NET_PROTOCOL_VERSION_0_2_0 = Gem::Version.new(Net::Protocol::VERSION) >= Gem::Version.new('0.2.0') + +module Gitlab + module HTTP_V2 + # Net::BufferedIO is overwritten by webmock but in order to test this class, + # it needs to inherit from the original BufferedIO. + # https://github.com/bblimke/webmock/blob/867f4b290fd133658aa9530cba4ba8b8c52c0d35/lib/webmock/http_lib_adapters/net_http.rb#L266 + parent_class = if const_defined?('WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetBufferedIO') && + Rails.env.test? + WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetBufferedIO + else + Net::BufferedIO + end + + class BufferedIo < parent_class + HEADER_READ_TIMEOUT = 20 + + # rubocop: disable Style/RedundantReturn + # rubocop: disable Cop/LineBreakAfterGuardClauses + # rubocop: disable Layout/EmptyLineAfterGuardClause + + # Original method: + # https://github.com/ruby/ruby/blob/cdb7d699d0641e8f081d590d06d07887ac09961f/lib/net/protocol.rb#L190-L200 + def readuntil(terminator, ignore_eof = false, start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)) + if NET_PROTOCOL_VERSION_0_2_0 + offset = @rbuf_offset + begin + until idx = @rbuf.index(terminator, offset) # rubocop:disable Lint/AssignmentInCondition + if (elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time) > HEADER_READ_TIMEOUT + raise Gitlab::HTTP_V2::HeaderReadTimeout, + "Request timed out after reading headers for #{elapsed} seconds" + end + + offset = @rbuf.bytesize + rbuf_fill + end + + return rbuf_consume(idx + terminator.bytesize - @rbuf_offset) + rescue EOFError + raise unless ignore_eof + return rbuf_consume(@rbuf.size) + end + else + begin + until idx = @rbuf.index(terminator) # rubocop:disable Lint/AssignmentInCondition + if (elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time) > HEADER_READ_TIMEOUT + raise Gitlab::HTTP_V2::HeaderReadTimeout, + "Request timed out after reading headers for #{elapsed} seconds" + end + + rbuf_fill + end + + return rbuf_consume(idx + terminator.size) + rescue EOFError + raise unless ignore_eof + return rbuf_consume(@rbuf.size) + end + end + end + # rubocop: enable Style/RedundantReturn + # rubocop: enable Cop/LineBreakAfterGuardClauses + # rubocop: enable Layout/EmptyLineAfterGuardClause + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/client.rb b/gems/gitlab-http/lib/gitlab/http_v2/client.rb new file mode 100644 index 00000000000..8daf19d7351 --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/client.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'httparty' +require 'net/http' +require 'active_support/all' +require_relative 'new_connection_adapter' +require_relative "exceptions" + +module Gitlab + module HTTP_V2 + class Client + DEFAULT_TIMEOUT_OPTIONS = { + open_timeout: 10, + read_timeout: 20, + write_timeout: 30 + }.freeze + DEFAULT_READ_TOTAL_TIMEOUT = 30.seconds + + SILENT_MODE_ALLOWED_METHODS = [ + Net::HTTP::Get, + Net::HTTP::Head, + Net::HTTP::Options, + Net::HTTP::Trace + ].freeze + + include HTTParty # rubocop:disable Gitlab/HTTParty + + class << self + alias_method :httparty_perform_request, :perform_request + end + + connection_adapter NewConnectionAdapter + + def self.perform_request(http_method, path, options, &block) + raise_if_blocked_by_silent_mode(http_method) if options.delete(:silent_mode_enabled) + + log_info = options.delete(:extra_log_info) + options_with_timeouts = + if !options.has_key?(:timeout) + options.with_defaults(DEFAULT_TIMEOUT_OPTIONS) + else + options + end + + if options[:stream_body] + httparty_perform_request(http_method, path, options_with_timeouts, &block) + else + begin + start_time = nil + read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT) + + httparty_perform_request(http_method, path, options_with_timeouts) do |fragment| + start_time ||= system_monotonic_time + elapsed = system_monotonic_time - start_time + + raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout + + yield fragment if block + end + rescue HTTParty::RedirectionTooDeep + raise RedirectionTooDeep + rescue *HTTP_ERRORS => e + extra_info = log_info || {} + extra_info = log_info.call(e, path, options) if log_info.respond_to?(:call) + configuration.log_exception(e, extra_info) + + raise e + end + end + end + + def self.try_get(path, options = {}, &block) + self.get(path, options, &block) # rubocop:disable Style/RedundantSelf + rescue *HTTP_ERRORS + nil + end + + def self.raise_if_blocked_by_silent_mode(http_method) + return if SILENT_MODE_ALLOWED_METHODS.include?(http_method) + + configuration.silent_mode_log_info('Outbound HTTP request blocked', http_method.to_s) + + raise SilentModeBlockedError, 'only get, head, options, and trace methods are allowed in silent mode' + end + + def self.system_monotonic_time + Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) + end + + def self.configuration + Gitlab::HTTP_V2.configuration + end + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/configuration.rb b/gems/gitlab-http/lib/gitlab/http_v2/configuration.rb new file mode 100644 index 00000000000..98b07d0cf27 --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/configuration.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Gitlab + module HTTP_V2 + class Configuration + attr_accessor :allowed_internal_uris, :log_exception_proc, :silent_mode_log_info_proc + + def log_exception(...) + log_exception_proc&.call(...) + end + + def silent_mode_log_info(...) + silent_mode_log_info_proc&.call(...) + end + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/domain_allowlist_entry.rb b/gems/gitlab-http/lib/gitlab/http_v2/domain_allowlist_entry.rb new file mode 100644 index 00000000000..5a08c891184 --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/domain_allowlist_entry.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module HTTP_V2 + class DomainAllowlistEntry + attr_reader :domain, :port + + def initialize(domain, port: nil) + @domain = domain + @port = port + end + + def match?(requested_domain, requested_port = nil) + return false unless domain == requested_domain + return true if port.nil? + + port == requested_port + end + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/exceptions.rb b/gems/gitlab-http/lib/gitlab/http_v2/exceptions.rb new file mode 100644 index 00000000000..5a34d0b9939 --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/exceptions.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'net/http' + +module Gitlab + module HTTP_V2 + BlockedUrlError = Class.new(StandardError) + RedirectionTooDeep = Class.new(StandardError) + ReadTotalTimeout = Class.new(Net::ReadTimeout) + HeaderReadTimeout = Class.new(Net::ReadTimeout) + SilentModeBlockedError = Class.new(StandardError) + + HTTP_TIMEOUT_ERRORS = [ + Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout, Gitlab::HTTP_V2::ReadTotalTimeout + ].freeze + + HTTP_ERRORS = HTTP_TIMEOUT_ERRORS + [ + EOFError, SocketError, OpenSSL::SSL::SSLError, OpenSSL::OpenSSLError, + Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Errno::ENETUNREACH, + Gitlab::HTTP_V2::BlockedUrlError, Gitlab::HTTP_V2::RedirectionTooDeep, + Net::HTTPBadResponse + ].freeze + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/ip_allowlist_entry.rb b/gems/gitlab-http/lib/gitlab/http_v2/ip_allowlist_entry.rb new file mode 100644 index 00000000000..ed5a2dba284 --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/ip_allowlist_entry.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Gitlab + module HTTP_V2 + class IpAllowlistEntry + attr_reader :ip, :port + + # Argument ip should be an IPAddr object + def initialize(ip, port: nil) + @ip = ip + @port = port + end + + def match?(requested_ip, requested_port = nil) + requested_ip = IPAddr.new(requested_ip) if requested_ip.is_a?(String) + + return false unless ip_include?(requested_ip) + return true if port.nil? + + port == requested_port + end + + private + + # Prior to ipaddr v1.2.3, if the allow list were the IPv4 to IPv6 + # mapped address ::ffff:169.254.168.100 and the requested IP were + # 169.254.168.100 or ::ffff:169.254.168.100, the IP would be + # considered in the allow list. However, with + # https://github.com/ruby/ipaddr/pull/31, IPAddr#include? will + # only match if the IP versions are the same. This method + # preserves backwards compatibility if the versions differ by + # checking inclusion by coercing an IPv4 address to its IPv6 + # mapped address. + def ip_include?(requested_ip) + return true if ip.include?(requested_ip) + return ip.include?(requested_ip.ipv4_mapped) if requested_ip.ipv4? && ip.ipv6? + return ip.ipv4_mapped.include?(requested_ip) if requested_ip.ipv6? && ip.ipv4? + + false + end + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/net_http_adapter.rb b/gems/gitlab-http/lib/gitlab/http_v2/net_http_adapter.rb new file mode 100644 index 00000000000..c6af2ed6aff --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/net_http_adapter.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'net/http' +require 'webmock' if Rails.env.test? +require_relative 'buffered_io' + +module Gitlab + module HTTP_V2 + # Webmock overwrites the Net::HTTP#request method with + # https://github.com/bblimke/webmock/blob/867f4b290fd133658aa9530cba4ba8b8c52c0d35/lib/webmock/http_lib_adapters/net_http.rb#L74 + # Net::HTTP#request usually calls Net::HTTP#connect but the Webmock overwrite doesn't. + # This makes sure that, in a test environment, the superclass is the Webmock overwrite. + parent_class = if defined?(WebMock) && Rails.env.test? + WebMock::HttpLibAdapters::NetHttpAdapter.instance_variable_get(:@webMockNetHTTP) + else + Net::HTTP + end + + class NetHttpAdapter < parent_class + private + + def connect + result = super + + @socket = BufferedIo.new(@socket.io, + read_timeout: @socket.read_timeout, + write_timeout: @socket.write_timeout, + continue_timeout: @socket.continue_timeout, + debug_output: @socket.debug_output) + + result + end + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb b/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb new file mode 100644 index 00000000000..ee4be97dc6d --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +# This class is part of the Gitlab::HTTP wrapper. It handles local requests and header timeouts +# +# 1. Local requests +# Depending on the value of the global setting allow_local_requests_from_web_hooks_and_services, +# this adapter will allow/block connection to internal IPs and/or urls. +# +# This functionality can be overridden by providing the setting the option +# allow_local_requests = true in the request. For example: +# Gitlab::HTTP.get('http://www.gitlab.com', allow_local_requests: true) +# +# This option will take precedence over the global setting. +# +# 2. Header timeouts +# When the use_read_total_timeout option is used, that means the receiver +# of the HTTP request cannot be trusted. Gitlab::BufferedIo will be used, +# to read header data. It is a modified version of Net::BufferedIO that +# raises a timeout error if reading header data takes too much time. + +require 'httparty' +require_relative 'net_http_adapter' +require_relative 'url_blocker' + +module Gitlab + module HTTP_V2 + class NewConnectionAdapter < HTTParty::ConnectionAdapter + def initialize(...) + super + + @allow_local_requests = options.delete(:allow_local_requests) + @extra_allowed_uris = options.delete(:extra_allowed_uris) + @deny_all_requests_except_allowed = options.delete(:deny_all_requests_except_allowed) + @outbound_local_requests_allowlist = options.delete(:outbound_local_requests_allowlist) + @dns_rebinding_protection_enabled = options.delete(:dns_rebinding_protection_enabled) + end + + def connection + result = validate_url_with_proxy!(uri) + @uri = result.uri + hostname = result.hostname + + http = super + http.hostname_override = hostname if hostname + + unless result.use_proxy + http.proxy_from_env = false + http.proxy_address = nil + end + + net_adapter = NetHttpAdapter.new(http.address, http.port) + + http.instance_variables.each do |variable| + net_adapter.instance_variable_set(variable, http.instance_variable_get(variable)) + end + + net_adapter + end + + private + + def validate_url_with_proxy!(url) + UrlBlocker.validate_url_with_proxy!(url, **url_blocker_options) + rescue UrlBlocker::BlockedUrlError => e + raise HTTP_V2::BlockedUrlError, "URL is blocked: #{e.message}" + end + + def url_blocker_options + { + allow_local_network: @allow_local_requests, + allow_localhost: @allow_local_requests, + extra_allowed_uris: @extra_allowed_uris, + schemes: %w[http https], + deny_all_requests_except_allowed: @deny_all_requests_except_allowed, + outbound_local_requests_allowlist: @outbound_local_requests_allowlist, + dns_rebind_protection: @dns_rebinding_protection_enabled + }.compact + end + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/patches.rb b/gems/gitlab-http/lib/gitlab/http_v2/patches.rb new file mode 100644 index 00000000000..3d26fbc6447 --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/patches.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +require_relative "../../hostname_override_patch" +require_relative "../../net_http/protocol_patch" +require_relative "../../net_http/response_patch" +require_relative "../../httparty/response_patch" diff --git a/gems/gitlab-http/lib/gitlab/http_v2/url_allowlist.rb b/gems/gitlab-http/lib/gitlab/http_v2/url_allowlist.rb new file mode 100644 index 00000000000..6e17315c87d --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/url_allowlist.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'gitlab/utils/all' +require_relative 'ip_allowlist_entry' +require_relative 'domain_allowlist_entry' + +module Gitlab + module HTTP_V2 + class UrlAllowlist + class << self + def ip_allowed?(ip_string, allowlist, port: nil) + return false if ip_string.blank? + + ip_allowlist, _ = outbound_local_requests_allowlist_arrays(allowlist) + ip_obj = ::Gitlab::Utils.string_to_ip_object(ip_string) + + ip_allowlist.any? do |ip_allowlist_entry| + ip_allowlist_entry.match?(ip_obj, port) + end + end + + def domain_allowed?(domain_string, allowlist, port: nil) + return false if domain_string.blank? + + _, domain_allowlist = outbound_local_requests_allowlist_arrays(allowlist) + + domain_allowlist.any? do |domain_allowlist_entry| + domain_allowlist_entry.match?(domain_string, port) + end + end + + private + + def outbound_local_requests_allowlist_arrays(allowlist) + return [[], []] if allowlist.blank? + + allowlist.reduce([[], []]) do |(ip_allowlist, domain_allowlist), string| + address, port = parse_addr_and_port(string) + + ip_obj = ::Gitlab::Utils.string_to_ip_object(address) + + if ip_obj + ip_allowlist << IpAllowlistEntry.new(ip_obj, port: port) + else + domain_allowlist << DomainAllowlistEntry.new(address, port: port) + end + + [ip_allowlist, domain_allowlist] + end + end + + def parse_addr_and_port(str) + case str + when /\A\[(?<address> .* )\]:(?<port> \d+ )\z/x # string like "[::1]:80" + address = $~[:address] + port = $~[:port] + when /\A(?<address> [^:]+ ):(?<port> \d+ )\z/x # string like "127.0.0.1:80" + address = $~[:address] + port = $~[:port] + else # string with no port number + address = str + port = nil + end + + [address, port&.to_i] + end + end + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb b/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb new file mode 100644 index 00000000000..a794ab2f443 --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb @@ -0,0 +1,409 @@ +# frozen_string_literal: true + +require 'resolv' +require 'ipaddress' +require_relative 'url_allowlist' + +module Gitlab + module HTTP_V2 + class UrlBlocker + BlockedUrlError = Class.new(StandardError) + HTTP_PROXY_ENV_VARS = %w[http_proxy https_proxy HTTP_PROXY HTTPS_PROXY].freeze + + # Result stores the validation result: + # uri - The original URI requested + # hostname - The hostname that should be used to connect. For DNS + # rebinding protection, this will be the resolved IP address of + # the hostname. + # use_proxy - + # If true, this means that the proxy server specified in the + # http_proxy/https_proxy environment variables should be used. + # + # If false, this either means that no proxy server was specified + # or that the hostname in the URL is exempt via the no_proxy + # environment variable. This allows the caller to disable usage + # of a proxy since the IP address may be used to + # connect. Otherwise, Net::HTTP may erroneously compare the IP + # address against the no_proxy list. + Result = Struct.new(:uri, :hostname, :use_proxy) + + class << self + # Validates the given url according to the constraints specified by arguments. + # + # ports - Raises error if the given URL port is not between given ports. + # allow_localhost - Raises error if URL resolves to a localhost IP address and argument is false. + # allow_local_network - Raises error if URL resolves to a link-local address and argument is false. + # extra_allowed_uris - Array of URI objects that are allowed in addition to hostname and IP constraints. + # This parameter is passed in this class when making the HTTP request. + # ascii_only - Raises error if URL has unicode characters and argument is true. + # enforce_user - Raises error if URL user doesn't start with alphanumeric characters and argument is true. + # enforce_sanitization - Raises error if URL includes any HTML/CSS/JS tags and argument is true. + # deny_all_requests_except_allowed - Raises error if URL is not in the allow list and argument is true. + # Can be Boolean or Proc. Defaults to instance app setting. + # dns_rebind_protection - Enforce DNS-rebinding attack protection. + # outbound_local_requests_allowlist - A list of trusted domains or IP addresses to which local requests are + # allowed when local requests for webhooks and integrations are disabled. This parameter is static and + # comes from the `outbound_local_requests_whitelist` application setting. # rubocop:disable Naming/InclusiveLanguage + # + # Returns a Result object. + # rubocop:disable Metrics/ParameterLists + def validate_url_with_proxy!( + url, + schemes:, + ports: [], + allow_localhost: false, + allow_local_network: true, + extra_allowed_uris: [], + ascii_only: false, + enforce_user: false, + enforce_sanitization: false, + deny_all_requests_except_allowed: false, + dns_rebind_protection: true, + outbound_local_requests_allowlist: [] + ) + # rubocop:enable Metrics/ParameterLists + + return Result.new(nil, nil, true) if url.nil? + + raise ArgumentError, 'The schemes is a required argument' if schemes.blank? + + # Param url can be a string, URI or Addressable::URI + uri = parse_url(url) + + validate_uri( + uri: uri, + schemes: schemes, + ports: ports, + enforce_sanitization: enforce_sanitization, + enforce_user: enforce_user, + ascii_only: ascii_only + ) + + begin + address_info = get_address_info(uri) + rescue SocketError + proxy_in_use = uri_under_proxy_setting?(uri, nil) + + unless enforce_address_info_retrievable?(uri, + dns_rebind_protection, + deny_all_requests_except_allowed, + outbound_local_requests_allowlist) + return Result.new(uri, nil, proxy_in_use) + end + + raise BlockedUrlError, 'Host cannot be resolved or invalid' + end + + ip_address = ip_address(address_info) + proxy_in_use = uri_under_proxy_setting?(uri, ip_address) + + # Ignore DNS rebind protection when a proxy is being used, as DNS + # rebinding is expected behavior. + dns_rebind_protection &&= !proxy_in_use + return Result.new(uri, nil, proxy_in_use) if domain_in_allow_list?(uri, outbound_local_requests_allowlist) + + protected_uri_with_hostname = enforce_uri_hostname(ip_address, uri, dns_rebind_protection, proxy_in_use) + + if ip_in_allow_list?(ip_address, outbound_local_requests_allowlist, port: get_port(uri)) + return protected_uri_with_hostname + end + + return protected_uri_with_hostname if allowed_uri?(uri, extra_allowed_uris) + + validate_deny_all_requests_except_allowed!(deny_all_requests_except_allowed) + + validate_local_request( + address_info: address_info, + allow_localhost: allow_localhost, + allow_local_network: allow_local_network + ) + + protected_uri_with_hostname + end + + def blocked_url?(url, **kwargs) + validate!(url, **kwargs) + + false + rescue BlockedUrlError + true + end + + # For backwards compatibility, Returns an array with [<uri>, <original-hostname>]. + # Issue for refactoring: https://gitlab.com/gitlab-org/gitlab/-/issues/410890 + def validate!(...) + result = validate_url_with_proxy!(...) + [result.uri, result.hostname] + end + + private + + # Returns the given URI with IP address as hostname and the original hostname respectively + # in an Array. + # + # It checks whether the resolved IP address matches with the hostname. If not, it changes + # the hostname to the resolved IP address. + # + # The original hostname is used to validate the SSL, given in that scenario + # we'll be making the request to the IP address, instead of using the hostname. + def enforce_uri_hostname(ip_address, uri, dns_rebind_protection, proxy_in_use) + unless dns_rebind_protection && ip_address && ip_address != uri.hostname + return Result.new(uri, nil, proxy_in_use) + end + + new_uri = uri.dup + new_uri.hostname = ip_address + Result.new(new_uri, uri.hostname, proxy_in_use) + end + + def ip_address(address_info) + address_info.first&.ip_address + end + + def validate_uri(uri:, schemes:, ports:, enforce_sanitization:, enforce_user:, ascii_only:) + validate_html_tags(uri) if enforce_sanitization + + return if internal?(uri) + + validate_scheme(uri.scheme, schemes) + validate_port(get_port(uri), ports) if ports.any? + validate_user(uri.user) if enforce_user + validate_hostname(uri.hostname) + validate_unicode_restriction(uri) if ascii_only + end + + def uri_under_proxy_setting?(uri, ip_address) + return false unless http_proxy_env? + # `no_proxy|NO_PROXY` specifies addresses for which the proxy is not + # used. If it's empty, there are no exceptions and this URI + # will be under proxy settings. + return true if no_proxy_env.blank? + + # `no_proxy|NO_PROXY` is being used. We must check whether it + # applies to this specific URI. + ::URI::Generic.use_proxy?(uri.hostname, ip_address, get_port(uri), no_proxy_env) + end + + # Returns addrinfo object for the URI. + # + # @param uri [Addressable::URI] + # + # @raise [Gitlab::UrlBlocker::BlockedUrlError, ArgumentError] - BlockedUrlError raised if host is too long. + # + # @return [Array<Addrinfo>] + def get_address_info(uri) + Addrinfo.getaddrinfo(uri.hostname, get_port(uri), nil, :STREAM).map do |addr| + addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr + end + rescue ArgumentError => e + # Addrinfo.getaddrinfo errors if the domain exceeds 1024 characters. + raise unless e.message.include?('hostname too long') + + raise BlockedUrlError, "Host is too long (maximum is 1024 characters)" + end + + def enforce_address_info_retrievable?( + uri, dns_rebind_protection, deny_all_requests_except_allowed, + outbound_local_requests_allowlist) + # Do not enforce if URI is in the allow list + return false if domain_in_allow_list?(uri, outbound_local_requests_allowlist) + + # Enforce if the instance should block requests + return true if deny_all_requests_except_allowed?(deny_all_requests_except_allowed) + + # Do not enforce if DNS rebinding protection is disabled + return false unless dns_rebind_protection + + # Do not enforce if proxy is used + return false if http_proxy_env? + + # In the test suite we use a lot of mocked urls that are either invalid or + # don't exist. In order to avoid modifying a ton of tests and factories + # we allow invalid urls unless the environment variable RSPEC_ALLOW_INVALID_URLS + # is not true + return false if Rails.env.test? && ENV['RSPEC_ALLOW_INVALID_URLS'] == 'true' + + true + end + + def validate_local_request( + address_info:, + allow_localhost:, + allow_local_network:) + return if allow_local_network && allow_localhost + + unless allow_localhost + validate_localhost(address_info) + validate_loopback(address_info) + end + + return if allow_local_network + + validate_local_network(address_info) + validate_link_local(address_info) + validate_shared_address(address_info) + validate_limited_broadcast_address(address_info) + end + + def validate_shared_address(addrs_info) + netmask = IPAddr.new('100.64.0.0/10') + return unless addrs_info.any? { |addr| netmask.include?(addr.ip_address) } + + raise BlockedUrlError, "Requests to the shared address space are not allowed" + end + + def validate_html_tags(uri) + uri_str = uri.to_s + sanitized_uri = ActionController::Base.helpers.sanitize(uri_str, tags: []) + return if sanitized_uri == uri_str + + raise BlockedUrlError, 'HTML/CSS/JS tags are not allowed' + end + + def parse_url(url) + Addressable::URI.parse(url).tap do |parsed_url| + raise Addressable::URI::InvalidURIError if multiline_blocked?(parsed_url) + end + rescue Addressable::URI::InvalidURIError, URI::InvalidURIError + raise BlockedUrlError, 'URI is invalid' + end + + def multiline_blocked?(parsed_url) + url = parsed_url.to_s + + return true if url =~ /\n|\r/ + # Google Cloud Storage uses a multi-line, encoded Signature query string + return false if %w[http https].include?(parsed_url.scheme&.downcase) + + CGI.unescape(url) =~ /\n|\r/ + end + + def validate_port(port, ports) + return if port.blank? + # Only ports under 1024 are restricted + return if port >= 1024 + return if ports.include?(port) + + raise BlockedUrlError, "Only allowed ports are #{ports.join(', ')}, and any over 1024" + end + + def validate_scheme(scheme, schemes) + return unless scheme.blank? || (schemes.any? && schemes.exclude?(scheme)) + + raise BlockedUrlError, "Only allowed schemes are #{schemes.join(', ')}" + end + + def validate_user(value) + return if value.blank? + return if value =~ /\A\p{Alnum}/ + + raise BlockedUrlError, "Username needs to start with an alphanumeric character" + end + + def validate_hostname(value) + return if value.blank? + return if IPAddress.valid?(value) + return if value =~ /\A\p{Alnum}/ + + raise BlockedUrlError, "Hostname or IP address invalid" + end + + def validate_unicode_restriction(uri) + return if uri.to_s.ascii_only? + + raise BlockedUrlError, "URI must be ascii only #{uri.to_s.dump}" + end + + def validate_localhost(addrs_info) + local_ips = ["::", "0.0.0.0"] + local_ips.concat(Socket.ip_address_list.map(&:ip_address)) + + return if (local_ips & addrs_info.map(&:ip_address)).empty? + + raise BlockedUrlError, "Requests to localhost are not allowed" + end + + def validate_loopback(addrs_info) + return unless addrs_info.any? { |addr| addr.ipv4_loopback? || addr.ipv6_loopback? } + + raise BlockedUrlError, "Requests to loopback addresses are not allowed" + end + + def validate_local_network(addrs_info) + return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? || addr.ipv6_unique_local? } + + raise BlockedUrlError, "Requests to the local network are not allowed" + end + + def validate_link_local(addrs_info) + netmask = IPAddr.new('169.254.0.0/16') + return unless addrs_info.any? { |addr| addr.ipv6_linklocal? || netmask.include?(addr.ip_address) } + + raise BlockedUrlError, "Requests to the link local network are not allowed" + end + + # Raises a BlockedUrlError if the instance is configured to deny all requests. + # + # This should only be called after allow list checks have been made. + def validate_deny_all_requests_except_allowed!(should_deny) + return unless deny_all_requests_except_allowed?(should_deny) + + raise BlockedUrlError, "Requests to hosts and IP addresses not on the Allow List are denied" + end + + # Raises a BlockedUrlError if any IP in `addrs_info` is the limited + # broadcast address. + # https://datatracker.ietf.org/doc/html/rfc919#section-7 + def validate_limited_broadcast_address(addrs_info) + blocked_ips = ["255.255.255.255"] + + return if (blocked_ips & addrs_info.map(&:ip_address)).empty? + + raise BlockedUrlError, "Requests to the limited broadcast address are not allowed" + end + + def allowed_uri?(uri, extra_allowed_uris) + internal?(uri) || check_uri(uri, extra_allowed_uris) + end + + # Allow url from the GitLab instance itself but only for the configured hostname and ports + def internal?(uri) + check_uri(uri, Gitlab::HTTP_V2.configuration.allowed_internal_uris) + end + + def check_uri(uri, allowlist) + allowlist.any? do |allowed_uri| + allowed_uri.scheme == uri.scheme && + allowed_uri.hostname == uri.hostname && + get_port(allowed_uri) == get_port(uri) + end + end + + def deny_all_requests_except_allowed?(should_deny) + should_deny.is_a?(Proc) ? should_deny.call : should_deny + end + + def domain_in_allow_list?(uri, outbound_local_requests_allowlist) + Gitlab::HTTP_V2::UrlAllowlist.domain_allowed?( + uri.normalized_host, outbound_local_requests_allowlist, port: get_port(uri)) + end + + def ip_in_allow_list?(ip_address, outbound_local_requests_allowlist, port: nil) + Gitlab::HTTP_V2::UrlAllowlist.ip_allowed?(ip_address, outbound_local_requests_allowlist, port: port) + end + + def no_proxy_env + ENV['no_proxy'] || ENV['NO_PROXY'] + end + + def http_proxy_env? + HTTP_PROXY_ENV_VARS.any? { |name| ENV[name].present? } + end + + def get_port(uri) + uri.port || uri.default_port + end + end + end + end +end diff --git a/gems/gitlab-http/lib/gitlab/http_v2/version.rb b/gems/gitlab-http/lib/gitlab/http_v2/version.rb new file mode 100644 index 00000000000..8a9a17de112 --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/version.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Gitlab + module HTTP_V2 + module Version + VERSION = "0.1.0" + end + end +end diff --git a/gems/gitlab-http/lib/hostname_override_patch.rb b/gems/gitlab-http/lib/hostname_override_patch.rb new file mode 100644 index 00000000000..c5799bf0682 --- /dev/null +++ b/gems/gitlab-http/lib/hostname_override_patch.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +# This override allows passing `@hostname_override` to the SNI protocol, +# which is used to lookup the correct SSL certificate in the +# request handshake process. +# +# Given we've forced the HTTP request to be sent to the resolved +# IP address in a few scenarios (e.g.: `Gitlab::HTTP_V2` through +# `UrlBlocker.validate!`), we need to provide the _original_ +# hostname via SNI in order to have a clean connection setup. +# +# This is ultimately needed in order to avoid DNS rebinding attacks +# through HTTP requests. + +require 'net/http' + +class OpenSSL::SSL::SSLContext + attr_accessor :hostname_override +end + +class OpenSSL::SSL::SSLSocket + module HostnameOverride + # rubocop: disable Gitlab/ModuleWithInstanceVariables + def hostname=(hostname) + super(@context.hostname_override || hostname) + end + + def post_connection_check(hostname) + super(@context.hostname_override || hostname) + end + # rubocop: enable Gitlab/ModuleWithInstanceVariables + end + + prepend HostnameOverride +end + +class Net::HTTP + attr_accessor :hostname_override + + SSL_IVNAMES << :@hostname_override + SSL_ATTRIBUTES << :hostname_override + + module HostnameOverride + def addr_port + return super unless hostname_override + + addr = hostname_override + default_port = use_ssl? ? Net::HTTP.https_default_port : Net::HTTP.http_default_port + default_port == port ? addr : "#{addr}:#{port}" + end + end + + prepend HostnameOverride +end diff --git a/gems/gitlab-http/lib/httparty/response_patch.rb b/gems/gitlab-http/lib/httparty/response_patch.rb new file mode 100644 index 00000000000..3488ff034b4 --- /dev/null +++ b/gems/gitlab-http/lib/httparty/response_patch.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'httparty' + +HTTParty::Response.class_eval do + # Original method: https://github.com/jnunemaker/httparty/blob/v0.20.0/lib/httparty/response.rb#L83-L86 + # Related issue: https://github.com/jnunemaker/httparty/issues/568 + # + # We need to override this method because `Concurrent::Promise` calls `nil?` on the response when + # calling the `value` method. And the `value` calls `nil?`. + # https://github.com/ruby-concurrency/concurrent-ruby/blob/v1.2.2/lib/concurrent-ruby/concurrent/concern/dereferenceable.rb#L64 + def nil? + response.nil? || response.body.blank? + end +end diff --git a/gems/gitlab-http/lib/net_http/protocol_patch.rb b/gems/gitlab-http/lib/net_http/protocol_patch.rb new file mode 100644 index 00000000000..8231423e1a5 --- /dev/null +++ b/gems/gitlab-http/lib/net_http/protocol_patch.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# Monkey patch Net::HTTP to fix missing URL decoding for username and password in proxy settings +# +# See proposed upstream fix https://github.com/ruby/net-http/pull/5 +# See Ruby-lang issue https://bugs.ruby-lang.org/issues/17542 +# See issue on GitLab https://gitlab.com/gitlab-org/gitlab/-/issues/289836 + +require 'net/http' + +# This file can be removed once Ruby 3.0 is no longer supported: +# https://gitlab.com/gitlab-org/gitlab/-/issues/396223 +return if Gem::Version.new(Net::HTTP::VERSION) >= Gem::Version.new('0.2.0') + +module Net + class HTTP < Protocol + def proxy_user + if environment_variable_is_multiuser_safe? && @proxy_from_env + user = proxy_uri&.user + CGI.unescape(user) unless user.nil? + else + @proxy_user + end + end + + def proxy_pass + if environment_variable_is_multiuser_safe? && @proxy_from_env + pass = proxy_uri&.password + CGI.unescape(pass) unless pass.nil? + else + @proxy_pass + end + end + + def environment_variable_is_multiuser_safe? + ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE + end + end +end diff --git a/gems/gitlab-http/lib/net_http/response_patch.rb b/gems/gitlab-http/lib/net_http/response_patch.rb new file mode 100644 index 00000000000..e5477a31318 --- /dev/null +++ b/gems/gitlab-http/lib/net_http/response_patch.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Net + class HTTPResponse + # rubocop: disable Cop/LineBreakAfterGuardClauses + # rubocop: disable Cop/LineBreakAroundConditionalBlock + # rubocop: disable Layout/EmptyLineAfterGuardClause + # rubocop: disable Style/AndOr + # rubocop: disable Style/CharacterLiteral + # rubocop: disable Style/InfiniteLoop + + # Original method: + # https://github.com/ruby/ruby/blob/v2_7_5/lib/net/http/response.rb#L54-L69 + # + # Our changes: + # - Pass along the `start_time` to `Gitlab::HTTP_V2::BufferedIo`, so we can raise a timeout + # if reading the headers takes too long. + # - Limit the regexes to avoid ReDoS attacks. + def self.each_response_header(sock) + start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC) + key = value = nil + while true + line = if sock.is_a?(Gitlab::HTTP_V2::BufferedIo) + sock.readuntil("\n", true, start_time) + else + sock.readuntil("\n", true) + end + line = line.sub(/\s{0,10}\z/, '') + break if line.empty? + if line[0] == ?\s or line[0] == ?\t and value + # rubocop:disable Gitlab/NoCodeCoverageComment + # :nocov: + value << ' ' unless value.empty? + value << line.strip + # :nocov: + # rubocop:enable Gitlab/NoCodeCoverageComment + else + yield key, value if key + key, value = line.strip.split(/\s{0,10}:\s{0,10}/, 2) + raise Net::HTTPBadResponse, 'wrong header line format' if value.nil? + end + end + yield key, value if key + end + # rubocop: enable Cop/LineBreakAfterGuardClauses + # rubocop: enable Cop/LineBreakAroundConditionalBlock + # rubocop: enable Layout/EmptyLineAfterGuardClause + # rubocop: enable Style/AndOr + # rubocop: enable Style/CharacterLiteral + # rubocop: enable Style/InfiniteLoop + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2/buffered_io_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/buffered_io_spec.rb new file mode 100644 index 00000000000..74fe8b18199 --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2/buffered_io_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::HTTP_V2::BufferedIo do + describe '#readuntil' do + let(:mock_io) { StringIO.new('a') } + let(:start_time) { Process.clock_gettime(Process::CLOCK_MONOTONIC) } + + before do + stub_const('Gitlab::HTTP_V2::BufferedIo::HEADER_READ_TIMEOUT', 0.1) + end + + subject(:readuntil) do + described_class.new(mock_io).readuntil('a', false, start_time) + end + + it 'does not raise a timeout error' do + expect { readuntil }.not_to raise_error + end + + context 'when the response contains infinitely long headers' do + before do + read_counter = 0 + + allow(mock_io).to receive(:read_nonblock) do |buffer_size, *_| + read_counter += 1 + raise 'Test did not raise HeaderReadTimeout' if read_counter > 10 + + sleep 0.01 + 'H' * buffer_size + end + end + + it 'raises a timeout error' do + expect do + readuntil + end.to raise_error(Gitlab::HTTP_V2::HeaderReadTimeout, + /Request timed out after reading headers for 0\.[0-9]+ seconds/) + end + + context 'when not passing start_time' do + subject(:readuntil) do + described_class.new(mock_io).readuntil('a', false) + end + + it 'raises a timeout error' do + expect do + readuntil + end.to raise_error(Gitlab::HTTP_V2::HeaderReadTimeout, + /Request timed out after reading headers for 0\.[0-9]+ seconds/) + end + end + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2/domain_allowlist_entry_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/domain_allowlist_entry_spec.rb new file mode 100644 index 00000000000..0f9d5bc550d --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2/domain_allowlist_entry_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::HTTP_V2::DomainAllowlistEntry do + let(:domain) { 'www.example.com' } + + describe '#initialize' do + it 'initializes without port' do + domain_allowlist_entry = described_class.new(domain) + + expect(domain_allowlist_entry.domain).to eq(domain) + expect(domain_allowlist_entry.port).to be(nil) + end + + it 'initializes with port' do + port = 8080 + domain_allowlist_entry = described_class.new(domain, port: port) + + expect(domain_allowlist_entry.domain).to eq(domain) + expect(domain_allowlist_entry.port).to eq(port) + end + end + + describe '#match?' do + it 'matches when domain and port are equal' do + port = 8080 + domain_allowlist_entry = described_class.new(domain, port: port) + + expect(domain_allowlist_entry).to be_match(domain, port) + end + + it 'matches any port when port is nil' do + domain_allowlist_entry = described_class.new(domain) + + expect(domain_allowlist_entry).to be_match(domain, 8080) + expect(domain_allowlist_entry).to be_match(domain, 9090) + end + + it 'does not match when port is present but requested_port is nil' do + domain_allowlist_entry = described_class.new(domain, port: 8080) + + expect(domain_allowlist_entry).not_to be_match(domain, nil) + end + + it 'matches when port and requested_port are nil' do + domain_allowlist_entry = described_class.new(domain) + + expect(domain_allowlist_entry).to be_match(domain) + end + + it 'does not match if domain is not equal' do + domain_allowlist_entry = described_class.new(domain) + + expect(domain_allowlist_entry).not_to be_match('www.gitlab.com', 8080) + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2/ip_allowlist_entry_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/ip_allowlist_entry_spec.rb new file mode 100644 index 00000000000..ad7d993ec62 --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2/ip_allowlist_entry_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::HTTP_V2::IpAllowlistEntry, feature_category: :shared do + let(:ipv4) { IPAddr.new('192.168.1.1') } + + describe '#initialize' do + it 'initializes without port' do + ip_allowlist_entry = described_class.new(ipv4) + + expect(ip_allowlist_entry.ip).to eq(ipv4) + expect(ip_allowlist_entry.port).to be(nil) + end + + it 'initializes with port' do + port = 8080 + ip_allowlist_entry = described_class.new(ipv4, port: port) + + expect(ip_allowlist_entry.ip).to eq(ipv4) + expect(ip_allowlist_entry.port).to eq(port) + end + end + + describe '#match?' do + it 'matches with equivalent IP and port' do + port = 8080 + ip_allowlist_entry = described_class.new(ipv4, port: port) + + expect(ip_allowlist_entry).to be_match(ipv4.to_s, port) + end + + it 'matches any port when port is nil' do + ip_allowlist_entry = described_class.new(ipv4) + + expect(ip_allowlist_entry).to be_match(ipv4.to_s, 8080) + expect(ip_allowlist_entry).to be_match(ipv4.to_s, 9090) + end + + it 'does not match when port is present but requested_port is nil' do + ip_allowlist_entry = described_class.new(ipv4, port: 8080) + + expect(ip_allowlist_entry).not_to be_match(ipv4.to_s, nil) + end + + it 'matches when port and requested_port are nil' do + ip_allowlist_entry = described_class.new(ipv4) + + expect(ip_allowlist_entry).to be_match(ipv4.to_s) + end + + it 'works with ipv6' do + ipv6 = IPAddr.new('fe80::c800:eff:fe74:8') + ip_allowlist_entry = described_class.new(ipv6) + + expect(ip_allowlist_entry).to be_match(ipv6.to_s, 8080) + end + + it 'matches ipv4 within IPv4 range' do + ipv4_range = IPAddr.new('127.0.0.0/28') + ip_allowlist_entry = described_class.new(ipv4_range) + + expect(ip_allowlist_entry).to be_match(ipv4_range.to_range.last.to_s, 8080) + expect(ip_allowlist_entry).not_to be_match('127.0.1.1', 8080) + end + + it 'matches IPv6 within IPv6 range' do + ipv6_range = IPAddr.new('::ffff:192.168.1.0/8') + ip_allowlist_entry = described_class.new(ipv6_range) + + expect(ip_allowlist_entry).to be_match(ipv6_range.to_range.last.to_s, 8080) + expect(ip_allowlist_entry).not_to be_match('fd84:6d02:f6d8:f::f', 8080) + end + + it 'matches IPv4 to IPv6 mapped addresses in allow list' do + ipv6_range = IPAddr.new('::ffff:192.168.1.1') + ip_allowlist_entry = described_class.new(ipv6_range) + + expect(ip_allowlist_entry).to be_match(ipv4, 8080) + expect(ip_allowlist_entry).to be_match(ipv6_range.to_range.last.to_s, 8080) + expect(ip_allowlist_entry).not_to be_match('::ffff:192.168.1.0', 8080) + expect(ip_allowlist_entry).not_to be_match('::ffff:169.254.168.101', 8080) + end + + it 'matches IPv4 to IPv6 mapped addresses in requested IP' do + ipv4_range = IPAddr.new('192.168.1.1/24') + ip_allowlist_entry = described_class.new(ipv4_range) + + expect(ip_allowlist_entry).to be_match(ipv4, 8080) + expect(ip_allowlist_entry).to be_match('::ffff:192.168.1.0', 8080) + expect(ip_allowlist_entry).to be_match('::ffff:192.168.1.1', 8080) + expect(ip_allowlist_entry).not_to be_match('::ffff:169.254.170.100/8', 8080) + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2/net_http_adapter_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/net_http_adapter_spec.rb new file mode 100644 index 00000000000..22998803cc8 --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2/net_http_adapter_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'net/http' + +RSpec.describe Gitlab::HTTP_V2::NetHttpAdapter, feature_category: :api do + describe '#connect' do + let(:url) { 'https://example.org' } + let(:net_http_adapter) { described_class.new(url) } + + subject(:connect) { net_http_adapter.send(:connect) } + + before do + allow(TCPSocket).to receive(:open).and_return(Socket.new(:INET, :STREAM)) + end + + it 'uses a Gitlab::HTTP_V2::BufferedIo instance as @socket' do + connect + + expect(net_http_adapter.instance_variable_get(:@socket)).to be_a(Gitlab::HTTP_V2::BufferedIo) + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2/net_http_patch_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/net_http_patch_spec.rb new file mode 100644 index 00000000000..b82646fb365 --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2/net_http_patch_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'net/http' + +RSpec.describe 'Net::HTTP patch proxy user and password encoding' do + let(:net_http) { Net::HTTP.new('hostname.example') } + + before do + # This file can be removed once Ruby 3.0 is no longer supported: + # https://gitlab.com/gitlab-org/gitlab/-/issues/396223 + skip if Gem::Version.new(Net::HTTP::VERSION) >= Gem::Version.new('0.2.0') + end + + describe '#proxy_user' do + subject { net_http.proxy_user } + + it { is_expected.to eq(nil) } + + context 'with http_proxy env' do + let(:http_proxy) { 'http://proxy.example:8000' } + + before do + stub_env('http_proxy', http_proxy) + end + + it { is_expected.to eq(nil) } + + context 'and user:password authentication' do + let(:http_proxy) { 'http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000' } + + context 'when on multiuser safe platform' do + # linux, freebsd, darwin are considered multi user safe platforms + # See https://github.com/ruby/net-http/blob/v0.1.1/lib/net/http.rb#L1174-L1178 + + before do + allow(net_http).to receive(:environment_variable_is_multiuser_safe?).and_return(true) + end + + it { is_expected.to eq 'Y\\X' } + end + + context 'when not on multiuser safe platform' do + before do + allow(net_http).to receive(:environment_variable_is_multiuser_safe?).and_return(false) + end + + it { is_expected.to be_nil } + end + end + end + end + + describe '#proxy_pass' do + subject { net_http.proxy_pass } + + it { is_expected.to eq(nil) } + + context 'with http_proxy env' do + let(:http_proxy) { 'http://proxy.example:8000' } + + before do + stub_env('http_proxy', http_proxy) + end + + it { is_expected.to eq(nil) } + + context 'and user:password authentication' do + let(:http_proxy) { 'http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000' } + + context 'when on multiuser safe platform' do + # linux, freebsd, darwin are considered multi user safe platforms + # See https://github.com/ruby/net-http/blob/v0.1.1/lib/net/http.rb#L1174-L1178 + + before do + allow(net_http).to receive(:environment_variable_is_multiuser_safe?).and_return(true) + end + + it { is_expected.to eq 'R%S] ?X' } + end + + context 'when not on multiuser safe platform' do + before do + allow(net_http).to receive(:environment_variable_is_multiuser_safe?).and_return(false) + end + + it { is_expected.to be_nil } + end + end + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2/net_http_response_patch_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/net_http_response_patch_spec.rb new file mode 100644 index 00000000000..f8d0f0a57fc --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2/net_http_response_patch_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Net::HTTPResponse patch header read timeout', feature_category: :shared do + describe '.each_response_header' do + let(:server_response) do + <<~HTTP + Content-Type: text/html + Header-Two: foo + + Hello World + HTTP + end + + before do + stub_const('Gitlab::HTTP_V2::BufferedIo::HEADER_READ_TIMEOUT', 0.1) + end + + subject(:each_response_header) { Net::HTTPResponse.each_response_header(socket) { |k, v| } } # rubocop:disable Lint/EmptyBlock + + context 'with Net::BufferedIO' do + let(:socket) { Net::BufferedIO.new(StringIO.new(server_response)) } + + it 'does not forward start time to the socket' do + allow(socket).to receive(:readuntil).and_call_original + expect(socket).to receive(:readuntil).with("\n", true) + + each_response_header + end + + context 'when the response contains many consecutive spaces' do + it 'has no regex backtracking issues' do + expect(socket).to receive(:readuntil).and_return( + "a: #{' ' * 100_000} b", + '' + ) + + Timeout.timeout(1) do + each_response_header + end + end + end + end + + context 'with Gitlab:HTTP_V2:::BufferedIo' do + let(:mock_io) { StringIO.new(server_response) } + let(:socket) { Gitlab::HTTP_V2::BufferedIo.new(mock_io) } + + it 'forwards start time to the socket' do + allow(socket).to receive(:readuntil).and_call_original + expect(socket).to receive(:readuntil).with("\n", true, kind_of(Numeric)) + + each_response_header + end + + context 'when the response contains an infinite number of headers' do + before do + read_counter = 0 + + allow(mock_io).to receive(:read_nonblock) do + read_counter += 1 + raise 'Test did not raise HeaderReadTimeout' if read_counter > 10 + + sleep 0.01 + +"Yet-Another-Header: foo\n" + end + end + + it 'raises a timeout error' do + expect { each_response_header }.to raise_error(Gitlab::HTTP_V2::HeaderReadTimeout, + /Request timed out after reading headers for 0\.[0-9]+ seconds/) + end + end + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2/new_connection_adapter_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/new_connection_adapter_spec.rb new file mode 100644 index 00000000000..852bafc5557 --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2/new_connection_adapter_spec.rb @@ -0,0 +1,157 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::HTTP_V2::NewConnectionAdapter, feature_category: :shared do + let(:uri) { URI('https://example.org') } + let(:options) { {} } + + subject(:connection) { described_class.new(uri, options).connection } + + describe '#connection' do + before do + stub_all_dns('https://example.org', ip_address: '93.184.216.34') + end + + context 'when local requests are allowed' do + let(:options) { { allow_local_requests: true } } + + it 'sets up the connection' do + expect(connection).to be_a(Gitlab::HTTP_V2::NetHttpAdapter) + expect(connection.address).to eq('93.184.216.34') + expect(connection.hostname_override).to eq('example.org') + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + end + + context 'when local requests are not allowed' do + let(:options) { { allow_local_requests: false } } + + it 'sets up the connection' do + expect(connection).to be_a(Gitlab::HTTP_V2::NetHttpAdapter) + expect(connection.address).to eq('93.184.216.34') + expect(connection.hostname_override).to eq('example.org') + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + + context 'when it is a request to local network' do + let(:uri) { URI('http://172.16.0.0/12') } + + it 'raises error' do + expect { subject }.to raise_error( + Gitlab::HTTP_V2::BlockedUrlError, + "URL is blocked: Requests to the local network are not allowed" + ) + end + + context 'when local request allowed' do + let(:options) { { allow_local_requests: true } } + + it 'sets up the connection' do + expect(connection).to be_a(Gitlab::HTTP_V2::NetHttpAdapter) + expect(connection.address).to eq('172.16.0.0') + expect(connection.hostname_override).to be(nil) + expect(connection.addr_port).to eq('172.16.0.0') + expect(connection.port).to eq(80) + end + end + end + + context 'when it is a request to local address' do + let(:uri) { URI('http://127.0.0.1') } + + it 'raises error' do + expect { subject }.to raise_error( + Gitlab::HTTP_V2::BlockedUrlError, + "URL is blocked: Requests to localhost are not allowed" + ) + end + + context 'when local request allowed' do + let(:options) { { allow_local_requests: true } } + + it 'sets up the connection' do + expect(connection).to be_a(Gitlab::HTTP_V2::NetHttpAdapter) + expect(connection.address).to eq('127.0.0.1') + expect(connection.hostname_override).to be(nil) + expect(connection.addr_port).to eq('127.0.0.1') + expect(connection.port).to eq(80) + end + end + end + + context 'when port different from URL scheme is used' do + let(:uri) { URI('https://example.org:8080') } + + it 'sets up the addr_port accordingly' do + expect(connection).to be_a(Gitlab::HTTP_V2::NetHttpAdapter) + expect(connection.address).to eq('93.184.216.34') + expect(connection.hostname_override).to eq('example.org') + expect(connection.addr_port).to eq('example.org:8080') + expect(connection.port).to eq(8080) + end + end + end + + context 'when DNS rebinding protection is disabled' do + let(:options) { { dns_rebinding_protection_enabled: false } } + + it 'sets up the connection' do + expect(connection).to be_a(Gitlab::HTTP_V2::NetHttpAdapter) + expect(connection.address).to eq('example.org') + expect(connection.hostname_override).to eq(nil) + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + end + + context 'when proxy is enabled' do + before do + stub_env('http_proxy', 'http://proxy.example.com') + end + + it 'proxy stays configured' do + expect(connection.proxy?).to be true + expect(connection.proxy_from_env?).to be true + expect(connection.proxy_address).to eq('proxy.example.com') + end + + context 'when no_proxy matches the request' do + before do + stub_env('no_proxy', 'example.org') + end + + it 'proxy is disabled' do + expect(connection.proxy?).to be false + expect(connection.proxy_from_env?).to be false + expect(connection.proxy_address).to be nil + end + end + + context 'when no_proxy does not match the request' do + before do + stub_env('no_proxy', 'example.com') + end + + it 'proxy stays configured' do + expect(connection.proxy?).to be true + expect(connection.proxy_from_env?).to be true + expect(connection.proxy_address).to eq('proxy.example.com') + end + end + end + + context 'when URL scheme is not HTTP/HTTPS' do + let(:uri) { URI('ssh://example.org') } + + it 'raises error' do + expect { subject }.to raise_error( + Gitlab::HTTP_V2::BlockedUrlError, + "URL is blocked: Only allowed schemes are http, https" + ) + end + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2/url_allowlist_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/url_allowlist_spec.rb new file mode 100644 index 00000000000..bac69a2c38c --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2/url_allowlist_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::HTTP_V2::UrlAllowlist do + let(:allowlist) { [] } + + describe '#domain_allowed?' do + let(:allowlist) { %w[www.example.com example.com] } + + it 'returns true if domains present in allowlist' do + not_allowed = %w[subdomain.example.com example.org] + + aggregate_failures do + allowlist.each do |domain| + expect(described_class).to be_domain_allowed(domain, allowlist) + end + + not_allowed.each do |domain| + expect(described_class).not_to be_domain_allowed(domain, allowlist) + end + end + end + + it 'returns false when domain is blank' do + expect(described_class).not_to be_domain_allowed(nil, allowlist) + end + + context 'with ports' do + let(:allowlist) { ['example.io:3000'] } + + it 'returns true if domain and ports present in allowlist' do + parsed_allowlist = [['example.io', 3000]] + not_allowed = [ + 'example.io', + ['example.io', 3001] + ] + + aggregate_failures do + parsed_allowlist.each do |domain, port| + expect(described_class).to be_domain_allowed(domain, allowlist, port: port) + end + + not_allowed.each do |domain, port| + expect(described_class).not_to be_domain_allowed(domain, allowlist, port: port) + end + end + end + end + end + + describe '#ip_allowed?' do + let(:allowlist) do + [ + '0.0.0.0', + '127.0.0.1', + '192.168.1.1', + '0:0:0:0:0:ffff:192.168.1.2', + '::ffff:c0a8:102', + 'fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa', + '0:0:0:0:0:ffff:169.254.169.254', + '::ffff:a9fe:a9fe', + '::ffff:a9fe:a864', + 'fe80::c800:eff:fe74:8' + ] + end + + it 'returns true if ips present in allowlist' do + aggregate_failures do + allowlist.each do |ip_address| + expect(described_class).to be_ip_allowed(ip_address, allowlist) + end + + %w[172.16.2.2 127.0.0.2 fe80::c800:eff:fe74:9].each do |ip_address| + expect(described_class).not_to be_ip_allowed(ip_address, allowlist) + end + end + end + + it 'returns false when ip is blank' do + expect(described_class).not_to be_ip_allowed(nil, allowlist) + end + + context 'with ip ranges in allowlist' do + let(:ipv4_range) { '127.0.0.0/28' } + let(:ipv6_range) { 'fd84:6d02:f6d8:c89e::/124' } + + let(:allowlist) do + [ + ipv4_range, + ipv6_range + ] + end + + it 'does not allowlist ipv4 range when not in allowlist' do + IPAddr.new(ipv4_range).to_range.to_a.each do |ip| + expect(described_class).not_to be_ip_allowed(ip.to_s, []) + end + end + + it 'allowlists all ipv4s in the range when in allowlist' do + IPAddr.new(ipv4_range).to_range.to_a.each do |ip| + expect(described_class).to be_ip_allowed(ip.to_s, allowlist) + end + end + + it 'does not allowlist ipv6 range when not in allowlist' do + IPAddr.new(ipv6_range).to_range.to_a.each do |ip| + expect(described_class).not_to be_ip_allowed(ip.to_s, []) + end + end + + it 'allowlists all ipv6s in the range when in allowlist' do + IPAddr.new(ipv6_range).to_range.to_a.each do |ip| + expect(described_class).to be_ip_allowed(ip.to_s, allowlist) + end + end + + it 'does not allowlist IPs outside the range' do + expect(described_class).not_to be_ip_allowed("fd84:6d02:f6d8:c89e:0:0:1:f", allowlist) + + expect(described_class).not_to be_ip_allowed("127.0.1.15", allowlist) + end + end + + context 'with ports' do + let(:allowlist) { %w[127.0.0.9:3000 [2001:db8:85a3:8d3:1319:8a2e:370:7348]:443] } + + it 'returns true if ip and ports present in allowlist' do + parsed_allowlist = [ + ['127.0.0.9', 3000], + ['[2001:db8:85a3:8d3:1319:8a2e:370:7348]', 443] + ] + not_allowed = [ + '127.0.0.9', + ['127.0.0.9', 3001], + '[2001:db8:85a3:8d3:1319:8a2e:370:7348]', + ['[2001:db8:85a3:8d3:1319:8a2e:370:7348]', 3001] + ] + + aggregate_failures do + parsed_allowlist.each do |ip, port| + expect(described_class).to be_ip_allowed(ip, allowlist, port: port) + end + + not_allowed.each do |ip, port| + expect(described_class).not_to be_ip_allowed(ip, allowlist, port: port) + end + end + end + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2/url_blocker_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/url_blocker_spec.rb new file mode 100644 index 00000000000..e47098e6f74 --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2/url_blocker_spec.rb @@ -0,0 +1,988 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::HTTP_V2::UrlBlocker, :stub_invalid_dns_only, feature_category: :shared do + let(:schemes) { %w[http https] } + + # This test ensures backward compatibliity for the validate! method. + # We shoud refactor all callers of validate! to handle a Result object: + # https://gitlab.com/gitlab-org/gitlab/-/issues/410890 + describe '#validate!' do + let(:options) { { schemes: schemes } } + + subject { described_class.validate!(import_url, **options) } + + shared_examples 'validates URI and hostname' do + it 'runs the url validations' do + uri, hostname = subject + + expect(uri).to eq(Addressable::URI.parse(expected_uri)) + expect(hostname).to eq(expected_hostname) + end + end + + context 'when the URL hostname is a domain' do + context 'when domain can be resolved' do + let(:import_url) { 'https://example.org' } + + before do + stub_dns(import_url, ip_address: '93.184.216.34') + end + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { 'https://93.184.216.34' } + let(:expected_hostname) { 'example.org' } + let(:expected_use_proxy) { false } + end + end + end + end + + describe '#validate_url_with_proxy!' do + let(:options) { { schemes: schemes } } + + subject { described_class.validate_url_with_proxy!(import_url, **options) } + + shared_examples 'validates URI and hostname' do + it 'runs the url validations' do + expect(subject.uri).to eq(Addressable::URI.parse(expected_uri)) + expect(subject.hostname).to eq(expected_hostname) + expect(subject.use_proxy).to eq(expected_use_proxy) + end + end + + shared_context 'when instance configured to deny all requests' do + let(:options) { super().merge(deny_all_requests_except_allowed: true) } + end + + shared_examples 'a URI denied by `deny_all_requests_except_allowed`' do + context 'when instance setting is enabled' do + include_context 'when instance configured to deny all requests' + + it 'blocks the request' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + + context 'when instance setting is not enabled' do + it 'does not block the request' do + expect { subject }.not_to raise_error + end + end + + context 'when passed as an argument' do + let(:options) { super().merge(deny_all_requests_except_allowed: arg_value) } + + context 'when argument is a proc that evaluates to true' do + let(:arg_value) { proc { true } } + + it 'blocks the request' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + + context 'when argument is a proc that evaluates to false' do + let(:arg_value) { proc { false } } + + it 'does not block the request' do + expect { subject }.not_to raise_error + end + end + + context 'when argument is true' do + let(:arg_value) { true } + + it 'blocks the request' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + + context 'when argument is false' do + let(:arg_value) { false } + + it 'does not block the request' do + expect { subject }.not_to raise_error + end + end + end + end + + shared_examples 'a URI exempt from `deny_all_requests_except_allowed`' do + include_context 'when instance configured to deny all requests' + + it 'does not block the request' do + expect { subject }.not_to raise_error + end + end + + context 'when URI is nil' do + let(:import_url) { nil } + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { nil } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { true } + end + + it_behaves_like 'a URI exempt from `deny_all_requests_except_allowed`' + end + + context 'when URI is internal' do + let(:import_url) { 'http://localhost' } + + before do + stub_dns(import_url, ip_address: '127.0.0.1') + end + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { 'http://127.0.0.1' } + let(:expected_hostname) { 'localhost' } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI exempt from `deny_all_requests_except_allowed`' + end + + context 'when URI is for a local object storage' do + let(:import_url) { "#{host}/external-diffs/merge_request_diffs/mr-1/diff-1" } + + context 'when extra_allowed_uris is passed' do + let(:options) { super().merge(extra_allowed_uris: [URI(host)]) } + + context 'with a local domain name' do + let(:host) { 'http://review-minio-svc.svc:9000' } + + before do + stub_dns(host, ip_address: '127.0.0.1') + end + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { 'http://127.0.0.1:9000/external-diffs/merge_request_diffs/mr-1/diff-1' } + let(:expected_hostname) { 'review-minio-svc.svc' } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI exempt from `deny_all_requests_except_allowed`' + end + + context 'with an IP address' do + let(:host) { 'http://127.0.0.1:9000' } + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { 'http://127.0.0.1:9000/external-diffs/merge_request_diffs/mr-1/diff-1' } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI exempt from `deny_all_requests_except_allowed`' + end + + context 'with an LFS object storage' do + let(:host) { 'http://127.0.0.1:9000' } + + context 'when extra_allowed_uris is not passed' do + let(:options) { super().merge(extra_allowed_uris: []) } + + it 'raises an error' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + end + end + + context 'when extra_allowed_uris is not passed' do + context 'with a local domain name' do + let(:host) { 'http://review-minio-svc.svc:9000' } + + before do + stub_dns(host, ip_address: '127.0.0.1') + end + + it 'raises an error' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + + context 'with an IP address' do + let(:host) { 'http://127.0.0.1:9000' } + + it 'raises an error' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + end + end + + context 'when the URL hostname is a domain' do + context 'when domain can be resolved' do + let(:import_url) { 'https://example.org' } + + before do + stub_dns(import_url, ip_address: '93.184.216.34') + end + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { 'https://93.184.216.34' } + let(:expected_hostname) { 'example.org' } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI denied by `deny_all_requests_except_allowed`' + end + + context 'when domain cannot be resolved' do + let(:import_url) { 'http://foobar.x' } + + before do + stub_env('RSPEC_ALLOW_INVALID_URLS', 'false') + end + + it 'raises an error' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + + context 'with HTTP_PROXY' do + let(:import_url) { 'http://foobar.x' } + + before do + stub_env('http_proxy', 'http://proxy.example.com') + end + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { import_url } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { true } + end + + context 'with no_proxy' do + before do + stub_env('no_proxy', 'foobar.x') + end + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { import_url } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { false } + end + end + end + end + + context 'when domain is too long' do + let(:import_url) { "https://example#{'a' * 1024}.com" } + + it 'raises an error' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + end + + context 'when the URL hostname is an IP address' do + let(:import_url) { 'https://93.184.216.34' } + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { import_url } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI denied by `deny_all_requests_except_allowed`' + + context 'when the address is invalid' do + let(:import_url) { 'http://1.1.1.1.1' } + + it 'raises an error' do + stub_env('RSPEC_ALLOW_INVALID_URLS', 'false') + + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + end + + context 'when DNS rebinding protection with IP allowed' do + let(:import_url) { 'http://a.192.168.0.120.3times.127.0.0.1.1time.repeat.rebind.network:9121/scrape?target=unix:///var/opt/gitlab/redis/redis.socket&check-keys=*' } + + before do + stub_dns(import_url, ip_address: '192.168.0.120') + + allow(Gitlab::HTTP_V2::UrlAllowlist).to receive(:ip_allowed?).and_return(true) + end + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { 'http://192.168.0.120:9121/scrape?target=unix:///var/opt/gitlab/redis/redis.socket&check-keys=*' } + let(:expected_hostname) { 'a.192.168.0.120.3times.127.0.0.1.1time.repeat.rebind.network' } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI exempt from `deny_all_requests_except_allowed`' + + context 'with HTTP_PROXY' do + before do + stub_env('http_proxy', 'http://proxy.example.com') + end + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { import_url } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { true } + end + + context 'when domain is in no_proxy env' do + before do + stub_env('no_proxy', 'a.192.168.0.120.3times.127.0.0.1.1time.repeat.rebind.network') + end + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { 'http://192.168.0.120:9121/scrape?target=unix:///var/opt/gitlab/redis/redis.socket&check-keys=*' } + let(:expected_hostname) { 'a.192.168.0.120.3times.127.0.0.1.1time.repeat.rebind.network' } + let(:expected_use_proxy) { false } + end + end + end + end + + context 'with disabled DNS rebinding protection' do + let(:options) { { dns_rebind_protection: false, schemes: schemes } } + + context 'when URI is internal' do + let(:import_url) { 'http://localhost' } + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { import_url } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI exempt from `deny_all_requests_except_allowed`' + end + + context 'when the URL hostname is a domain' do + let(:import_url) { 'https://example.org' } + + before do + stub_env('RSPEC_ALLOW_INVALID_URLS', 'false') + end + + context 'when domain can be resolved' do + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { import_url } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI denied by `deny_all_requests_except_allowed`' + end + + context 'when domain cannot be resolved' do + let(:import_url) { 'http://foobar.x' } + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { import_url } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI denied by `deny_all_requests_except_allowed`' + end + end + + context 'when the URL hostname is an IP address' do + let(:import_url) { 'https://93.184.216.34' } + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { import_url } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI denied by `deny_all_requests_except_allowed`' + + context 'when it is invalid' do + let(:import_url) { 'http://1.1.1.1.1' } + + it_behaves_like 'validates URI and hostname' do + let(:expected_uri) { import_url } + let(:expected_hostname) { nil } + let(:expected_use_proxy) { false } + end + + it_behaves_like 'a URI denied by `deny_all_requests_except_allowed`' + end + end + end + end + + describe '#blocked_url?' do + let(:ports) { [80, 443] } + + it 'allows imports from configured web host and port' do + import_url = "http://localhost:80/t.git" + expect(described_class.blocked_url?(import_url, schemes: schemes)).to be false + end + + it 'allows mirroring from configured SSH host and port' do + import_url = "ssh://localhost:22/t.git" + expect(described_class.blocked_url?(import_url, schemes: schemes)).to be false + end + + it 'returns true for bad localhost hostname' do + expect(described_class.blocked_url?('https://localhost:65535/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for bad port' do + expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git', ports: ports, schemes: schemes)) + .to be true + end + + it 'returns true for bad scheme' do + expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', schemes: ['https'])).to be false + expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', schemes: ['http'])).to be true + end + + it 'returns true for bad protocol on configured web/SSH host and ports' do + web_url = "javascript://localhost:80/t.git%0aalert(1)" + expect(described_class.blocked_url?(web_url, schemes: schemes)).to be true + + ssh_url = "javascript://localhost:22/t.git%0aalert(1)" + expect(described_class.blocked_url?(ssh_url, schemes: schemes)).to be true + end + + it 'returns true for localhost IPs' do + expect(described_class.blocked_url?('https://[0:0:0:0:0:0:0:0]/foo/foo.git', schemes: schemes)).to be true + expect(described_class.blocked_url?('https://0.0.0.0/foo/foo.git', schemes: schemes)).to be true + expect(described_class.blocked_url?('https://[::]/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for loopback IP' do + expect(described_class.blocked_url?('https://127.0.0.2/foo/foo.git', schemes: schemes)).to be true + expect(described_class.blocked_url?('https://127.0.0.1/foo/foo.git', schemes: schemes)).to be true + expect(described_class.blocked_url?('https://[::1]/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (0177.1)' do + expect(described_class.blocked_url?('https://0177.1:65535/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (017700000001)' do + expect(described_class.blocked_url?('https://017700000001:65535/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (0x7f.1)' do + expect(described_class.blocked_url?('https://0x7f.1:65535/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (0x7f.0.0.1)' do + expect(described_class.blocked_url?('https://0x7f.0.0.1:65535/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (0x7f000001)' do + expect(described_class.blocked_url?('https://0x7f000001:65535/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (2130706433)' do + expect(described_class.blocked_url?('https://2130706433:65535/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (127.000.000.001)' do + expect(described_class.blocked_url?('https://127.000.000.001:65535/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (127.0.1)' do + expect(described_class.blocked_url?('https://127.0.1:65535/foo/foo.git', schemes: schemes)).to be true + end + + context 'with ipv6 mapped address' do + it 'returns true for localhost IPs' do + expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:0.0.0.0]/foo/foo.git', schemes: schemes)) + .to be true + expect(described_class.blocked_url?('https://[::ffff:0.0.0.0]/foo/foo.git', schemes: schemes)).to be true + expect(described_class.blocked_url?('https://[::ffff:0:0]/foo/foo.git', schemes: schemes)).to be true + end + + it 'returns true for loopback IPs' do + expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.1]/foo/foo.git', schemes: schemes)) + .to be true + expect(described_class.blocked_url?('https://[::ffff:127.0.0.1]/foo/foo.git', schemes: schemes)).to be true + expect(described_class.blocked_url?('https://[::ffff:7f00:1]/foo/foo.git', schemes: schemes)).to be true + expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.2]/foo/foo.git', schemes: schemes)) + .to be true + expect(described_class.blocked_url?('https://[::ffff:127.0.0.2]/foo/foo.git', schemes: schemes)).to be true + expect(described_class.blocked_url?('https://[::ffff:7f00:2]/foo/foo.git', schemes: schemes)).to be true + end + end + + it 'returns true for a non-alphanumeric hostname' do + aggregate_failures do + expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami/a', schemes: ['ssh']) + + # The leading character here is a Unicode "soft hyphen" + expect(described_class).to be_blocked_url('ssh://oProxyCommand=whoami/a', schemes: ['ssh']) + + # Unicode alphanumerics are allowed + expect(described_class).not_to be_blocked_url('ssh://ğitlab.com/a', schemes: ['ssh']) + end + end + + it 'returns true for invalid URL' do + expect(described_class.blocked_url?('http://:8080', schemes: schemes)).to be true + end + + it 'returns false for legitimate URL' do + expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', schemes: schemes)).to be false + end + + describe 'allow_local_network' do + let(:shared_address_space_ips) { ['100.64.0.0', '100.64.127.127', '100.64.255.255'] } + + let(:local_ips) do + [ + '192.168.1.2', + '[0:0:0:0:0:ffff:192.168.1.2]', + '[::ffff:c0a8:102]', + '10.0.0.2', + '[0:0:0:0:0:ffff:10.0.0.2]', + '[::ffff:a00:2]', + '172.16.0.2', + '[0:0:0:0:0:ffff:172.16.0.2]', + '[::ffff:ac10:20]', + '[feef::1]', + '[fee2::]', + '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]', + *shared_address_space_ips + ] + end + + let(:limited_broadcast_address_variants) do + [ + '255.255.255.255', # "normal" dotted decimal + '0377.0377.0377.0377', # Octal + '0377.00000000377.00377.0000377', # Still octal + '0xff.0xff.0xff.0xff', # hex + '0xffffffff', # still hex + '0xBaaaaaaaaaaaaaaaaffffffff', # padded hex + '255.255.255.255:65535', # with a port + '4294967295', # as an integer / dword + '[::ffff:ffff:ffff]', # short IPv6 + '[0000:0000:0000:0000:0000:ffff:ffff:ffff]' # long IPv6 + ] + end + + let(:fake_domain) { 'www.fakedomain.fake' } + + shared_examples 'allows local requests' do + it 'does not block urls from private networks' do + local_ips.each do |ip| + stub_domain_resolv(fake_domain, ip) do + expect(described_class).not_to be_blocked_url("http://#{fake_domain}", **url_blocker_attributes) + end + + expect(described_class).not_to be_blocked_url("http://#{ip}", **url_blocker_attributes) + end + end + + it 'allows localhost endpoints' do + expect(described_class).not_to be_blocked_url('http://0.0.0.0', **url_blocker_attributes) + expect(described_class).not_to be_blocked_url('http://localhost', **url_blocker_attributes) + expect(described_class).not_to be_blocked_url('http://127.0.0.1', **url_blocker_attributes) + end + + it 'allows loopback endpoints' do + expect(described_class).not_to be_blocked_url('http://127.0.0.2', **url_blocker_attributes) + end + + it 'allows IPv4 link-local endpoints' do + expect(described_class).not_to be_blocked_url('http://169.254.169.254', **url_blocker_attributes) + expect(described_class).not_to be_blocked_url('http://169.254.168.100', **url_blocker_attributes) + end + + it 'allows IPv6 link-local endpoints' do + expect(described_class).not_to be_blocked_url( + 'http://[0:0:0:0:0:ffff:169.254.169.254]', **url_blocker_attributes) + expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.169.254]', **url_blocker_attributes) + expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a9fe]', **url_blocker_attributes) + expect(described_class).not_to be_blocked_url( + 'http://[0:0:0:0:0:ffff:169.254.168.100]', **url_blocker_attributes) + expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.168.100]', **url_blocker_attributes) + expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a864]', **url_blocker_attributes) + expect(described_class).not_to be_blocked_url('http://[fe80::c800:eff:fe74:8]', **url_blocker_attributes) + end + + it 'allows limited broadcast address 255.255.255.255 and variants' do + limited_broadcast_address_variants.each do |variant| + expect(described_class).not_to be_blocked_url("https://#{variant}", **url_blocker_attributes), + "Expected #{variant} to be allowed" + end + end + end + + context 'when true (default)' do + let(:url_blocker_attributes) do + options.merge( + allow_localhost: true, + allow_local_network: true + ) + end + + let(:options) { { schemes: schemes } } + + it_behaves_like 'allows local requests', + { allow_localhost: true, allow_local_network: true, schemes: %w[http https] } + end + + context 'when false' do + it 'blocks urls from private networks' do + local_ips.each do |ip| + stub_domain_resolv(fake_domain, ip) do + expect(described_class).to be_blocked_url( + "http://#{fake_domain}", allow_local_network: false, schemes: schemes) + end + + expect(described_class).to be_blocked_url("http://#{ip}", allow_local_network: false, schemes: schemes) + end + end + + it 'blocks IPv4 link-local endpoints' do + expect(described_class).to be_blocked_url( + 'http://169.254.169.254', allow_local_network: false, schemes: schemes) + expect(described_class).to be_blocked_url( + 'http://169.254.168.100', allow_local_network: false, schemes: schemes) + end + + it 'blocks IPv6 link-local endpoints' do + expect(described_class).to be_blocked_url( + 'http://[0:0:0:0:0:ffff:169.254.169.254]', allow_local_network: false, schemes: schemes) + expect(described_class).to be_blocked_url( + 'http://[::ffff:169.254.169.254]', allow_local_network: false, schemes: schemes) + expect(described_class).to be_blocked_url( + 'http://[::ffff:a9fe:a9fe]', allow_local_network: false, schemes: schemes) + expect(described_class).to be_blocked_url( + 'http://[0:0:0:0:0:ffff:169.254.168.100]', allow_local_network: false, schemes: schemes) + expect(described_class).to be_blocked_url( + 'http://[::ffff:169.254.168.100]', allow_local_network: false, schemes: schemes) + expect(described_class).to be_blocked_url( + 'http://[::ffff:a9fe:a864]', allow_local_network: false, schemes: schemes) + expect(described_class).to be_blocked_url( + 'http://[fe80::c800:eff:fe74:8]', allow_local_network: false, schemes: schemes) + end + + it 'blocks limited broadcast address 255.255.255.255 and variants' do + # Raise BlockedUrlError for invalid URLs. + # The padded hex version, for example, is a valid URL on Mac but + # not on Ubuntu. + stub_env('RSPEC_ALLOW_INVALID_URLS', 'false') + + limited_broadcast_address_variants.each do |variant| + expect(described_class).to be_blocked_url( + "https://#{variant}", allow_local_network: false, schemes: schemes), + "Expected #{variant} to be blocked" + end + end + + context 'when local domain/IP is allowed' do + let(:url_blocker_attributes) do + options.merge( + allow_localhost: false, + allow_local_network: false + ) + end + + let(:options) { { schemes: schemes, outbound_local_requests_allowlist: allowlist } } + + context 'with IPs in allowlist' do + let(:allowlist) do + [ + '0.0.0.0', + '127.0.0.1', + '127.0.0.2', + '192.168.1.1', + *local_ips, + '0:0:0:0:0:ffff:169.254.169.254', + '::ffff:a9fe:a9fe', + '::ffff:169.254.168.100', + '::ffff:a9fe:a864', + 'fe80::c800:eff:fe74:8', + '255.255.255.255', + + # garbage IPs + '45645632345', + 'garbage456:more345gar:bage' + ] + end + + it_behaves_like 'allows local requests', + { allow_localhost: false, allow_local_network: false, schemes: %w[http https] } + + it 'allows IP when dns_rebind_protection is disabled' do + url = "http://example.com" + attrs = url_blocker_attributes.merge(dns_rebind_protection: false) + + stub_domain_resolv('example.com', '192.168.1.2') do + expect(described_class).not_to be_blocked_url(url, **attrs) + end + + stub_domain_resolv('example.com', '192.168.1.3') do + expect(described_class).to be_blocked_url(url, **attrs) + end + end + + it 'allows the limited broadcast address 255.255.255.255' do + expect(described_class).not_to be_blocked_url('http://255.255.255.255', **url_blocker_attributes) + end + end + + context 'with domains in allowlist' do + let(:allowlist) do + [ + 'www.example.com', + 'example.com', + 'xn--itlab-j1a.com', + 'garbage$^$%#$^&$' + ] + end + + it 'allows domains present in allowlist' do + domain = 'example.com' + subdomain1 = 'www.example.com' + subdomain2 = 'subdomain.example.com' + + stub_domain_resolv(domain, '192.168.1.1') do + expect(described_class).not_to be_blocked_url("http://#{domain}", + **url_blocker_attributes) + end + + stub_domain_resolv(subdomain1, '192.168.1.1') do + expect(described_class).not_to be_blocked_url("http://#{subdomain1}", + **url_blocker_attributes) + end + + # subdomain2 is not part of the allowlist so it should be blocked + stub_domain_resolv(subdomain2, '192.168.1.1') do + expect(described_class).to be_blocked_url("http://#{subdomain2}", + **url_blocker_attributes) + end + end + + it 'works with unicode and idna encoded domains' do + unicode_domain = 'ğitlab.com' + idna_encoded_domain = 'xn--itlab-j1a.com' + + stub_domain_resolv(unicode_domain, '192.168.1.1') do + expect(described_class).not_to be_blocked_url("http://#{unicode_domain}", + **url_blocker_attributes) + end + + stub_domain_resolv(idna_encoded_domain, '192.168.1.1') do + expect(described_class).not_to be_blocked_url("http://#{idna_encoded_domain}", + **url_blocker_attributes) + end + end + + shared_examples 'dns rebinding checks' do + shared_examples 'allowlists the domain' do + let(:allowlist) { [domain] } + let(:url) { "http://#{domain}" } + + before do + stub_env('RSPEC_ALLOW_INVALID_URLS', 'false') + end + + it do + expect(described_class).not_to be_blocked_url(url, **options, dns_rebind_protection: dns_rebind_value) + end + end + + describe 'dns_rebinding_setting' do + context 'when enabled' do + let(:dns_rebind_value) { true } + + it_behaves_like 'allowlists the domain' + end + + context 'when disabled' do + let(:dns_rebind_value) { false } + + it_behaves_like 'allowlists the domain' + end + end + end + + context 'when the domain cannot be resolved' do + let(:domain) { 'foobar.x' } + + it_behaves_like 'dns rebinding checks' + end + + context 'when the domain can be resolved' do + let(:domain) { 'example.com' } + + before do + stub_dns(url, ip_address: '93.184.216.34') + end + + it_behaves_like 'dns rebinding checks' + end + end + + context 'with ports' do + let(:allowlist) do + ["127.0.0.1:2000"] + end + + it 'allows domain with port when resolved ip has port allowed' do + stub_domain_resolv("www.resolve-domain.com", '127.0.0.1', 2000) do + expect(described_class).not_to be_blocked_url( + "http://www.resolve-domain.com:2000", **url_blocker_attributes) + end + end + end + end + end + end + + describe 'enforce_user' do + context 'when false (default)' do + it 'does not block urls with a non-alphanumeric username' do + expect(described_class).not_to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a', schemes: ['ssh']) + + # The leading character here is a Unicode "soft hyphen" + expect(described_class).not_to be_blocked_url('ssh://oProxyCommand=whoami@example.com/a', schemes: ['ssh']) + + # Unicode alphanumerics are allowed + expect(described_class).not_to be_blocked_url('ssh://ğitlab@example.com/a', schemes: ['ssh']) + end + end + + context 'when true' do + it 'blocks urls with a non-alphanumeric username' do + aggregate_failures do + expect(described_class).to be_blocked_url( + 'ssh://-oProxyCommand=whoami@example.com/a', enforce_user: true, schemes: ['ssh']) + + # The leading character here is a Unicode "soft hyphen" + expect(described_class).to be_blocked_url( + 'ssh://oProxyCommand=whoami@example.com/a', enforce_user: true, schemes: ['ssh']) + + # Unicode alphanumerics are allowed + expect(described_class).not_to be_blocked_url( + 'ssh://ğitlab@example.com/a', enforce_user: true, schemes: ['ssh']) + end + end + end + end + + context 'when ascii_only is true' do + it 'returns true for unicode domain' do + expect(described_class.blocked_url?( + 'https://𝕘itⅼαƄ.com/foo/foo.bar', ascii_only: true, schemes: schemes)).to be true + end + + it 'returns true for unicode tld' do + expect(described_class.blocked_url?( + 'https://gitlab.ᴄοm/foo/foo.bar', ascii_only: true, schemes: schemes)).to be true + end + + it 'returns true for unicode path' do + expect(described_class.blocked_url?( + 'https://gitlab.com/𝒇οο/𝒇οο.Ƅαꮁ', ascii_only: true, schemes: schemes)).to be true + end + + it 'returns true for IDNA deviations' do + expect(described_class.blocked_url?( + 'https://mißile.com/foo/foo.bar', ascii_only: true, schemes: schemes)).to be true + expect(described_class.blocked_url?( + 'https://miςςile.com/foo/foo.bar', ascii_only: true, schemes: schemes)).to be true + expect(described_class.blocked_url?( + 'https://gitlab.com/foo/foo.bar', ascii_only: true, schemes: schemes)).to be true + expect(described_class.blocked_url?( + 'https://gitlab.com/foo/foo.bar', ascii_only: true, schemes: schemes)).to be true + end + end + + it 'blocks urls with invalid ip address' do + stub_env('RSPEC_ALLOW_INVALID_URLS', 'false') + + expect(described_class).to be_blocked_url('http://8.8.8.8.8', schemes: schemes) + end + + it 'blocks urls whose hostname cannot be resolved' do + stub_env('RSPEC_ALLOW_INVALID_URLS', 'false') + + expect(described_class).to be_blocked_url('http://foobar.x', schemes: schemes) + end + + context 'when gitlab is running on a non-default port' do + let(:gitlab_port) { 3000 } + + before do + Gitlab::HTTP_V2.configuration.allowed_internal_uris = [ + URI::HTTP.build( + scheme: 'http', + host: 'gitlab.local', + port: gitlab_port + ) + ] + end + + it 'returns true for url targeting the wrong port' do + stub_domain_resolv('gitlab.local', '127.0.0.1') do + expect(described_class).to be_blocked_url("http://gitlab.local/foo", schemes: schemes) + end + end + + it 'does not block url on gitlab port' do + stub_domain_resolv('gitlab.local', '127.0.0.1', gitlab_port) do + expect(described_class).not_to be_blocked_url("http://gitlab.local:#{gitlab_port}/foo", schemes: schemes) + end + end + end + + def stub_domain_resolv(domain, ip, port = 80) + address = instance_double(Addrinfo, + ip_address: ip, + ipv4_private?: true, + ipv6_linklocal?: false, + ipv4_loopback?: false, + ipv6_loopback?: false, + ipv4?: false, + ip_port: port + ) + allow(Addrinfo).to receive(:getaddrinfo).with(domain, port, any_args).and_return([address]) + allow(address).to receive(:ipv6_v4mapped?).and_return(false) + + yield + + allow(Addrinfo).to receive(:getaddrinfo).and_call_original + end + end + + describe '#validate_hostname' do + let(:ip_addresses) do + [ + '2001:db8:1f70::999:de8:7648:6e8', + 'FE80::C800:EFF:FE74:8', + '::ffff:127.0.0.1', + '::ffff:169.254.168.100', + '::ffff:7f00:1', + '0:0:0:0:0:ffff:0.0.0.0', + 'localhost', + '127.0.0.1', + '127.000.000.001', + '0x7f000001', + '0x7f.0.0.1', + '0x7f.0.0.1', + '017700000001', + '0177.1', + '2130706433', + '::', + '::1' + ] + end + + it 'does not raise error for valid Ip addresses' do + ip_addresses.each do |ip| + expect { described_class.send(:validate_hostname, ip) }.not_to raise_error + end + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2_spec.rb new file mode 100644 index 00000000000..bfa1dcd2633 --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/http_v2_spec.rb @@ -0,0 +1,453 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::HTTP_V2, feature_category: :shared do + context 'when allow_local_requests' do + it 'sends the request to the correct URI' do + stub_full_request('https://example.org:8080', ip_address: '8.8.8.8').to_return(status: 200) + + described_class.get('https://example.org:8080', allow_local_requests: false) + + expect(WebMock).to have_requested(:get, 'https://8.8.8.8:8080').once + end + end + + context 'when not allow_local_requests' do + it 'sends the request to the correct URI' do + stub_full_request('https://example.org:8080') + + described_class.get('https://example.org:8080', allow_local_requests: true) + + expect(WebMock).to have_requested(:get, 'https://8.8.8.9:8080').once + end + end + + context 'when reading the response is too slow' do + before(:all) do + # Override Net::HTTP to add a delay between sending each response chunk + mocked_http = Class.new(Net::HTTP) do + def request(*) + super do |response| + response.instance_eval do + def read_body(*) + mock_stream = @body.split(' ') + mock_stream.each do |fragment| + sleep 0.002.seconds + + yield fragment if block_given? + end + + @body + end + end + + yield response if block_given? + + response + end + end + end + + @original_net_http = Net.send(:remove_const, :HTTP) + @webmock_net_http = WebMock::HttpLibAdapters::NetHttpAdapter.instance_variable_get(:@webMockNetHTTP) + + Net.send(:const_set, :HTTP, mocked_http) + WebMock::HttpLibAdapters::NetHttpAdapter.instance_variable_set(:@webMockNetHTTP, mocked_http) + + # Reload Gitlab::NetHttpAdapter + described_class.send(:remove_const, :NetHttpAdapter) + load "gitlab/http_v2/net_http_adapter.rb" + end + + before do + stub_const("#{described_class}::Client::DEFAULT_READ_TOTAL_TIMEOUT", 0.001.seconds) + + WebMock.stub_request(:post, /.*/).to_return do + { body: "chunk-1 chunk-2", status: 200 } + end + end + + after(:all) do + Net.send(:remove_const, :HTTP) + Net.send(:const_set, :HTTP, @original_net_http) # rubocop:disable RSpec/InstanceVariable + WebMock::HttpLibAdapters::NetHttpAdapter.instance_variable_set(:@webMockNetHTTP, @webmock_net_http) # rubocop:disable RSpec/InstanceVariable + + # Reload Gitlab::NetHttpAdapter + described_class.send(:remove_const, :NetHttpAdapter) + load "gitlab/http_v2/net_http_adapter.rb" + end + + let(:options) { {} } + + subject(:request_slow_responder) { described_class.post('http://example.org', **options) } + + it 'raises an error' do + expect do + request_slow_responder + end.to raise_error(Gitlab::HTTP_V2::ReadTotalTimeout, + /Request timed out after ?([0-9]*[.])?[0-9]+ seconds/) + end + + context 'and timeout option is greater than DEFAULT_READ_TOTAL_TIMEOUT' do + let(:options) { { timeout: 10.seconds } } + + it 'does not raise an error' do + expect { request_slow_responder }.not_to raise_error + end + end + + context 'and stream_body option is truthy' do + let(:options) { { stream_body: true } } + + it 'does not raise an error' do + expect { request_slow_responder }.not_to raise_error + end + end + end + + it 'calls a block' do + WebMock.stub_request(:post, /.*/) + + expect { |b| described_class.post('http://example.org', &b) }.to yield_with_args + end + + describe 'allow_local_requests' do + before do + WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success') + end + + context 'when it is disabled' do + it 'deny requests to localhost' do + expect do + described_class.get('http://localhost:3003', allow_local_requests: false) + end.to raise_error(Gitlab::HTTP_V2::BlockedUrlError) + end + + it 'deny requests to private network' do + expect do + described_class.get('http://192.168.1.2:3003', allow_local_requests: false) + end.to raise_error(Gitlab::HTTP_V2::BlockedUrlError) + end + + context 'if allow_local_requests set to true' do + it 'override the global value and allow requests to localhost or private network' do + stub_full_request('http://localhost:3003') + + expect { described_class.get('http://localhost:3003', allow_local_requests: true) }.not_to raise_error + end + end + end + + context 'when it is enabled' do + it 'allow requests to localhost' do + stub_full_request('http://localhost:3003') + + expect { described_class.get('http://localhost:3003', allow_local_requests: true) }.not_to raise_error + end + + it 'allow requests to private network' do + expect { described_class.get('http://192.168.1.2:3003', allow_local_requests: true) }.not_to raise_error + end + + context 'if allow_local_requests set to false' do + it 'override the global value and ban requests to localhost or private network' do + expect do + described_class.get('http://localhost:3003', + allow_local_requests: false) + end.to raise_error(Gitlab::HTTP_V2::BlockedUrlError) + end + end + end + end + + describe 'handle redirect loops' do + before do + stub_full_request("http://example.org", method: :any) + .to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep")) + end + + it 'handles GET requests' do + expect { described_class.get('http://example.org') }.to raise_error(Gitlab::HTTP_V2::RedirectionTooDeep) + end + + it 'handles POST requests' do + expect { described_class.post('http://example.org') }.to raise_error(Gitlab::HTTP_V2::RedirectionTooDeep) + end + + it 'handles PUT requests' do + expect { described_class.put('http://example.org') }.to raise_error(Gitlab::HTTP_V2::RedirectionTooDeep) + end + + it 'handles DELETE requests' do + expect { described_class.delete('http://example.org') }.to raise_error(Gitlab::HTTP_V2::RedirectionTooDeep) + end + + it 'handles HEAD requests' do + expect { described_class.head('http://example.org') }.to raise_error(Gitlab::HTTP_V2::RedirectionTooDeep) + end + end + + describe 'setting default timeouts' do + let(:default_timeout_options) { described_class::Client::DEFAULT_TIMEOUT_OPTIONS } + + before do + stub_full_request('http://example.org', method: :any) + end + + context 'when no timeouts are set' do + it 'sets default open and read and write timeouts' do + expect(described_class::Client).to receive(:httparty_perform_request).with( + Net::HTTP::Get, 'http://example.org', default_timeout_options + ).and_call_original + + described_class.get('http://example.org') + end + end + + context 'when :timeout is set' do + it 'does not set any default timeouts' do + expect(described_class::Client).to receive(:httparty_perform_request).with( + Net::HTTP::Get, 'http://example.org', { timeout: 1 } + ).and_call_original + + described_class.get('http://example.org', timeout: 1) + end + end + + context 'when :open_timeout is set' do + it 'only sets default read and write timeout' do + expect(described_class::Client).to receive(:httparty_perform_request).with( + Net::HTTP::Get, 'http://example.org', default_timeout_options.merge(open_timeout: 1) + ).and_call_original + + described_class.get('http://example.org', open_timeout: 1) + end + end + + context 'when :read_timeout is set' do + it 'only sets default open and write timeout' do + expect(described_class::Client).to receive(:httparty_perform_request).with( + Net::HTTP::Get, 'http://example.org', default_timeout_options.merge(read_timeout: 1) + ).and_call_original + + described_class.get('http://example.org', read_timeout: 1) + end + end + + context 'when :write_timeout is set' do + it 'only sets default open and read timeout' do + expect(described_class::Client).to receive(:httparty_perform_request).with( + Net::HTTP::Put, 'http://example.org', default_timeout_options.merge(write_timeout: 1) + ).and_call_original + + described_class.put('http://example.org', write_timeout: 1) + end + end + end + + describe '.try_get' do + let(:path) { 'http://example.org' } + let(:default_timeout_options) { described_class::Client::DEFAULT_TIMEOUT_OPTIONS } + + let(:extra_log_info_proc) do + proc do |error, url, options| + { klass: error.class, url: url, options: options } + end + end + + let(:request_options) do + { + **default_timeout_options, + verify: false, + basic_auth: { username: 'user', password: 'pass' } + } + end + + described_class::HTTP_ERRORS.each do |exception_class| + context "with #{exception_class}" do + let(:klass) { exception_class } + + context 'with path' do + before do + expect(described_class::Client).to receive(:httparty_perform_request) # rubocop:disable RSpec/ExpectInHook + .with(Net::HTTP::Get, path, default_timeout_options) + .and_raise(klass) + end + + it 'handles requests without extra_log_info' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(klass), {}) + + expect(described_class.try_get(path)).to be_nil + end + + it 'handles requests with extra_log_info as hash' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(klass), { a: :b }) + + expect(described_class.try_get(path, extra_log_info: { a: :b })).to be_nil + end + + it 'handles requests with extra_log_info as proc' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(klass), { url: path, klass: klass, options: {} }) + + expect(described_class.try_get(path, extra_log_info: extra_log_info_proc)).to be_nil + end + end + + context 'with path and options' do + before do + expect(described_class::Client).to receive(:httparty_perform_request) # rubocop:disable RSpec/ExpectInHook + .with(Net::HTTP::Get, path, request_options) + .and_raise(klass) + end + + it 'handles requests without extra_log_info' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(klass), {}) + + expect(described_class.try_get(path, request_options)).to be_nil + end + + it 'handles requests with extra_log_info as hash' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(klass), { a: :b }) + + expect(described_class.try_get(path, **request_options, extra_log_info: { a: :b })).to be_nil + end + + it 'handles requests with extra_log_info as proc' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(klass), { klass: klass, url: path, options: request_options }) + + expect(described_class.try_get(path, **request_options, extra_log_info: extra_log_info_proc)).to be_nil + end + end + + context 'with path, options, and block' do + let(:block) do + proc {} + end + + before do + expect(described_class::Client).to receive(:httparty_perform_request) # rubocop:disable RSpec/ExpectInHook + .with(Net::HTTP::Get, path, request_options, &block) + .and_raise(klass) + end + + it 'handles requests without extra_log_info' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(klass), {}) + + expect(described_class.try_get(path, request_options, &block)).to be_nil + end + + it 'handles requests with extra_log_info as hash' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(klass), { a: :b }) + + expect(described_class.try_get(path, **request_options, extra_log_info: { a: :b }, &block)).to be_nil + end + + it 'handles requests with extra_log_info as proc' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(klass), { klass: klass, url: path, options: request_options }) + + expect( + described_class.try_get(path, **request_options, extra_log_info: extra_log_info_proc, &block) + ).to be_nil + end + end + end + end + end + + describe 'silent mode', feature_category: :geo_replication do + before do + stub_full_request("http://example.org", method: :any) + end + + context 'when silent mode is enabled' do + let(:silent_mode) { true } + + it 'allows GET requests' do + expect { described_class.get('http://example.org', silent_mode_enabled: silent_mode) }.not_to raise_error + end + + it 'allows HEAD requests' do + expect { described_class.head('http://example.org', silent_mode_enabled: silent_mode) }.not_to raise_error + end + + it 'allows OPTIONS requests' do + expect { described_class.options('http://example.org', silent_mode_enabled: silent_mode) }.not_to raise_error + end + + it 'blocks POST requests' do + expect do + described_class.post('http://example.org', silent_mode_enabled: silent_mode) + end.to raise_error(Gitlab::HTTP_V2::SilentModeBlockedError) + end + + it 'blocks PUT requests' do + expect do + described_class.put('http://example.org', silent_mode_enabled: silent_mode) + end.to raise_error(Gitlab::HTTP_V2::SilentModeBlockedError) + end + + it 'blocks DELETE requests' do + expect do + described_class.delete('http://example.org', silent_mode_enabled: silent_mode) + end.to raise_error(Gitlab::HTTP_V2::SilentModeBlockedError) + end + + it 'logs blocked requests' do + expect(described_class.configuration).to receive(:silent_mode_log_info).with( + "Outbound HTTP request blocked", 'Net::HTTP::Post' + ) + + expect do + described_class.post('http://example.org', silent_mode_enabled: silent_mode) + end.to raise_error(Gitlab::HTTP_V2::SilentModeBlockedError) + end + end + + context 'when silent mode is disabled' do + let(:silent_mode) { false } + + it 'allows GET requests' do + expect { described_class.get('http://example.org', silent_mode_enabled: silent_mode) }.not_to raise_error + end + + it 'allows HEAD requests' do + expect { described_class.head('http://example.org', silent_mode_enabled: silent_mode) }.not_to raise_error + end + + it 'allows OPTIONS requests' do + expect { described_class.options('http://example.org', silent_mode_enabled: silent_mode) }.not_to raise_error + end + + it 'blocks POST requests' do + expect { described_class.post('http://example.org', silent_mode_enabled: silent_mode) }.not_to raise_error + end + + it 'blocks PUT requests' do + expect { described_class.put('http://example.org', silent_mode_enabled: silent_mode) }.not_to raise_error + end + + it 'blocks DELETE requests' do + expect { described_class.delete('http://example.org', silent_mode_enabled: silent_mode) }.not_to raise_error + end + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/stub_requests.rb b/gems/gitlab-http/spec/gitlab/stub_requests.rb new file mode 100644 index 00000000000..ea4a6865251 --- /dev/null +++ b/gems/gitlab-http/spec/gitlab/stub_requests.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Gitlab + module StubRequests + IP_ADDRESS_STUB = '8.8.8.9' + + # Fully stubs a request using WebMock class. This class also + # stubs the IP address the URL is translated to (DNS lookup). + # + # It expects the final request to go to the `ip_address` instead the given url. + # That's primarily a DNS rebind attack prevention of Gitlab::HTTP + # (see: Gitlab::HTTP_V2::UrlBlocker). + # + def stub_full_request(url, ip_address: IP_ADDRESS_STUB, port: 80, method: :get) + stub_dns(url, ip_address: ip_address, port: port) + + url = stubbed_hostname(url, hostname: ip_address) + WebMock.stub_request(method, url) + end + + def stub_dns(url, ip_address:, port: 80) + url = parse_url(url) + socket = Socket.sockaddr_in(port, ip_address) + addr = Addrinfo.new(socket) + + # See Gitlab::UrlBlocker + allow(Addrinfo).to receive(:getaddrinfo) + .with(url.hostname, url.port, nil, :STREAM) + .and_return([addr]) + end + + def stub_all_dns(url, ip_address:) + url = URI(url) + port = 80 # arbitarily chosen, does not matter as we are not going to connect + socket = Socket.sockaddr_in(port, ip_address) + addr = Addrinfo.new(socket) + + # See Gitlab::UrlBlocker + allow(Addrinfo).to receive(:getaddrinfo).and_call_original + allow(Addrinfo).to receive(:getaddrinfo) + .with(url.hostname, anything, nil, :STREAM) + .and_return([addr]) + end + + def stubbed_hostname(url, hostname: IP_ADDRESS_STUB) + url = parse_url(url) + url.hostname = hostname + url.to_s + end + + private + + def parse_url(url) + url.is_a?(URI) ? url : URI(url) + end + end +end diff --git a/gems/gitlab-http/spec/spec_helper.rb b/gems/gitlab-http/spec/spec_helper.rb new file mode 100644 index 00000000000..a9bfc471aef --- /dev/null +++ b/gems/gitlab-http/spec/spec_helper.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'rails' +require 'rspec/mocks' + +require 'gitlab/rspec/all' +require 'gitlab/http_v2' +require 'gitlab/http_v2/configuration' +require 'gitlab/stub_requests' +require 'webmock/rspec' + +ENV["RSPEC_ALLOW_INVALID_URLS"] = 'true' # rubocop: disable RSpec/EnvAssignment + +RSpec.configure do |config| + config.include StubENV + config.include Gitlab::StubRequests + + # Enable flags like --only-failures and --next-failure + config.example_status_persistence_file_path = ".rspec_status" + + # Disable RSpec exposing methods globally on `Module` and `main` + config.disable_monkey_patching! + + config.expect_with :rspec do |c| + c.syntax = :expect + end +end + +Gitlab::HTTP_V2.configure do |config| + config.allowed_internal_uris = [ + URI::HTTP.build( + scheme: 'http', + host: 'localhost', + port: '80' + ), + URI::Generic.build( + scheme: 'ssh', + host: 'localhost', + port: '22' + ) + ] + + config.log_exception_proc = ->(exception, extra_info) do + # no-op + end + + config.silent_mode_log_info_proc = ->(message, http_method) do + # no-op + end +end diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/inconsistency.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/inconsistency.rb index 13799b8b9ff..503e05f12e9 100644 --- a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/inconsistency.rb +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/inconsistency.rb @@ -36,6 +36,17 @@ module Gitlab Diffy::Diff.new(structure_sql_statement, database_statement) end + def to_h + { + type: type, + object_type: object_type, + table_name: table_name, + object_name: object_name, + structure_sql_statement: structure_sql_statement, + database_statement: database_statement + } + end + def display <<~MSG #{'-' * 54} diff --git a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/inconsistency_spec.rb b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/inconsistency_spec.rb index 268bb4556e3..300383d5909 100644 --- a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/inconsistency_spec.rb +++ b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/inconsistency_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Schema::Validation::Inconsistency do +RSpec.describe Gitlab::Schema::Validation::Inconsistency, feature_category: :database do let(:validator) { Gitlab::Schema::Validation::Validators::DifferentDefinitionIndexes } let(:database_statement) { 'CREATE INDEX index_name ON public.achievements USING btree (namespace_id)' } @@ -44,6 +44,23 @@ RSpec.describe Gitlab::Schema::Validation::Inconsistency do end end + describe '#to_h' do + let(:result) do + { + database_statement: inconsistency.database_statement, + object_name: inconsistency.object_name, + object_type: inconsistency.object_type, + structure_sql_statement: inconsistency.structure_sql_statement, + table_name: inconsistency.table_name, + type: inconsistency.type + } + end + + it 'returns the to_h of the validator' do + expect(inconsistency.to_h).to eq(result) + end + end + describe '#table_name' do it 'returns the table name' do expect(inconsistency.table_name).to eq('achievements') |