Commit 5a70276c authored by Mayra Cabrera's avatar Mayra Cabrera

Merge remote-tracking branch 'security/master'

parents 562f3b1f 10627a13
Please view this file on the master branch, on stable branches it's out of date.
## 13.8.2 (2021-02-01)
### Security (2 changes)
- Remove Kubernetes IP address from error messages returned in Threat Monitoring.
- Sanitize XSS in Epic milestone due date.
## 13.8.1 (2021-01-26)
### Fixed (2 changes)
......@@ -119,6 +127,14 @@ Please view this file on the master branch, on stable branches it's out of date.
- Enable DevOps Adoption Report feature flag if any Segments already exist. !51602
## 13.7.6 (2021-02-01)
### Security (2 changes)
- Remove Kubernetes IP address from error messages returned in Threat Monitoring.
- Sanitize XSS in Epic milestone due date.
## 13.7.5 (2021-01-25)
### Fixed (1 change)
......@@ -300,6 +316,14 @@ Please view this file on the master branch, on stable branches it's out of date.
- Rename code coverage analytics sections. !49931
## 13.6.6 (2021-02-01)
### Security (2 changes)
- Remove Kubernetes IP address from error messages returned in Threat Monitoring.
- Sanitize XSS in Epic milestone due date.
## 13.6.5 (2021-01-13)
- No changes.
......
......@@ -2,6 +2,17 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
## 13.8.2 (2021-02-01)
### Security (5 changes)
- Filter sensitive GraphQL variables from logs.
- Avoid exposing release links when the user cannot read git-tag/repository.
- Sanitize target branch on MR page.
- Fix DNS rebinding protection bypass when allowing an IP address in Outbound Requests setting.
- Add routes for unmatched url for not-get requests.
## 13.8.1 (2021-01-26)
### Fixed (3 changes)
......@@ -368,6 +379,17 @@ entry.
- Add verbiage + link sast to show it's in core. !51935
## 13.7.6 (2021-02-01)
### Security (5 changes)
- Filter sensitive GraphQL variables from logs.
- Avoid exposing release links when the user cannot read git-tag/repository.
- Sanitize target branch on MR page.
- Fix DNS rebinding protection bypass when allowing an IP address in Outbound Requests setting.
- Add routes for unmatched url for not-get requests.
## 13.7.5 (2021-01-25)
### Fixed (2 changes, 1 of them is from the community)
......@@ -878,6 +900,17 @@ entry.
- Update GitLab Workhorse to v8.57.0.
## 13.6.6 (2021-02-01)
### Security (5 changes)
- Filter sensitive GraphQL variables from logs.
- Avoid exposing release links when the user cannot read git-tag/repository.
- Sanitize target branch on MR page.
- Fix DNS rebinding protection bypass when allowing an IP address in Outbound Requests setting.
- Add routes for unmatched url for not-get requests.
## 13.6.5 (2021-01-13)
### Security (1 change)
......
<script>
import { isNumber } from 'lodash';
import { sanitize } from '~/lib/dompurify';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import ArtifactsApp from './artifacts_list_app.vue';
import MrWidgetContainer from './mr_widget_container.vue';
......@@ -40,7 +41,7 @@ export default {
return this.isPostMerge ? this.mr.targetBranch : this.mr.sourceBranch;
},
branchLink() {
return this.isPostMerge ? this.mr.targetBranch : this.mr.sourceBranchLink;
return this.isPostMerge ? sanitize(this.mr.targetBranch) : this.mr.sourceBranchLink;
},
deployments() {
return this.isPostMerge ? this.mr.postMergeDeployments : this.mr.deployments;
......
......@@ -5,6 +5,9 @@ class Projects::ReleasesController < Projects::ApplicationController
before_action :require_non_empty_project, except: [:index]
before_action :release, only: %i[edit show update downloads]
before_action :authorize_read_release!
# We have to check `download_code` permission because detail URL path
# contains git-tag name.
before_action :authorize_download_code!, except: [:index]
before_action do
push_frontend_feature_flag(:graphql_release_data, project, default_enabled: true)
push_frontend_feature_flag(:graphql_milestone_stats, project, default_enabled: true)
......
......@@ -85,10 +85,18 @@ module TokenAuthenticatableStrategies
end
def find_by_encrypted_token(token, unscoped)
encrypted_value = Gitlab::CryptoHelper.aes256_gcm_encrypt(token)
nonce = Feature.enabled?(:dynamic_nonce_creation) ? find_hashed_iv(token) : Gitlab::CryptoHelper::AES256_GCM_IV_STATIC
encrypted_value = Gitlab::CryptoHelper.aes256_gcm_encrypt(token, nonce: nonce)
relation(unscoped).find_by(encrypted_field => encrypted_value)
end
def find_hashed_iv(token)
token_record = TokenWithIv.find_by_plaintext_token(token)
token_record&.iv || Gitlab::CryptoHelper::AES256_GCM_IV_STATIC
end
def insecure_strategy
@insecure_strategy ||= TokenAuthenticatableStrategies::Insecure
.new(klass, token_field, options)
......
# frozen_string_literal: true
# rubocop: todo Gitlab/NamespacedClass
class TokenWithIv < ApplicationRecord
validates :hashed_token, presence: true
validates :iv, presence: true
validates :hashed_plaintext_token, presence: true
def self.find_by_hashed_token(value)
find_by(hashed_token: ::Digest::SHA256.digest(value))
end
def self.find_by_plaintext_token(value)
find_by(hashed_plaintext_token: ::Digest::SHA256.digest(value))
end
def self.find_nonce_by_hashed_token(value)
return unless table_exists?
token_record = find_by_hashed_token(value)
token_record&.iv
end
end
......@@ -20,6 +20,8 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated
end
def self_url
return unless can_download_code?
project_release_url(project, release)
end
......
---
title: Add token_with_iv table
merge_request:
author:
type: security
---
name: dynamic_nonce_creation
introduced_by_url:
rollout_issue_url:
milestone: '13.9'
type: development
group: group::manage
default_enabled: false
......@@ -277,6 +277,7 @@ Rails.application.routes.draw do
draw :dashboard
draw :user
draw :project
draw :unmatched_project
# Issue https://gitlab.com/gitlab-org/gitlab/-/issues/210024
scope as: 'deprecated' do
......
# frozen_string_literal: true
scope(path: '*namespace_id',
as: :namespace,
namespace_id: Gitlab::PathRegex.full_namespace_route_regex) do
scope(path: ':project_id',
constraints: { project_id: Gitlab::PathRegex.project_route_regex },
as: :project) do
post '*all', to: 'application#route_not_found'
put '*all', to: 'application#route_not_found'
patch '*all', to: 'application#route_not_found'
delete '*all', to: 'application#route_not_found'
post '/', to: 'application#route_not_found'
put '/', to: 'application#route_not_found'
patch '/', to: 'application#route_not_found'
delete '/', to: 'application#route_not_found'
end
end
# frozen_string_literal: true
class CreateTokensWithIv < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
create_table :token_with_ivs do |t|
t.binary :hashed_token, null: false
t.binary :hashed_plaintext_token, null: false
t.binary :iv, null: false
t.index :hashed_token, name: 'index_token_with_ivs_on_hashed_token', unique: true, using: :btree
t.index :hashed_plaintext_token, name: 'index_token_with_ivs_on_hashed_plaintext_token', unique: true, using: :btree
end
end
end
......@@ -10,7 +10,7 @@ class EncryptFeatureFlagsClientsTokens < ActiveRecord::Migration[5.1]
def up
say_with_time("Encrypting tokens from operations_feature_flags_clients") do
FeatureFlagsClient.where('token_encrypted is NULL AND token IS NOT NULL').find_each do |feature_flags_client|
token_encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(feature_flags_client.token)
token_encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(feature_flags_client.token, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC)
feature_flags_client.update!(token_encrypted: token_encrypted)
end
end
......
......@@ -10,7 +10,7 @@ class EncryptDeployTokensTokens < ActiveRecord::Migration[5.1]
def up
say_with_time("Encrypting tokens from deploy_tokens") do
DeploymentTokens.where('token_encrypted is NULL AND token IS NOT NULL').find_each(batch_size: 10000) do |deploy_token|
token_encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(deploy_token.token)
token_encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(deploy_token.token, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC)
deploy_token.update!(token_encrypted: token_encrypted)
end
end
......
dde424c434c78e22087123fa30eec75c07268a9079fea44339915747aae235e0
\ No newline at end of file
......@@ -17439,6 +17439,22 @@ CREATE SEQUENCE todos_id_seq
ALTER SEQUENCE todos_id_seq OWNED BY todos.id;
CREATE TABLE token_with_ivs (
id bigint NOT NULL,
hashed_token bytea NOT NULL,
hashed_plaintext_token bytea NOT NULL,
iv bytea NOT NULL
);
CREATE SEQUENCE token_with_ivs_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE token_with_ivs_id_seq OWNED BY token_with_ivs.id;
CREATE TABLE trending_projects (
id integer NOT NULL,
project_id integer NOT NULL
......@@ -19161,6 +19177,8 @@ ALTER TABLE ONLY timelogs ALTER COLUMN id SET DEFAULT nextval('timelogs_id_seq':
ALTER TABLE ONLY todos ALTER COLUMN id SET DEFAULT nextval('todos_id_seq'::regclass);
ALTER TABLE ONLY token_with_ivs ALTER COLUMN id SET DEFAULT nextval('token_with_ivs_id_seq'::regclass);
ALTER TABLE ONLY trending_projects ALTER COLUMN id SET DEFAULT nextval('trending_projects_id_seq'::regclass);
ALTER TABLE ONLY u2f_registrations ALTER COLUMN id SET DEFAULT nextval('u2f_registrations_id_seq'::regclass);
......@@ -20689,6 +20707,9 @@ ALTER TABLE ONLY timelogs
ALTER TABLE ONLY todos
ADD CONSTRAINT todos_pkey PRIMARY KEY (id);
ALTER TABLE ONLY token_with_ivs
ADD CONSTRAINT token_with_ivs_pkey PRIMARY KEY (id);
ALTER TABLE ONLY trending_projects
ADD CONSTRAINT trending_projects_pkey PRIMARY KEY (id);
......@@ -23225,6 +23246,10 @@ CREATE INDEX index_todos_on_user_id_and_id_done ON todos USING btree (user_id, i
CREATE INDEX index_todos_on_user_id_and_id_pending ON todos USING btree (user_id, id) WHERE ((state)::text = 'pending'::text);
CREATE UNIQUE INDEX index_token_with_ivs_on_hashed_plaintext_token ON token_with_ivs USING btree (hashed_plaintext_token);
CREATE UNIQUE INDEX index_token_with_ivs_on_hashed_token ON token_with_ivs USING btree (hashed_token);
CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects USING btree (project_id);
CREATE INDEX index_u2f_registrations_on_key_handle ON u2f_registrations USING btree (key_handle);
......
......@@ -6,6 +6,7 @@ import { __, s__, sprintf } from '~/locale';
import createGqClient, { fetchPolicies } from '~/lib/graphql';
import { parseBoolean } from '~/lib/utils/common_utils';
import { dateInWords, parsePikadayDate } from '~/lib/utils/datetime_utility';
import { sanitize } from '~/lib/dompurify';
import { dateTypes } from '../constants';
......@@ -54,8 +55,9 @@ const getDateFromMilestonesTooltip = ({
dueDateSourcingMilestoneDates,
dueDateTimeFromMilestones,
}) => {
const dateSourcingMilestoneTitle =
dateType === dateTypes.start ? startDateSourcingMilestoneTitle : dueDateSourcingMilestoneTitle;
const dateSourcingMilestoneTitle = sanitize(
dateType === dateTypes.start ? startDateSourcingMilestoneTitle : dueDateSourcingMilestoneTitle,
);
const sourcingMilestoneDates =
dateType === dateTypes.start ? startDateSourcingMilestoneDates : dueDateSourcingMilestoneDates;
......
......@@ -23,7 +23,7 @@ module NetworkPolicies
ServiceResponse.success
rescue Kubeclient::HttpError => e
kubernetes_error_response(e)
kubernetes_error_response(e.message)
end
end
end
......@@ -26,7 +26,7 @@ module NetworkPolicies
load_policy_from_resource
ServiceResponse.success(payload: policy)
rescue Kubeclient::HttpError => e
kubernetes_error_response(e)
kubernetes_error_response(e.message)
end
private
......
......@@ -16,7 +16,7 @@ module NetworkPolicies
ServiceResponse.success(payload: get_policy)
rescue Kubeclient::HttpError => e
kubernetes_error_response(e)
kubernetes_error_response(e.message)
end
private
......
......@@ -23,7 +23,7 @@ RSpec.describe 'Geo read-only message', :geo do
context 'when in maintenance mode' do
before do
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
end
it_behaves_like 'Read-only instance', /This GitLab instance is undergoing maintenance and is operating in read\-only mode./
......
......@@ -57,4 +57,23 @@ describe('epicUtils', () => {
expect(Cookies.get('collapsed_gutter')).toBe(`${collapsedGutterVal}`); // Cookie value will always be string
});
});
describe('getDateFromMilestonesTooltip', () => {
it('Sanitizes html in milestone title', () => {
const tooltipText = epicUtils.getDateFromMilestonesTooltip({
dateType: 'start',
startDateSourcingMilestoneTitle:
'<svg width="100"><use xlink:href="/h5bp/html5-boilerplate/-/raw/master/demo.svg#x" /></svg>',
startDateSourcingMilestoneDates: {
startDate: '2020-12-23',
dueDate: '2021-01-28',
},
startDateTimeFromMilestones: '2020-12-22T18:30:00.000Z',
dueDateTimeFromMilestones: '2021-01-27T18:30:00.000Z',
});
const sanitizedTitle = '<svg width="100"><use></use></svg>';
expect(tooltipText.startsWith(sanitizedTitle)).toBe(true);
});
});
});
......@@ -22,7 +22,7 @@ RSpec.describe ApplicationHelper do
context 'maintenance mode' do
context 'enabled' do
before do
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
end
it 'returns default message' do
......@@ -48,7 +48,7 @@ RSpec.describe ApplicationHelper do
context 'disabled' do
it 'returns nil' do
stub_application_setting(maintenance_mode: false)
stub_maintenance_mode_setting(false)
expect(helper.read_only_message).to be_nil
end
......@@ -60,7 +60,7 @@ RSpec.describe ApplicationHelper do
context 'maintenance mode on' do
it 'returns messages for both' do
expect(Gitlab::Geo).to receive(:secondary?).twice { true }
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
expect(helper.read_only_message).to match(/you must visit the primary site/)
expect(helper.read_only_message).to match(/#{default_maintenance_mode_message}/)
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::CryptoHelper do
include ::EE::GeoHelpers
describe '.read_only?' do
context 'with Geo enabled' do
before do
allow(Gitlab::Geo).to receive(:enabled?) { true }
allow(Gitlab::Geo).to receive(:current_node) { geo_node }
end
context 'is Geo secondary node' do
let(:geo_node) { create(:geo_node) }
it 'returns true' do
expect(described_class.read_only?).to be_truthy
end
end
context 'is Geo primary node' do
let(:geo_node) { create(:geo_node, :primary) }
it 'returns false when is Geo primary node' do
expect(described_class.read_only?).to be_falsey
end
end
end
end
end
......@@ -37,7 +37,7 @@ RSpec.describe Gitlab::Database do
context 'in maintenance mode' do
before do
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
end
it 'returns true' do
......
......@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe Gitlab::Middleware::ReadOnly do
context 'when maintenance mode is on' do
before do
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
end
it_behaves_like 'write access for a read-only GitLab (EE) instance in maintenance mode'
......@@ -13,7 +13,7 @@ RSpec.describe Gitlab::Middleware::ReadOnly do
context 'when maintenance mode is not on' do
before do
stub_application_setting(maintenance_mode: false)
stub_maintenance_mode_setting(false)
end
it_behaves_like 'write access for a read-only GitLab (EE) instance'
......
......@@ -758,7 +758,7 @@ RSpec.describe Gitlab::GitAccess do
context 'when maintenance mode is enabled' do
before do
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
end
it 'blocks git push' do
......@@ -770,7 +770,7 @@ RSpec.describe Gitlab::GitAccess do
context 'when maintenance mode is disabled' do
before do
stub_application_setting(maintenance_mode: false)
stub_maintenance_mode_setting(false)
end
it 'allows git push' do
......
......@@ -12,8 +12,8 @@ RSpec.describe NullifyFeatureFlagPlaintextTokens do
let!(:project1) { projects.create!(namespace_id: namespace.id, name: 'Project 1') }
let!(:project2) { projects.create!(namespace_id: namespace.id, name: 'Project 2') }
let(:secret1_encrypted) { Gitlab::CryptoHelper.aes256_gcm_encrypt('secret1') }
let(:secret2_encrypted) { Gitlab::CryptoHelper.aes256_gcm_encrypt('secret2') }
let(:secret1_encrypted) { Gitlab::CryptoHelper.aes256_gcm_encrypt('secret1', nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC) }
let(:secret2_encrypted) { Gitlab::CryptoHelper.aes256_gcm_encrypt('secret2', nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC) }
before do
feature_flags_clients.create!(token: 'secret1', token_encrypted: secret1_encrypted, project_id: project1.id)
......
......@@ -248,7 +248,7 @@ RSpec.describe API::Internal::Base do
let_it_be(:project) { create(:project, :repository) }
before do
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
project.add_developer(user)
end
......
......@@ -9,5 +9,11 @@ RSpec.describe 'EE git_http routing' do
let(:container_path) { '/gitlab-org/gitlab-test' }
let(:params) { { geo_node_id: 'node', repository_path: 'gitlab-org/gitlab-test.git' } }
end
it_behaves_like 'git repository routes with fallback for git-upload-pack' do
let(:path) { '/-/push_from_secondary/node/gitlab-org/gitlab-test.git' }
let(:container_path) { '/gitlab-org/gitlab-test' }
let(:params) { { geo_node_id: 'node', repository_path: 'gitlab-org/gitlab-test.git' } }
end
end
end
......@@ -19,7 +19,7 @@ RSpec.describe Auth::ContainerRegistryAuthenticationService do
end
before do
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
project.add_developer(current_user)
end
......
......@@ -49,8 +49,11 @@ RSpec.describe NetworkPolicies::DeleteResourceService do
end
context 'with Kubeclient::HttpError' do
let(:request_url) { 'https://kubernetes.local' }
let(:response) { RestClient::Response.create('', {}, RestClient::Request.new(url: request_url, method: :get)) }
before do
allow(kubeclient).to receive(:delete_network_policy).and_raise(Kubeclient::HttpError.new(500, 'system failure', nil))
allow(kubeclient).to receive(:delete_network_policy).and_raise(Kubeclient::HttpError.new(500, 'system failure', response))
end
it 'returns error response' do
......@@ -58,6 +61,10 @@ RSpec.describe NetworkPolicies::DeleteResourceService do
expect(subject.http_status).to eq(:bad_request)
expect(subject.message).not_to be_nil
end
it 'returns error message without request url' do
expect(subject.message).not_to include(request_url)
end
end
context 'with CiliumNetworkPolicy' do
......
......@@ -94,8 +94,11 @@ RSpec.describe NetworkPolicies::DeployResourceService do
end
context 'with Kubeclient::HttpError' do
let(:request_url) { 'https://kubernetes.local' }
let(:response) { RestClient::Response.create('', {}, RestClient::Request.new(url: request_url, method: :get)) }
before do
allow(kubeclient).to receive(:create_network_policy).and_raise(Kubeclient::HttpError.new(500, 'system failure', nil))
allow(kubeclient).to receive(:create_network_policy).and_raise(Kubeclient::HttpError.new(500, 'system failure', response))
end
it 'returns error response' do
......@@ -103,6 +106,10 @@ RSpec.describe NetworkPolicies::DeployResourceService do
expect(subject.http_status).to eq(:bad_request)
expect(subject.message).not_to be_nil
end
it 'returns error message without request url' do
expect(subject.message).not_to include(request_url)
end
end
context 'with cilium network policy' do
......
......@@ -62,8 +62,11 @@ RSpec.describe NetworkPolicies::FindResourceService do
end
context 'with Kubeclient::HttpError' do
let(:request_url) { 'https://kubernetes.local' }
let(:response) { RestClient::Response.create('', {}, RestClient::Request.new(url: request_url, method: :get)) }
before do
allow(kubeclient).to receive(:get_network_policy).and_raise(Kubeclient::HttpError.new(500, 'system failure', nil))
allow(kubeclient).to receive(:get_network_policy).and_raise(Kubeclient::HttpError.new(500, 'system failure', response))
end
it 'returns error response' do
......@@ -71,6 +74,10 @@ RSpec.describe NetworkPolicies::FindResourceService do
expect(subject.http_status).to eq(:bad_request)
expect(subject.message).not_to be_nil
end
it 'returns error message without request url' do
expect(subject.message).not_to include(request_url)
end
end
end
end
......@@ -7,7 +7,7 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in main
include_context 'with a mocked GitLab instance'
before do
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
end
context 'normal requests to a read-only GitLab instance' do
......
......@@ -118,6 +118,7 @@ module Gitlab
def self.maintenance_mode?
return false unless ::Feature.enabled?(:maintenance_mode)
return false unless ::Gitlab::CurrentSettings.current_application_settings?
::Gitlab::CurrentSettings.maintenance_mode
end
......
......@@ -6,25 +6,44 @@ module Gitlab
AES256_GCM_OPTIONS = {
algorithm: 'aes-256-gcm',
key: Settings.attr_encrypted_db_key_base_32,
iv: Settings.attr_encrypted_db_key_base_12
key: Settings.attr_encrypted_db_key_base_32
}.freeze
AES256_GCM_IV_STATIC = Settings.attr_encrypted_db_key_base_12
def sha256(value)
salt = Settings.attr_encrypted_db_key_base_truncated
::Digest::SHA256.base64digest("#{value}#{salt}")
end
def aes256_gcm_encrypt(value)
encrypted_token = Encryptor.encrypt(AES256_GCM_OPTIONS.merge(value: value))
Base64.strict_encode64(encrypted_token)
def aes256_gcm_encrypt(value, nonce: nil)
aes256_gcm_encrypt_using_static_nonce(value)
end
def aes256_gcm_decrypt(value)
return unless value
nonce = Feature.enabled?(:dynamic_nonce_creation) ? dynamic_nonce(value) : AES256_GCM_IV_STATIC
encrypted_token = Base64.decode64(value)
Encryptor.decrypt(AES256_GCM_OPTIONS.merge(value: encrypted_token))
decrypted_token = Encryptor.decrypt(AES256_GCM_OPTIONS.merge(value: encrypted_token, iv: nonce))
decrypted_token
end
def dynamic_nonce(value)
TokenWithIv.find_nonce_by_hashed_token(value) || AES256_GCM_IV_STATIC
end
def aes256_gcm_encrypt_using_static_nonce(value)
create_encrypted_token(value, AES256_GCM_IV_STATIC)
end
def read_only?
Gitlab::Database.read_only?
end
def create_encrypted_token(value, iv)
encrypted_token = Encryptor.encrypt(AES256_GCM_OPTIONS.merge(value: value, iv: iv))
Base64.strict_encode64(encrypted_token)
end
end
end
......@@ -7,6 +7,10 @@ module Gitlab
Gitlab::SafeRequestStore.fetch(:current_application_settings) { ensure_application_settings! }
end
def current_application_settings?
Gitlab::SafeRequestStore.exist?(:current_application_settings) || ::ApplicationSetting.current.present?
end
def expire_current_application_settings
::ApplicationSetting.expire
Gitlab::SafeRequestStore.delete(:current_application_settings)
......
......@@ -49,11 +49,19 @@ module Gitlab
private
def process_variables(variables)
if variables.respond_to?(:to_s)
variables.to_s
filtered_variables = filter_sensitive_variables(variables)
if filtered_variables.respond_to?(:to_s)
filtered_variables.to_s
else
variables
filtered_variables
end
end
def filter_sensitive_variables(variables)
ActiveSupport::ParameterFilter
.new(::Rails.application.config.filter_parameters)
.filter(variables)
end
def duration(time_started)
......
......@@ -49,10 +49,12 @@ module Gitlab
return [uri, nil] unless address_info
ip_address = ip_address(address_info)
return [uri, nil] if domain_allowed?(uri) || ip_allowed?(ip_address, port: get_port(uri))
return [uri, nil] if domain_allowed?(uri)
protected_uri_with_hostname = enforce_uri_hostname(ip_address, uri, dns_rebind_protection)
return protected_uri_with_hostname if ip_allowed?(ip_address, port: get_port(uri))
# Allow url from the GitLab instance itself but only for the configured hostname and ports
return protected_uri_with_hostname if internal?(uri)
......
......@@ -27,7 +27,8 @@ RSpec.describe Admin::RunnersController do
# There is still an N+1 query for `runner.builds.count`
# We also need to add 1 because it takes 2 queries to preload tags
expect { get :index }.not_to exceed_query_limit(control_count + 6)
# also looking for token nonce requires database queries
expect { get :index }.not_to exceed_query_limit(control_count + 16)
expect(response).to have_gitlab_http_status(:ok)
expect(response.body).to have_content('tag1')
......
......@@ -9,6 +9,7 @@ RSpec.describe Projects::ReleasesController do
let_it_be(:private_project) { create(:project, :repository, :private) }
let_it_be(:developer) { create(:user) }
let_it_be(:reporter) { create(:user) }
let_it_be(:guest) { create(:user) }
let_it_be(:user) { developer }
let!(:release_1) { create(:release, project: project, released_at: Time.zone.parse('2018-10-18')) }
let!(:release_2) { create(:release, project: project, released_at: Time.zone.parse('2019-10-19')) }
......@@ -16,6 +17,7 @@ RSpec.describe Projects::ReleasesController do
before do
project.add_developer(developer)
project.add_reporter(reporter)
project.add_guest(guest)
end
shared_examples_for 'successful request' do
......@@ -199,6 +201,13 @@ RSpec.describe Projects::ReleasesController do
it_behaves_like 'not found'
end
context 'when user is a guest' do
let(:project) { private_project }
let(:user) { guest }
it_behaves_like 'not found'
end
end
# `GET #downloads` is addressed in spec/requests/projects/releases_controller_spec.rb
......
# frozen_string_literal: true
FactoryBot.define do
factory :token_with_iv do
hashed_token { ::Digest::SHA256.digest(SecureRandom.hex(50)) }
iv { ::Digest::SHA256.digest(SecureRandom.hex(50)) }
hashed_plaintext_token { ::Digest::SHA256.digest(SecureRandom.hex(50)) }
end
end
......@@ -78,6 +78,18 @@ describe('MrWidgetPipelineContainer', () => {
});
});
it('sanitizes the targetBranch', () => {
factory({
isPostMerge: true,
mr: {
...mockStore,
targetBranch: 'Foo<script>alert("XSS")</script>',
},
});
expect(wrapper.find(MrWidgetPipeline).props().sourceBranchLink).toBe('Foo');
});
it('renders deployments', () => {
const expectedProps = mockStore.postMergeDeployments.map((dep) =>
expect.objectContaining({
......
......@@ -19,10 +19,25 @@ RSpec.describe Gitlab::CryptoHelper do
expect(encrypted).to match %r{\A[A-Za-z0-9+/=]+\z}
expect(encrypted).not_to include "\n"
end
it 'does not save hashed token with iv value in database' do
expect { described_class.aes256_gcm_encrypt('some-value') }.not_to change { TokenWithIv.count }
end
it 'encrypts using static iv' do
expect(Encryptor).to receive(:encrypt).with(described_class::AES256_GCM_OPTIONS.merge(value: 'some-value', iv: described_class::AES256_GCM_IV_STATIC)).and_return('hashed_value')
described_class.aes256_gcm_encrypt('some-value')
end
end
describe '.aes256_gcm_decrypt' do
let(:encrypted) { described_class.aes256_gcm_encrypt('some-value') }
before do
stub_feature_flags(dynamic_nonce_creation: false)
end
context 'when token was encrypted using static nonce' do
let(:encrypted) { described_class.aes256_gcm_encrypt('some-value', nonce: described_class::AES256_GCM_IV_STATIC) }
it 'correctly decrypts encrypted string' do
decrypted = described_class.aes256_gcm_decrypt(encrypted)
......@@ -35,5 +50,54 @@ RSpec.describe Gitlab::CryptoHelper do
expect(decrypted).to eq 'some-value'
end
it 'does not save hashed token with iv value in database' do
expect { described_class.aes256_gcm_decrypt(encrypted) }.not_to change { TokenWithIv.count }
end
context 'with feature flag switched on' do
before do
stub_feature_flags(dynamic_nonce_creation: true)
end
it 'correctly decrypts encrypted string' do
decrypted = described_class.aes256_gcm_decrypt(encrypted)
expect(decrypted).to eq 'some-value'
end
end
end
context 'when token was encrypted using random nonce' do
let(:value) { 'random-value' }
# for compatibility with tokens encrypted using dynamic nonce
let!(:encrypted) do
iv = create_nonce
encrypted_token = described_class.create_encrypted_token(value, iv)
TokenWithIv.create!(hashed_token: Digest::SHA256.digest(encrypted_token), hashed_plaintext_token: Digest::SHA256.digest(encrypted_token), iv: iv)
encrypted_token
end
before do
stub_feature_flags(dynamic_nonce_creation: true)
end
it 'correctly decrypts encrypted string' do
decrypted = described_class.aes256_gcm_decrypt(encrypted)
expect(decrypted).to eq value
end
it 'does not save hashed token with iv value in database' do
expect { described_class.aes256_gcm_decrypt(encrypted) }.not_to change { TokenWithIv.count }
end
end
end
def create_nonce
cipher = OpenSSL::Cipher.new('aes-256-gcm')
cipher.encrypt # Required before '#random_iv' can be called
cipher.random_iv # Ensures that the IV is the correct length respective to the algorithm used.
end
end
......@@ -194,4 +194,32 @@ RSpec.describe Gitlab::CurrentSettings do
end
end
end
describe '#current_application_settings?', :use_clean_rails_memory_store_caching do
before do
allow(Gitlab::CurrentSettings).to receive(:current_application_settings?).and_call_original
end
it 'returns true when settings exist' do
create(:application_setting,
home_page_url: 'http://mydomain.com',
signup_enabled: false)
expect(described_class.current_application_settings?).to eq(true)
end
it 'returns false when settings do not exist' do
expect(described_class.current_application_settings?).to eq(false)
end
context 'with cache', :request_store do
include_context 'with settings in cache'
it 'returns an in-memory ApplicationSetting object' do
expect(ApplicationSetting).not_to receive(:current)
expect(described_class.current_application_settings?).to eq(true)
end
end
end
end
......@@ -40,4 +40,22 @@ RSpec.describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do
end
end
end
describe '#initial_value' do
it 'filters out sensitive variables' do
doc = GraphQL.parse <<-GRAPHQL
mutation createNote($body: String!) {
createNote(input: {noteableId: "1", body: $body}) {
note {
id
}
}
}
GRAPHQL
query = GraphQL::Query.new(GitlabSchema, document: doc, context: {}, variables: { body: "some note" })
expect(subject.initial_value(query)[:variables]).to eq('{:body=>"[FILTERED]"}')
end
end
end
......@@ -91,6 +91,21 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
end
end
context '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&amp;check-keys=*' }
before do
stub_dns(import_url, ip_address: '192.168.0.120')
allow(Gitlab::UrlBlockers::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&amp;check-keys=*' }
let(:expected_hostname) { 'a.192.168.0.120.3times.127.0.0.1.1time.repeat.rebind.network' }
end
end
context 'disabled DNS rebinding protection' do
subject { described_class.validate!(import_url, dns_rebind_protection: false) }
......
......@@ -332,13 +332,13 @@ RSpec.describe Gitlab do
describe '.maintenance_mode?' do
it 'returns true when maintenance mode is enabled' do
stub_application_setting(maintenance_mode: true)
stub_maintenance_mode_setting(true)
expect(described_class.maintenance_mode?).to eq(true)
end
it 'returns false when maintenance mode is disabled' do
stub_application_setting(maintenance_mode: false)
stub_maintenance_mode_setting(false)
expect(described_class.maintenance_mode?).to eq(false)
end
......
......@@ -8,7 +8,7 @@ RSpec.describe EncryptFeatureFlagsClientsTokens do
let(:feature_flags_clients) { table(:operations_feature_flags_clients) }
let(:projects) { table(:projects) }
let(:plaintext) { "secret-token" }
let(:ciphertext) { Gitlab::CryptoHelper.aes256_gcm_encrypt(plaintext) }
let(:ciphertext) { Gitlab::CryptoHelper.aes256_gcm_encrypt(plaintext, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC) }
describe '#up' do
it 'keeps plaintext token the same and populates token_encrypted if not present' do
......
......@@ -358,7 +358,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
it 'calls .destroy_sessions' do
expect(ActiveSession).to(
receive(:destroy_sessions)
.with(anything, user, [active_session.public_id, rack_session.public_id, rack_session.private_id]))
.with(anything, user, [encrypted_active_session_id, rack_session.public_id, rack_session.private_id]))
subject
end
......
......@@ -54,7 +54,7 @@ RSpec.describe ApplicationSetting, 'TokenAuthenticatable' do
it 'persists new token as an encrypted string' do
expect(subject).to eq settings.reload.runners_registration_token
expect(settings.read_attribute('runners_registration_token_encrypted'))
.to eq Gitlab::CryptoHelper.aes256_gcm_encrypt(subject)
.to eq Gitlab::CryptoHelper.aes256_gcm_encrypt(subject, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC)
expect(settings).to be_persisted
end
......@@ -243,7 +243,7 @@ RSpec.describe Ci::Build, 'TokenAuthenticatable' do
it 'persists new token as an encrypted string' do
build.ensure_token!
encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(build.token)
encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(build.token, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC)
expect(build.read_attribute('token_encrypted')).to eq encrypted
end
......
......@@ -68,6 +68,10 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do
context 'when using optional strategy' do
let(:options) { { encrypted: :optional } }
before do
stub_feature_flags(dynamic_nonce_creation: false)
end
it 'returns decrypted token when an encrypted token is present' do
allow(instance).to receive(:read_attribute)
.with('some_field_encrypted')
......@@ -124,7 +128,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do
it 'writes encrypted token and removes plaintext token and returns it' do
expect(instance).to receive(:[]=)
.with('some_field_encrypted', encrypted)
.with('some_field_encrypted', any_args)
expect(instance).to receive(:[]=)
.with('some_field', nil)
......@@ -137,7 +141,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do
it 'writes encrypted token and writes plaintext token' do
expect(instance).to receive(:[]=)
.with('some_field_encrypted', encrypted)
.with('some_field_encrypted', any_args)
expect(instance).to receive(:[]=)
.with('some_field', 'my-value')
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe TokenWithIv do
describe 'validations' do
it { is_expected.to validate_presence_of :hashed_token }
it { is_expected.to validate_presence_of :iv }
it { is_expected.to validate_presence_of :hashed_plaintext_token }
end
describe '.find_by_hashed_token' do
it 'only includes matching record' do
matching_record = create(:token_with_iv, hashed_token: ::Digest::SHA256.digest('hashed-token'))
create(:token_with_iv)
expect(described_class.find_by_hashed_token('hashed-token')).to eq(matching_record)
end
end
describe '.find_by_plaintext_token' do
it 'only includes matching record' do
matching_record = create(:token_with_iv, hashed_plaintext_token: ::Digest::SHA256.digest('hashed-token'))
create(:token_with_iv)
expect(described_class.find_by_plaintext_token('hashed-token')).to eq(matching_record)
end
end
end
......@@ -62,6 +62,12 @@ RSpec.describe ReleasePresenter do
it 'returns its own url' do
is_expected.to eq(project_release_url(project, release))
end
context 'when user is guest' do
let(:user) { guest }
it { is_expected.to be_nil }
end
end
describe '#opened_merge_requests_url' do
......
......@@ -159,13 +159,17 @@ RSpec.describe 'Git HTTP requests' do
context "POST git-upload-pack" do
it "fails to find a route" do
expect { clone_post(repository_path) }.to raise_error(ActionController::RoutingError)
clone_post(repository_path) do |response|
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
context "POST git-receive-pack" do
it "fails to find a route" do
expect { push_post(repository_path) }.to raise_error(ActionController::RoutingError)
push_post(repository_path) do |response|
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
end
......
......@@ -7,6 +7,10 @@ RSpec.describe 'git_http routing' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org/gitlab-test.git' }
end
it_behaves_like 'git repository routes with fallback for git-upload-pack' do
let(:path) { '/gitlab-org/gitlab-test.git' }
end
end
describe 'wiki repositories' do
......@@ -14,6 +18,7 @@ RSpec.describe 'git_http routing' do
let(:path) { '/gitlab-org/gitlab-test.wiki.git' }
it_behaves_like 'git repository routes'
it_behaves_like 'git repository routes with fallback for git-upload-pack'
describe 'redirects', type: :request do
let(:web_path) { '/gitlab-org/gitlab-test/-/wikis' }
......@@ -37,12 +42,20 @@ RSpec.describe 'git_http routing' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org.wiki.git' }
end
it_behaves_like 'git repository routes with fallback for git-upload-pack' do
let(:path) { '/gitlab-org.wiki.git' }
end
end
context 'in child group' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org/child.wiki.git' }
end
it_behaves_like 'git repository routes with fallback for git-upload-pack' do
let(:path) { '/gitlab-org/child.wiki.git' }
end
end
end
......@@ -51,12 +64,20 @@ RSpec.describe 'git_http routing' do
it_behaves_like 'git repository routes' do
let(:path) { '/snippets/123.git' }
end
it_behaves_like 'git repository routes without fallback' do
let(:path) { '/snippets/123.git' }
end
end
context 'project snippet' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org/gitlab-test/snippets/123.git' }
end
it_behaves_like 'git repository routes with fallback' do
let(:path) { '/gitlab-org/gitlab-test/snippets/123.git' }
end
end
end
end
......@@ -876,4 +876,73 @@ RSpec.describe 'project routing' do
)
end
end
context 'with a non-existent project' do
it 'routes to 404 with get request' do
expect(get: "/gitlab/not_exist").to route_to(
'application#route_not_found',
unmatched_route: 'gitlab/not_exist'
)
end
it 'routes to 404 with delete request' do
expect(delete: "/gitlab/not_exist").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist'
)
end
it 'routes to 404 with post request' do
expect(post: "/gitlab/not_exist").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist'
)
end
it 'routes to 404 with put request' do
expect(put: "/gitlab/not_exist").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist'
)
end
context 'with route to some action' do
it 'routes to 404 with get request to' do
expect(get: "/gitlab/not_exist/some_action").to route_to(
'application#route_not_found',
unmatched_route: 'gitlab/not_exist/some_action'
)
end
it 'routes to 404 with delete request' do
expect(delete: "/gitlab/not_exist/some_action").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist',
all: 'some_action'
)
end
it 'routes to 404 with post request' do
expect(post: "/gitlab/not_exist/some_action").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist',
all: 'some_action'
)
end
it 'routes to 404 with put request' do
expect(put: "/gitlab/not_exist/some_action").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist',
all: 'some_action'
)
end
end
end
end
......@@ -284,6 +284,8 @@ RSpec.configure do |config|
current_user_mode.send(:user)&.admin?
end
end
allow(Gitlab::CurrentSettings).to receive(:current_application_settings?).and_return(false)
end
config.around(:example, :quarantine) do |example|
......
......@@ -121,6 +121,12 @@ module StubConfiguration
allow(::Gitlab.config.packages).to receive_messages(to_settings(messages))
end
def stub_maintenance_mode_setting(value)
allow(Gitlab::CurrentSettings).to receive(:current_application_settings?).and_return(true)
stub_application_setting(maintenance_mode: value)
end
private
# Modifies stubbed messages to also stub possible predicate versions
......
# frozen_string_literal: true
RSpec::Matchers.define :route_to_route_not_found do
match do |actual|
expect(actual).to route_to(controller: 'application', action: 'route_not_found')
rescue RSpec::Expectations::ExpectationNotMetError => e
# `route_to` matcher requires providing all params for exact match. As we use it in shared examples and we provide different paths,
# this matcher checks if provided route matches controller and action, without checking params.
expect(e.message).to include("-{\"controller\"=>\"application\", \"action\"=>\"route_not_found\"}\n+{\"controller\"=>\"application\", \"action\"=>\"route_not_found\",")
end
failure_message do |_|
"expected #{actual} to route to route_not_found"
end
end
......@@ -16,10 +16,6 @@ RSpec.shared_examples 'git repository routes' do
expect(get("#{container_path}/info/refs?service=git-upload-pack")).to redirect_to("#{container_path}.git/info/refs?service=git-upload-pack")
expect(get("#{container_path}/info/refs?service=git-receive-pack")).to redirect_to("#{container_path}.git/info/refs?service=git-receive-pack")
end
it 'does not redirect other requests' do
expect(post("#{container_path}/git-upload-pack")).not_to be_routable
end
end
it 'routes LFS endpoints' do
......@@ -35,6 +31,56 @@ RSpec.shared_examples 'git repository routes' do
expect(get("#{path}/gitlab-lfs/objects/#{oid}")).to route_to('repositories/lfs_storage#download', oid: oid, **params)
expect(put("#{path}/gitlab-lfs/objects/#{oid}/456/authorize")).to route_to('repositories/lfs_storage#upload_authorize', oid: oid, size: '456', **params)
expect(put("#{path}/gitlab-lfs/objects/#{oid}/456")).to route_to('repositories/lfs_storage#upload_finalize', oid: oid, size: '456', **params)
end
end
RSpec.shared_examples 'git repository routes without fallback' do
let(:container_path) { path.delete_suffix('.git') }
context 'requests without .git format' do
it 'does not redirect other requests' do
expect(post("#{container_path}/git-upload-pack")).not_to be_routable
end
end
it 'routes LFS endpoints for unmatched routes' do
oid = generate(:oid)
expect(put("#{path}/gitlab-lfs/objects/foo")).not_to be_routable
expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).not_to be_routable
expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo/authorize")).not_to be_routable
end
end
RSpec.shared_examples 'git repository routes with fallback' do
let(:container_path) { path.delete_suffix('.git') }
context 'requests without .git format' do
it 'does not redirect other requests' do
expect(post("#{container_path}/git-upload-pack")).to route_to_route_not_found
end
end
it 'routes LFS endpoints' do
oid = generate(:oid)
expect(put("#{path}/gitlab-lfs/objects/foo")).to route_to_route_not_found
expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).to route_to_route_not_found
expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo/authorize")).to route_to_route_not_found
end
end
RSpec.shared_examples 'git repository routes with fallback for git-upload-pack' do
let(:container_path) { path.delete_suffix('.git') }
context 'requests without .git format' do
it 'does not redirect other requests' do
expect(post("#{container_path}/git-upload-pack")).to route_to_route_not_found
end
end
it 'routes LFS endpoints for unmatched routes' do
oid = generate(:oid)
expect(put("#{path}/gitlab-lfs/objects/foo")).not_to be_routable
expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).not_to be_routable
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment