summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2022-06-29 14:10:04 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2022-06-29 14:10:04 +0000
commit2059bbcc3ac8a3f05d625669bc9e03a126b24ff8 (patch)
tree506b2726fa70023442bbd2d12255ba7dbe4ffaed
parent0847e5f57325e0518d8cd4ffe303e5b7a2cec307 (diff)
downloadgitlab-ce-2059bbcc3ac8a3f05d625669bc9e03a126b24ff8.tar.gz
Add latest changes from gitlab-org/security/gitlab@14-10-stable-ee
-rw-r--r--app/graphql/resolvers/ci/config_resolver.rb2
-rw-r--r--app/models/concerns/enums/ci/commit_status.rb3
-rw-r--r--app/presenters/commit_status_presenter.rb3
-rw-r--r--config/initializers/net_http_response_patch.rb46
-rw-r--r--doc/update/index.md3
-rw-r--r--lib/gitlab/buffered_io.rb6
-rw-r--r--lib/gitlab/ci/status/build/failed.rb3
-rw-r--r--spec/graphql/resolvers/ci/config_resolver_spec.rb98
-rw-r--r--spec/initializers/net_http_response_patch_spec.rb79
-rw-r--r--spec/lib/gitlab/buffered_io_spec.rb64
10 files changed, 224 insertions, 83 deletions
diff --git a/app/graphql/resolvers/ci/config_resolver.rb b/app/graphql/resolvers/ci/config_resolver.rb
index f9d60650443..ccde39522c4 100644
--- a/app/graphql/resolvers/ci/config_resolver.rb
+++ b/app/graphql/resolvers/ci/config_resolver.rb
@@ -12,7 +12,7 @@ module Resolvers
Should not be requested more than once per request.
MD
- authorize :read_pipeline
+ authorize :create_pipeline
argument :project_path, GraphQL::Types::ID,
required: true,
diff --git a/app/models/concerns/enums/ci/commit_status.rb b/app/models/concerns/enums/ci/commit_status.rb
index 312b88a4d6d..445277a7a7c 100644
--- a/app/models/concerns/enums/ci/commit_status.rb
+++ b/app/models/concerns/enums/ci/commit_status.rb
@@ -35,7 +35,8 @@ module Enums
bridge_pipeline_is_child_pipeline: 1_006, # not used anymore, but cannot be deleted because of old data
downstream_pipeline_creation_failed: 1_007,
secrets_provider_not_found: 1_008,
- reached_max_descendant_pipelines_depth: 1_009
+ reached_max_descendant_pipelines_depth: 1_009,
+ ip_restriction_failure: 1_010
}
end
end
diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb
index fdfcc896bf8..675288da35b 100644
--- a/app/presenters/commit_status_presenter.rb
+++ b/app/presenters/commit_status_presenter.rb
@@ -30,7 +30,8 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
trace_size_exceeded: 'The job log size limit was reached',
builds_disabled: 'The CI/CD is disabled for this project',
environment_creation_failure: 'This job could not be executed because it would create an environment with an invalid parameter.',
- deployment_rejected: 'This deployment job was rejected.'
+ deployment_rejected: 'This deployment job was rejected.',
+ ip_restriction_failure: "This job could not be executed because group IP address restrictions are enabled, and the runner's IP address is not in the allowed range."
}.freeze
TROUBLESHOOTING_DOC = {
diff --git a/config/initializers/net_http_response_patch.rb b/config/initializers/net_http_response_patch.rb
new file mode 100644
index 00000000000..4ffe227a3fd
--- /dev/null
+++ b/config/initializers/net_http_response_patch.rb
@@ -0,0 +1,46 @@
+# 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::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 = sock.is_a?(Gitlab::BufferedIo) ? sock.readuntil("\n", true, start_time) : sock.readuntil("\n", true)
+ line = line.sub(/\s{0,10}\z/, '')
+ break if line.empty?
+ if line[0] == ?\s or line[0] == ?\t and value
+ # :nocov:
+ value << ' ' unless value.empty?
+ value << line.strip
+ # :nocov:
+ 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/doc/update/index.md b/doc/update/index.md
index 003c0fc02f7..b2d31489ed4 100644
--- a/doc/update/index.md
+++ b/doc/update/index.md
@@ -407,6 +407,9 @@ and [Helm Chart deployments](https://docs.gitlab.com/charts/). They come with ap
- The upgrade to GitLab 14.10 executes a [concurrent index drop](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84308) of unneeded
entries from the `ci_job_artifacts` database table. This could potentially run for multiple minutes, especially if the table has a lot of
traffic and the migration is unable to acquire a lock. It is advised to let this process finish as restarting may result in data loss.
+- Unauthenticated requests to the [`ciConfig` GraphQL field](../api/graphql/reference/index.md#queryciconfig) are no longer supported.
+ Before you upgrade to GitLab 15.1, add an [access token](../api/index.md#authentication) to your requests.
+ The user creating the token must have [permission](../user/permissions.md) to create pipelines in the project.
### 14.9.0
diff --git a/lib/gitlab/buffered_io.rb b/lib/gitlab/buffered_io.rb
index 91d5d8acc52..b76c8191fa2 100644
--- a/lib/gitlab/buffered_io.rb
+++ b/lib/gitlab/buffered_io.rb
@@ -14,6 +14,7 @@ module Gitlab
HEADER_READ_TIMEOUT = 20
+ # rubocop: disable Style/RedundantBegin
# rubocop: disable Style/RedundantReturn
# rubocop: disable Cop/LineBreakAfterGuardClauses
# rubocop: disable Layout/EmptyLineAfterGuardClause
@@ -21,9 +22,7 @@ module Gitlab
# Original method:
# https://github.com/ruby/ruby/blob/cdb7d699d0641e8f081d590d06d07887ac09961f/lib/net/protocol.rb#L190-L200
override :readuntil
- def readuntil(terminator, ignore_eof = false)
- start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
-
+ def readuntil(terminator, ignore_eof = false, start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC))
begin
until idx = @rbuf.index(terminator)
if (elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time) > HEADER_READ_TIMEOUT
@@ -39,6 +38,7 @@ module Gitlab
return rbuf_consume(@rbuf.size)
end
end
+ # rubocop: disable Style/RedundantBegin
# rubocop: enable Style/RedundantReturn
# rubocop: enable Cop/LineBreakAfterGuardClauses
# rubocop: enable Layout/EmptyLineAfterGuardClause
diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb
index 5dd28157b1f..1a074c1af53 100644
--- a/lib/gitlab/ci/status/build/failed.rb
+++ b/lib/gitlab/ci/status/build/failed.rb
@@ -35,7 +35,8 @@ module Gitlab
trace_size_exceeded: 'log size limit exceeded',
builds_disabled: 'project builds are disabled',
environment_creation_failure: 'environment creation failure',
- deployment_rejected: 'deployment rejected'
+ deployment_rejected: 'deployment rejected',
+ ip_restriction_failure: 'IP address restriction failure'
}.freeze
private_constant :REASONS
diff --git a/spec/graphql/resolvers/ci/config_resolver_spec.rb b/spec/graphql/resolvers/ci/config_resolver_spec.rb
index 3ff6d8f4347..570f547292c 100644
--- a/spec/graphql/resolvers/ci/config_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/config_resolver_spec.rb
@@ -7,24 +7,13 @@ RSpec.describe Resolvers::Ci::ConfigResolver do
describe '#resolve' do
let_it_be(:user) { create(:user) }
- let_it_be(:project) { create(:project, :repository, creator: user, namespace: user.namespace) }
+ let_it_be(:project) { create(:project, :repository) }
let_it_be(:sha) { nil }
let_it_be(:content) do
File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_includes.yml'))
end
- let(:ci_lint) do
- ci_lint_double = instance_double(::Gitlab::Ci::Lint)
- allow(ci_lint_double).to receive(:validate).and_return(fake_result)
-
- ci_lint_double
- end
-
- before do
- allow(::Gitlab::Ci::Lint).to receive(:new).and_return(ci_lint)
- end
-
subject(:response) do
resolve(described_class,
args: { project_path: project.full_path, content: content, sha: sha },
@@ -49,51 +38,76 @@ RSpec.describe Resolvers::Ci::ConfigResolver do
end
end
- context 'with a valid .gitlab-ci.yml' do
- context 'with a sha' do
- let(:sha) { '1231231' }
+ context 'when the user can create a pipeline' do
+ let(:ci_lint) do
+ ci_lint_double = instance_double(::Gitlab::Ci::Lint)
+ allow(ci_lint_double).to receive(:validate).and_return(fake_result)
- it_behaves_like 'a valid config file'
+ ci_lint_double
end
- context 'without a sha' do
- it_behaves_like 'a valid config file'
+ before do
+ allow(::Gitlab::Ci::Lint).to receive(:new).and_return(ci_lint)
+
+ project.add_developer(user)
end
- end
- context 'with an invalid .gitlab-ci.yml' do
- let(:content) { 'invalid' }
+ context 'with a valid .gitlab-ci.yml' do
+ context 'with a sha' do
+ let(:sha) { '1231231' }
- let(:fake_result) do
- Gitlab::Ci::Lint::Result.new(
- jobs: [],
- merged_yaml: content,
- errors: ['Invalid configuration format'],
- warnings: []
- )
+ it_behaves_like 'a valid config file'
+ end
+
+ context 'without a sha' do
+ it_behaves_like 'a valid config file'
+ end
end
- it 'responds with errors about invalid syntax' do
- expect(response[:status]).to eq(:invalid)
- expect(response[:errors]).to eq(['Invalid configuration format'])
+ context 'with an invalid .gitlab-ci.yml' do
+ let(:content) { 'invalid' }
+
+ let(:fake_result) do
+ Gitlab::Ci::Lint::Result.new(
+ jobs: [],
+ merged_yaml: content,
+ errors: ['Invalid configuration format'],
+ warnings: []
+ )
+ end
+
+ it 'responds with errors about invalid syntax' do
+ expect(response[:status]).to eq(:invalid)
+ expect(response[:errors]).to match_array(['Invalid configuration format'])
+ end
end
- end
- context 'with an invalid SHA' do
- let_it_be(:sha) { ':' }
+ context 'with an invalid SHA' do
+ let_it_be(:sha) { ':' }
- let(:ci_lint) do
- ci_lint_double = instance_double(::Gitlab::Ci::Lint)
- allow(ci_lint_double).to receive(:validate).and_raise(GRPC::InvalidArgument)
+ let(:ci_lint) do
+ ci_lint_double = instance_double(::Gitlab::Ci::Lint)
+ allow(ci_lint_double).to receive(:validate).and_raise(GRPC::InvalidArgument)
- ci_lint_double
+ ci_lint_double
+ end
+
+ it 'logs the invalid SHA to Sentry' do
+ expect(Gitlab::ErrorTracking).to receive(:track_and_raise_exception)
+ .with(GRPC::InvalidArgument, sha: ':')
+
+ response
+ end
end
+ end
- it 'logs the invalid SHA to Sentry' do
- expect(Gitlab::ErrorTracking).to receive(:track_and_raise_exception)
- .with(GRPC::InvalidArgument, sha: ':')
+ context 'when the user cannot create a pipeline' do
+ before do
+ project.add_guest(user)
+ end
- response
+ it 'returns an error stating that the user cannot access the linting' do
+ expect { response }.to raise_error(::Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
end
diff --git a/spec/initializers/net_http_response_patch_spec.rb b/spec/initializers/net_http_response_patch_spec.rb
new file mode 100644
index 00000000000..3bd0d8c3907
--- /dev/null
+++ b/spec/initializers/net_http_response_patch_spec.rb
@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Net::HTTPResponse patch header read timeout' do
+ describe '.each_response_header' do
+ let(:server_response) do
+ <<~EOS
+ Content-Type: text/html
+ Header-Two: foo
+
+ Hello World
+ EOS
+ end
+
+ before do
+ stub_const('Gitlab::BufferedIo::HEADER_READ_TIMEOUT', 0.1)
+ end
+
+ subject(:each_response_header) { Net::HTTPResponse.each_response_header(socket) { |k, v| } }
+
+ 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
+ before do
+ expect(socket).to receive(:readuntil).and_return(
+ "a: #{' ' * 100_000} b",
+ ''
+ )
+ end
+
+ it 'has no regex backtracking issues' do
+ Timeout.timeout(1) do
+ each_response_header
+ end
+ end
+ end
+ end
+
+ context 'with Gitlab::BufferedIo' do
+ let(:mock_io) { StringIO.new(server_response) }
+ let(:socket) { Gitlab::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::HeaderReadTimeout,
+ /Request timed out after reading headers for 0\.[0-9]+ seconds/)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/buffered_io_spec.rb b/spec/lib/gitlab/buffered_io_spec.rb
index f8896abd46e..c6939b819e2 100644
--- a/spec/lib/gitlab/buffered_io_spec.rb
+++ b/spec/lib/gitlab/buffered_io_spec.rb
@@ -1,54 +1,50 @@
-# rubocop:disable Style/FrozenStringLiteralComment
+# frozen_string_literal: true
+
require 'spec_helper'
RSpec.describe Gitlab::BufferedIo do
describe '#readuntil' do
- let(:never_ending_tcp_socket) do
- Class.new do
- def initialize(*_)
- @read_counter = 0
- end
+ let(:mock_io) { StringIO.new('a') }
+ let(:start_time) { Process.clock_gettime(Process::CLOCK_MONOTONIC) }
- def setsockopt(*_); end
+ before do
+ stub_const('Gitlab::BufferedIo::HEADER_READ_TIMEOUT', 0.1)
+ end
- def closed?
- false
- end
+ subject(:readuntil) do
+ Gitlab::BufferedIo.new(mock_io).readuntil('a', false, start_time)
+ end
- def close
- true
- end
+ it 'does not raise a timeout error' do
+ expect { readuntil }.not_to raise_error
+ end
- def to_io
- StringIO.new('Hello World!')
- end
+ context 'when the response contains infinitely long headers' do
+ before do
+ read_counter = 0
- def write_nonblock(data, *_)
- data.size
- end
+ allow(mock_io).to receive(:read_nonblock) do |buffer_size, *_|
+ read_counter += 1
+ raise 'Test did not raise HeaderReadTimeout' if read_counter > 10
- def read_nonblock(buffer_size, *_)
sleep 0.01
- @read_counter += 1
-
- raise 'Test did not raise HeaderReadTimeout' if @read_counter > 10
-
'H' * buffer_size
end
end
- end
- before do
- stub_const('Gitlab::BufferedIo::HEADER_READ_TIMEOUT', 0.1)
- end
+ it 'raises a timeout error' do
+ expect { readuntil }.to raise_error(Gitlab::HTTP::HeaderReadTimeout, /Request timed out after reading headers for 0\.[0-9]+ seconds/)
+ end
- subject(:readuntil) do
- Gitlab::BufferedIo.new(never_ending_tcp_socket.new).readuntil('a')
- end
+ context 'when not passing start_time' do
+ subject(:readuntil) do
+ Gitlab::BufferedIo.new(mock_io).readuntil('a', false)
+ end
- it 'raises a timeout error' do
- expect { readuntil }.to raise_error(Gitlab::HTTP::HeaderReadTimeout, /Request timed out after reading headers for 0\.[0-9]+ seconds/)
+ it 'raises a timeout error' do
+ expect { readuntil }.to raise_error(Gitlab::HTTP::HeaderReadTimeout, /Request timed out after reading headers for 0\.[0-9]+ seconds/)
+ end
+ end
end
end
end
-# rubocop:enable Style/FrozenStringLiteralComment