This instance will be upgraded to Heptapod 0.31.0rc on 2022-05-19 at 11:00 UTC+2 (a few minutes of down time)

Commit 591c3b5f authored by GitLab Bot's avatar GitLab Bot
Browse files

Add latest changes from gitlab-org/security/gitlab@13-4-stable-ee

parent db707a44cee3
......@@ -15,7 +15,7 @@ def connect
private
def find_user_from_session_store
session = ActiveSession.sessions_from_ids([session_id]).first
session = ActiveSession.sessions_from_ids([session_id.private_id]).first
Warden::SessionSerializer.new('rack.session' => session).fetch(:user)
end
......
......@@ -6,7 +6,9 @@ def index
end
def destroy
ActiveSession.destroy_with_public_id(current_user, params[:id])
# params[:id] can be either an Rack::Session::SessionId#private_id
# or an encrypted Rack::Session::SessionId#public_id
ActiveSession.destroy_with_deprecated_encryption(current_user, params[:id])
current_user.forget_me!
respond_to do |format|
......
......@@ -22,4 +22,13 @@ def active_session_device_type_icon(active_session)
sprite_icon(icon_name, css_class: 'gl-mt-2')
end
def revoke_session_path(active_session)
if active_session.session_private_id
profile_active_session_path(active_session.session_private_id)
else
# TODO: remove in 13.7
profile_active_session_path(active_session.public_id)
end
end
end
......@@ -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 @@ def human_device_type
# 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 @@ def self.set(user, request)
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 @@ def self.list(user)
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 @@ def self.destroy_sessions(redis, user, session_ids)
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 @@ def self.list_sessions(user)
# 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 @@ def self.sessions_from_ids(session_ids)
# 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 @@ def self.clean_up_old_sessions(redis, user)
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 @@ def self.cleaned_up_lookup_entries(redis, user)
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
......
......@@ -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],
......
......@@ -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
......
......@@ -6,7 +6,9 @@ class Link < ApplicationRecord
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 }
......
......@@ -19,6 +19,7 @@ def can_admin_issuable?(issuable)
def filter_params(issuable)
unless can_admin_issuable?(issuable)
params.delete(:milestone)
params.delete(:milestone_id)
params.delete(:labels)
params.delete(:add_label_ids)
......
......@@ -3,6 +3,7 @@
module Issues
class UpdateService < Issues::BaseService
include SpamCheckMethods
extend ::Gitlab::Utils::Override
def execute(issue)
handle_move_between_ids(issue)
......@@ -17,6 +18,17 @@ def update(issue)
super
end
override :filter_params
def filter_params(issue)
super
# filter confidential in `Issues::UpdateService` and not in `IssuableBaseService#filtr_params`
# because we do allow users that cannot admin issues to set confidential flag when creating an issue
unless can_admin_issuable?(issue)
params.delete(:confidential)
end
end
def before_update(issue, skip_spam_check: false)
spam_check(issue, current_user, action: :update) unless skip_spam_check
end
......
# frozen_string_literal: true
# FutureDateValidator
# Validates that a date is in the future.
#
# Example:
#
# class Member < ActiveRecord::Base
# validates :expires_at, allow_blank: true, future_date: true
# end
class FutureDateValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
record.errors.add(attribute, _('cannot be a date in the past')) if value < Date.current
end
end
......@@ -27,6 +27,9 @@
- unless is_current_session
.float-right
= link_to profile_active_session_path(active_session.public_id), data: { confirm: _('Are you sure? The device will be signed out of GitLab and all remember me tokens revoked.') }, method: :delete, class: "btn btn-danger gl-ml-3" do
= link_to(revoke_session_path(active_session),
{ data: { confirm: _('Are you sure? The device will be signed out of GitLab and all remember me tokens revoked.') },
method: :delete,
class: "btn btn-danger gl-ml-3" }) do
%span.sr-only= _('Revoke')
= _('Revoke')
......@@ -315,6 +315,14 @@
:weight: 1
:idempotent:
:tags: []
- :name: cronjob:remove_unaccepted_member_invites
:feature_category: :authentication_and_authorization
:has_external_dependencies:
:urgency: :low
:resource_boundary: :unknown
:weight: 1
:idempotent: true
:tags: []
- :name: cronjob:remove_unreferenced_lfs_objects
:feature_category: :git_lfs
:has_external_dependencies:
......
# frozen_string_literal: true
class RemoveUnacceptedMemberInvitesWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :authentication_and_authorization
urgency :low
idempotent!
EXPIRATION_THRESHOLD = 90.days
BATCH_SIZE = 10_000
def perform
# We need to check for user_id IS NULL because we have accepted invitations
# in the database where we did not clear the invite_token. We do not
# want to accidentally delete those members.
loop do
# rubocop: disable CodeReuse/ActiveRecord
inner_query = Member
.select(:id)
.invite
.created_before(EXPIRATION_THRESHOLD.ago)
.where(user_id: nil)
.limit(BATCH_SIZE)
records_deleted = Member.where(id: inner_query).delete_all
# rubocop: enable CodeReuse/ActiveRecord
break if records_deleted == 0
end
end
end
---
title: Do not store session id in Redis
merge_request:
author:
type: security
---
title: Fix permission checks when updating confidentiality and milestone on issues
or merge requests
merge_request:
author:
type: security
---
title: Purge unaccepted member invitations older than 90 days
merge_request:
author:
type: security
---
title: Adds feature flags plan limits
merge_request:
author:
type: security
---
title: Do not bypass admin mode when authenticated with deploy token
merge_request:
author:
type: security
---
title: Fixes release asset link filepath ReDoS
merge_request:
author:
type: security
---
title: Validate that membership expiry dates are not in the past
merge_request:
author:
type: security
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