Add system check for authorized_keys file perm

This check is being removed from gitlab-shell as the file
is now being managed by gitlab-rails.

5 files changed, 138 insertions, 7 deletions

diff --git a/lib/gitlab/authorized_keys.rb b/lib/gitlab/authorized_keys.rb
index 3fe72f5fd43..ca9b65b7c44 100644
--- a/lib/gitlab/authorized_keys.rb
+++ b/lib/gitlab/authorized_keys.rb
@@ -13,6 +13,15 @@ module Gitlab
       @logger = logger
     end
 
+    # Checks if the file is accessible or not
+    #
+    # @return [Boolean]
+    def accessible?
+      open_authorized_keys_file('r') { true }
+    rescue Errno::ENOENT, Errno::EACCES
+      false
+    end
+
     # Add id and its key to the authorized_keys file
     #
     # @param [String] id identifier of key prefixed by `key-`
@@ -102,10 +111,14 @@ module Gitlab
       []
     end
 
+    def file
+      @file ||= Gitlab.config.gitlab_shell.authorized_keys_file
+    end
+
     private
 
     def lock(timeout = 10)
-      File.open("#{authorized_keys_file}.lock", "w+") do |f|
+      File.open("#{file}.lock", "w+") do |f|
         f.flock File::LOCK_EX
         Timeout.timeout(timeout) { yield }
       ensure
@@ -114,7 +127,7 @@ module Gitlab
     end
 
     def open_authorized_keys_file(mode)
-      File.open(authorized_keys_file, mode, 0o600) do |file|
+      File.open(file, mode, 0o600) do |file|
         file.chmod(0o600)
         yield file
       end
@@ -141,9 +154,5 @@ module Gitlab
     def strip(key)
       key.split(/[ ]+/)[0, 2].join(' ')
     end
-
-    def authorized_keys_file
-      Gitlab.config.gitlab_shell.authorized_keys_file
-    end
   end
 end
diff --git a/lib/system_check/app/authorized_keys_permission_check.rb b/lib/system_check/app/authorized_keys_permission_check.rb
new file mode 100644
index 00000000000..1c581f88abc
--- /dev/null
+++ b/lib/system_check/app/authorized_keys_permission_check.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module SystemCheck
+  module App
+    class AuthorizedKeysPermissionCheck < SystemCheck::BaseCheck
+      set_name 'Is authorized keys file accessible?'
+      set_skip_reason 'skipped (authorized keys not enabled)'
+
+      def skip?
+        !authorized_keys_enabled?
+      end
+
+      def check?
+        authorized_keys.accessible?
+      end
+
+      def show_error
+        try_fixing_it([
+          "sudo chmod 700 #{File.dirname(authorized_keys.file)}",
+          "touch #{authorized_keys.file}",
+          "sudo chmod 600 #{authorized_keys.file}"
+        ])
+        fix_and_rerun
+      end
+
+      private
+
+      def authorized_keys_enabled?
+        Gitlab::CurrentSettings.current_application_settings.authorized_keys_enabled
+      end
+
+      def authorized_keys
+        @authorized_keys ||= Gitlab::AuthorizedKeys.new
+      end
+    end
+  end
+end
diff --git a/lib/system_check/rake_task/app_task.rb b/lib/system_check/rake_task/app_task.rb
index cc32feb8604..e98cee510ff 100644
--- a/lib/system_check/rake_task/app_task.rb
+++ b/lib/system_check/rake_task/app_task.rb
@@ -30,7 +30,8 @@ module SystemCheck
           SystemCheck::App::RubyVersionCheck,
           SystemCheck::App::GitVersionCheck,
           SystemCheck::App::GitUserDefaultSSHConfigCheck,
-          SystemCheck::App::ActiveUsersCheck
+          SystemCheck::App::ActiveUsersCheck,
+          SystemCheck::App::AuthorizedKeysPermissionCheck
         ]
       end
     end
diff --git a/spec/lib/gitlab/authorized_keys_spec.rb b/spec/lib/gitlab/authorized_keys_spec.rb
index 42bc509eeef..85d1cc3aaa3 100644
--- a/spec/lib/gitlab/authorized_keys_spec.rb
+++ b/spec/lib/gitlab/authorized_keys_spec.rb
@@ -7,6 +7,40 @@ describe Gitlab::AuthorizedKeys do
 
   subject { described_class.new(logger) }
 
+  describe '#accessible?' do
+    context 'authorized_keys file exists' do
+      before do
+        create_authorized_keys_fixture
+      end
+
+      after do
+        delete_authorized_keys_file
+      end
+
+      context 'can open file' do
+        it 'returns true' do
+          expect(subject.accessible?).to eq(true)
+        end
+      end
+
+      context 'cannot open file' do
+        before do
+          allow(File).to receive(:open).and_raise(Errno::EACCES)
+        end
+
+        it 'returns false' do
+          expect(subject.accessible?).to eq(false)
+        end
+      end
+    end
+
+    context 'authorized_keys file does not exist' do
+      it 'returns false' do
+        expect(subject.accessible?).to eq(false)
+      end
+    end
+  end
+
   describe '#add_key' do
     context 'authorized_keys file exists' do
       before do
diff --git a/spec/lib/system_check/app/authorized_keys_permission_check_spec.rb b/spec/lib/system_check/app/authorized_keys_permission_check_spec.rb
new file mode 100644
index 00000000000..0aa3539e2bd
--- /dev/null
+++ b/spec/lib/system_check/app/authorized_keys_permission_check_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe SystemCheck::App::AuthorizedKeysPermissionCheck do
+  subject { described_class.new }
+
+  describe '#skip?' do
+    context 'authorized keys enabled' do
+      it 'returns false' do
+        expect(subject.skip?).to eq(false)
+      end
+    end
+
+    context 'authorized keys not enabled' do
+      before do
+        stub_application_setting(authorized_keys_enabled: false)
+      end
+
+      it 'returns true' do
+        expect(subject.skip?).to eq(true)
+      end
+    end
+  end
+
+  describe '#check?' do
+    let(:authorized_keys) { double }
+
+    before do
+      allow(Gitlab::AuthorizedKeys).to receive(:new).and_return(authorized_keys)
+      allow(authorized_keys).to receive(:accessible?).and_return(accessible?)
+    end
+
+    context 'authorized keys is accessible' do
+      let(:accessible?) { true }
+
+      it 'returns true' do
+        expect(subject.check?).to eq(true)
+      end
+    end
+
+    context 'authorized keys is not accessible' do
+      let(:accessible?) { false }
+
+      it 'returns false' do
+        expect(subject.check?).to eq(false)
+      end
+    end
+  end
+end