Introduce a more-complete implementation of bin/authorized_keys

bin/authorized_keys doesn't check that the requesting user matches the expected
user, so to enable database authorized keys lookups, we currently ask the admin
to create a custom script for that purpose.

Better is to have a complete script that can perform the whole task. This commit
introduces bin/gitlab-shell-authorized-keys-check which does so.

5 files changed, 197 insertions, 39 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ad511bd..c04e7f8 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -10,7 +10,7 @@ before_script:
 
 rspec:
   script:
-    - bundle exec rspec spec
+    - bundle exec rspec -f d spec
   tags:
     - ruby
   except:
@@ -28,7 +28,7 @@ rubocop:
 rspec:ruby2.2:
   image: ruby:2.2
   script:
-    - bundle exec rspec spec
+    - bundle exec rspec -f d spec
   tags:
     - ruby
   except:
@@ -38,7 +38,7 @@ rspec:ruby2.2:
 rspec:ruby2.1:
   image: ruby:2.1
   script:
-    - bundle exec rspec spec
+    - bundle exec rspec -f d spec
   tags:
     - ruby
   except:
diff --git a/bin/gitlab-shell-authorized-keys-check b/bin/gitlab-shell-authorized-keys-check
new file mode 100755
index 0000000..2ea1a74
--- /dev/null
+++ b/bin/gitlab-shell-authorized-keys-check
@@ -0,0 +1,42 @@
+#!/usr/bin/env ruby
+
+#
+# GitLab shell authorized_keys helper. Query GitLab API to get the authorized
+# command for a given ssh key fingerprint
+#
+# Ex.
+#   bin/gitlab-shell-authorized-keys-check <username> <public-key>
+#
+# Returns
+#   command="/bin/gitlab-shell key-#",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAA...
+#
+# Expects to be called by the SSH daemon, via configuration like:
+#     AuthorizedKeysCommandUser git
+#     AuthorizedKeysCommand /bin/gitlab-shell-authorized-keys-check git %u %k
+
+abort "# Wrong number of arguments. #{ARGV.size}. Usage:
+#     gitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>" unless ARGV.size == 3
+
+expected_username = ARGV[0]
+abort '# No username provided' if expected_username.nil? || expected_username == ''
+
+actual_username = ARGV[1]
+abort '# No username provided' if actual_username.nil? || actual_username == ''
+
+# Only check access if the requested username matches the configured username.
+# Normally, these would both be 'git', but it can be configured by the user
+exit 0 unless expected_username == actual_username
+
+key = ARGV[2]
+abort "# No key provided" if key.nil? || key == ''
+
+require_relative '../lib/gitlab_init'
+require_relative '../lib/gitlab_net'
+require_relative '../lib/gitlab_keys'
+
+authorized_key = GitlabNet.new.authorized_key(key)
+if authorized_key.nil?
+  puts "# No key was found for #{key}"
+else
+  puts GitlabKeys.key_line("key-#{authorized_key['id']}", authorized_key['key'])
+end
diff --git a/spec/gitlab_shell_authorized_keys_check_spec.rb b/spec/gitlab_shell_authorized_keys_check_spec.rb
new file mode 100644
index 0000000..166e675
--- /dev/null
+++ b/spec/gitlab_shell_authorized_keys_check_spec.rb
@@ -0,0 +1,106 @@
+require_relative 'spec_helper'
+
+describe 'bin/gitlab-shell-authorized-keys-check' do
+  def config_path
+    File.join(ROOT_PATH, 'config.yml')
+  end
+
+  def tmp_config_path
+    config_path + ".#{$$}"
+  end
+
+  def tmp_socket_path
+    File.join(ROOT_PATH, 'tmp', 'gitlab-shell-authorized-keys-check-socket')
+  end
+
+  before(:all) do
+    FileUtils.mkdir_p(File.dirname(tmp_socket_path))
+    FileUtils.touch(File.join(ROOT_PATH, '.gitlab_shell_secret'))
+
+    @server = HTTPUNIXServer.new(BindAddress: tmp_socket_path)
+    @server.mount_proc('/api/v4/internal/authorized_keys') do |req, res|
+      if req.query['key'] == 'known-rsa-key'
+        res.status = 200
+        res.content_type = 'application/json'
+        res.body = '{"key":"known-rsa-key", "id": 1}'
+      else
+        res.status = 404
+      end
+    end
+
+    @webrick_thread = Thread.new { @server.start }
+
+    sleep(0.1) while @webrick_thread.alive? && @server.status != :Running
+    raise "Couldn't start stub GitlabNet server" unless @server.status == :Running
+
+    FileUtils.mv(config_path, tmp_config_path) if File.exist?(config_path)
+    File.open(config_path, 'w') do |f|
+      f.write("---\ngitlab_url: http+unix://#{CGI.escape(tmp_socket_path)}\n")
+    end
+  end
+
+  after(:all) do
+    @server.shutdown if @server
+    @webrick_thread.join if @webrick_thread
+    FileUtils.rm_f(config_path)
+    FileUtils.mv(tmp_config_path, config_path) if File.exist?(tmp_config_path)
+  end
+
+  let(:gitlab_shell_path) { File.join(ROOT_PATH, 'bin', 'gitlab-shell') }
+  let(:authorized_keys_check_path) { File.join(ROOT_PATH, 'bin', 'gitlab-shell-authorized-keys-check') }
+
+  it 'succeeds when a valid key is given' do
+    output, status = run!
+
+    expect(output).to eq("command=\"#{gitlab_shell_path} key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty known-rsa-key\n")
+    expect(status).to be_success
+  end
+
+  it 'returns nothing when an unknown key is given' do
+    output, status = run!(key: 'unknown-key')
+
+    expect(output).to eq("# No key was found for unknown-key\n")
+    expect(status).to be_success
+  end
+
+  it' fails when not enough arguments are given' do
+    output, status = run!(key: nil)
+
+    expect(output).to eq('')
+    expect(status).not_to be_success
+  end
+
+  it' fails when too many arguments are given' do
+    output, status = run!(key: ['a', 'b'])
+
+    expect(output).to eq('')
+    expect(status).not_to be_success
+  end
+
+  it 'skips when run as the wrong user' do
+    output, status = run!(expected_user: 'unknown-user')
+
+    expect(output).to eq('')
+    expect(status).to be_success
+  end
+
+  it 'skips when the wrong users connects' do
+    output, status = run!(actual_user: 'unknown-user')
+
+    expect(output).to eq('')
+    expect(status).to be_success
+  end
+
+  def run!(expected_user: 'git', actual_user: 'git', key: 'known-rsa-key')
+    cmd = [
+      authorized_keys_check_path,
+      expected_user,
+      actual_user,
+      key
+    ].flatten.compact
+
+    output = IO.popen(cmd, &:read)
+
+    [output, $?]
+  end
+end
diff --git a/spec/httpunix_spec.rb b/spec/httpunix_spec.rb
index 77235e8..4c59340 100644
--- a/spec/httpunix_spec.rb
+++ b/spec/httpunix_spec.rb
@@ -1,6 +1,5 @@
 require_relative 'spec_helper'
 require_relative '../lib/httpunix'
