Commit 4722c45e authored by Robert Speicher's avatar Robert Speicher

Merge branch 'strip-key-comments-for-gitlab-shell' into 'master'

Strip comments before sending keys to gitlab-shell

## Why was this MR needed?

https://gitlab.com/gitlab-org/gitlab-ce/issues/22167 encoding issues in comment text.

## What are the relevant issue numbers?

https://gitlab.com/gitlab-org/gitlab-ce/issues/22167

See merge request !6381
parents 791079e2 67ec96e3
...@@ -58,6 +58,7 @@ v 8.12.0 (unreleased) ...@@ -58,6 +58,7 @@ v 8.12.0 (unreleased)
- Add hover color to emoji icon (ClemMakesApps) - Add hover color to emoji icon (ClemMakesApps)
- Increase ci_builds artifacts_size column to 8-byte integer to allow larger files - Increase ci_builds artifacts_size column to 8-byte integer to allow larger files
- Add textarea autoresize after comment (ClemMakesApps) - Add textarea autoresize after comment (ClemMakesApps)
- Do not write SSH public key 'comments' to authorized_keys !6381
- Refresh todos count cache when an Issue/MR is deleted - Refresh todos count cache when an Issue/MR is deleted
- Fix branches page dropdown sort alignment (ClemMakesApps) - Fix branches page dropdown sort alignment (ClemMakesApps)
- Hides merge request button on branches page is user doesn't have permissions - Hides merge request button on branches page is user doesn't have permissions
......
...@@ -6,7 +6,12 @@ module Gitlab ...@@ -6,7 +6,12 @@ module Gitlab
KeyAdder = Struct.new(:io) do KeyAdder = Struct.new(:io) do
def add_key(id, key) def add_key(id, key)
key.gsub!(/[[:space:]]+/, ' ').strip! key = Gitlab::Shell.strip_key(key)
# Newline and tab are part of the 'protocol' used to transmit id+key to the other end
if key.include?("\t") || key.include?("\n")
raise Error.new("Invalid key: #{key.inspect}")
end
io.puts("#{id}\t#{key}") io.puts("#{id}\t#{key}")
end end
end end
...@@ -16,6 +21,10 @@ module Gitlab ...@@ -16,6 +21,10 @@ module Gitlab
@version_required ||= File.read(Rails.root. @version_required ||= File.read(Rails.root.
join('GITLAB_SHELL_VERSION')).strip join('GITLAB_SHELL_VERSION')).strip
end end
def strip_key(key)
key.split(/ /)[0, 2].join(' ')
end
end end
# Init new repository # Init new repository
...@@ -107,7 +116,7 @@ module Gitlab ...@@ -107,7 +116,7 @@ module Gitlab
# #
def add_key(key_id, key_content) def add_key(key_id, key_content)
Gitlab::Utils.system_silent([gitlab_shell_keys_path, Gitlab::Utils.system_silent([gitlab_shell_keys_path,
'add-key', key_id, key_content]) 'add-key', key_id, self.class.strip_key(key_content)])
end end
# Batch-add keys to authorized_keys # Batch-add keys to authorized_keys
......
require 'spec_helper' require 'spec_helper'
require 'stringio'
describe Gitlab::Shell, lib: true do describe Gitlab::Shell, lib: true do
let(:project) { double('Project', id: 7, path: 'diaspora') } let(:project) { double('Project', id: 7, path: 'diaspora') }
...@@ -44,15 +45,38 @@ describe Gitlab::Shell, lib: true do ...@@ -44,15 +45,38 @@ describe Gitlab::Shell, lib: true do
end end
end 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::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
describe Gitlab::Shell::KeyAdder, lib: true do describe Gitlab::Shell::KeyAdder, lib: true do
describe '#add_key' do describe '#add_key' do
it 'normalizes space characters in the key' do it 'removes trailing garbage' do
io = spy io = spy(:io)
adder = described_class.new(io) adder = described_class.new(io)
adder.add_key('key-42', "sha-rsa foo\tbar\tbaz") adder.add_key('key-42', "ssh-rsa foo bar\tbaz")
expect(io).to have_received(:puts).with("key-42\tssh-rsa foo")
end
it 'raises an exception if the key contains a tab' do
expect do
described_class.new(StringIO.new).add_key('key-42', "ssh-rsa\tfoobar")
end.to raise_error(Gitlab::Shell::Error)
end
expect(io).to have_received(:puts).with("key-42\tsha-rsa foo bar baz") it 'raises an exception if the key contains a newline' do
expect do
described_class.new(StringIO.new).add_key('key-42', "ssh-rsa foobar\nssh-rsa pawned")
end.to raise_error(Gitlab::Shell::Error)
end end
end end
end end
......
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