Backport option to disable writing to `authorized_keys` file

Originally branch 'mk-toggle-writing-to-auth-keys-1631'

See merge request !2004

Squashed commits:
Add authorized_keys_enabled to Application Settings
Ensure default settings are exposed in UI
Without this change, `authorized_keys_enabled` is unchecked when it is nil, even if it should be checked by default.
Add “Speed up SSH operations” documentation
Clarify the reasons for disabling writes
Add "How to go back" section
Tweak copy
Update Application Setting screenshot

11 files changed, 217 insertions, 13 deletions

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 4dfb397e82c..4de808eb71f 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -52,7 +52,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
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index b12ea760668..45f7d29eb05 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -146,6 +146,7 @@ module ApplicationSettingsHelper
       :after_sign_up_text,
       :akismet_api_key,
       :akismet_enabled,
+      :authorized_keys_enabled,
       :auto_devops_enabled,
       :circuitbreaker_access_retries,
       :circuitbreaker_check_interval,
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 253e213af81..8ab338d873d 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -261,6 +261,7 @@ class ApplicationSetting < ActiveRecord::Base
     {
       after_sign_up_text: nil,
       akismet_enabled: false,
+      authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
       container_registry_token_expire_delay: 5,
       default_artifacts_expire_in: '30 days',
       default_branch_protection: Settings.gitlab['default_branch_protection'],
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 3e2dbb07a6c..ba4ca88a8a9 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -775,6 +775,22 @@
           = 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/fast_ssh_key_lookup')
+
+  %fieldset
     %legend User and IP Rate Limits
     .form-group
       .col-sm-offset-2.col-sm-10
diff --git a/db/migrate/20170531180233_add_authorized_keys_enabled_to_application_settings.rb b/db/migrate/20170531180233_add_authorized_keys_enabled_to_application_settings.rb
new file mode 100644
index 00000000000..fdae309946c
--- /dev/null
+++ b/db/migrate/20170531180233_add_authorized_keys_enabled_to_application_settings.rb
@@ -0,0 +1,15 @@
+# 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
diff --git a/db/schema.rb b/db/schema.rb
index 740e80ccfd4..11273d2a82e 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -154,6 +154,7 @@ ActiveRecord::Schema.define(version: 20171230123729) do
     t.integer "gitaly_timeout_default", default: 55, null: false
     t.integer "gitaly_timeout_medium", default: 30, null: false
     t.integer "gitaly_timeout_fast", default: 10, null: false
+    t.boolean "authorized_keys_enabled"
   end
 
   create_table "audit_events", force: :cascade do |t|
diff --git a/doc/administration/operations/img/write_to_authorized_keys_setting.png b/doc/administration/operations/img/write_to_authorized_keys_setting.png
new file mode 100644
index 00000000000..232765f1917
--- /dev/null
+++ b/doc/administration/operations/img/write_to_authorized_keys_setting.png
diff --git a/doc/administration/operations/index.md b/doc/administration/operations/index.md
index 320d71a9527..f96a084c853 100644
--- a/doc/administration/operations/index.md
+++ b/doc/administration/operations/index.md
@@ -13,4 +13,5 @@ by GitLab to another file system or another server.
 that to prioritize important jobs.
 - [Sidekiq MemoryKiller](sidekiq_memory_killer.md): Configure Sidekiq MemoryKiller
 to restart Sidekiq.
-- [Unicorn](unicorn.md): Understand Unicorn and unicorn-worker-killer.
\ No newline at end of file
+- [Unicorn](unicorn.md): Understand Unicorn and unicorn-worker-killer.
+- [Speed up SSH operations](speed_up_ssh.md)
diff --git a/doc/administration/operations/speed_up_ssh.md b/doc/administration/operations/speed_up_ssh.md
new file mode 100644
index 00000000000..9b260beb34f
--- /dev/null
+++ b/doc/administration/operations/speed_up_ssh.md
@@ -0,0 +1,69 @@
+# 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
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index 564047bbd34..20df19c734d 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -183,6 +183,8 @@ module Gitlab
     #   add_key("key-42", "sha-rsa ...")
     #
     def add_key(key_id, key_content)
+      return unless self.authorized_keys_enabled?
+
       gitlab_shell_fast_execute([gitlab_shell_keys_path,
                                  'add-key', key_id, self.class.strip_key(key_content)])
     end
@@ -192,6 +194,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
@@ -203,6 +207,8 @@ module Gitlab
     #   remove_key("key-342", "sha-rsa ...")
     #
     def remove_key(key_id, key_content)
+      return unless self.authorized_keys_enabled?
+
       args = [gitlab_shell_keys_path, 'rm-key', key_id]
       args << key_content if key_content
 
@@ -215,6 +221,8 @@ module Gitlab
     #   remove_all_keys
     #
     def remove_all_keys
+      return unless self.authorized_keys_enabled?
+
       gitlab_shell_fast_execute([gitlab_shell_keys_path, 'clear'])
     end
 
@@ -410,5 +418,9 @@ module Gitlab
       # need to do the same here...
       raise Error, e
     end
+
+    def authorized_keys_enabled?
+      current_application_settings.authorized_keys_enabled
+    end
   end
 end
diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb
index 81d9e6a8f82..6d50f537430 100644
--- a/spec/lib/gitlab/shell_spec.rb
+++ b/spec/lib/gitlab/shell_spec.rb
@@ -51,6 +51,105 @@ describe Gitlab::Shell do
     end
   end
 
+  describe '#add_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, '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
+
+  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
+
   describe Gitlab::Shell::KeyAdder do
     describe '#add_key' do
       it 'removes trailing garbage' do
@@ -96,17 +195,6 @@ describe Gitlab::Shell do
       allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800)
     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_shell).to receive(:gitlab_shell_fast_execute).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 '#add_repository' do
       shared_examples '#add_repository' do
         let(:repository_storage) { 'default' }