-require 'webrick'
 
 describe URI::HTTPUNIX do
   describe :parse do
@@ -14,47 +13,29 @@ describe URI::HTTPUNIX do
   end
 end
 
-
-# like WEBrick::HTTPServer, but listens on UNIX socket
-class HTTPUNIXServer < WEBrick::HTTPServer
-  def listen(address, port)
-    socket = Socket.unix_server_socket(address)
-    socket.autoclose = false
-    server = UNIXServer.for_fd(socket.fileno)
-    socket.close
-    @listeners << server
+describe Net::HTTPUNIX do
+  def tmp_socket_path
+    File.join(ROOT_PATH, 'tmp/test-socket')
   end
 
-  # Workaround:
-  # https://bugs.ruby-lang.org/issues/10956
-  # Affecting Ruby 2.2
-  # Fix for 2.2 is at https://github.com/ruby/ruby/commit/ab0a64e1
-  # However, this doesn't work with 2.1. The following should work for both:
-  def start(&block)
-    @shutdown_pipe = IO.pipe
-    shutdown_pipe = @shutdown_pipe
-    super(&block)
-  end
+  before(:all) do
+    # "hello world" over unix socket server in background thread
+    FileUtils.mkdir_p(File.dirname(tmp_socket_path))
+    @server = HTTPUNIXServer.new(BindAddress: tmp_socket_path)
+    @server.mount_proc '/' do |req, resp|
+      resp.body = "Hello World  (at #{req.path})"
+    end
 
-  def cleanup_shutdown_pipe(shutdown_pipe)
-    @shutdown_pipe = nil
-    return if !shutdown_pipe
-    super(shutdown_pipe)
-  end
-end
+    @webrick_thread = Thread.new { @server.start }
 
-def tmp_socket_path
-  'tmp/test-socket'
-end
+    sleep(0.1) while @webrick_thread.alive? && @server.status != :Running
+    raise "Couldn't start HTTPUNIXServer" unless @server.status == :Running
+  end
 
-describe Net::HTTPUNIX do
-  # "hello world" over unix socket server in background thread
-  FileUtils.mkdir_p(File.dirname(tmp_socket_path))
-  server = HTTPUNIXServer.new(:BindAddress => tmp_socket_path)
-  server.mount_proc '/' do |req, resp|
-    resp.body = "Hello World  (at #{req.path})"
+  after(:all) do
+    @server.shutdown if @server
+    @webrick_thread.join if @webrick_thread
   end
-  Thread.start { server.start }
 
   it "talks via HTTP ok" do
     VCR.turned_off do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 612af76..aeebd98 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -10,9 +10,38 @@ end
 
 require 'vcr'
 require 'webmock'
+require 'webrick'
 
 VCR.configure do |c|
   c.cassette_library_dir = 'spec/vcr_cassettes'
   c.hook_into :webmock
   c.configure_rspec_metadata!
 end
+
+# like WEBrick::HTTPServer, but listens on UNIX socket
+class HTTPUNIXServer < WEBrick::HTTPServer
+  def listen(address, port)
+    socket = Socket.unix_server_socket(address)
+    socket.autoclose = false
+    server = UNIXServer.for_fd(socket.fileno)
+    socket.close
+    @listeners << server
+  end
+
+  # Workaround:
+  # https://bugs.ruby-lang.org/issues/10956
+  # Affecting Ruby 2.2
+  # Fix for 2.2 is at https://github.com/ruby/ruby/commit/ab0a64e1
+  # However, this doesn't work with 2.1. The following should work for both:
+  def start(&block)
+    @shutdown_pipe = IO.pipe
+    shutdown_pipe = @shutdown_pipe
+    super(&block)
+  end
+
+  def cleanup_shutdown_pipe(shutdown_pipe)
+    @shutdown_pipe = nil
+    return if !shutdown_pipe
+    super(shutdown_pipe)
+  end
+end