diff options
| -rw-r--r-- | Gemfile | 10 | ||||
| -rw-r--r-- | Gemfile.lock | 10 | ||||
| -rw-r--r-- | app/models/project.rb | 5 | ||||
| -rw-r--r-- | app/models/project_import_data.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/key_fingerprint.rb | 71 | ||||
| -rw-r--r-- | lib/gitlab/shell.rb | 24 | ||||
| -rw-r--r-- | spec/lib/gitlab/bitbucket_import/importer_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/key_fingerprint_spec.rb | 84 | ||||
| -rw-r--r-- | spec/lib/gitlab/shell_spec.rb | 90 | ||||
| -rw-r--r-- | spec/models/key_spec.rb | 10 |
10 files changed, 233 insertions, 75 deletions
@@ -390,6 +390,16 @@ gem 'health_check', '~> 2.6.0' gem 'vmstat', '~> 2.3.0' gem 'sys-filesystem', '~> 1.1.6' +# SSH host key support +gem 'net-ssh', '~> 4.1.0' + +# Required for ED25519 SSH host key support +group :ed25519 do + gem 'rbnacl-libsodium' + gem 'rbnacl', '~> 3.2' + gem 'bcrypt_pbkdf', '~> 1.0' +end + # Gitaly GRPC client gem 'gitaly', '~> 0.24.0' diff --git a/Gemfile.lock b/Gemfile.lock index 04d17d54636..8bbd8cce322 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -75,6 +75,7 @@ GEM babosa (1.0.2) base32 (0.3.2) bcrypt (3.1.11) + bcrypt_pbkdf (1.0.0) benchmark-ips (2.3.0) better_errors (2.1.1) coderay (>= 1.0.0) @@ -475,6 +476,7 @@ GEM mustermann (~> 1.0.0) mysql2 (0.4.5) net-ldap (0.16.0) + net-ssh (4.1.0) netrc (0.11.0) nokogiri (1.6.8.1) mini_portile2 (~> 2.1.0) @@ -662,6 +664,10 @@ GEM rake (12.0.0) rblineprof (0.3.6) debugger-ruby_core_source (~> 1.3) + rbnacl (3.4.0) + ffi + rbnacl-libsodium (1.0.11) + rbnacl (>= 3.0.1) rdoc (4.2.2) json (~> 1.4) re2 (1.1.1) @@ -924,6 +930,7 @@ DEPENDENCIES awesome_print (~> 1.2.0) babosa (~> 1.0.2) base32 (~> 0.3.0) + bcrypt_pbkdf (~> 1.0) benchmark-ips (~> 2.3.0) better_errors (~> 2.1.0) binding_of_caller (~> 0.7.2) @@ -1016,6 +1023,7 @@ DEPENDENCIES mousetrap-rails (~> 1.4.6) mysql2 (~> 0.4.5) net-ldap + net-ssh (~> 4.1.0) nokogiri (~> 1.6.7, >= 1.6.7.2) oauth2 (~> 1.4) octokit (~> 4.6.2) @@ -1062,6 +1070,8 @@ DEPENDENCIES rainbow (~> 2.2) raindrops (~> 0.18) rblineprof (~> 0.3.6) + rbnacl (~> 3.2) + rbnacl-libsodium rdoc (~> 4.2) re2 (~> 1.1.1) recaptcha (~> 3.0) diff --git a/app/models/project.rb b/app/models/project.rb index 09b1305739c..36b5d644ca3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -163,7 +163,7 @@ class Project < ActiveRecord::Base has_many :todos has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent - has_one :import_data, class_name: 'ProjectImportData' + has_one :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true has_one :project_feature has_one :statistics, class_name: 'ProjectStatistics' @@ -192,6 +192,7 @@ class Project < ActiveRecord::Base accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :project_feature + accepts_nested_attributes_for :import_data delegate :name, to: :owner, allow_nil: true, prefix: true delegate :count, to: :forks, prefix: true @@ -588,8 +589,6 @@ class Project < ActiveRecord::Base project_import_data.credentials ||= {} project_import_data.credentials = project_import_data.credentials.merge(credentials) end - - project_import_data.save end def import? diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index 37730474324..6da6632f4f2 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -1,7 +1,7 @@ require 'carrierwave/orm/activerecord' class ProjectImportData < ActiveRecord::Base - belongs_to :project + belongs_to :project, inverse_of: :import_data attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true, diff --git a/lib/gitlab/key_fingerprint.rb b/lib/gitlab/key_fingerprint.rb index b75ae512d92..d9a79f7c291 100644 --- a/lib/gitlab/key_fingerprint.rb +++ b/lib/gitlab/key_fingerprint.rb @@ -1,55 +1,48 @@ module Gitlab class KeyFingerprint - include Gitlab::Popen + attr_reader :key, :ssh_key - attr_accessor :key + # Unqualified MD5 fingerprint for compatibility + delegate :fingerprint, to: :ssh_key, allow_nil: true def initialize(key) @key = key - end - - def fingerprint - cmd_status = 0 - cmd_output = '' - - Tempfile.open('gitlab_key_file') do |file| - file.puts key - file.rewind - - cmd = [] - cmd.push('ssh-keygen') - cmd.push('-E', 'md5') if explicit_fingerprint_algorithm? - cmd.push('-lf', file.path) - - cmd_output, cmd_status = popen(cmd, '/tmp') - end - - return nil unless cmd_status.zero? - # 16 hex bytes separated by ':', optionally starting with "MD5:" - fingerprint_matches = cmd_output.match(/(MD5:)?(?<fingerprint>(\h{2}:){15}\h{2})/) - return nil unless fingerprint_matches - - fingerprint_matches[:fingerprint] + @ssh_key = + begin + Net::SSH::KeyFactory.load_data_public_key(key) + rescue Net::SSH::Exception, NotImplementedError + end end - private - - def explicit_fingerprint_algorithm? - # OpenSSH 6.8 introduces a new default output format for fingerprints. - # Check the version and decide which command to use. - - version_output, version_status = popen(%w(ssh -V)) - return false unless version_status.zero? + def valid? + ssh_key.present? + end - version_matches = version_output.match(/OpenSSH_(?<major>\d+)\.(?<minor>\d+)/) - return false unless version_matches + def type + return unless valid? - version_info = Gitlab::VersionInfo.new(version_matches[:major].to_i, version_matches[:minor].to_i) + parts = ssh_key.ssh_type.split('-') + parts.shift if parts[0] == 'ssh' - required_version_info = Gitlab::VersionInfo.new(6, 8) + parts[0].upcase + end - version_info >= required_version_info + def bits + return unless valid? + + case type + when 'RSA' + ssh_key.n.num_bits + when 'DSS', 'DSA' + ssh_key.p.num_bits + when 'ECDSA' + ssh_key.group.order.num_bits + when 'ED25519' + 256 + else + raise "Unsupported key type: #{type}" + end end end end diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 4366ff336ef..0cb28732402 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -105,12 +105,24 @@ module Gitlab # fetch_remote("gitlab/gitlab-ci", "upstream") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 - def fetch_remote(storage, name, remote, forced: false, no_tags: false) + def fetch_remote(storage, name, remote, ssh_auth: nil, forced: false, no_tags: false) args = [gitlab_shell_projects_path, 'fetch-remote', storage, "#{name}.git", remote, "#{Gitlab.config.gitlab_shell.git_timeout}"] args << '--force' if forced args << '--no-tags' if no_tags - gitlab_shell_fast_execute_raise_error(args) + vars = {} + + if ssh_auth&.ssh_import? + if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? + vars['GITLAB_SHELL_SSH_KEY'] = ssh_auth.ssh_private_key + end + + if ssh_auth.ssh_known_hosts.present? + vars['GITLAB_SHELL_KNOWN_HOSTS'] = ssh_auth.ssh_known_hosts + end + end + + gitlab_shell_fast_execute_raise_error(args, vars) end # Move repository @@ -293,15 +305,15 @@ module Gitlab false end - def gitlab_shell_fast_execute_raise_error(cmd) - output, status = gitlab_shell_fast_execute_helper(cmd) + def gitlab_shell_fast_execute_raise_error(cmd, vars = {}) + output, status = gitlab_shell_fast_execute_helper(cmd, vars) raise Error, output unless status.zero? true end - def gitlab_shell_fast_execute_helper(cmd) - vars = ENV.to_h.slice(*GITLAB_SHELL_ENV_VARS) + def gitlab_shell_fast_execute_helper(cmd, vars = {}) + vars.merge!(ENV.to_h.slice(*GITLAB_SHELL_ENV_VARS)) # Don't pass along the entire parent environment to prevent gitlab-shell # from wasting I/O by searching through GEM_PATH diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index d7d6a37f7cf..a66347ead76 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -54,7 +54,7 @@ describe Gitlab::BitbucketImport::Importer do create( :project, import_source: project_identifier, - import_data: ProjectImportData.new(credentials: data) + import_data_attributes: { credentials: data } ) end diff --git a/spec/lib/gitlab/key_fingerprint_spec.rb b/spec/lib/gitlab/key_fingerprint_spec.rb index d7bebaca675..f5fd5a96bc9 100644 --- a/spec/lib/gitlab/key_fingerprint_spec.rb +++ b/spec/lib/gitlab/key_fingerprint_spec.rb @@ -1,12 +1,82 @@ -require "spec_helper" +require 'spec_helper' -describe Gitlab::KeyFingerprint do - let(:key) { "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=" } - let(:fingerprint) { "3f:a2:ee:de:b5:de:53:c3:aa:2f:9c:45:24:4c:47:7b" } +describe Gitlab::KeyFingerprint, lib: true do + KEYS = { + rsa: + 'example.com ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC5z65PwQ1GE6foJgwk' \ + '9rmQi/glaXbUeVa5uvQpnZ3Z5+forcI7aTngh3aZ/H2UDP2L70TGy7kKNyp0J3a8/OdG' \ + 'Z08y5yi3JlbjFARO1NyoFEjw2H1SJxeJ43L6zmvTlu+hlK1jSAlidl7enS0ufTlzEEj4' \ + 'iJcuTPKdVzKRgZuTRVm9woWNVKqIrdRC0rJiTinERnfSAp/vNYERMuaoN4oJt8p/NEek' \ + 'rmFoDsQOsyDW5RAnCnjWUU+jFBKDpfkJQ1U2n6BjJewC9dl6ODK639l3yN4WOLZEk4tN' \ + 'UysfbGeF3rmMeflaD6O1Jplpv3YhwVGFNKa7fMq6k3Z0tszTJPYh', + ecdsa: + 'example.com ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAI' \ + 'bmlzdHAyNTYAAABBBKTJy43NZzJSfNxpv/e2E6Zy3qoHoTQbmOsU5FEfpWfWa1MdTeXQ' \ + 'YvKOi+qz/1AaNx6BK421jGu74JCDJtiZWT8=', + ed25519: + '@revoked example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAfuCHKVTjq' \ + 'uxvt6CM6tdG4SLp1Btn/nOeHHE5UOzRdf', + dss: + 'example.com ssh-dss AAAAB3NzaC1kc3MAAACBAP1/U4EddRIpUt9KnC7s5Of2EbdS' \ + 'PO9EAMMeP4C2USZpRV1AIlH7WT2NWPq/xfW6MPbLm1Vs14E7gB00b/JmYLdrmVClpJ+f' \ + '6AR7ECLCT7up1/63xhv4O1fnxqimFQ8E+4P208UewwI1VBNaFpEy9nXzrith1yrv8iID' \ + 'GZ3RSAHHAAAAFQCXYFCPFSMLzLKSuYKi64QL8Fgc9QAAAIEA9+GghdabPd7LvKtcNrhX' \ + 'uXmUr7v6OuqC+VdMCz0HgmdRWVeOutRZT+ZxBxCBgLRJFnEj6EwoFhO3zwkyjMim4TwW' \ + 'eotUfI0o4KOuHiuzpnWRbqN/C/ohNWLx+2J6ASQ7zKTxvqhRkImog9/hWuWfBpKLZl6A' \ + 'e1UlZAFMO/7PSSoAAACBAJcQ4JODqhuGbXIEpqxetm7PWbdbCcr3y/GzIZ066pRovpL6' \ + 'qm3qCVIym4cyChxWwb8qlyCIi+YRUUWm1z/wiBYT2Vf3S4FXBnyymCkKEaV/EY7+jd4X' \ + '1bXI58OD2u+bLCB/sInM4fGB8CZUIWT9nJH0Ve9jJUge2ms348/QOJ1+' + }.freeze - describe "#fingerprint" do - it "generates the key's fingerprint" do - expect(described_class.new(key).fingerprint).to eq(fingerprint) + MD5_FINGERPRINTS = { + rsa: '06:b2:8a:92:df:0e:11:2c:ca:7b:8f:a4:ba:6e:4b:fd', + ecdsa: '45:ff:5b:98:9a:b6:8a:41:13:c1:30:8b:09:5e:7b:4e', + ed25519: '2e:65:6a:c8:cf:bf:b2:8b:9a:bd:6d:9f:11:5c:12:16', + dss: '57:98:86:02:5f:9c:f4:9b:ad:5a:1e:51:92:0e:fd:2b' + }.freeze + + BIT_COUNTS = { + rsa: 2048, + ecdsa: 256, + ed25519: 256, + dss: 1024 + }.freeze + + describe '#type' do + KEYS.each do |type, key| + it "calculates the type of #{type} keys" do + calculated_type = described_class.new(key).type + + expect(calculated_type).to eq(type.to_s.upcase) + end + end + end + + describe '#fingerprint' do + KEYS.each do |type, key| + it "calculates the MD5 fingerprint for #{type} keys" do + fp = described_class.new(key).fingerprint + + expect(fp).to eq(MD5_FINGERPRINTS[type]) + end + end + end + + describe '#bits' do + KEYS.each do |type, key| + it "calculates the number of bits in #{type} keys" do + bits = described_class.new(key).bits + + expect(bits).to eq(BIT_COUNTS[type]) + end + end + end + + describe '#key' do + it 'carries the unmodified key data' do + key = described_class.new(KEYS[:rsa]).key + + expect(key).to eq(KEYS[:rsa]) end end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index b90d8dede0f..2345874cf10 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -174,20 +174,94 @@ describe Gitlab::Shell do end describe '#fetch_remote' do + def fetch_remote(ssh_auth = nil) + gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage', ssh_auth: ssh_auth) + end + + def expect_popen(vars = {}) + popen_args = [ + projects_path, + 'fetch-remote', + 'current/storage', + 'project/path.git', + 'new/storage', + Gitlab.config.gitlab_shell.git_timeout.to_s + ] + + expect(Gitlab::Popen).to receive(:popen).with(popen_args, nil, popen_vars.merge(vars)) + end + + def build_ssh_auth(opts = {}) + defaults = { + ssh_import?: true, + ssh_key_auth?: false, + ssh_known_hosts: nil, + ssh_private_key: nil + } + + double(:ssh_auth, defaults.merge(opts)) + end + it 'returns true when the command succeeds' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'fetch-remote', 'current/storage', 'project/path.git', 'new/storage', '800'], - nil, popen_vars).and_return([nil, 0]) + expect_popen.and_return([nil, 0]) - expect(gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage')).to be true + expect(fetch_remote).to be_truthy end it 'raises an exception when the command fails' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'fetch-remote', 'current/storage', 'project/path.git', 'new/storage', '800'], - nil, popen_vars).and_return(["error", 1]) + expect_popen.and_return(["error", 1]) + + expect { fetch_remote }.to raise_error(Gitlab::Shell::Error, "error") + end + + context 'SSH auth' do + it 'passes the SSH key if specified' do + expect_popen('GITLAB_SHELL_SSH_KEY' => 'foo').and_return([nil, 0]) + + ssh_auth = build_ssh_auth(ssh_key_auth?: true, ssh_private_key: 'foo') + + expect(fetch_remote(ssh_auth)).to be_truthy + end + + it 'does not pass an empty SSH key' do + expect_popen.and_return([nil, 0]) + + ssh_auth = build_ssh_auth(ssh_key_auth: true, ssh_private_key: '') + + expect(fetch_remote(ssh_auth)).to be_truthy + end + + it 'does not pass the key unless SSH key auth is to be used' do + expect_popen.and_return([nil, 0]) + + ssh_auth = build_ssh_auth(ssh_key_auth: false, ssh_private_key: 'foo') + + expect(fetch_remote(ssh_auth)).to be_truthy + end + + it 'passes the known_hosts data if specified' do + expect_popen('GITLAB_SHELL_KNOWN_HOSTS' => 'foo').and_return([nil, 0]) + + ssh_auth = build_ssh_auth(ssh_known_hosts: 'foo') + + expect(fetch_remote(ssh_auth)).to be_truthy + end + + it 'does not pass empty known_hosts data' do + expect_popen.and_return([nil, 0]) + + ssh_auth = build_ssh_auth(ssh_known_hosts: '') + + expect(fetch_remote(ssh_auth)).to be_truthy + end + + it 'does not pass known_hosts data unless SSH is to be used' do + expect_popen(popen_vars).and_return([nil, 0]) + + ssh_auth = build_ssh_auth(ssh_import?: false, ssh_known_hosts: 'foo') - expect { gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage') }.to raise_error(Gitlab::Shell::Error, "error") + expect(fetch_remote(ssh_auth)).to be_truthy + end end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 0daeb337168..3508391c721 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -83,15 +83,6 @@ describe Key, :mailer do expect(build(:key)).to be_valid end - it 'rejects an unfingerprintable key that contains a space' do - key = build(:key) - - # Not always the middle, but close enough - key.key = key.key[0..100] + ' ' + key.key[101..-1] - - expect(key).not_to be_valid - end - it 'accepts a key with newline charecters after stripping them' do key = build(:key) key.key = key.key.insert(100, "\n") @@ -102,7 +93,6 @@ describe Key, :mailer do it 'rejects the unfingerprintable key (not a key)' do expect(build(:key, key: 'ssh-rsa an-invalid-key==')).not_to be_valid end - end context 'callbacks' do |