diff options
author | Jan Provaznik <jprovaznik@gitlab.com> | 2018-10-23 13:50:54 +0300 |
---|---|---|
committer | Thiago Presa <tpresa@gitlab.com> | 2018-10-25 03:01:41 +0300 |
commit | daed01a5ca348e7d267b50e325bf58185617a0ad (patch) | |
tree | 7a579442d7b8a5740f3b22613c929d285d7e3e23 /app | |
parent | 9266cb278c006f763b891f9bc4c04053e38be41b (diff) |
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
Diffstat (limited to 'app')
-rw-r--r-- | app/models/concerns/token_authenticatable.rb | 55 | ||||
-rw-r--r-- | app/models/concerns/token_authenticatable_strategies/base.rb | 69 | ||||
-rw-r--r-- | app/models/concerns/token_authenticatable_strategies/digest.rb | 50 | ||||
-rw-r--r-- | app/models/concerns/token_authenticatable_strategies/insecure.rb | 23 | ||||
-rw-r--r-- | app/models/personal_access_token.rb | 16 | ||||
-rw-r--r-- | app/models/user.rb | 11 |
6 files changed, 178 insertions, 46 deletions
diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb index 522b65e4205..66db4bd92de 100644 --- a/app/models/concerns/token_authenticatable.rb +++ b/app/models/concerns/token_authenticatable.rb @@ -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 diff --git a/app/models/concerns/token_authenticatable_strategies/base.rb b/app/models/concerns/token_authenticatable_strategies/base.rb new file mode 100644 index 00000000000..f0f7107d627 --- /dev/null +++ b/app/models/concerns/token_authenticatable_strategies/base.rb @@ -0,0 +1,69 @@ +# 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 diff --git a/app/models/concerns/token_authenticatable_strategies/digest.rb b/app/models/concerns/token_authenticatable_strategies/digest.rb new file mode 100644 index 00000000000..9926662ed66 --- /dev/null +++ b/app/models/concerns/token_authenticatable_strategies/digest.rb @@ -0,0 +1,50 @@ +# 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 diff --git a/app/models/concerns/token_authenticatable_strategies/insecure.rb b/app/models/concerns/token_authenticatable_strategies/insecure.rb new file mode 100644 index 00000000000..5f915259521 --- /dev/null +++ b/app/models/concerns/token_authenticatable_strategies/insecure.rb @@ -0,0 +1,23 @@ +# 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 diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index 207146479c0..73a58f2420e 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index cd3b1c95b7e..b7f7abbe184 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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 |