Merge branch 'mk-toggle-writing-to-auth-keys-1631' into 'master'

Add option to disable writing to `authorized_keys` file

Closes #1631

See merge request !2004

app/controllers/admin/application_settings_controller.rb

@@ -53,7 +53,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   private
 
   def set_application_setting
-    @application_setting = ApplicationSetting.current
+    @application_setting = current_application_settings
   end
 
   def application_setting_params
@@ -177,7 +177,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :check_namespace_plan,
       :mirror_max_delay,
       :mirror_max_capacity,
-      :mirror_capacity_threshold
+      :mirror_capacity_threshold,
+      :authorized_keys_enabled
     ]
   end
 end

app/models/ee/application_setting.rb

@@ -32,6 +32,7 @@ module EE
     module ClassMethods
       def defaults
         super.merge(
+          authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
           elasticsearch_url: ENV['ELASTIC_URL'] || 'http://localhost:9200',
           elasticsearch_aws: false,
           elasticsearch_aws_region: ENV['ELASTIC_REGION'] || 'us-east-1',

app/views/admin/application_settings/_form.html.haml

@@ -637,6 +637,22 @@
           installations. Set to 0 to completely disable polling.
           = link_to icon('question-circle'), help_page_path('administration/polling')
 
+  %fieldset
+    %legend Performance optimization
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :authorized_keys_enabled do
+            = f.check_box :authorized_keys_enabled
+            Write to "authorized_keys" file
+          .help-block
+            By default, we write to the "authorized_keys" file to support Git
+            over SSH without additional configuration. GitLab can be optimized
+            to authenticate SSH keys via the database file. Only uncheck this
+            if you have configured your OpenSSH server to use the
+            AuthorizedKeysCommand. Click on the help icon for more details.
+            = link_to icon('question-circle'), help_page_path('administration/operations/speed_up_ssh', anchor: 'the-solution')
+
   - if Gitlab::Geo.license_allows?
     %fieldset
       %legend GitLab Geo

db/migrate/20170531180233_add_authorized_keys_enabled_to_application_settings.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddAuthorizedKeysEnabledToApplicationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    # allow_null: true because we want to set the default based on if the
+    # instance is configured to use AuthorizedKeysCommand
+    add_column :application_settings, :authorized_keys_enabled, :boolean, allow_null: true
+  end
+end

db/schema.rb

@@ -140,6 +140,7 @@ ActiveRecord::Schema.define(version: 20170602003304) do
     t.integer "mirror_max_delay", default: 5, null: false
     t.integer "mirror_max_capacity", default: 100, null: false
     t.integer "mirror_capacity_threshold", default: 50, null: false
+    t.boolean "authorized_keys_enabled"
   end
 
   create_table "approvals", force: :cascade do |t|

doc/administration/operations.md

@@ -6,3 +6,4 @@
 - [Cleaning up Redis sessions](operations/cleaning_up_redis_sessions.md)
 - [Understanding Unicorn and unicorn-worker-killer](operations/unicorn.md)
 - [Moving repositories to a new location](operations/moving_repositories.md)
+- [Speed up SSH operations](operations/speed_up_ssh.md)

doc/administration/operations/speed_up_ssh.md

+# Speed up SSH operations
+
+## The problem
+
+SSH operations become slow as the number of users grows.
+
+## The reason
+
+OpenSSH searches for a key to authorize a user via a linear search. In the worst case, such as when the user is not authorized to access GitLab, OpenSSH will scan the entire file to search for a key. This can take significant time and disk I/O, which will delay users attempting to push or pull to a repository. Making matters worse, if users add or remove keys frequently, the operating system may not be able to cache the authorized_keys file, which causes the disk to be accessed repeatedly.
+
+## The solution
+
+GitLab Shell provides a way to authorize SSH users via a fast, indexed lookup to the GitLab database. GitLab Shell uses the fingerprint of the SSH key to check whether the user is authorized to access GitLab.
+
+> **Warning:** OpenSSH version 6.9+ is required because `AuthorizedKeysCommand` must be able to accept a fingerprint. These instructions will break installations using older versions of OpenSSH, such as those included with CentOS as of May 2017.
+
+Create this file at `/opt/gitlab-shell/authorized_keys`:
+
+```
+#!/bin/bash
+
+if [[ "$1" == "git" ]]; then
+  /opt/gitlab/embedded/service/gitlab-shell/bin/authorized_keys $2
+fi
+```
+
+Set appropriate ownership and permissions:
+
+```
+sudo chown root:git /opt/gitlab-shell/authorized_keys
+sudo chmod 0650 /opt/gitlab-shell/authorized_keys
+```
+
+Add the following to `/etc/ssh/sshd_config`:
+
+```
+AuthorizedKeysCommand /opt/gitlab-shell/authorized_keys %u %k
+AuthorizedKeysCommandUser git
+```
+
+Reload the sshd service:
+
+```
+sudo service sshd reload
+```
+
+Confirm that SSH is working by removing your user's SSH key in the UI, adding a new one, and attempting to pull a repo.
+
+> **Warning:** Do not disable writes until SSH is confirmed to be working perfectly because the file will quickly become out-of-date.
+
+In the case of lookup failures (which are not uncommon), the `authorized_keys` file will still be scanned. So git SSH performance will still be slow for many users as long as a large file exists.
+
+You can disable any more writes to the `authorized_keys` file by unchecking `Write to "authorized_keys" file` in the Application Settings of your GitLab installation.
+
+![Write to authorized keys setting](img/write_to_authorized_keys_setting.png)
+
+Again, confirm that SSH is working by removing your user's SSH key in the UI, adding a new one, and attempting to pull a repo.
+
+Then you can backup and delete your `authorized_keys` file for best performance.
+
+## How to go back to using the `authorized_keys` file
+
+This is a brief overview. Please refer to the above instructions for more context.
+
+1. Rebuild the `authorized_keys` file. See https://docs.gitlab.com/ce/administration/raketasks/maintenance.html#rebuild-authorized_keys-file
+1. Enable writes to the `authorized_keys` file
+1. Remove the `AuthorizedKeysCommand` lines from `/etc/ssh/sshd_config`
+1. Reload the sshd service
+1. Remove the `/opt/gitlab-shell/authorized_keys` file

lib/gitlab/shell.rb

@@ -197,6 +197,8 @@ module Gitlab
     #   add_key("key-42", "sha-rsa ...")
     #
     def add_key(key_id, key_content)
+      return unless self.authorized_keys_enabled?
+
       Gitlab::Utils.system_silent([gitlab_shell_keys_path,
                                    'add-key', key_id, self.class.strip_key(key_content)])
     end
@@ -206,6 +208,8 @@ module Gitlab
     # Ex.
     #   batch_add_keys { |adder| adder.add_key("key-42", "sha-rsa ...") }
     def batch_add_keys(&block)
+      return unless self.authorized_keys_enabled?
+
       IO.popen(%W(#{gitlab_shell_path}/bin/gitlab-keys batch-add-keys), 'w') do |io|
         yield(KeyAdder.new(io))
       end
@@ -217,6 +221,8 @@ module Gitlab
     #   remove_key("key-342", "sha-rsa ...")
     #
     def remove_key(key_id, key_content)
+      return unless self.authorized_keys_enabled?
+
       Gitlab::Utils.system_silent([gitlab_shell_keys_path,
                                    'rm-key', key_id, key_content])
     end
@@ -227,6 +233,8 @@ module Gitlab
     #   remove_all_keys
     #
     def remove_all_keys
+      return unless self.authorized_keys_enabled?
+
       Gitlab::Utils.system_silent([gitlab_shell_keys_path, 'clear'])
     end
 
@@ -356,5 +364,9 @@ module Gitlab
     def gitlab_shell_keys_path
       File.join(gitlab_shell_path, 'bin', 'gitlab-keys')
     end
+
+    def authorized_keys_enabled?
+      current_application_settings.authorized_keys_enabled
+    end
   end
 end

spec/lib/gitlab/shell_spec.rb

@@ -104,13 +104,101 @@ describe Gitlab::Shell, lib: true do
   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']
-      )
+    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
 
-      gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage')
+  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