Commit 80163b97 authored by Nick Thomas's avatar Nick Thomas

Merge branch 'fix/gb/encrypt-runners-tokens' into 'master'

Encrypt runners tokens

Closes #51232 and #52931

See merge request gitlab-org/gitlab-ce!23412
parents 42a7d3b7 a1bd34e9
......@@ -7,7 +7,7 @@ class ApplicationSetting < ActiveRecord::Base
include IgnorableColumn
include ChronicDurationAttribute
add_authentication_token_field :runners_registration_token
add_authentication_token_field :runners_registration_token, encrypted: true, fallback: true
add_authentication_token_field :health_check_access_token
DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace
......
......@@ -8,6 +8,9 @@ module Ci
include RedisCacheable
include ChronicDurationAttribute
include FromUnion
include TokenAuthenticatable
add_authentication_token_field :token, encrypted: true, migrating: true
enum access_level: {
not_protected: 0,
......@@ -39,7 +42,7 @@ module Ci
has_one :last_build, ->() { order('id DESC') }, class_name: 'Ci::Build'
before_validation :set_default_values
before_save :ensure_token
scope :active, -> { where(active: true) }
scope :paused, -> { where(active: false) }
......@@ -145,10 +148,6 @@ module Ci
end
end
def set_default_values
self.token = SecureRandom.hex(15) if self.token.blank?
end
def assign_to(project, current_user = nil)
if instance_type?
self.runner_type = :project_type
......
......@@ -9,24 +9,18 @@ module TokenAuthenticatable
private # rubocop:disable Lint/UselessAccessModifier
def add_authentication_token_field(token_field, options = {})
@token_fields = [] unless @token_fields
unique = options.fetch(:unique, true)
if @token_fields.include?(token_field)
if token_authenticatable_fields.include?(token_field)
raise ArgumentError.new("#{token_field} already configured via add_authentication_token_field")
end
@token_fields << token_field
token_authenticatable_fields.push(token_field)
attr_accessor :cleartext_tokens
strategy = if options[:digest]
TokenAuthenticatableStrategies::Digest.new(self, token_field, options)
else
TokenAuthenticatableStrategies::Insecure.new(self, token_field, options)
end
strategy = TokenAuthenticatableStrategies::Base
.fabricate(self, token_field, options)
if unique
if options.fetch(:unique, true)
define_singleton_method("find_by_#{token_field}") do |token|
strategy.find_token_authenticatable(token)
end
......@@ -59,5 +53,9 @@ module TokenAuthenticatable
token.present? && ActiveSupport::SecurityUtils.variable_size_secure_compare(other_token, token)
end
end
def token_authenticatable_fields
@token_authenticatable_fields ||= []
end
end
end
......@@ -2,6 +2,8 @@
module TokenAuthenticatableStrategies
class Base
attr_reader :klass, :token_field, :options
def initialize(klass, token_field, options)
@klass = klass
@token_field = token_field
......@@ -22,6 +24,7 @@ module TokenAuthenticatableStrategies
def ensure_token(instance)
write_new_token(instance) unless token_set?(instance)
get_token(instance)
end
# Returns a token, but only saves when the database is in read & write mode
......@@ -36,6 +39,36 @@ module TokenAuthenticatableStrategies
instance.save! if Gitlab::Database.read_write?
end
def fallback?
unless options[:fallback].in?([true, false, nil])
raise ArgumentError, 'fallback: needs to be a boolean value!'
end
options[:fallback] == true
end
def migrating?
unless options[:migrating].in?([true, false, nil])
raise ArgumentError, 'migrating: needs to be a boolean value!'
end
options[:migrating] == true
end
def self.fabricate(model, field, options)
if options[:digest] && options[:encrypted]
raise ArgumentError, 'Incompatible options set!'
end
if options[:digest]
TokenAuthenticatableStrategies::Digest.new(model, field, options)
elsif options[:encrypted]
TokenAuthenticatableStrategies::Encrypted.new(model, field, options)
else
TokenAuthenticatableStrategies::Insecure.new(model, field, options)
end
end
protected
def write_new_token(instance)
......@@ -65,9 +98,5 @@ module TokenAuthenticatableStrategies
def token_set?(instance)
raise NotImplementedError
end
def token_field_name
@token_field
end
end
end
# frozen_string_literal: true
module TokenAuthenticatableStrategies
class Encrypted < Base
def initialize(*)
super
if migrating? && fallback?
raise ArgumentError, '`fallback` and `migrating` options are not compatible!'
end
end
def find_token_authenticatable(token, unscoped = false)
return if token.blank?
if fully_encrypted?
return find_by_encrypted_token(token, unscoped)
end
if fallback?
find_by_encrypted_token(token, unscoped) ||
find_by_plaintext_token(token, unscoped)
elsif migrating?
find_by_plaintext_token(token, unscoped)
else
raise ArgumentError, 'Unknown encryption phase!'
end
end
def ensure_token(instance)
# TODO, tech debt, because some specs are testing migrations, but are still
# using factory bot to create resources, it might happen that a database
# schema does not have "#{token_name}_encrypted" field yet, however a bunch
# of models call `ensure_#{token_name}` in `before_save`.
#
# In that case we are using insecure strategy, but this should only happen
# in tests, because otherwise `encrypted_field` is going to exist.
#
# Another use case is when we are caching resources / columns, like we do
# in case of ApplicationSetting.
return super if instance.has_attribute?(encrypted_field)
if fully_encrypted?
raise ArgumentError, 'Using encrypted strategy when encrypted field is missing!'
else
insecure_strategy.ensure_token(instance)
end
end
def get_token(instance)
return insecure_strategy.get_token(instance) if migrating?
encrypted_token = instance.read_attribute(encrypted_field)
token = Gitlab::CryptoHelper.aes256_gcm_decrypt(encrypted_token)
token || (insecure_strategy.get_token(instance) if fallback?)
end
def set_token(instance, token)
raise ArgumentError unless token.present?
instance[encrypted_field] = Gitlab::CryptoHelper.aes256_gcm_encrypt(token)
instance[token_field] = token if migrating?
instance[token_field] = nil if fallback?
token
end
def fully_encrypted?
!migrating? && !fallback?
end
protected
def find_by_plaintext_token(token, unscoped)
insecure_strategy.find_token_authenticatable(token, unscoped)
end
def find_by_encrypted_token(token, unscoped)
encrypted_value = Gitlab::CryptoHelper.aes256_gcm_encrypt(token)
relation(unscoped).find_by(encrypted_field => encrypted_value)
end
def insecure_strategy
@insecure_strategy ||= TokenAuthenticatableStrategies::Insecure
.new(klass, token_field, options)
end
def token_set?(instance)
raw_token = instance.read_attribute(encrypted_field)
unless fully_encrypted?
raw_token ||= insecure_strategy.get_token(instance)
end
raw_token.present?
end
def encrypted_field
@encrypted_field ||= "#{@token_field}_encrypted"
end
end
end
......@@ -55,7 +55,7 @@ class Group < Namespace
validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 }
add_authentication_token_field :runners_token
add_authentication_token_field :runners_token, encrypted: true, migrating: true
after_create :post_create_hook
after_destroy :post_destroy_hook
......
......@@ -85,7 +85,7 @@ class Project < ActiveRecord::Base
default_value_for :snippets_enabled, gitlab_config_features.snippets
default_value_for :only_allow_merge_if_all_discussions_are_resolved, false
add_authentication_token_field :runners_token
add_authentication_token_field :runners_token, encrypted: true, migrating: true
before_validation :mark_remote_mirrors_for_removal, if: -> { RemoteMirror.table_exists? }
......
---
title: Encrypt runners tokens
merge_request: 23412
author:
type: security
......@@ -95,6 +95,14 @@ class Settings < Settingslogic
Gitlab::Application.secrets.db_key_base[0..31]
end
def attr_encrypted_db_key_base_32
Gitlab::Utils.ensure_utf8_size(attr_encrypted_db_key_base, bytes: 32.bytes)
end
def attr_encrypted_db_key_base_12
Gitlab::Utils.ensure_utf8_size(attr_encrypted_db_key_base, bytes: 12.bytes)
end
# This should be used for :per_attribute_salt_and_iv mode. There is no
# need to truncate the key because the encryptor will use the salt to
# generate a hash of the password:
......
# frozen_string_literal: true
class AddEncryptedRunnersTokenToSettings < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :application_settings, :runners_registration_token_encrypted, :string
end
end
# frozen_string_literal: true
class AddEncryptedRunnersTokenToNamespaces < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :namespaces, :runners_token_encrypted, :string
end
end
# frozen_string_literal: true
class AddEncryptedRunnersTokenToProjects < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :projects, :runners_token_encrypted, :string
end
end
# frozen_string_literal: true
class AddTokenEncryptedToCiRunners < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :ci_runners, :token_encrypted, :string
end
end
# frozen_string_literal: true
class ScheduleRunnersTokenEncryption < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
BATCH_SIZE = 10000
RANGE_SIZE = 2000
MIGRATION = 'EncryptRunnersTokens'
MODELS = [
::Gitlab::BackgroundMigration::Models::EncryptColumns::Settings,
::Gitlab::BackgroundMigration::Models::EncryptColumns::Namespace,
::Gitlab::BackgroundMigration::Models::EncryptColumns::Project,
::Gitlab::BackgroundMigration::Models::EncryptColumns::Runner
].freeze
disable_ddl_transaction!
def up
MODELS.each do |model|
model.each_batch(of: BATCH_SIZE) do |relation, index|
delay = index * 4.minutes
relation.each_batch(of: RANGE_SIZE) do |relation|
range = relation.pluck('MIN(id)', 'MAX(id)').first
args = [model.name.demodulize.downcase, *range]
BackgroundMigrationWorker.perform_in(delay, MIGRATION, args)
end
end
end
end
def down
# no-op
end
end
......@@ -166,6 +166,7 @@ ActiveRecord::Schema.define(version: 20181126153547) do
t.integer "diff_max_patch_bytes", default: 102400, null: false
t.integer "archive_builds_in_seconds"
t.string "commit_email_hostname"
t.string "runners_registration_token_encrypted"
t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree
end
......@@ -520,6 +521,7 @@ ActiveRecord::Schema.define(version: 20181126153547) do
t.string "ip_address"
t.integer "maximum_timeout"
t.integer "runner_type", limit: 2, null: false
t.string "token_encrypted"
t.index ["contacted_at"], name: "index_ci_runners_on_contacted_at", using: :btree
t.index ["is_shared"], name: "index_ci_runners_on_is_shared", using: :btree
t.index ["locked"], name: "index_ci_runners_on_locked", using: :btree
......@@ -1335,6 +1337,7 @@ ActiveRecord::Schema.define(version: 20181126153547) do
t.integer "two_factor_grace_period", default: 48, null: false
t.integer "cached_markdown_version"
t.string "runners_token"
t.string "runners_token_encrypted"
t.index ["created_at"], name: "index_namespaces_on_created_at", using: :btree
t.index ["name", "parent_id"], name: "index_namespaces_on_name_and_parent_id", unique: true, using: :btree
t.index ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"}
......@@ -1675,6 +1678,7 @@ ActiveRecord::Schema.define(version: 20181126153547) do
t.boolean "pages_https_only", default: true
t.boolean "remote_mirror_available_overridden"
t.bigint "pool_repository_id"
t.string "runners_token_encrypted"
t.index ["ci_id"], name: "index_projects_on_ci_id", using: :btree
t.index ["created_at"], name: "index_projects_on_created_at", using: :btree
t.index ["creator_id"], name: "index_projects_on_creator_id", using: :btree
......
......@@ -19,7 +19,6 @@ module API
optional :tag_list, type: Array[String], desc: %q(List of Runner's tags)
optional :maximum_timeout, type: Integer, desc: 'Maximum timeout set when this Runner will handle the job'
end
# rubocop: disable CodeReuse/ActiveRecord
post '/' do
attributes = attributes_for_keys([:description, :active, :locked, :run_untagged, :tag_list, :maximum_timeout])
.merge(get_runner_details_from_request)
......@@ -28,10 +27,10 @@ module API
if runner_registration_token_valid?
# Create shared runner. Requires admin access
attributes.merge(runner_type: :instance_type)
elsif project = Project.find_by(runners_token: params[:token])
elsif project = Project.find_by_runners_token(params[:token])
# Create a specific runner for the project
attributes.merge(runner_type: :project_type, projects: [project])
elsif group = Group.find_by(runners_token: params[:token])
elsif group = Group.find_by_runners_token(params[:token])
# Create a specific runner for the group
attributes.merge(runner_type: :group_type, groups: [group])
else
......@@ -46,7 +45,6 @@ module API
render_validation_error!(runner)
end
end
# rubocop: enable CodeReuse/ActiveRecord
desc 'Deletes a registered Runner' do
http_codes [[204, 'Runner was deleted'], [403, 'Forbidden']]
......
......@@ -5,15 +5,17 @@ module Gitlab
# EncryptColumn migrates data from an unencrypted column - `foo`, say - to
# an encrypted column - `encrypted_foo`, say.
#
# To avoid depending on a particular version of the model in app/, add a
# model to `lib/gitlab/background_migration/models/encrypt_columns` and use
# it in the migration that enqueues the jobs, so code can be shared.
#
# For this background migration to work, the table that is migrated _has_ to
# have an `id` column as the primary key. Additionally, the encrypted column
# should be managed by attr_encrypted, and map to an attribute with the same
# name as the unencrypted column (i.e., the unencrypted column should be
# shadowed).
# shadowed), unless you want to define specific methods / accessors in the
# temporary model in `/models/encrypt_columns/your_model.rb`.
#
# To avoid depending on a particular version of the model in app/, add a
# model to `lib/gitlab/background_migration/models/encrypt_columns` and use
# it in the migration that enqueues the jobs, so code can be shared.
class EncryptColumns
def perform(model, attributes, from, to)
model = model.constantize if model.is_a?(String)
......@@ -36,6 +38,10 @@ module Gitlab
end
end
def clear_migrated_values?
true
end
private
# Build a hash of { attribute => encrypted column name }
......@@ -72,7 +78,10 @@ module Gitlab
if instance.changed?
instance.save!
instance.update_columns(to_clear)
if clear_migrated_values?
instance.update_columns(to_clear)
end
end
end
......
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# EncryptColumn migrates data from an unencrypted column - `foo`, say - to
# an encrypted column - `encrypted_foo`, say.
#
# We only create a subclass here because we want to isolate this migration
# (migrating unencrypted runner registration tokens to encrypted columns)
# from other `EncryptColumns` migration. This class name is going to be
# serialized and stored in Redis and later picked by Sidekiq, so we need to
# create a separate class name in order to isolate these migration tasks.
#
# We can solve this differently, see tech debt issue:
#
# https://gitlab.com/gitlab-org/gitlab-ce/issues/54328
#
class EncryptRunnersTokens < EncryptColumns
def perform(model, from, to)
resource = "::Gitlab::BackgroundMigration::Models::EncryptColumns::#{model.to_s.capitalize}"
model = resource.constantize
attributes = model.encrypted_attributes.keys
super(model, attributes, from, to)
end
def clear_migrated_values?
false
end
end
end
end
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
module Models
module EncryptColumns
# This model is shared between synchronous and background migrations to
# encrypt the `runners_token` column in `namespaces` table.
#
class Namespace < ActiveRecord::Base
include ::EachBatch
self.table_name = 'namespaces'
self.inheritance_column = :_type_disabled
def runners_token=(value)
self.runners_token_encrypted =
::Gitlab::CryptoHelper.aes256_gcm_encrypt(value)
end
def self.encrypted_attributes
{ runners_token: { attribute: :runners_token_encrypted } }
end
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
module Models
module EncryptColumns
# This model is shared between synchronous and background migrations to
# encrypt the `runners_token` column in `projects` table.
#
class Project < ActiveRecord::Base
include ::EachBatch
self.table_name = 'projects'
self.inheritance_column = :_type_disabled
def runners_token=(value)
self.runners_token_encrypted =
::Gitlab::CryptoHelper.aes256_gcm_encrypt(value)
end
def self.encrypted_attributes
{ runners_token: { attribute: :runners_token_encrypted } }
end
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
module Models
module EncryptColumns
# This model is shared between synchronous and background migrations to
# encrypt the `token` column in `ci_runners` table.
#
class Runner < ActiveRecord::Base
include ::EachBatch
self.table_name = 'ci_runners'
self.inheritance_column = :_type_disabled
def token=(value)
self.token_encrypted =
::Gitlab::CryptoHelper.aes256_gcm_encrypt(value)
end
def self.encrypted_attributes
{ token: { attribute: :token_encrypted } }
end
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
module Models
module EncryptColumns
# This model is shared between synchronous and background migrations to
# encrypt the `runners_token` column in `application_settings` table.
#
class Settings < ActiveRecord::Base
include ::EachBatch
include ::CacheableAttributes
self.table_name = 'application_settings'
self.inheritance_column = :_type_disabled
after_commit do
::ApplicationSetting.expire
end
def runners_registration_token=(value)
self.runners_registration_token_encrypted =
::Gitlab::CryptoHelper.aes256_gcm_encrypt(value)
end
def self.encrypted_attributes
{
runners_registration_token: {
attribute: :runners_registration_token_encrypted
}
}
end
end
end
end
end
end
......@@ -15,12 +15,12 @@ module Gitlab
attr_encrypted :token,
mode: :per_attribute_iv,
algorithm: 'aes-256-gcm',
key: Settings.attr_encrypted_db_key_base_truncated
key: ::Settings.attr_encrypted_db_key_base_truncated
attr_encrypted :url,
mode: :per_attribute_iv,
algorithm: 'aes-256-gcm',
key: Settings.attr_encrypted_db_key_base_truncated
key: ::Settings.attr_encrypted_db_key_base_truncated
end
end
end
......
......@@ -6,8 +6,8 @@ module Gitlab
AES256_GCM_OPTIONS = {
algorithm: 'aes-256-gcm',
key: Settings.attr_encrypted_db_key_base_truncated,
iv: Settings.attr_encrypted_db_key_base_truncated[0..11]
key: Settings.attr_encrypted_db_key_base_32,
iv: Settings.attr_encrypted_db_key_base_12
}.freeze
def sha256(value)
......@@ -17,7 +17,7 @@ module Gitlab
def aes256_gcm_encrypt(value)
encrypted_token = Encryptor.encrypt(AES256_GCM_OPTIONS.merge(value: value))
Base64.encode64(encrypted_token)
Base64.strict_encode64(encrypted_token)
end
def aes256_gcm_decrypt(value)
......
......@@ -100,6 +100,7 @@ excluded_attributes:
- :import_source
- :mirror
- :runners_token
- :runners_token_encrypted
- :repository_storage
- :repository_read_only
- :lfs_enabled
......@@ -114,6 +115,9 @@ excluded_attributes:
- :remote_mirror_available_overridden
- :description_html
- :repository_languages
namespaces:
- :runners_token
- :runners_token_encrypted
project_import_state:
- :last_error
- :jid
......@@ -155,6 +159,9 @@ excluded_attributes:
- :encrypted_token_iv
- :encrypted_url
- :encrypted_url_iv
runners:
- :token
- :token_encrypted
services:
- :template
......
......@@ -10,6 +10,7 @@ module Gitlab
triggers: 'Ci::Trigger',
pipeline_schedules: 'Ci::PipelineSchedule',
builds: 'Ci::Build',
runners: 'Ci::Runner',