Merge branch '4090-authorized-keys-check' into 'master'

Added Authorized Keys specific checks for Geo

Closes #4090

See merge request gitlab-org/gitlab-ee!3728

changelogs/unreleased-ee/4090-authorized-keys-check.yml

+---
+title: 'Geo: Added Authorized Keys specific checks'
+merge_request: 3728
+author:
+type: added

ee/lib/system_check/geo/authorized_keys_check.rb

+module SystemCheck
+  module Geo
+    class AuthorizedKeysCheck < ::SystemCheck::BaseCheck
+      set_name 'OpenSSH configured to use AuthorizedKeysCommand'
+
+      AUTHORIZED_KEYS_DOCS = 'doc/administration/operations/speed_up_ssh.md'.freeze
+      OPENSSH_AUTHORIZED_KEYS_CMD_REGEXP = %r{
+        ^AuthorizedKeysCommand # line starts with
+        \s+                    # one space or more
+        (?<quote>['"]?)        # detect optional quotes
+        (?<content>[^#'"]+)    # content should be at least 1 char, non quotes or start-comment symbol
+        \k<quote>              # boundary for command, backtracks the same detected quote, or none
+        \s*                    # optional any amount of space character
+        (?:\#.*)?$             # optional start-comment symbol followed by optionally any character until end of line
+      }x
+      OPENSSH_AUTHORIZED_KEYS_USER_REGEXP = %r{
+        ^AuthorizedKeysCommandUser # line starts with
+        \s+                        # one space or more
+        (?<quote>['"]?)            # detect optional quotes
+        (?<content>[^#'"]+)        # content should be at least 1 char, non quotes or start-comment symbol
+        \k<quote>                  # boundary for command, backtracks the same detected quote, or none
+        \s*                        # optional any amount of space character
+        (?:\#.*)?$                 # optional start-comment symbol followed by optionally any character until end of line
+      }x
+      OPENSSH_EXPECTED_COMMAND = '/opt/gitlab-shell/authorized_keys %u %k'.freeze
+
+      def multi_check
+        unless openssh_config_exists?
+          print_failure("Cannot find OpenSSH configuration file at: #{openssh_config_path}")
+
+          if in_docker?
+            try_fixing_it(
+              'If you are not using our official docker containers,',
+              'make sure you have OpenSSH server installed and configured correctly on this system'
+            )
+
+            for_more_information(AUTHORIZED_KEYS_DOCS)
+          else
+            try_fixing_it(
+              'Make sure you have OpenSSH server installed on this system'
+            )
+          end
+
+          return
+        end
+
+        unless openssh_config_readable?
+          print_skipped('Cannot access OpenSSH configuration file')
+
+          try_fixing_it(
+            'This is expected if you are using SELinux. You may want to check configuration manually'
+          )
+
+          for_more_information(AUTHORIZED_KEYS_DOCS)
+          return
+        end
+
+        authorized_keys_command = extract_authorized_keys_command
+        unless authorized_keys_command
+          print_failure('OpenSSH configuration file does not contain a AuthorizedKeysCommand')
+
+          try_fixing_it(
+            'Change your OpenSSH configuration file pointing to the correct command'
+          )
+
+          for_more_information(AUTHORIZED_KEYS_DOCS)
+          return
+        end
+
+        unless openssh_is_expected_command?(authorized_keys_command)
+          print_warning('OpenSSH configuration file points to a different AuthorizedKeysCommand')
+
+          try_fixing_it(
+            "We were expecting AuthorizedKeysCommand to be: #{OPENSSH_EXPECTED_COMMAND}",
+            "but instead it is: #{authorized_keys_command}",
+            'If you made a custom command, make sure it behaves according to GitLab\'s Documentation'
+          )
+
+          for_more_information(AUTHORIZED_KEYS_DOCS)
+          # this check should not block the others
+        end
+
+        authorized_keys_command_path = openssh_extract_command_path(authorized_keys_command)
+        unless File.file?(authorized_keys_command_path)
+          print_failure("Cannot find configured AuthorizedKeysCommand: #{authorized_keys_command_path}")
+
+          try_fixing_it(
+            'You need to create the file and add the correct content to it'
+          )
+
+          for_more_information(AUTHORIZED_KEYS_DOCS)
+          return
+        end
+
+        authorized_keys_command_user = extract_authorized_keys_command_user
+        unless authorized_keys_command_user
+          print_failure('OpenSSH configuration file does not contain a AuthorizedKeysCommandUser')
+
+          try_fixing_it(
+            'Change your OpenSSH configuration file pointing to the correct user'
+          )
+
+          for_more_information(AUTHORIZED_KEYS_DOCS)
+          return
+        end
+
+        unless authorized_keys_command_user == gitlab_user
+          print_warning('OpenSSH configuration file points to a different AuthorizedKeysCommandUser')
+
+          try_fixing_it(
+            "We were expecting AuthorizedKeysCommandUser to be: #{gitlab_user}",
+            "but instead it is: #{authorized_keys_command_user}",
+            'Fix your OpenSSH configuration file pointing to the correct user'
+          )
+
+          for_more_information(AUTHORIZED_KEYS_DOCS)
+          return
+        end
+
+        $stdout.puts 'yes'.color(:green)
+        true
+      end
+
+      def extract_authorized_keys_command
+        extract_openssh_config(OPENSSH_AUTHORIZED_KEYS_CMD_REGEXP)
+      end
+
+      def extract_authorized_keys_command_user
+        extract_openssh_config(OPENSSH_AUTHORIZED_KEYS_USER_REGEXP)
+      end
+
+      def openssh_config_path
+        @openssh_config_path ||= begin
+          if in_docker?
+            '/assets/sshd_config' # path in our official docker containers
+          else
+            '/etc/ssh/sshd_config'
+          end
+        end
+      end
+
+      private
+
+      def print_skipped(reason)
+        $stdout.puts 'skipped'.color(:magenta)
+
+        $stdout.puts '  Reason:'.color(:blue)
+        $stdout.puts "  #{reason}"
+      end
+
+      def print_warning(reason)
+        $stdout.puts 'warning'.color(:magenta)
+
+        $stdout.puts '  Reason:'.color(:blue)
+        $stdout.puts "  #{reason}"
+      end
+
+      def print_failure(reason)
+        $stdout.puts 'no'.color(:red)
+
+        $stdout.puts '  Reason:'.color(:blue)
+        $stdout.puts "  #{reason}"
+      end
+
+      def openssh_config_exists?
+        File.file?(openssh_config_path)
+      end
+
+      def openssh_config_readable?
+        File.readable?(openssh_config_path)
+      end
+
+      def openssh_extract_command_path(cmd_with_params)
+        cmd_with_params.split(' ').first
+      end
+
+      def openssh_is_expected_command?(authorized_keys_command)
+        authorized_keys_command.squeeze(' ') == OPENSSH_EXPECTED_COMMAND
+      end
+
+      def in_docker?
+        File.file?('/.dockerenv')
+      end
+
+      def gitlab_user
+        Gitlab.config.gitlab.user
+      end
+
+      def extract_openssh_config(regexp)
+        return false unless openssh_config_exists? && openssh_config_readable?
+
+        File.open(openssh_config_path) do |f|
+          f.each_line do |line|
+            if (match = line.match(regexp))
+              raw_content = match[:content]
+              # remove linebreak, and lead and trailing spaces
+              return raw_content.chomp.strip
+            end
+          end
+        end
+
+        nil
+      end
+    end
+  end
+end

ee/lib/system_check/geo/authorized_keys_flag_check.rb

+module SystemCheck
+  module Geo
+    class AuthorizedKeysFlagCheck < ::SystemCheck::BaseCheck
+      set_name 'GitLab configured to disable writing to authorized_keys file'
+
+      def check?
+        !Gitlab::CurrentSettings.current_application_settings.authorized_keys_enabled
+      end
+
+      def show_error
+        try_fixing_it(
+          "You need to disable `Write to authorized_keys file` in GitLab's Admin panel"
+        )
+
+        for_more_information(AUTHORIZED_KEYS_DOCS)
+      end
+    end
+  end
+end

lib/tasks/gitlab/check.rake

@@ -464,7 +464,9 @@ namespace :gitlab do
         SystemCheck::Geo::HttpConnectionCheck,
         SystemCheck::Geo::HTTPCloneEnabledCheck,
         SystemCheck::Geo::ClocksSynchronizationCheck,
-        SystemCheck::App::GitUserDefaultSSHConfigCheck
+        SystemCheck::App::GitUserDefaultSSHConfigCheck,
+        SystemCheck::Geo::AuthorizedKeysCheck,
+        SystemCheck::Geo::AuthorizedKeysFlagCheck
       ]
 
       SystemCheck.run('Geo', checks)

spec/ee/fixtures/system_check/sshd_config

+# Package generated configuration file
+# See the sshd_config(5) manpage for details
+
+# What ports, IPs and protocols we listen for
+Port 22
+# Use these options to restrict which interfaces/protocols sshd will bind to
+#ListenAddress ::
+#ListenAddress 0.0.0.0
+Protocol 2
+# HostKeys for protocol version 2
+HostKey /etc/ssh/ssh_host_rsa_key
+HostKey /etc/ssh/ssh_host_dsa_key
+HostKey /etc/ssh/ssh_host_ecdsa_key
+HostKey /etc/ssh/ssh_host_ed25519_key
+#Privilege Separation is turned on for security
+UsePrivilegeSeparation yes
+
+# Lifetime and size of ephemeral version 1 server key
+KeyRegenerationInterval 3600
+ServerKeyBits 1024
+
+# Logging
+SyslogFacility AUTH
+LogLevel INFO
+
+# Authentication:
+LoginGraceTime 120
+PermitRootLogin yes
+StrictModes yes
+
+RSAAuthentication yes
+PubkeyAuthentication yes
+#AuthorizedKeysFile	%h/.ssh/authorized_keys
+#AuthorizedKeysCommand /opt/gitlab-shell/invalid_authorized_keys %u %k
+AuthorizedKeysCommand /opt/gitlab-shell/authorized_keys %u %k
+AuthorizedKeysCommandUser git
+
+# Don't read the user's ~/.rhosts and ~/.shosts files
+IgnoreRhosts yes
+# For this to work you will also need host keys in /etc/ssh_known_hosts
+RhostsRSAAuthentication no
+# similar for protocol version 2
+HostbasedAuthentication no
+# Uncomment if you don't trust ~/.ssh/known_hosts for RhostsRSAAuthentication
+#IgnoreUserKnownHosts yes
+
+# To enable empty passwords, change to yes (NOT RECOMMENDED)
+PermitEmptyPasswords no
+
+# Change to yes to enable challenge-response passwords (beware issues with
+# some PAM modules and threads)
+ChallengeResponseAuthentication no
+
+# Change to no to disable tunnelled clear text passwords
+PasswordAuthentication yes
+
+# Kerberos options
+#KerberosAuthentication no
+#KerberosGetAFSToken no
+#KerberosOrLocalPasswd yes
+#KerberosTicketCleanup yes
+
+# GSSAPI options
+#GSSAPIAuthentication no
+#GSSAPICleanupCredentials yes
+
+X11Forwarding yes
+X11DisplayOffset 10
+PrintMotd no
+PrintLastLog yes
+TCPKeepAlive yes
+#UseLogin no
+
+#MaxStartups 10:30:60
+#Banner /etc/issue.net
+
+# Allow client to pass locale environment variables
+AcceptEnv LANG LC_*
+
+Subsystem sftp /usr/lib/openssh/sftp-server
+
+# Set this to 'yes' to enable PAM authentication, account processing,
+# and session processing. If this is enabled, PAM authentication will
+# be allowed through the ChallengeResponseAuthentication and
+# PasswordAuthentication.  Depending on your PAM configuration,
+# PAM authentication via ChallengeResponseAuthentication may bypass
+# the setting of "PermitRootLogin yes
+# If you just want the PAM account and session checks to run without
+# PAM authentication, then enable this but set PasswordAuthentication
+# and ChallengeResponseAuthentication to 'no'.
+UsePAM yes

spec/ee/fixtures/system_check/sshd_config_invalid_command

+# Package generated configuration file
+# See the sshd_config(5) manpage for details
+
+RSAAuthentication yes
+PubkeyAuthentication yes
+#AuthorizedKeysFile	%h/.ssh/authorized_keys
+AuthorizedKeysCommand "/opt/gitlab-shell/invalid_authorized_keys   %u      %k"         # comment
+#AuthorizedKeysCommand /opt/gitlab-shell/authorized_keys %u %k
+AuthorizedKeysCommandUser anotheruser  #comment with more stuff#

spec/ee/fixtures/system_check/sshd_config_invalid_user

+# Package generated configuration file
+# See the sshd_config(5) manpage for details
+
+RSAAuthentication yes
+PubkeyAuthentication yes
+#AuthorizedKeysFile	%h/.ssh/authorized_keys
+AuthorizedKeysCommand /opt/gitlab-shell/authorized_keys %u %k          # comment
+AuthorizedKeysCommandUser anotheruser  #comment with more stuff#

spec/ee/fixtures/system_check/sshd_config_no_command

+# Package generated configuration file
+# See the sshd_config(5) manpage for details
+
+RSAAuthentication yes
+PubkeyAuthentication yes
+# AuthorizedKeysFile	%h/.ssh/authorized_keys
+# AuthorizedKeysCommand /opt/gitlab-shell/invalid_authorized_keys %u %k
+# AuthorizedKeysCommand /opt/gitlab-shell/authorized_keys %u %k
+# AuthorizedKeysCommandUser git

spec/ee/fixtures/system_check/sshd_config_no_user

+# Package generated configuration file
+# See the sshd_config(5) manpage for details
+
+RSAAuthentication yes
+PubkeyAuthentication yes
+#AuthorizedKeysFile	%h/.ssh/authorized_keys
+#AuthorizedKeysCommand /opt/gitlab-shell/invalid_authorized_keys %u %k
+AuthorizedKeysCommand /opt/gitlab-shell/authorized_keys %u %k
+#AuthorizedKeysCommandUser git

spec/ee/spec/lib/system_check/geo/authorized_keys_check_spec.rb

+require 'spec_helper'
+require 'rake_helper'
+
+describe SystemCheck::Geo::AuthorizedKeysCheck do
+  describe '#multi_check' do
+    subject { described_class.new }
+
+    before do
+      allow(File).to receive(:file?).and_call_original # provides a default behavior when mocking
+      allow(File).to receive(:file?).with('/opt/gitlab-shell/authorized_keys') { true }
+    end
+
+    context 'OpenSSH config file' do
+      context 'in docker' do
+        it 'fails when config file does not exist' do
+          allow(subject).to receive(:in_docker?) { true }
+          allow(File).to receive(:file?).with('/assets/sshd_config') { false }
+
+          expect_failure('Cannot find OpenSSH configuration file at: /assets/sshd_config')
+
+          subject.multi_check
+        end
+      end
+
+      it 'fails when config file does not exist' do
+        allow(subject).to receive(:in_docker?) { false }
+        allow(File).to receive(:file?).with('/etc/ssh/sshd_config') { false }
+
+        expect_failure('Cannot find OpenSSH configuration file at: /etc/ssh/sshd_config')
+
+        subject.multi_check
+      end
+
+      it 'skips when config file is not readable' do
+        override_sshd_config('system_check/sshd_config')
+        allow(File).to receive(:readable?).with(expand_fixture_ee_path('system_check/sshd_config')) { false }
+
+        expect_skipped('Cannot access OpenSSH configuration file')
+
+        subject.multi_check
+      end
+    end
+
+    context 'AuthorizedKeysCommand' do
+      it 'fails when config file does not contain the AuthorizedKeysCommand' do
+        override_sshd_config('system_check/sshd_config_no_command')
+
+        expect_failure('OpenSSH configuration file does not contain a AuthorizedKeysCommand')
+
+        subject.multi_check
+      end
+
+      it 'warns when config file does not contain the correct AuthorizedKeysCommand' do
+        override_sshd_config('system_check/sshd_config_invalid_command')
+
+        expect_warning('OpenSSH configuration file points to a different AuthorizedKeysCommand')
+
+        subject.multi_check
+      end
+
+      it 'fails when cannot find referred authorized keys file on disk' do
+        override_sshd_config('system_check/sshd_config')
+        allow(subject).to receive(:extract_authorized_keys_command) { '/tmp/nonexistent/authorized_keys' }
+
+        expect_failure('Cannot find configured AuthorizedKeysCommand: /tmp/nonexistent/authorized_keys')
+
+        subject.multi_check
+      end
+    end
+
+    context 'AuthorizedKeysCommandUser' do
+      it 'fails when config file does not contain the AuthorizedKeysCommandUser' do
+        override_sshd_config('system_check/sshd_config_no_user')
+
+        expect_failure('OpenSSH configuration file does not contain a AuthorizedKeysCommandUser')
+
+        subject.multi_check
+      end
+
+      it 'fails when config file does not contain the correct AuthorizedKeysCommandUser' do
+        override_sshd_config('system_check/sshd_config_invalid_user')
+
+        expect_warning('OpenSSH configuration file points to a different AuthorizedKeysCommandUser')
+
+        subject.multi_check
+      end
+    end
+
+    it 'succeed when all conditions are met' do
+      override_sshd_config('system_check/sshd_config')
+      allow(subject).to receive(:gitlab_user) { 'git' }
+
+      result = subject.multi_check
+      expect($stdout.string).to include('yes')
+      expect(result).to be_truthy
+    end
+  end
+
+  describe '#extract_authorized_keys_command' do
+    it 'returns false when no command is available' do
+      override_sshd_config('system_check/sshd_config_no_command')
+
+      expect(subject.extract_authorized_keys_command).to be_falsey
+    end
+
+    it 'returns correct (uncommented) command' do
+      override_sshd_config('system_check/sshd_config')
+
+      expect(subject.extract_authorized_keys_command).to eq('/opt/gitlab-shell/authorized_keys %u %k')
+    end
+
+    it 'returns command without comments and without quotes' do
+      override_sshd_config('system_check/sshd_config_invalid_command')
+
+      expect(subject.extract_authorized_keys_command).to eq('/opt/gitlab-shell/invalid_authorized_keys   %u      %k')
+    end
+  end
+
+  describe '#extract_authorized_keys_command_user' do
+    it 'returns false when no command user is available' do
+      override_sshd_config('system_check/sshd_config_no_command')
+
+      expect(subject.extract_authorized_keys_command_user).to be_falsey
+    end
+
+    it 'returns correct (uncommented) command' do
+      override_sshd_config('system_check/sshd_config')
+
+      expect(subject.extract_authorized_keys_command_user).to eq('git')
+    end
+
+    it 'returns command without comments' do
+      override_sshd_config('system_check/sshd_config_invalid_command')
+
+      expect(subject.extract_authorized_keys_command_user).to eq('anotheruser')
+    end
+  end
+
+  describe '#openssh_config_path' do
+    context 'when in docker container' do
+      it 'returns /assets/sshd_config' do
+        allow(subject).to receive(:in_docker?) { true }
+
+        expect(subject.openssh_config_path).to eq('/assets/sshd_config')
+      end
+    end
+
+    context 'when not in docker container' do
+      it 'returns /etc/ssh/sshd_config' do
+        allow(subject).to receive(:in_docker?) { false }
+
+        expect(subject.openssh_config_path).to eq('/etc/ssh/sshd_config')
+      end
+    end
+  end
+
+  def expect_failure(reason)
+    expect(subject).to receive(:print_failure).with(reason)
+  end
+
+  def expect_warning(reason)
+    expect(subject).to receive(:print_warning).with(reason)
+  end
+
+  def expect_skipped(reason)
+    expect(subject).to receive(:print_skipped).with(reason)
+  end
+
+  def override_sshd_config(relative_path)
+    allow(subject).to receive(:openssh_config_path) { expand_fixture_ee_path(relative_path) }
+  end
+end

spec/ee/spec/lib/system_check/geo/authorized_keys_flag_check_spec.rb

+require 'spec_helper'
+require 'rake_helper'
+
+describe SystemCheck::Geo::AuthorizedKeysFlagCheck do
+  before do
+    silence_output
+  end
+
+  describe '#check?' do
+    it 'fails when write to authorized_keys still enabled' do
+      stub_application_setting(authorized_keys_enabled: true)
+
+      expect(subject.check?).to be_falsey
+    end
+
+    it 'succeed when write to authorized_keys is disabled' do
+      stub_application_setting(authorized_keys_enabled: false)
+
+      expect(subject.check?).to be_truthy
+    end
+  end
+end

spec/support/fixture_helpers.rb

@@ -5,9 +5,19 @@ module FixtureHelpers
     File.read(expand_fixture_path(filename))
   end
 
+  def fixture_file_ee(filename)
+    return '' if filename.blank?
+
+    File.read(expand_fixture_ee_path(filename))
+  end
+
   def expand_fixture_path(filename)
     File.expand_path(Rails.root.join('spec/fixtures/', filename))
   end
+
+  def expand_fixture_ee_path(filename)
+    File.expand_path(Rails.root.join('spec/ee/fixtures/', filename))
+  end
 end
 
 RSpec.configure do |config|