Commit 2c62adfb authored by Andreas Brandl's avatar Andreas Brandl

Ignore errors from invalid keys

This aims to make the migration more robust to invalid keys.

See https://gitlab.com/gitlab-org/gitlab/issues/196741
parent eac596f7
...@@ -24,12 +24,14 @@ module Gitlab ...@@ -24,12 +24,14 @@ module Gitlab
fingerprints = [] fingerprints = []
Key.where(id: start_id..stop_id, fingerprint_sha256: nil).find_each do |regular_key| Key.where(id: start_id..stop_id, fingerprint_sha256: nil).find_each do |regular_key|
fingerprint = Base64.decode64(generate_ssh_public_key(regular_key.key)) if fingerprint = generate_ssh_public_key(regular_key.key)
bytea = ActiveRecord::Base.connection.escape_bytea(Base64.decode64(fingerprint))
fingerprints << {
id: regular_key.id, fingerprints << {
fingerprint_sha256: ActiveRecord::Base.connection.escape_bytea(fingerprint) id: regular_key.id,
} fingerprint_sha256: bytea
}
end
end end
Gitlab::Database.bulk_insert(TEMP_TABLE, fingerprints) Gitlab::Database.bulk_insert(TEMP_TABLE, fingerprints)
...@@ -48,7 +50,7 @@ module Gitlab ...@@ -48,7 +50,7 @@ module Gitlab
private private
def generate_ssh_public_key(regular_key) def generate_ssh_public_key(regular_key)
Gitlab::SSHPublicKey.new(regular_key).fingerprint("SHA256").gsub("SHA256:", "") Gitlab::SSHPublicKey.new(regular_key).fingerprint("SHA256")&.gsub("SHA256:", "")
end end
def execute(query) def execute(query)
......
...@@ -37,6 +37,25 @@ describe Gitlab::BackgroundMigration::MigrateFingerprintSha256WithinKeys, :migra ...@@ -37,6 +37,25 @@ describe Gitlab::BackgroundMigration::MigrateFingerprintSha256WithinKeys, :migra
expect(key_2.fingerprint_sha256).to eq('zMNbLekgdjtcgDv8VSC0z5lpdACMG3Q4PUoIz5+H2jM') expect(key_2.fingerprint_sha256).to eq('zMNbLekgdjtcgDv8VSC0z5lpdACMG3Q4PUoIz5+H2jM')
end end
context 'with invalid keys' do
before do
key = Key.find(1017)
# double space after "ssh-rsa" leads to a
# OpenSSL::PKey::PKeyError in Net::SSH::KeyFactory.load_data_public_key
key.update_column(:key, key.key.gsub('ssh-rsa ', 'ssh-rsa '))
end
it 'ignores errors and does not set the fingerprint' do
fingerprint_migrator.perform(1, 10000)
key_1 = Key.find(1017)
key_2 = Key.find(1027)
expect(key_1.fingerprint_sha256).to be_nil
expect(key_2.fingerprint_sha256).not_to be_nil
end
end
it 'migrates all keys' do it 'migrates all keys' do
expect(Key.where(fingerprint_sha256: nil).count).to eq(Key.all.count) expect(Key.where(fingerprint_sha256: nil).count).to eq(Key.all.count)
......
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