diff options
| -rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 2 | ||||
| -rw-r--r-- | app/helpers/application_settings_helper.rb | 1 | ||||
| -rw-r--r-- | app/models/application_setting.rb | 1 | ||||
| -rw-r--r-- | app/views/admin/application_settings/_form.html.haml | 16 | ||||
| -rw-r--r-- | db/migrate/20170531180233_add_authorized_keys_enabled_to_application_settings.rb | 15 | ||||
| -rw-r--r-- | db/schema.rb | 1 | ||||
| -rw-r--r-- | doc/administration/operations/img/write_to_authorized_keys_setting.png | bin | 0 -> 94218 bytes | |||
| -rw-r--r-- | doc/administration/operations/index.md | 3 | ||||
| -rw-r--r-- | doc/administration/operations/speed_up_ssh.md | 69 | ||||
| -rw-r--r-- | lib/gitlab/shell.rb | 12 | ||||
| -rw-r--r-- | spec/lib/gitlab/shell_spec.rb | 110 |
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 Binary files differnew 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. + + + +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' } |
