diff options
author | Nick Thomas <nick@gitlab.com> | 2018-07-31 19:29:45 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-07-31 19:29:45 +0000 |
commit | 768314049c43af40a65c4138dee20441138cc1d0 (patch) | |
tree | 010b07cbb9f4167d730a0e4277573ad787a6ebcd | |
parent | 15fde478ccc6e464e175da7b8d8148a5835d4076 (diff) | |
parent | 2e8b67027067761034f36dadb3c2208ce66d2552 (diff) | |
download | gitlab-shell-768314049c43af40a65c4138dee20441138cc1d0.tar.gz |
Merge branch 'user-argument-2/upstream' into 'master'
Add support for ssh certificates
Closes gitlab-ce#34572
See merge request gitlab-org/gitlab-shell!207
-rwxr-xr-x | bin/gitlab-shell | 6 | ||||
-rwxr-xr-x | bin/gitlab-shell-authorized-principals-check | 36 | ||||
-rw-r--r-- | lib/gitlab_access_status.rb | 6 | ||||
-rw-r--r-- | lib/gitlab_keys.rb | 26 | ||||
-rw-r--r-- | lib/gitlab_net.rb | 62 | ||||
-rw-r--r-- | lib/gitlab_shell.rb | 39 | ||||
-rw-r--r-- | spec/gitlab_access_spec.rb | 2 | ||||
-rw-r--r-- | spec/gitlab_keys_spec.rb | 32 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 38 |
9 files changed, 192 insertions, 55 deletions
diff --git a/bin/gitlab-shell b/bin/gitlab-shell index 6ef572f..1016570 100755 --- a/bin/gitlab-shell +++ b/bin/gitlab-shell @@ -5,19 +5,19 @@ unless ENV['SSH_CONNECTION'] exit end -key_id = /key-[0-9]+/.match(ARGV.join).to_s original_cmd = ENV.delete('SSH_ORIGINAL_COMMAND') require_relative '../lib/gitlab_init' # # -# GitLab shell, invoked from ~/.ssh/authorized_keys +# GitLab shell, invoked from ~/.ssh/authorized_keys or from an +# AuthorizedPrincipalsCommand in the key-less SSH CERT mode. # # require File.join(ROOT_PATH, 'lib', 'gitlab_shell') -if GitlabShell.new(key_id).exec(original_cmd) +if GitlabShell.new(ARGV.join).exec(original_cmd) exit 0 else exit 1 diff --git a/bin/gitlab-shell-authorized-principals-check b/bin/gitlab-shell-authorized-principals-check new file mode 100755 index 0000000..aa6d427 --- /dev/null +++ b/bin/gitlab-shell-authorized-principals-check @@ -0,0 +1,36 @@ +#!/usr/bin/env ruby + +# +# GitLab shell authorized principals helper. Emits the same sort of +# command="..." line as gitlab-shell-authorized-principals-check, with +# the right options. +# +# Ex. +# bin/gitlab-shell-authorized-keys-check <key-id> <principal1> [<principal2>...] +# +# Returns one line per principal passed in, e.g.: +# command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL} +# [command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL2}] +# +# Expects to be called by the SSH daemon, via configuration like: +# AuthorizedPrincipalsCommandUser root +# AuthorizedPrincipalsCommand /bin/gitlab-shell-authorized-principals-check git %i sshUsers + +abort "# Wrong number of arguments. #{ARGV.size}. Usage: +# gitlab-shell-authorized-principals-check <key-id> <principal1> [<principal2>...]" unless ARGV.size >= 2 + +key_id = ARGV[0] +abort '# No key_id provided' if key_id.nil? || key_id == '' + +principals = ARGV[1..-1] +principals.each { |principal| + abort '# An invalid principal was provided' if principal.nil? || principal == '' +} + +require_relative '../lib/gitlab_init' +require_relative '../lib/gitlab_net' +require_relative '../lib/gitlab_keys' + +principals.each { |principal| + puts GitlabKeys.principal_line("username-#{key_id}", principal.dup) +} diff --git a/lib/gitlab_access_status.rb b/lib/gitlab_access_status.rb index 783bc0c..44225aa 100644 --- a/lib/gitlab_access_status.rb +++ b/lib/gitlab_access_status.rb @@ -1,12 +1,13 @@ require 'json' class GitAccessStatus - attr_reader :message, :gl_repository, :gl_username, :repository_path, :gitaly + attr_reader :message, :gl_repository, :gl_id, :gl_username, :repository_path, :gitaly - def initialize(status, message, gl_repository:, gl_username:, repository_path:, gitaly:) + def initialize(status, message, gl_repository:, gl_id:, gl_username:, repository_path:, gitaly:) @status = status @message = message @gl_repository = gl_repository + @gl_id = gl_id @gl_username = gl_username @repository_path = repository_path @gitaly = gitaly @@ -17,6 +18,7 @@ class GitAccessStatus new(values["status"], values["message"], gl_repository: values["gl_repository"], + gl_id: values["gl_id"], gl_username: values["gl_username"], repository_path: values["repository_path"], gitaly: values["gitaly"]) diff --git a/lib/gitlab_keys.rb b/lib/gitlab_keys.rb index 30444c3..3ee2882 100644 --- a/lib/gitlab_keys.rb +++ b/lib/gitlab_keys.rb @@ -9,12 +9,20 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength attr_accessor :auth_file, :key - def self.command(key_id) + def self.command(whatever) + "#{ROOT_PATH}/bin/gitlab-shell #{whatever}" + end + + def self.command_key(key_id) unless /\A[a-z0-9-]+\z/ =~ key_id raise KeyError, "Invalid key_id: #{key_id.inspect}" end - "#{ROOT_PATH}/bin/gitlab-shell #{key_id}" + command(key_id) + end + + def self.whatever_line(command, trailer) + "command=\"#{command}\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty #{trailer}" end def self.key_line(key_id, public_key) @@ -24,7 +32,17 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength raise KeyError, "Invalid public_key: #{public_key.inspect}" end - "command=\"#{command(key_id)}\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty #{public_key}" + whatever_line(command_key(key_id), public_key) + end + + def self.principal_line(username_key_id, principal) + principal.chomp! + + if principal.include?("\n") + raise KeyError, "Invalid principal: #{principal.inspect}" + end + + whatever_line(command_key(username_key_id), principal) end def initialize @@ -119,7 +137,7 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength $logger.info('Removing key', key_id: @key_id) open_auth_file('r+') do |f| while line = f.gets # rubocop:disable Style/AssignmentInCondition - next unless line.start_with?("command=\"#{self.class.command(@key_id)}\"") + next unless line.start_with?("command=\"#{self.class.command_key(@key_id)}\"") f.seek(-line.length, IO::SEEK_CUR) # Overwrite the line with #'s. Because the 'line' variable contains # a terminating '\n', we write line.length - 1 '#' characters. diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 584dd93..7013d79 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -17,7 +17,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength CHECK_TIMEOUT = 5 - def check_access(cmd, gl_repository, repo, actor, changes, protocol, env: {}) + def check_access(cmd, gl_repository, repo, who, changes, protocol, env: {}) changes = changes.join("\n") unless changes.is_a?(String) params = { @@ -29,11 +29,8 @@ class GitlabNet # rubocop:disable Metrics/ClassLength env: env } - if actor =~ /\Akey\-\d+\Z/ - params[:key_id] = actor.gsub("key-", "") - elsif actor =~ /\Auser\-\d+\Z/ - params[:user_id] = actor.gsub("user-", "") - end + who_sym, _, who_v = self.class.parse_who(who) + params[who_sym] = who_v url = "#{internal_api_endpoint}/allowed" resp = post(url, params) @@ -44,23 +41,37 @@ class GitlabNet # rubocop:disable Metrics/ClassLength GitAccessStatus.new(false, 'API is not accessible', gl_repository: nil, + gl_id: nil, gl_username: nil, repository_path: nil, gitaly: nil) end end - def discover(key) - key_id = key.gsub("key-", "") - resp = get("#{internal_api_endpoint}/discover?key_id=#{key_id}") + def discover(who) + _, who_k, who_v = self.class.parse_who(who) + + resp = get("#{internal_api_endpoint}/discover?#{who_k}=#{who_v}") + JSON.parse(resp.body) rescue nil end - def lfs_authenticate(key, repo) - params = { - project: sanitize_path(repo), - key_id: key.gsub('key-', '') - } + def lfs_authenticate(gl_id, repo) + id_sym, _, id = self.class.parse_who(gl_id) + + if id_sym == :key_id + params = { + project: sanitize_path(repo), + key_id: id + } + elsif id_sym == :user_id + params = { + project: sanitize_path(repo), + user_id: id + } + else + raise ArgumentError, "lfs_authenticate() got unsupported GL_ID='#{gl_id}'!" + end resp = post("#{internal_api_endpoint}/lfs_authenticate", params) @@ -101,9 +112,10 @@ class GitlabNet # rubocop:disable Metrics/ClassLength nil end - def two_factor_recovery_codes(key) - key_id = key.gsub('key-', '') - resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", key_id: key_id) + def two_factor_recovery_codes(gl_id) + id_sym, _, id = self.class.parse_who(gl_id) + + resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", id_sym => id) JSON.parse(resp.body) if resp.code == '200' rescue @@ -140,6 +152,22 @@ class GitlabNet # rubocop:disable Metrics/ClassLength JSON.parse(resp.body) if resp.code == '200' end + def self.parse_who(who) + if who.start_with?("key-") + value = who.gsub("key-", "") + raise ArgumentError, "who='#{who}' is invalid!" unless value =~ /\A[0-9]+\z/ + [:key_id, 'key_id', value] + elsif who.start_with?("user-") + value = who.gsub("user-", "") + raise ArgumentError, "who='#{who}' is invalid!" unless value =~ /\A[0-9]+\z/ + [:user_id, 'user_id', value] + elsif who.start_with?("username-") + [:username, 'username', who.gsub("username-", "")] + else + raise ArgumentError, "who='#{who}' is invalid!" + end + end + protected def sanitize_path(repo) diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index 9644cf4..9f05004 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -18,11 +18,16 @@ class GitlabShell # rubocop:disable Metrics/ClassLength API_COMMANDS = %w(2fa_recovery_codes).freeze GL_PROTOCOL = 'ssh'.freeze - attr_accessor :key_id, :gl_repository, :repo_name, :command, :git_access + attr_accessor :gl_id, :gl_repository, :repo_name, :command, :git_access attr_reader :repo_path - def initialize(key_id) - @key_id = key_id + def initialize(who) + who_sym, = GitlabNet.parse_who(who) + if who_sym == :username + @who = who + else + @gl_id = who + end @config = GitlabConfig.new end @@ -40,6 +45,12 @@ class GitlabShell # rubocop:disable Metrics/ClassLength if GIT_COMMANDS.include?(args.first) GitlabMetrics.measure('verify-access') { verify_access } + elsif !defined?(@gl_id) + # We're processing an API command like 2fa_recovery_codes, but + # don't have a @gl_id yet, that means we're in the "username" + # mode and need to materialize it, calling the "user" method + # will do that and call the /discover method. + user end process_cmd(args) @@ -101,7 +112,7 @@ class GitlabShell # rubocop:disable Metrics/ClassLength end def verify_access - status = api.check_access(@git_access, nil, @repo_name, @key_id, '_any', GL_PROTOCOL) + status = api.check_access(@git_access, nil, @repo_name, @who || @gl_id, '_any', GL_PROTOCOL) raise AccessDeniedError, status.message unless status.allowed? @@ -109,6 +120,9 @@ class GitlabShell # rubocop:disable Metrics/ClassLength @gl_repository = status.gl_repository @gitaly = status.gitaly @username = status.gl_username + if defined?(@who) + @gl_id = status.gl_id + end end def process_cmd(args) @@ -135,7 +149,7 @@ class GitlabShell # rubocop:disable Metrics/ClassLength gitaly_request = { 'repository' => @gitaly['repository'], 'gl_repository' => @gl_repository, - 'gl_id' => @key_id, + 'gl_id' => @gl_id, 'gl_username' => @username } @@ -161,7 +175,7 @@ class GitlabShell # rubocop:disable Metrics/ClassLength 'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'LANG' => ENV['LANG'], - 'GL_ID' => @key_id, + 'GL_ID' => @gl_id, 'GL_PROTOCOL' => GL_PROTOCOL, 'GL_REPOSITORY' => @gl_repository, 'GL_USERNAME' => @username @@ -190,7 +204,12 @@ class GitlabShell # rubocop:disable Metrics/ClassLength return @user if defined?(@user) begin - @user = api.discover(@key_id) + if defined?(@who) + @user = api.discover(@who) + @gl_id = "user-#{@user['id']}" + else + @user = api.discover(@gl_id) + end rescue GitlabNet::ApiUnreachableError @user = nil end @@ -208,11 +227,11 @@ class GitlabShell # rubocop:disable Metrics/ClassLength # User identifier to be used in log messages. def log_username - @config.audit_usernames ? username : "user with key #{@key_id}" + @config.audit_usernames ? username : "user with id #{@gl_id}" end def lfs_authenticate - lfs_access = api.lfs_authenticate(@key_id, @repo_name) + lfs_access = api.lfs_authenticate(@gl_id, @repo_name) return unless lfs_access @@ -240,7 +259,7 @@ class GitlabShell # rubocop:disable Metrics/ClassLength return end - resp = api.two_factor_recovery_codes(key_id) + resp = api.two_factor_recovery_codes(@gl_id) if resp['success'] codes = resp['recovery_codes'].join("\n") puts "Your two-factor authentication recovery codes are:\n\n" \ diff --git a/spec/gitlab_access_spec.rb b/spec/gitlab_access_spec.rb index 8882e01..d082176 100644 --- a/spec/gitlab_access_spec.rb +++ b/spec/gitlab_access_spec.rb @@ -10,6 +10,7 @@ describe GitlabAccess do api.stub(check_access: GitAccessStatus.new(true, 'ok', gl_repository: 'project-1', + gl_id: 'user-123', gl_username: 'testuser', repository_path: '/home/git/repositories', gitaly: nil)) @@ -47,6 +48,7 @@ describe GitlabAccess do false, 'denied', gl_repository: nil, + gl_id: nil, gl_username: nil, repository_path: nil, gitaly: nil diff --git a/spec/gitlab_keys_spec.rb b/spec/gitlab_keys_spec.rb index d6583b8..7011ca0 100644 --- a/spec/gitlab_keys_spec.rb +++ b/spec/gitlab_keys_spec.rb @@ -8,13 +8,25 @@ describe GitlabKeys do end describe '.command' do + it 'the internal "command" utility function' do + command = "#{ROOT_PATH}/bin/gitlab-shell does-not-validate" + expect(described_class.command('does-not-validate')).to eq(command) + end + + it 'does not raise a KeyError on invalid input' do + command = "#{ROOT_PATH}/bin/gitlab-shell foo\nbar\nbaz\n" + expect(described_class.command("foo\nbar\nbaz\n")).to eq(command) + end + end + + describe '.command_key' 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) + expect(described_class.command_key('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) + expect { described_class.command_key("\nssh-rsa AAA") }.to raise_error(described_class::KeyError) end end @@ -34,6 +46,22 @@ describe GitlabKeys do end end + describe '.principal_line' do + let(:line) { %(command="#{ROOT_PATH}/bin/gitlab-shell username-someuser",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty sshUsers) } + + it 'returns the key line' do + expect(described_class.principal_line('username-someuser', 'sshUsers')).to eq(line) + end + + it 'silently removes a trailing newline' do + expect(described_class.principal_line('username-someuser', "sshUsers\n")).to eq(line) + end + + it 'raises KeyError on invalid input' do + expect { described_class.principal_line('username-someuser', "sshUsers\nloginUsers") }.to raise_error(described_class::KeyError) + end + end + describe :initialize do let(:gitlab_keys) { build_gitlab_keys('add-key', 'key-741', 'ssh-rsa AAAAB3NzaDAxx2E') } diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index af84b29..3f7c962 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -13,8 +13,8 @@ describe GitlabShell do end subject do - ARGV[0] = key_id - GitlabShell.new(key_id).tap do |shell| + ARGV[0] = gl_id + GitlabShell.new(gl_id).tap do |shell| shell.stub(exec_cmd: :exec_called) shell.stub(api: api) end @@ -24,6 +24,7 @@ describe GitlabShell do true, 'ok', gl_repository: gl_repository, + gl_id: gl_id, gl_username: gl_username, repository_path: repo_path, gitaly: { 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default'} , 'address' => 'unix:gitaly.socket' } @@ -37,6 +38,7 @@ describe GitlabShell do true, 'ok', gl_repository: gl_repository, + gl_id: gl_id, gl_username: gl_username, repository_path: repo_path, gitaly: nil)) @@ -47,13 +49,14 @@ describe GitlabShell do end end - let(:key_id) { "key-#{rand(100) + 100}" } + let(:gl_id) { "key-#{rand(100) + 100}" } let(:ssh_cmd) { nil } let(:tmp_repos_path) { File.join(ROOT_PATH, 'tmp', 'repositories') } let(:repo_name) { 'gitlab-ci.git' } let(:repo_path) { File.join(tmp_repos_path, repo_name) } let(:gl_repository) { 'project-1' } + let(:gl_id) { 'user-1' } let(:gl_username) { 'testuser' } before do @@ -63,7 +66,7 @@ describe GitlabShell do describe :initialize do let(:ssh_cmd) { 'git-receive-pack' } - its(:key_id) { should == key_id } + its(:gl_id) { should == gl_id } end describe :parse_cmd do @@ -146,7 +149,7 @@ describe GitlabShell do end describe :exec do - let(:gitaly_message) { JSON.dump({ 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default' }, 'gl_repository' => gl_repository, 'gl_id' => key_id, 'gl_username' => gl_username}) } + let(:gitaly_message) { JSON.dump({ 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default' }, 'gl_repository' => gl_repository, 'gl_id' => gl_id, 'gl_username' => gl_username}) } shared_examples_for 'upload-pack' do |command| let(:ssh_cmd) { "#{command} gitlab-ci.git" } @@ -162,7 +165,7 @@ describe GitlabShell do it "should log the command execution" do message = "executing git command" - user_string = "user with key #{key_id}" + user_string = "user with id #{gl_id}" $logger.should_receive(:info).with(message, command: "git-upload-pack #{repo_path}", user: user_string) end @@ -197,7 +200,7 @@ describe GitlabShell do it "should log the command execution" do message = "executing git command" - user_string = "user with key #{key_id}" + user_string = "user with id #{gl_id}" $logger.should_receive(:info).with(message, command: "gitaly-upload-pack unix:gitaly.socket #{gitaly_message}", user: user_string) end @@ -221,7 +224,7 @@ describe GitlabShell do it "should log the command execution" do message = "executing git command" - user_string = "user with key #{key_id}" + user_string = "user with id #{gl_id}" $logger.should_receive(:info).with(message, command: "git-receive-pack #{repo_path}", user: user_string) end end @@ -243,7 +246,7 @@ describe GitlabShell do it "should log the command execution" do message = "executing git command" - user_string = "user with key #{key_id}" + user_string = "user with id #{gl_id}" $logger.should_receive(:info).with(message, command: "gitaly-receive-pack unix:gitaly.socket #{gitaly_message}", user: user_string) end @@ -270,7 +273,7 @@ describe GitlabShell do it "should log the command execution" do message = "executing git command" - user_string = "user with key #{key_id}" + user_string = "user with id #{gl_id}" $logger.should_receive(:info).with(message, command: exec_cmd_log_params.join(' '), user: user_string) end @@ -322,7 +325,7 @@ describe GitlabShell do it "should log the attempt" do message = 'Denied disallowed command' - user_string = "user with key #{key_id}" + user_string = "user with id #{gl_id}" $logger.should_receive(:warn).with(message, command: 'arbitrary command', user: user_string) end end @@ -331,7 +334,7 @@ describe GitlabShell do after { subject.exec(nil) } it "should call api.discover" do - api.should_receive(:discover).with(key_id) + api.should_receive(:discover).with(gl_id) end end @@ -398,7 +401,7 @@ describe GitlabShell do after { subject.exec(ssh_cmd) } it "should call api.check_access" do - api.should_receive(:check_access).with('git-upload-pack', nil, 'gitlab-ci.git', key_id, '_any', 'ssh') + api.should_receive(:check_access).with('git-upload-pack', nil, 'gitlab-ci.git', gl_id, '_any', 'ssh') end it "should disallow access and log the attempt if check_access returns false status" do @@ -406,11 +409,12 @@ describe GitlabShell do false, 'denied', gl_repository: nil, + gl_id: nil, gl_username: nil, repository_path: nil, gitaly: nil)) message = 'Access denied' - user_string = "user with key #{key_id}" + user_string = "user with id #{gl_id}" $logger.should_receive(:warn).with(message, command: 'git-upload-pack gitlab-ci.git', user: user_string) end end @@ -436,14 +440,14 @@ describe GitlabShell do end describe :exec_cmd do - let(:shell) { GitlabShell.new(key_id) } + let(:shell) { GitlabShell.new(gl_id) } let(:env) do { 'HOME' => ENV['HOME'], 'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'LANG' => ENV['LANG'], - 'GL_ID' => key_id, + 'GL_ID' => gl_id, 'GL_PROTOCOL' => 'ssh', 'GL_REPOSITORY' => gl_repository, 'GL_USERNAME' => 'testuser' @@ -543,7 +547,7 @@ describe GitlabShell do end describe :api do - let(:shell) { GitlabShell.new(key_id) } + let(:shell) { GitlabShell.new(gl_id) } subject { shell.send :api } it { should be_a(GitlabNet) } |