Commit daed01a5 authored by Jan Provaznik's avatar Jan Provaznik Committed by Thiago Presa

Merge branch 'security-if-51113-hash_tokens-11-4' into 'security-11-4'

[11.4] Persist only SHA digest of PersonalAccessToken#token

See merge request gitlab/gitlabhq!2551
parent 9266cb27
......@@ -5,57 +5,50 @@ module TokenAuthenticatable
private
def write_new_token(token_field)
new_token = generate_available_token(token_field)
write_attribute(token_field, new_token)
end
def generate_available_token(token_field)
loop do
token = generate_token(token_field)
break token unless self.class.unscoped.find_by(token_field => token)
end
end
def generate_token(token_field)
Devise.friendly_token
end
class_methods do
def authentication_token_fields
@token_fields || []
end
private # rubocop:disable Lint/UselessAccessModifier
def add_authentication_token_field(token_field)
def add_authentication_token_field(token_field, options = {})
@token_fields = [] unless @token_fields
if @token_fields.include?(token_field)
raise ArgumentError.new("#{token_field} already configured via add_authentication_token_field")
end
@token_fields << 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
define_singleton_method("find_by_#{token_field}") do |token|
find_by(token_field => token) if token
strategy.find_token_authenticatable(token)
end
define_method("ensure_#{token_field}") do
current_token = read_attribute(token_field)
current_token.blank? ? write_new_token(token_field) : current_token
define_method(token_field) do
strategy.get_token(self)
end
define_method("set_#{token_field}") do |token|
write_attribute(token_field, token) if token
strategy.set_token(self, token)
end
define_method("ensure_#{token_field}") do
strategy.ensure_token(self)
end
# Returns a token, but only saves when the database is in read & write mode
define_method("ensure_#{token_field}!") do
send("reset_#{token_field}!") if read_attribute(token_field).blank? # rubocop:disable GitlabSecurity/PublicSend
read_attribute(token_field)
strategy.ensure_token!(self)
end
# Resets the token, but only saves when the database is in read & write mode
define_method("reset_#{token_field}!") do
write_new_token(token_field)
save! if Gitlab::Database.read_write?
strategy.reset_token!(self)
end
end
end
......
# frozen_string_literal: true
module TokenAuthenticatableStrategies
class Base
def initialize(klass, token_field, options)
@klass = klass
@token_field = token_field
@options = options
end
def find_token_authenticatable(instance, unscoped = false)
raise NotImplementedError
end
def get_token(instance)
raise NotImplementedError
end
def set_token(instance)
raise NotImplementedError
end
def ensure_token(instance)
write_new_token(instance) unless token_set?(instance)
end
# Returns a token, but only saves when the database is in read & write mode
def ensure_token!(instance)
reset_token!(instance) unless token_set?(instance)
get_token(instance)
end
# Resets the token, but only saves when the database is in read & write mode
def reset_token!(instance)
write_new_token(instance)
instance.save! if Gitlab::Database.read_write?
end
protected
def write_new_token(instance)
new_token = generate_available_token
set_token(instance, new_token)
end
def generate_available_token
loop do
token = generate_token
break token unless find_token_authenticatable(token, true)
end
end
def generate_token
@options[:token_generator] ? @options[:token_generator].call : Devise.friendly_token
end
def relation(unscoped)
unscoped ? @klass.unscoped : @klass
end
def token_set?(instance)
raise NotImplementedError
end
def token_field_name
@token_field
end
end
end
# frozen_string_literal: true
module TokenAuthenticatableStrategies
class Digest < Base
def find_token_authenticatable(token, unscoped = false)
return unless token
token_authenticatable = relation(unscoped).find_by(token_field_name => Gitlab::CryptoHelper.sha256(token))
if @options[:fallback]
token_authenticatable ||= fallback_strategy.find_token_authenticatable(token)
end
token_authenticatable
end
def get_token(instance)
token = instance.cleartext_tokens&.[](@token_field)
token ||= fallback_strategy.get_token(instance) if @options[:fallback]
token
end
def set_token(instance, token)
return unless token
instance.cleartext_tokens ||= {}
instance.cleartext_tokens[@token_field] = token
instance[token_field_name] = Gitlab::CryptoHelper.sha256(token)
instance[@token_field] = nil if @options[:fallback]
end
protected
def fallback_strategy
@fallback_strategy ||= TokenAuthenticatableStrategies::Insecure.new(@klass, @token_field, @options)
end
def token_set?(instance)
token_digest = instance.read_attribute(token_field_name)
token_digest ||= instance.read_attribute(@token_field) if @options[:fallback]
token_digest.present?
end
def token_field_name
"#{@token_field}_digest"
end
end
end
# frozen_string_literal: true
module TokenAuthenticatableStrategies
class Insecure < Base
def find_token_authenticatable(token, unscoped = false)
relation(unscoped).find_by(@token_field => token) if token
end
def get_token(instance)
instance.read_attribute(@token_field)
end
def set_token(instance, token)
instance[@token_field] = token if token
end
protected
def token_set?(instance)
instance.read_attribute(@token_field).present?
end
end
end
......@@ -3,7 +3,7 @@
class PersonalAccessToken < ActiveRecord::Base
include Expirable
include TokenAuthenticatable
add_authentication_token_field :token
add_authentication_token_field :token, digest: true, fallback: true
REDIS_EXPIRY_TIME = 3.minutes
......@@ -33,16 +33,22 @@ class PersonalAccessToken < ActiveRecord::Base
def self.redis_getdel(user_id)
Gitlab::Redis::SharedState.with do |redis|
token = redis.get(redis_shared_state_key(user_id))
encrypted_token = redis.get(redis_shared_state_key(user_id))
redis.del(redis_shared_state_key(user_id))
token
begin
Gitlab::CryptoHelper.aes256_gcm_decrypt(encrypted_token)
rescue => ex
logger.warn "Failed to decrypt PersonalAccessToken value stored in Redis for User ##{user_id}: #{ex.class}"
encrypted_token
end
end
end
def self.redis_store!(user_id, token)
encrypted_token = Gitlab::CryptoHelper.aes256_gcm_encrypt(token)
Gitlab::Redis::SharedState.with do |redis|
redis.set(redis_shared_state_key(user_id), token, ex: REDIS_EXPIRY_TIME)
token
redis.set(redis_shared_state_key(user_id), encrypted_token, ex: REDIS_EXPIRY_TIME)
end
end
......
......@@ -28,7 +28,7 @@ class User < ActiveRecord::Base
ignore_column :email_provider
ignore_column :authentication_token
add_authentication_token_field :incoming_email_token
add_authentication_token_field :incoming_email_token, token_generator: -> { SecureRandom.hex.to_i(16).to_s(36) }
add_authentication_token_field :feed_token
default_value_for :admin, false
......@@ -1451,15 +1451,6 @@ class User < ActiveRecord::Base
end
end
def generate_token(token_field)
if token_field == :incoming_email_token
# Needs to be all lowercase and alphanumeric because it's gonna be used in an email address.
SecureRandom.hex.to_i(16).to_s(36)
else
super
end
end
def self.unique_internal(scope, username, email_pattern, &block)
scope.first || create_unique_internal(scope, username, email_pattern, &block)
end
......
---
title: Persist only SHA digest of PersonalAccessToken#token
merge_request:
author:
type: security
# frozen_string_literal: true
class AddTokenDigestToPersonalAccessTokens < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
change_column :personal_access_tokens, :token, :string, null: true
add_column :personal_access_tokens, :token_digest, :string
end
def down
remove_column :personal_access_tokens, :token_digest
change_column :personal_access_tokens, :token, :string, null: false
end
end
# frozen_string_literal: true
class AddIndexToTokenDigestOnPersonalAccessTokens < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_index :personal_access_tokens, :token_digest, unique: true
end
def down
remove_concurrent_index :personal_access_tokens, :token_digest if index_exists?(:personal_access_tokens, :token_digest)
end
end
class ScheduleDigestPersonalAccessTokens < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
BATCH_SIZE = 10_000
MIGRATION = 'DigestColumn'
DELAY_INTERVAL = 5.minutes.to_i
disable_ddl_transaction!
class PersonalAccessToken < ActiveRecord::Base
include EachBatch
self.table_name = 'personal_access_tokens'
end
def up
PersonalAccessToken.where('token is NOT NULL').each_batch(of: BATCH_SIZE) do |batch, index|
range = batch.pluck('MIN(id)', 'MAX(id)').first
BackgroundMigrationWorker.perform_in(index * DELAY_INTERVAL, MIGRATION, ['PersonalAccessToken', :token, :token_digest, *range])
end
end
def down
# raise ActiveRecord::IrreversibleMigration
end
end
......@@ -1503,7 +1503,7 @@ ActiveRecord::Schema.define(version: 20181002172433) do
create_table "personal_access_tokens", force: :cascade do |t|
t.integer "user_id", null: false
t.string "token", null: false
t.string "token"
t.string "name", null: false
t.boolean "revoked", default: false
t.date "expires_at"
......@@ -1511,9 +1511,11 @@ ActiveRecord::Schema.define(version: 20181002172433) do
t.datetime "updated_at", null: false
t.string "scopes", default: "--- []\n", null: false
t.boolean "impersonation", default: false, null: false
t.string "token_digest"
end
add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree
add_index "personal_access_tokens", ["token_digest"], name: "index_personal_access_tokens_on_token_digest", unique: true, using: :btree
add_index "personal_access_tokens", ["user_id"], name: "index_personal_access_tokens_on_user_id", using: :btree
create_table "programming_languages", force: :cascade do |t|
......
......@@ -71,7 +71,6 @@ module Gitlab
end
end
# rubocop: disable CodeReuse/ActiveRecord
def find_personal_access_token
token =
current_request.params[PRIVATE_TOKEN_PARAM].presence ||
......@@ -80,9 +79,8 @@ module Gitlab
return unless token
# Expiration, revocation and scopes are verified in `validate_access_token!`
PersonalAccessToken.find_by(token: token) || raise(UnauthorizedError)
PersonalAccessToken.find_by_token(token) || raise(UnauthorizedError)
end
# rubocop: enable CodeReuse/ActiveRecord
def find_oauth_access_token
token = Doorkeeper::OAuth::Token.from_request(current_request, *Doorkeeper.configuration.access_token_methods)
......
# frozen_string_literal: true
# rubocop:disable Style/Documentation
module Gitlab
module BackgroundMigration
class DigestColumn
class PersonalAccessToken < ActiveRecord::Base
self.table_name = 'personal_access_tokens'
end
def perform(model, attribute_from, attribute_to, start_id, stop_id)
model = model.constantize if model.is_a?(String)
model.transaction do
relation = model.where(id: start_id..stop_id).where.not(attribute_from => nil).lock
relation.each do |instance|
instance.update_columns(attribute_to => Gitlab::CryptoHelper.sha256(instance.read_attribute(attribute_from)),
attribute_from => nil)
end
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
module CryptoHelper
extend self
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]
}.freeze
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.encode64(encrypted_token)
end
def aes256_gcm_decrypt(value)
return unless value
encrypted_token = Base64.decode64(value)
Encryptor.decrypt(AES256_GCM_OPTIONS.merge(value: encrypted_token))
end
end
end
require_relative '../../app/models/concerns/token_authenticatable.rb'
require_relative '../../app/models/concerns/token_authenticatable_strategies/base.rb'
require_relative '../../app/models/concerns/token_authenticatable_strategies/insecure.rb'
require_relative '../../app/models/concerns/token_authenticatable_strategies/digest.rb'
namespace :tokens do
desc "Reset all GitLab incoming email tokens"
......@@ -26,13 +29,6 @@ class TmpUser < ActiveRecord::Base
self.table_name = 'users'
def reset_incoming_email_token!
write_new_token(:incoming_email_token)
save!(validate: false)
end
def reset_feed_token!
write_new_token(:feed_token)
save!(validate: false)
end
add_authentication_token_field :incoming_email_token, token_generator: -> { SecureRandom.hex.to_i(16).to_s(36) }
add_authentication_token_field :feed_token
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::BackgroundMigration::DigestColumn, :migration, schema: 20180913142237 do
let(:personal_access_tokens) { table(:personal_access_tokens) }
let(:users) { table(:users) }
subject { described_class.new }
describe '#perform' do
context 'token is not yet hashed' do
before do
users.create(id: 1, email: 'user@example.com', projects_limit: 10)
personal_access_tokens.create!(id: 1, user_id: 1, name: 'pat-01', token: 'token-01')
end
it 'saves token digest' do
expect { subject.perform(PersonalAccessToken, :token, :token_digest, 1, 2) }.to(
change { PersonalAccessToken.find(1).token_digest }.from(nil).to(Gitlab::CryptoHelper.sha256('token-01')))
end
it 'erases token' do
expect { subject.perform(PersonalAccessToken, :token, :token_digest, 1, 2) }.to(
change { PersonalAccessToken.find(1).token }.from('token-01').to(nil))
end
end
context 'token is already hashed' do
before do
users.create(id: 1, email: 'user@example.com', projects_limit: 10)
personal_access_tokens.create!(id: 1, user_id: 1, name: 'pat-01', token_digest: 'token-digest-01')
end
it 'does not change existing token digest' do
expect { subject.perform(PersonalAccessToken, :token, :token_digest, 1, 2) }.not_to(
change { PersonalAccessToken.find(1).token_digest })
end
it 'leaves token empty' do
expect { subject.perform(PersonalAccessToken, :token, :token_digest, 1, 2) }.not_to(
change { PersonalAccessToken.find(1).token }.from(nil))
end
end
end
end
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20180913142237_schedule_digest_personal_access_tokens.rb')
describe ScheduleDigestPersonalAccessTokens, :migration, :sidekiq do
let(:personal_access_tokens) { table(:personal_access_tokens) }
let(:users) { table(:users) }
before do
stub_const("#{described_class.name}::BATCH_SIZE", 4)
users.create(id: 1, email: 'user@example.com', projects_limit: 10)
personal_access_tokens.create!(id: 1, user_id: 1, name: 'pat-01', token: 'token-01')
personal_access_tokens.create!(id: 2, user_id: 1, name: 'pat-02', token: 'token-02')
personal_access_tokens.create!(id: 3, user_id: 1, name: 'pat-03', token_digest: 'token_digest')
personal_access_tokens.create!(id: 4, user_id: 1, name: 'pat-04', token: 'token-04')
personal_access_tokens.create!(id: 5, user_id: 1, name: 'pat-05', token: 'token-05')
personal_access_tokens.create!(id: 6, user_id: 1, name: 'pat-06', token: 'token-06')
end
it 'correctly schedules background migrations' do
Sidekiq::Testing.fake! do
migrate!
expect(described_class::MIGRATION).to(
be_scheduled_delayed_migration(
5.minutes, 'PersonalAccessToken', 'token', 'token_digest', 1, 5))
expect(described_class::MIGRATION).to(
be_scheduled_delayed_migration(
10.minutes, 'PersonalAccessToken', 'token', 'token_digest', 6, 6))
expect(BackgroundMigrationWorker.jobs.size).to eq 2
end
end
it 'schedules background migrations' do
perform_enqueued_jobs do
plain_text_token = 'token IS NOT NULL'
expect(personal_access_tokens.where(plain_text_token).count).to eq 5
migrate!
expect(personal_access_tokens.where(plain_text_token).count).to eq 0
end
end
end
......@@ -2,8 +2,6 @@ require 'spec_helper'
shared_examples 'TokenAuthenticatable' do
describe 'dynamically defined methods' do
it { expect(described_class).to be_private_method_defined(:generate_token) }
it { expect(described_class).to be_private_method_defined(:write_new_token) }
it { expect(described_class).to respond_to("find_by_#{token_field}") }
it { is_expected.to respond_to("ensure_#{token_field}") }
it { is_expected.to respond_to("set_#{token_field}") }
......@@ -66,13 +64,275 @@ describe ApplicationSetting, 'TokenAuthenticatable' do
end
describe 'multiple token fields' do
before do
before(:all) do
described_class.send(:add_authentication_token_field, :yet_another_token)
end
describe '.token_fields' do
subject { described_class.authentication_token_fields }
it { is_expected.to include(:runners_registration_token, :yet_another_token) }
it { is_expected.to respond_to(:ensure_runners_registration_token) }
it { is_expected.to respond_to(:ensure_yet_another_token) }
end
describe 'setting same token field multiple times' do
subject { described_class.send(:add_authentication_token_field, :runners_registration_token) }
it 'raises error' do
expect {subject}.to raise_error(ArgumentError)
end
end
end
describe PersonalAccessToken, 'TokenAuthenticatable' do
let(:personal_access_token_name) { 'test-pat-01' }
let(:token_value) { 'token' }
let(:user) { create(:user) }
let(:personal_access_token) do
described_class.new(name: personal_access_token_name,
user_id: user.id,
scopes: [:api],
token: token,
token_digest: token_digest)
end
before do
allow(Devise).to receive(:friendly_token).and_return(token_value)
end
describe '.find_by_token' do
subject { PersonalAccessToken.find_by_token(token_value) }
before do
personal_access_token.save
end
context 'token_digest already exists' do
let(:token) { nil }
let(:token_digest) { Gitlab::CryptoHelper.sha256(token_value) }
it 'finds the token' do
expect(subject).not_to be_nil
expect(subject.name).to eql(personal_access_token_name)
end
end
context 'token_digest does not exist' do
let(:token) { token_value }
let(:token_digest) { nil }
it 'finds the token' do
expect(subject).not_to be_nil
expect(subject.name).to eql(personal_access_token_name)
end
end
end
describe '#set_token' do
let(:new_token_value) { 'new-token' }
subject { personal_access_token.set_token(new_token_value) }
context 'token_digest already exists' do
let(:token) { nil }
let(:token_digest) { Gitlab::CryptoHelper.sha256(token_value) }
it 'overwrites token_digest' do
subject
expect(personal_access_token.read_attribute('token')).to be_nil
expect(personal_access_token.token).to eql(new_token_value)
expect(personal_access_token.token_digest).to eql( Gitlab::CryptoHelper.sha256(new_token_value))
end
end
context 'token_digest does not exist but token does' do
let(:token) { token_value }
let(:token_digest) { nil }
it 'creates new token_digest and clears token' do
subject
expect(personal_access_token.read_attribute('token')).to be_nil
expect(personal_access_token.token).to eql(new_token_value)
expect(personal_access_token.token_digest).to eql(Gitlab::CryptoHelper.sha256(new_token_value))
end
end
context 'token_digest does not exist, nor token' do
let(:token) { nil }
let(:token_digest) { nil }
it 'creates new token_digest' do
subject