Commit 0e33f16b authored by Patrick Bajao's avatar Patrick Bajao

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.
parent 95ffd22f
...@@ -13,6 +13,15 @@ module Gitlab ...@@ -13,6 +13,15 @@ module Gitlab
@logger = logger @logger = logger
end 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 # Add id and its key to the authorized_keys file
# #
# @param [String] id identifier of key prefixed by `key-` # @param [String] id identifier of key prefixed by `key-`
...@@ -102,10 +111,14 @@ module Gitlab ...@@ -102,10 +111,14 @@ module Gitlab
[] []
end end
def file
@file ||= Gitlab.config.gitlab_shell.authorized_keys_file
end
private private
def lock(timeout = 10) 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 f.flock File::LOCK_EX
Timeout.timeout(timeout) { yield } Timeout.timeout(timeout) { yield }
ensure ensure
...@@ -114,7 +127,7 @@ module Gitlab ...@@ -114,7 +127,7 @@ module Gitlab
end end
def open_authorized_keys_file(mode) 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) file.chmod(0o600)
yield file yield file
end end
...@@ -141,9 +154,5 @@ module Gitlab ...@@ -141,9 +154,5 @@ module Gitlab
def strip(key) def strip(key)
key.split(/[ ]+/)[0, 2].join(' ') key.split(/[ ]+/)[0, 2].join(' ')
end end
def authorized_keys_file
Gitlab.config.gitlab_shell.authorized_keys_file
end
end end
end end
# 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
...@@ -30,7 +30,8 @@ module SystemCheck ...@@ -30,7 +30,8 @@ module SystemCheck
SystemCheck::App::RubyVersionCheck, SystemCheck::App::RubyVersionCheck,
SystemCheck::App::GitVersionCheck, SystemCheck::App::GitVersionCheck,
SystemCheck::App::GitUserDefaultSSHConfigCheck, SystemCheck::App::GitUserDefaultSSHConfigCheck,
SystemCheck::App::ActiveUsersCheck SystemCheck::App::ActiveUsersCheck,
SystemCheck::App::AuthorizedKeysPermissionCheck
] ]
end end
end end
......
...@@ -7,6 +7,40 @@ describe Gitlab::AuthorizedKeys do ...@@ -7,6 +7,40 @@ describe Gitlab::AuthorizedKeys do
subject { described_class.new(logger) } 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 describe '#add_key' do
context 'authorized_keys file exists' do context 'authorized_keys file exists' do
before do before do
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment