Skip to content
Snippets Groups Projects
Verified Commit 88787a72 authored by Stan Hu's avatar Stan Hu
Browse files

Fix move deploy keys during project imports in FIPS mode

When a project is imported, it may overwrite a project via
`OverwriteProjectService`, which then calls
`MoveDeployKeysProjectsService` to move deploy keys. Previously in
FIPS mode the deploy keys would fail to move over because the query
relied on a join with MD5 fingerprints, which aren't present in FIPS.

To fix this, we use SHA256 fingerprints instead. Since deploy keys may
be deleted, we ensure that SHA256 fingerprints exist for all the keys
in question.

Relates to #364562

Changelog: fixed
parent ee521edd
No related branches found
No related tags found
1 merge request!92809Fix move deploy keys during project imports in FIPS mode
......@@ -121,6 +121,12 @@ def public_key
@public_key ||= Gitlab::SSHPublicKey.new(key)
end
def ensure_sha256_fingerprint!
return if self.fingerprint_sha256
save if generate_fingerprint
end
private
def generate_fingerprint
......
......@@ -5,6 +5,10 @@ class MoveDeployKeysProjectsService < BaseMoveRelationsService
def execute(source_project, remove_remaining_elements: true)
return unless super
# The SHA256 fingerprint should be there, but just in case it isn't
# we want to make sure it's generated. Otherwise we might delete keys.
ensure_sha256_fingerprints
Project.transaction do
move_deploy_keys_projects
remove_remaining_deploy_keys_projects if remove_remaining_elements
......@@ -15,6 +19,11 @@ def execute(source_project, remove_remaining_elements: true)
private
def ensure_sha256_fingerprints
@project.deploy_keys.each(&:ensure_sha256_fingerprint!)
source_project.deploy_keys.each(&:ensure_sha256_fingerprint!)
end
def move_deploy_keys_projects
non_existent_deploy_keys_projects.update_all(project_id: @project.id)
end
......@@ -23,7 +32,7 @@ def move_deploy_keys_projects
def non_existent_deploy_keys_projects
source_project.deploy_keys_projects
.joins(:deploy_key)
.where.not(keys: { fingerprint: @project.deploy_keys.select(:fingerprint) })
.where.not(keys: { fingerprint_sha256: @project.deploy_keys.select(:fingerprint_sha256) })
end
# rubocop: enable CodeReuse/ActiveRecord
......
......@@ -240,6 +240,39 @@
end
end
describe '#ensure_sha256_fingerprint!' do
let_it_be_with_reload(:user_key) { create(:personal_key) }
context 'with a valid SHA256 fingerprint' do
it 'does nothing' do
expect(user_key).not_to receive(:generate_fingerprint)
user_key.ensure_sha256_fingerprint!
end
end
context 'with a missing SHA256 fingerprint' do
before do
user_key.update_column(:fingerprint_sha256, nil)
user_key.ensure_sha256_fingerprint!
end
it 'fingerprints are present' do
expect(user_key.reload.fingerprint_sha256).to be_present
end
end
context 'with an invalid public key' do
before do
user_key.update_column(:key, 'a')
end
it 'does not throw an exception' do
expect { user_key.ensure_sha256_fingerprint! }.not_to raise_error
end
end
end
context 'fingerprint generation' do
it 'generates both md5 and sha256 fingerprints' do
key = build(:rsa_key_4096)
......
......@@ -56,5 +56,22 @@
expect(project_with_deploy_keys.deploy_keys_projects.count).not_to eq 0
end
end
context 'when SHA256 fingerprint is missing' do
before do
create(:deploy_keys_project, project: target_project)
DeployKey.all.update_all(fingerprint_sha256: nil)
end
it 'moves the user\'s deploy keys from one project to another' do
combined_keys = project_with_deploy_keys.deploy_keys + target_project.deploy_keys
subject.execute(project_with_deploy_keys)
expect(project_with_deploy_keys.deploy_keys.reload).to be_empty
expect(target_project.deploy_keys.reload).to match_array(combined_keys)
expect(DeployKey.all.select(:fingerprint_sha256)).to all(be_present)
end
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment