Commit 48115be5 authored by Nick Thomas's avatar Nick Thomas 🔴

Add a system check for the git user's custom SSH configuration

parent 25a443d6
Pipeline #11436033 failed with stages
in 130 minutes and 25 seconds
---
title: Deprecate custom SSH client configuration for the git user
merge_request: 13930
author:
type: deprecated
......@@ -193,6 +193,38 @@ How to add your SSH key to Eclipse: https://wiki.eclipse.org/EGit/User_Guide#Ecl
[winputty]: https://the.earth.li/~sgtatham/putty/0.67/htmldoc/Chapter8.html#pubkey-puttygen
## SSH on the GitLab server
GitLab integrates with the system-installed SSH daemon, designating a user
(typically named `git`) through which all access requests are handled. Users
connecting to the GitLab server over SSH are identified by their SSH key instead
of their username.
SSH *client* operations performed on the GitLab server wil be executed as this
user. Although it is possible to modify the SSH configuration for this user to,
e.g., provide a private SSH key to authenticate these requests by, this practice
is **not supported** and is strongly discouraged as it presents significant
security risks.
The GitLab check process includes a check for this condition, and will direct you
to this section if your server is configured like this, e.g.:
```
$ gitlab-rake gitlab:check
# ...
Git user has default SSH configuration? ... no
Try fixing it:
mkdir ~/gitlab-check-backup-1504540051
sudo mv /var/lib/git/.ssh/id_rsa ~/gitlab-check-backup-1504540051
sudo mv /var/lib/git/.ssh/id_rsa.pub ~/gitlab-check-backup-1504540051
For more information see:
doc/ssh/README.md in section "SSH on the GitLab server"
Please fix the error above and rerun the checks.
```
Remove the custom configuration as soon as you're able to. These customizations
are *explicitly not supported* and may stop working at any time.
## Troubleshooting
If on Git clone you are prompted for a password like `git@gitlab.com's password:`
......
module SystemCheck
module App
class GitUserDefaultSSHConfigCheck < SystemCheck::BaseCheck
# These files are allowed in the .ssh directory. The `config` file is not
# whitelisted as it may change the SSH client's behaviour dramatically.
WHITELIST = %w[
authorized_keys
authorized_keys2
known_hosts
].freeze
set_name 'Git user has default SSH configuration?'
set_skip_reason 'skipped (git user is not present or configured)'
def skip?
!home_dir || !File.directory?(home_dir)
end
def check?
forbidden_files.empty?
end
def show_error
backup_dir = "~/gitlab-check-backup-#{Time.now.to_i}"
instructions = forbidden_files.map do |filename|
"sudo mv #{Shellwords.escape(filename)} #{backup_dir}"
end
try_fixing_it("mkdir #{backup_dir}", *instructions)
for_more_information('doc/ssh/README.md in section "SSH on the GitLab server"')
fix_and_rerun
end
private
def git_user
Gitlab.config.gitlab.user
end
def home_dir
return @home_dir if defined?(@home_dir)
@home_dir =
begin
File.expand_path("~#{git_user}")
rescue ArgumentError
nil
end
end
def ssh_dir
return nil unless home_dir
File.join(home_dir, '.ssh')
end
def forbidden_files
@forbidden_files ||=
begin
present = Dir[File.join(ssh_dir, '*')]
whitelisted = WHITELIST.map { |basename| File.join(ssh_dir, basename) }
present - whitelisted
end
end
end
end
end
......@@ -33,6 +33,7 @@ namespace :gitlab do
SystemCheck::App::RedisVersionCheck,
SystemCheck::App::RubyVersionCheck,
SystemCheck::App::GitVersionCheck,
SystemCheck::App::GitUserDefaultSSHConfigCheck,
SystemCheck::App::ActiveUsersCheck
]
......
require 'spec_helper'
describe SystemCheck::App::GitUserDefaultSSHConfigCheck do
let(:username) { '_this_user_will_not_exist_unless_it_is_stubbed' }
let(:base_dir) { Dir.mktmpdir }
let(:home_dir) { File.join(base_dir, "/var/lib/#{username}") }
let(:ssh_dir) { File.join(home_dir, '.ssh') }
let(:forbidden_file) { 'id_rsa' }
before do
allow(Gitlab.config.gitlab).to receive(:user).and_return(username)
end
after do
FileUtils.rm_rf(base_dir)
end
it 'only whitelists safe files' do
expect(described_class::WHITELIST).to contain_exactly('authorized_keys', 'authorized_keys2', 'known_hosts')
end
describe '#skip?' do
subject { described_class.new.skip? }
where(user_exists: [true, false], home_dir_exists: [true, false])
with_them do
let(:expected_result) { !user_exists || !home_dir_exists }
before do
stub_user if user_exists
stub_home_dir if home_dir_exists
end
it { is_expected.to eq(expected_result) }
end
end
describe '#check?' do
subject { described_class.new.check? }
before do
stub_user
end
it 'fails if a forbidden file exists' do
stub_ssh_file(forbidden_file)
is_expected.to be_falsy
end
it "succeeds if the SSH directory doesn't exist" do
FileUtils.rm_rf(ssh_dir)
is_expected.to be_truthy
end
it 'succeeds if all the whitelisted files exist' do
described_class::WHITELIST.each do |filename|
stub_ssh_file(filename)
end
is_expected.to be_truthy
end
end
def stub_user
allow(File).to receive(:expand_path).with("~#{username}").and_return(home_dir)
end
def stub_home_dir
FileUtils.mkdir_p(home_dir)
end
def stub_ssh_file(filename)
FileUtils.mkdir_p(ssh_dir)
FileUtils.touch(File.join(ssh_dir, filename))
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