diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-30 22:02:13 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-30 22:02:13 +0000 |
commit | 516fba52cf280b9d5bad08dce9f0150f859b6cea (patch) | |
tree | 4dad71be856651af62c9a281b01087ae15480810 /app/models | |
parent | c90be62bdefdb6bb67c73a9c4a6d164c9f78a28d (diff) | |
download | gitlab-ce-516fba52cf280b9d5bad08dce9f0150f859b6cea.tar.gz |
Add latest changes from gitlab-org/security/gitlab@13-4-stable-ee
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/active_session.rb | 114 | ||||
-rw-r--r-- | app/models/member.rb | 2 | ||||
-rw-r--r-- | app/models/operations/feature_flag.rb | 3 | ||||
-rw-r--r-- | app/models/releases/link.rb | 4 |
4 files changed, 70 insertions, 53 deletions
diff --git a/app/models/active_session.rb b/app/models/active_session.rb index 4908290e06b..dded0eb1dc3 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -9,14 +9,14 @@ class ActiveSession attr_accessor :created_at, :updated_at, :ip_address, :browser, :os, :device_name, :device_type, - :is_impersonated, :session_id + :is_impersonated, :session_id, :session_private_id - def current?(session) - return false if session_id.nil? || session.id.nil? + def current?(rack_session) + return false if session_private_id.nil? || rack_session.id.nil? # Rack v2.0.8+ added private_id, which uses the hash of the # public_id to avoid timing attacks. - session_id.private_id == session.id.private_id + session_private_id == rack_session.id.private_id end def human_device_type @@ -25,13 +25,14 @@ class ActiveSession # This is not the same as Rack::Session::SessionId#public_id, but we # need to preserve this for backwards compatibility. + # TODO: remove in 13.7 def public_id - Gitlab::CryptoHelper.aes256_gcm_encrypt(session_id.public_id) + Gitlab::CryptoHelper.aes256_gcm_encrypt(session_id) end def self.set(user, request) Gitlab::Redis::SharedState.with do |redis| - session_id = request.session.id.public_id + session_private_id = request.session.id.private_id client = DeviceDetector.new(request.user_agent) timestamp = Time.current @@ -43,25 +44,37 @@ class ActiveSession device_type: client.device_type, created_at: user.current_sign_in_at || timestamp, updated_at: timestamp, - session_id: session_id, + # TODO: remove in 13.7 + session_id: request.session.id.public_id, + session_private_id: session_private_id, is_impersonated: request.session[:impersonator_id].present? ) redis.pipelined do redis.setex( - key_name(user.id, session_id), + key_name(user.id, session_private_id), Settings.gitlab['session_expire_delay'] * 60, Marshal.dump(active_user_session) ) redis.sadd( lookup_key_name(user.id), - session_id + session_private_id ) + + # We remove the ActiveSession stored by using public_id to avoid + # duplicate entries + remove_deprecated_active_sessions_with_public_id(redis, user.id, request.session.id.public_id) end end end + # TODO: remove in 13.7 + private_class_method def self.remove_deprecated_active_sessions_with_public_id(redis, user_id, rack_session_public_id) + redis.srem(lookup_key_name(user_id), rack_session_public_id) + redis.del(key_name(user_id, rack_session_public_id)) + end + def self.list(user) Gitlab::Redis::SharedState.with do |redis| cleaned_up_lookup_entries(redis, user).map do |raw_session| @@ -70,27 +83,29 @@ class ActiveSession end end - def self.destroy(user, session_id) - return unless session_id - + def self.cleanup(user) Gitlab::Redis::SharedState.with do |redis| - destroy_sessions(redis, user, [session_id]) + clean_up_old_sessions(redis, user) + cleaned_up_lookup_entries(redis, user) end end - def self.destroy_with_public_id(user, public_id) - decrypted_id = decrypt_public_id(public_id) + # TODO: remove in 13.7 + # After upgrade there might be a duplicate ActiveSessions: + # - one with the public_id stored in #session_id + # - another with private_id stored in #session_private_id + def self.destroy_with_rack_session_id(user, rack_session_id) + return unless rack_session_id - return if decrypted_id.nil? - - session_id = Rack::Session::SessionId.new(decrypted_id) - destroy(user, session_id) + Gitlab::Redis::SharedState.with do |redis| + destroy_sessions(redis, user, [rack_session_id.public_id, rack_session_id.private_id]) + end end def self.destroy_sessions(redis, user, session_ids) - key_names = session_ids.map { |session_id| key_name(user.id, session_id.public_id) } + key_names = session_ids.map { |session_id| key_name(user.id, session_id) } - redis.srem(lookup_key_name(user.id), session_ids.map(&:public_id)) + redis.srem(lookup_key_name(user.id), session_ids) Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do redis.del(key_names) @@ -98,19 +113,29 @@ class ActiveSession end end - def self.cleanup(user) + # TODO: remove in 13.7 + # After upgrade, .destroy might be called with the session id encrypted + # by .public_id. + def self.destroy_with_deprecated_encryption(user, session_id) + return unless session_id + + decrypted_session_id = decrypt_public_id(session_id) + rack_session_private_id = if decrypted_session_id + Rack::Session::SessionId.new(decrypted_session_id).private_id + end + Gitlab::Redis::SharedState.with do |redis| - clean_up_old_sessions(redis, user) - cleaned_up_lookup_entries(redis, user) + destroy_sessions(redis, user, [session_id, decrypted_session_id, rack_session_private_id].compact) end end - def self.destroy_all_but_current(user, current_session) - session_ids = not_impersonated(user) - session_ids.reject! { |session| session.current?(current_session) } if current_session + def self.destroy_all_but_current(user, current_rack_session) + sessions = not_impersonated(user) + sessions.reject! { |session| session.current?(current_rack_session) } if current_rack_session Gitlab::Redis::SharedState.with do |redis| - destroy_sessions(redis, user, session_ids.map(&:session_id)) if session_ids.any? + session_ids = (sessions.map(&:session_id) | sessions.map(&:session_private_id)).compact + destroy_sessions(redis, user, session_ids) if session_ids.any? end end @@ -132,17 +157,16 @@ class ActiveSession # Lists the relevant session IDs for the user. # - # Returns an array of Rack::Session::SessionId objects + # Returns an array of strings def self.session_ids_for_user(user_id) Gitlab::Redis::SharedState.with do |redis| - session_ids = redis.smembers(lookup_key_name(user_id)) - session_ids.map { |id| Rack::Session::SessionId.new(id) } + redis.smembers(lookup_key_name(user_id)) end end # Lists the session Hash objects for the given session IDs. # - # session_ids - An array of Rack::Session::SessionId objects + # session_ids - An array of strings # # Returns an array of ActiveSession objects def self.sessions_from_ids(session_ids) @@ -168,27 +192,12 @@ class ActiveSession # Returns an ActiveSession object def self.load_raw_session(raw_session) # rubocop:disable Security/MarshalLoad - session = Marshal.load(raw_session) + Marshal.load(raw_session) # rubocop:enable Security/MarshalLoad + end - # Older ActiveSession models serialize `session_id` as strings, To - # avoid breaking older sessions, we keep backwards compatibility - # with older Redis keys and initiate Rack::Session::SessionId here. - session.session_id = Rack::Session::SessionId.new(session.session_id) if session.try(:session_id).is_a?(String) - session - end - - def self.rack_session_keys(session_ids) - session_ids.each_with_object([]) do |session_id, arr| - # This is a redis-rack implementation detail - # (https://github.com/redis-store/redis-rack/blob/master/lib/rack/session/redis.rb#L88) - # - # We need to delete session keys based on the legacy public key name - # and the newer private ID keys, but there's no well-defined interface - # so we have to do it directly. - arr << "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id.public_id}" - arr << "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id.private_id}" - end + def self.rack_session_keys(rack_session_ids) + rack_session_ids.map { |session_id| "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id}" } end def self.raw_active_session_entries(redis, session_ids, user_id) @@ -220,7 +229,7 @@ class ActiveSession sessions = active_session_entries(session_ids, user.id, redis) sessions.sort_by! {|session| session.updated_at }.reverse! destroyable_sessions = sessions.drop(ALLOWED_NUMBER_OF_ACTIVE_SESSIONS) - destroyable_session_ids = destroyable_sessions.map { |session| session.session_id } + destroyable_session_ids = destroyable_sessions.flat_map { |session| [session.session_id, session.session_private_id] }.compact destroy_sessions(redis, user, destroyable_session_ids) if destroyable_session_ids.any? end @@ -244,6 +253,7 @@ class ActiveSession entries.compact end + # TODO: remove in 13.7 private_class_method def self.decrypt_public_id(public_id) Gitlab::CryptoHelper.aes256_gcm_decrypt(public_id) rescue diff --git a/app/models/member.rb b/app/models/member.rb index 5a084a3a2e6..7ea9caa45d3 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -5,6 +5,7 @@ class Member < ApplicationRecord include AfterCommitQueue include Sortable include Importable + include CreatedAtFilterable include Expirable include Gitlab::Access include Presentable @@ -20,6 +21,7 @@ class Member < ApplicationRecord delegate :name, :username, :email, to: :user, prefix: true + validates :expires_at, allow_blank: true, future_date: true validates :user, presence: true, unless: :invite? validates :source, presence: true validates :user_id, uniqueness: { scope: [:source_type, :source_id], diff --git a/app/models/operations/feature_flag.rb b/app/models/operations/feature_flag.rb index 586e9d689a1..104338b80d1 100644 --- a/app/models/operations/feature_flag.rb +++ b/app/models/operations/feature_flag.rb @@ -4,8 +4,11 @@ module Operations class FeatureFlag < ApplicationRecord include AtomicInternalId include IidRoutes + include Limitable self.table_name = 'operations_feature_flags' + self.limit_scope = :project + self.limit_name = 'project_feature_flags' belongs_to :project diff --git a/app/models/releases/link.rb b/app/models/releases/link.rb index e1dc3b904b9..82272f4857a 100644 --- a/app/models/releases/link.rb +++ b/app/models/releases/link.rb @@ -6,7 +6,9 @@ module Releases belongs_to :release - FILEPATH_REGEX = %r{\A/(?:[\-\.\w]+/?)*[\da-zA-Z]+\z}.freeze + # See https://gitlab.com/gitlab-org/gitlab/-/issues/218753 + # Regex modified to prevent catastrophic backtracking + FILEPATH_REGEX = %r{\A\/[^\/](?!.*\/\/.*)[\-\.\w\/]+[\da-zA-Z]+\z}.freeze validates :url, presence: true, addressable_url: { schemes: %w(http https ftp) }, uniqueness: { scope: :release } validates :name, presence: true, uniqueness: { scope: :release } |