From ac86b2043dea8f15cb0670db98c2bf21e1843d4b Mon Sep 17 00:00:00 2001 From: Pablo Carranza Date: Tue, 1 Mar 2016 17:53:01 +0000 Subject: Backport authorized_keys branch 'find-key-by-fingerprint' Add find key by base64 key or fingerprint to the internal API See merge request !250 Squashed changes: Add unique index to fingerprint Add new index to schema Add internal api to get ssh key by fingerprint Change API endpoint to authorized_keys Add InsecureKeyFingerprint that calculates the fingerprint without shelling out Add require for gitlab key fingerprint Remove uniqueness of fingerprint index Remove unique option from migration Fix spec style in fingerprint test Fix rubocop complain Extract insecure key fingerprint to separate file Change migration to support building index concurrently Remove those hideous tabs --- spec/lib/gitlab/insecure_key_fingerprint_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 spec/lib/gitlab/insecure_key_fingerprint_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/insecure_key_fingerprint_spec.rb b/spec/lib/gitlab/insecure_key_fingerprint_spec.rb new file mode 100644 index 00000000000..6532579b1c9 --- /dev/null +++ b/spec/lib/gitlab/insecure_key_fingerprint_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe Gitlab::InsecureKeyFingerprint do + let(:key) do + 'ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn' \ + '1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qk' \ + 'r8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMg' \ + 'Jw0=' + end + + let(:fingerprint) { "3f:a2:ee:de:b5:de:53:c3:aa:2f:9c:45:24:4c:47:7b" } + + describe "#fingerprint" do + it "generates the key's fingerprint" do + expect(described_class.new(key.split[1]).fingerprint).to eq(fingerprint) + end + end +end -- cgit v1.2.1 From 255a0f85e3b62845b58f5a4aa189e57f36992c77 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Tue, 30 May 2017 16:24:45 -0700 Subject: Backport option to disable writing to `authorized_keys` file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Originally branch 'mk-toggle-writing-to-auth-keys-1631' See merge request !2004 Squashed commits: Add authorized_keys_enabled to Application Settings Ensure default settings are exposed in UI Without this change, `authorized_keys_enabled` is unchecked when it is nil, even if it should be checked by default. Add “Speed up SSH operations” documentation Clarify the reasons for disabling writes Add "How to go back" section Tweak copy Update Application Setting screenshot --- spec/lib/gitlab/shell_spec.rb | 110 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 99 insertions(+), 11 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 81d9e6a8f82..6d50f537430 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -51,6 +51,105 @@ describe Gitlab::Shell do end end + describe '#add_key' do + context 'when authorized_keys_enabled is true' do + it 'removes trailing garbage' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(Gitlab::Utils).to receive(:system_silent).with( + [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] + ) + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end + end + + context 'when authorized_keys_enabled is false' do + before do + stub_application_setting(authorized_keys_enabled: false) + end + + it 'does nothing' do + expect(Gitlab::Utils).not_to receive(:system_silent) + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end + end + end + + describe '#batch_add_keys' do + context 'when authorized_keys_enabled is true' do + it 'instantiates KeyAdder' do + expect_any_instance_of(Gitlab::Shell::KeyAdder).to receive(:add_key).with('key-123', 'ssh-rsa foobar') + + gitlab_shell.batch_add_keys do |adder| + adder.add_key('key-123', 'ssh-rsa foobar') + end + end + end + + context 'when authorized_keys_enabled is false' do + before do + stub_application_setting(authorized_keys_enabled: false) + end + + it 'does nothing' do + expect_any_instance_of(Gitlab::Shell::KeyAdder).not_to receive(:add_key) + + gitlab_shell.batch_add_keys do |adder| + adder.add_key('key-123', 'ssh-rsa foobar') + end + end + end + end + + describe '#remove_key' do + context 'when authorized_keys_enabled is true' do + it 'removes trailing garbage' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(Gitlab::Utils).to receive(:system_silent).with( + [:gitlab_shell_keys_path, 'rm-key', 'key-123', 'ssh-rsa foobar'] + ) + + gitlab_shell.remove_key('key-123', 'ssh-rsa foobar') + end + end + + context 'when authorized_keys_enabled is false' do + before do + stub_application_setting(authorized_keys_enabled: false) + end + + it 'does nothing' do + expect(Gitlab::Utils).not_to receive(:system_silent) + + gitlab_shell.remove_key('key-123', 'ssh-rsa foobar') + end + end + end + + describe '#remove_all_keys' do + context 'when authorized_keys_enabled is true' do + it 'removes trailing garbage' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(Gitlab::Utils).to receive(:system_silent).with([:gitlab_shell_keys_path, 'clear']) + + gitlab_shell.remove_all_keys + end + end + + context 'when authorized_keys_enabled is false' do + before do + stub_application_setting(authorized_keys_enabled: false) + end + + it 'does nothing' do + expect(Gitlab::Utils).not_to receive(:system_silent) + + gitlab_shell.remove_all_keys + end + end + end + describe Gitlab::Shell::KeyAdder do describe '#add_key' do it 'removes trailing garbage' do @@ -96,17 +195,6 @@ describe Gitlab::Shell do allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800) end - describe '#add_key' do - it 'removes trailing garbage' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( - [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] - ) - - gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') - end - end - describe '#add_repository' do shared_examples '#add_repository' do let(:repository_storage) { 'default' } -- cgit v1.2.1 From 797fe0a6e6ce2f1241d2bd22c4b491c367e796fd Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Mon, 26 Jun 2017 14:40:08 -0700 Subject: Backport authorized_keys_enabled defaults to true' Originally from branch 'fix-authorized-keys-enabled-default-2738' via merge request https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2240 Removed background migrations which were intended to fix state after using Gitlab without a default having been set Squashed commits: Locally, if Spring was not restarted, `current_application_settings` was still cached, which prevented the migration from editing the file. This will also ensure that any app server somehow hitting old cache data will properly default this setting regardless. Retroactively fix migration This allows us to identify customers who ran the broken migration. Their `authorized_keys_enabled` column does not have a default at this point. We will fix the column after we fix the `authorized_keys` file. Fix authorized_keys file if needed Add default to authorized_keys_enabled setting Reminder: The original migration was fixed retroactively a few commits ago, so people who did not ever run GitLab 9.3.0 already have a column that defaults to true and disallows nulls. I have tested on PostgreSQL and MySQL that it is safe to run this migration regardless. Affected customers who did run 9.3.0 are the ones who need this migration to fix the authorized_keys_enabled column. The reason for the retroactive fix plus this migration is that it allows us to run a migration in between to fix the authorized_keys file only for those who ran 9.3.0. Tweaks to address feedback Extract work into background migration Move batch-add-logic to background migration Do the work synchronously to avoid multiple workers attempting to add batches of keys at the same time. Also, make the delete portion wait until after adding is done. Do read and delete work in background migration Fix Rubocop offenses Add changelog entry Inform the user of actions taken or not taken Prevent unnecessary `select`s and `remove_key`s Add logs for action taken Fix optimization Reuse `Gitlab::ShellAdapter` Guarantee the earliest key Fix migration spec for MySQL --- spec/lib/gitlab/shell_spec.rb | 212 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 212 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 6d50f537430..e452fbb757d 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -74,6 +74,21 @@ describe Gitlab::Shell do gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') end end + + context 'when authorized_keys_enabled is nil' do + before do + stub_application_setting(authorized_keys_enabled: nil) + end + + it 'removes trailing garbage' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(Gitlab::Utils).to receive(:system_silent).with( + [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] + ) + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end + end end describe '#batch_add_keys' do @@ -100,6 +115,20 @@ describe Gitlab::Shell do end end end + + context 'when authorized_keys_enabled is nil' do + before do + stub_application_setting(authorized_keys_enabled: nil) + end + + it 'instantiates KeyAdder' do + expect_any_instance_of(Gitlab::Shell::KeyAdder).to receive(:add_key).with('key-123', 'ssh-rsa foobar') + + gitlab_shell.batch_add_keys do |adder| + adder.add_key('key-123', 'ssh-rsa foobar') + end + end + end end describe '#remove_key' do @@ -125,6 +154,32 @@ describe Gitlab::Shell do gitlab_shell.remove_key('key-123', 'ssh-rsa foobar') end end + + context 'when authorized_keys_enabled is nil' do + before do + stub_application_setting(authorized_keys_enabled: nil) + end + + it 'removes trailing garbage' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(Gitlab::Utils).to receive(:system_silent).with( + [:gitlab_shell_keys_path, 'rm-key', 'key-123', 'ssh-rsa foobar'] + ) + + gitlab_shell.remove_key('key-123', 'ssh-rsa foobar') + end + end + + context 'when key content is not given' do + it 'calls rm-key with only one argument' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(Gitlab::Utils).to receive(:system_silent).with( + [:gitlab_shell_keys_path, 'rm-key', 'key-123'] + ) + + gitlab_shell.remove_key('key-123') + end + end end describe '#remove_all_keys' do @@ -148,6 +203,155 @@ describe Gitlab::Shell do gitlab_shell.remove_all_keys end end + + context 'when authorized_keys_enabled is nil' do + before do + stub_application_setting(authorized_keys_enabled: nil) + end + + it 'removes trailing garbage' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(Gitlab::Utils).to receive(:system_silent).with([:gitlab_shell_keys_path, 'clear']) + + gitlab_shell.remove_all_keys + end + end + end + + describe '#remove_keys_not_found_in_db' do + context 'when keys are in the file that are not in the DB' do + before do + gitlab_shell.remove_all_keys + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + gitlab_shell.add_key('key-9876', 'ssh-rsa ASDFASDF') + @another_key = create(:key) # this one IS in the DB + end + + it 'removes the keys' do + expect(find_in_authorized_keys_file(1234)).to be_truthy + expect(find_in_authorized_keys_file(9876)).to be_truthy + expect(find_in_authorized_keys_file(@another_key.id)).to be_truthy + gitlab_shell.remove_keys_not_found_in_db + expect(find_in_authorized_keys_file(1234)).to be_falsey + expect(find_in_authorized_keys_file(9876)).to be_falsey + expect(find_in_authorized_keys_file(@another_key.id)).to be_truthy + end + end + + context 'when keys there are duplicate keys in the file that are not in the DB' do + before do + gitlab_shell.remove_all_keys + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + end + + it 'removes the keys' do + expect(find_in_authorized_keys_file(1234)).to be_truthy + gitlab_shell.remove_keys_not_found_in_db + expect(find_in_authorized_keys_file(1234)).to be_falsey + end + + it 'does not run remove more than once per key (in a batch)' do + expect(gitlab_shell).to receive(:remove_key).with('key-1234').once + gitlab_shell.remove_keys_not_found_in_db + end + end + + context 'when keys there are duplicate keys in the file that ARE in the DB' do + before do + gitlab_shell.remove_all_keys + @key = create(:key) + gitlab_shell.add_key(@key.shell_id, @key.key) + end + + it 'does not remove the key' do + gitlab_shell.remove_keys_not_found_in_db + expect(find_in_authorized_keys_file(@key.id)).to be_truthy + end + + it 'does not need to run a SELECT query for that batch, on account of that key' do + expect_any_instance_of(ActiveRecord::Relation).not_to receive(:pluck) + gitlab_shell.remove_keys_not_found_in_db + end + end + + unless ENV['CI'] # Skip in CI, it takes 1 minute + context 'when the first batch can be skipped, but the next batch has keys that are not in the DB' do + before do + gitlab_shell.remove_all_keys + 100.times { |i| create(:key) } # first batch is all in the DB + gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') + end + + it 'removes the keys not in the DB' do + expect(find_in_authorized_keys_file(1234)).to be_truthy + gitlab_shell.remove_keys_not_found_in_db + expect(find_in_authorized_keys_file(1234)).to be_falsey + end + end + end + end + + describe '#batch_read_key_ids' do + context 'when there are keys in the authorized_keys file' do + before do + gitlab_shell.remove_all_keys + (1..4).each do |i| + gitlab_shell.add_key("key-#{i}", "ssh-rsa ASDFASDF#{i}") + end + end + + it 'iterates over the key IDs in the file, in batches' do + loop_count = 0 + first_batch = [1, 2] + second_batch = [3, 4] + + gitlab_shell.batch_read_key_ids(batch_size: 2) do |batch| + expected = (loop_count == 0 ? first_batch : second_batch) + expect(batch).to eq(expected) + loop_count += 1 + end + end + end + end + + describe '#list_key_ids' do + context 'when there are keys in the authorized_keys file' do + before do + gitlab_shell.remove_all_keys + (1..4).each do |i| + gitlab_shell.add_key("key-#{i}", "ssh-rsa ASDFASDF#{i}") + end + end + + it 'outputs the key IDs in the file, separated by newlines' do + ids = [] + gitlab_shell.list_key_ids do |io| + io.each do |line| + ids << line + end + end + + expect(ids).to eq(%W{1\n 2\n 3\n 4\n}) + end + end + + context 'when there are no keys in the authorized_keys file' do + before do + gitlab_shell.remove_all_keys + end + + it 'outputs nothing, not even an empty string' do + ids = [] + gitlab_shell.list_key_ids do |io| + io.each do |line| + ids << line + end + end + + expect(ids).to eq([]) + end + end end describe Gitlab::Shell::KeyAdder do @@ -484,4 +688,12 @@ describe Gitlab::Shell do end end end + + def find_in_authorized_keys_file(key_id) + gitlab_shell.batch_read_key_ids do |ids| + return true if ids.include?(key_id) + end + + false + end end -- cgit v1.2.1 From d9557e43e4eb91eede069c045a962e417ae6e852 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Tue, 4 Jul 2017 19:06:18 +0300 Subject: Backport spec fixes in spec/lib/gitlab/shell_spec.rb --- spec/lib/gitlab/shell_spec.rb | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index e452fbb757d..24fc17861dd 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -55,7 +55,7 @@ describe Gitlab::Shell do context 'when authorized_keys_enabled is true' do it 'removes trailing garbage' do allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(Gitlab::Utils).to receive(:system_silent).with( + expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] ) @@ -69,7 +69,7 @@ describe Gitlab::Shell do end it 'does nothing' do - expect(Gitlab::Utils).not_to receive(:system_silent) + expect(Gitlab::Utils).not_to receive(:gitlab_shell_fast_execute) gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') end @@ -82,7 +82,7 @@ describe Gitlab::Shell do it 'removes trailing garbage' do allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(Gitlab::Utils).to receive(:system_silent).with( + expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] ) @@ -135,7 +135,7 @@ describe Gitlab::Shell do context 'when authorized_keys_enabled is true' do it 'removes trailing garbage' do allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(Gitlab::Utils).to receive(:system_silent).with( + expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( [:gitlab_shell_keys_path, 'rm-key', 'key-123', 'ssh-rsa foobar'] ) @@ -149,7 +149,7 @@ describe Gitlab::Shell do end it 'does nothing' do - expect(Gitlab::Utils).not_to receive(:system_silent) + expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute) gitlab_shell.remove_key('key-123', 'ssh-rsa foobar') end @@ -162,7 +162,7 @@ describe Gitlab::Shell do it 'removes trailing garbage' do allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(Gitlab::Utils).to receive(:system_silent).with( + expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( [:gitlab_shell_keys_path, 'rm-key', 'key-123', 'ssh-rsa foobar'] ) @@ -173,7 +173,7 @@ describe Gitlab::Shell do context 'when key content is not given' do it 'calls rm-key with only one argument' do allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(Gitlab::Utils).to receive(:system_silent).with( + expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( [:gitlab_shell_keys_path, 'rm-key', 'key-123'] ) @@ -186,7 +186,7 @@ describe Gitlab::Shell do context 'when authorized_keys_enabled is true' do it 'removes trailing garbage' do allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(Gitlab::Utils).to receive(:system_silent).with([:gitlab_shell_keys_path, 'clear']) + expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with([:gitlab_shell_keys_path, 'clear']) gitlab_shell.remove_all_keys end @@ -198,7 +198,7 @@ describe Gitlab::Shell do end it 'does nothing' do - expect(Gitlab::Utils).not_to receive(:system_silent) + expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute) gitlab_shell.remove_all_keys end @@ -211,7 +211,9 @@ describe Gitlab::Shell do it 'removes trailing garbage' do allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(Gitlab::Utils).to receive(:system_silent).with([:gitlab_shell_keys_path, 'clear']) + expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( + [:gitlab_shell_keys_path, 'clear'] + ) gitlab_shell.remove_all_keys end -- cgit v1.2.1 From bd9ead683c3bade57a59d06530a1973768622a78 Mon Sep 17 00:00:00 2001 From: James Edwards-Jones Date: Mon, 8 Jan 2018 19:43:32 +0000 Subject: Fix spec in shell_spec.rb The spec for "#add_key does nothing" would always have passed, since the expectation was on both the wrong object and message. --- spec/lib/gitlab/shell_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 24fc17861dd..eb90f53468f 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -69,7 +69,7 @@ describe Gitlab::Shell do end it 'does nothing' do - expect(Gitlab::Utils).not_to receive(:gitlab_shell_fast_execute) + expect(gitlab_shell).not_to receive(:gitlab_shell_fast_execute) gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') end -- cgit v1.2.1