diff options
author | Robert Speicher <robert@gitlab.com> | 2016-08-10 16:52:49 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2016-08-10 16:52:49 +0000 |
commit | 0b73855f1b83818683f5a1de83090bb043a51616 (patch) | |
tree | 4c61731f3af587dafd424b13b71b6fefb4f3b5a1 | |
parent | 7837894a8a740f8ed9a4884fa7faee566eb9b6c2 (diff) | |
parent | 2f10eec142c0e70c06289f093f4057cc96eb3d44 (diff) | |
download | gitlab-shell-0b73855f1b83818683f5a1de83090bb043a51616.tar.gz |
Merge branch 'key-validations' into 'master'
Defense in depth for authorized_keys lines
Validate the key_id and public_key inputs when rendering the actual
'line' we append to authorized_keys. Although these inputs are either
trusted (key_id) or validated earlier (public_key) it does not hurt to
take a little extra care that we do not write unintended data to the
authorized_keys file.
See merge request !82
-rw-r--r-- | lib/gitlab_keys.rb | 5 | ||||
-rw-r--r-- | spec/gitlab_keys_spec.rb | 27 |
2 files changed, 32 insertions, 0 deletions
diff --git a/lib/gitlab_keys.rb b/lib/gitlab_keys.rb index dc654fd..d4c4102 100644 --- a/lib/gitlab_keys.rb +++ b/lib/gitlab_keys.rb @@ -4,13 +4,18 @@ require_relative 'gitlab_config' require_relative 'gitlab_logger' class GitlabKeys + class KeyError < StandardError ; end + attr_accessor :auth_file, :key def self.command(key_id) + raise KeyError.new("Invalid key_id: #{key_id.inspect}") unless /\A[a-z0-9-]+\z/ =~ key_id "#{ROOT_PATH}/bin/gitlab-shell #{key_id}" end def self.key_line(key_id, public_key) + public_key.chomp! + raise KeyError.new("Invalid public_key: #{public_key.inspect}") if public_key.include?("\n") "command=\"#{command(key_id)}\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty #{public_key}" end diff --git a/spec/gitlab_keys_spec.rb b/spec/gitlab_keys_spec.rb index d761cc4..adff6b4 100644 --- a/spec/gitlab_keys_spec.rb +++ b/spec/gitlab_keys_spec.rb @@ -7,6 +7,33 @@ describe GitlabKeys do $logger = double('logger').as_null_object end + describe '.command' do + it 'returns the "command" part of the key line' do + command = "#{ROOT_PATH}/bin/gitlab-shell key-123" + expect(described_class.command('key-123')).to eq(command) + end + + it 'raises KeyError on invalid input' do + expect { described_class.command("\nssh-rsa AAA") }.to raise_error(described_class::KeyError) + end + end + + describe '.key_line' do + let(:line) { %(command="#{ROOT_PATH}/bin/gitlab-shell key-741",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAAB3NzaDAxx2E) } + + it 'returns the key line' do + expect(described_class.key_line('key-741', 'ssh-rsa AAAAB3NzaDAxx2E')).to eq(line) + end + + it 'silently removes a trailing newline' do + expect(described_class.key_line('key-741', "ssh-rsa AAAAB3NzaDAxx2E\n")).to eq(line) + end + + it 'raises KeyError on invalid input' do + expect { described_class.key_line('key-741', "ssh-rsa AAA\nssh-rsa AAA") }.to raise_error(described_class::KeyError) + end + end + describe :initialize do let(:gitlab_keys) { build_gitlab_keys('add-key', 'key-741', 'ssh-rsa AAAAB3NzaDAxx2E') } |