Automatic merge of gitlab-org/gitlab-ce master

changelogs/unreleased/support-gitaly-tls.yml

+---
+title: Support tls communication in gitaly
+merge_request: 22602
+author:
+type: added

config/gitlab.yml.example

@@ -748,7 +748,7 @@ production: &base
     storages: # You must have at least a `default` storage path.
       default:
         path: /home/git/repositories/
-        gitaly_address: unix:/home/git/gitlab/tmp/sockets/private/gitaly.socket # TCP connections are supported too (e.g. tcp://host:port)
+        gitaly_address: unix:/home/git/gitlab/tmp/sockets/private/gitaly.socket # TCP connections are supported too (e.g. tcp://host:port). TLS connections are also supported using the system certificate pool (eg: tls://host:port).
         # gitaly_token: 'special token' # Optional: override global gitaly.token for this storage.
 
   ## Backup settings

doc/administration/gitaly/index.md

 # Gitaly
 
-[Gitaly](https://gitlab.com/gitlab-org/gitaly) is the service that 
+[Gitaly](https://gitlab.com/gitlab-org/gitaly) is the service that
 provides high-level RPC access to Git repositories. Without it, no other
 components can read or write Git data.
 
@@ -23,7 +23,7 @@ gitaly['prometheus_listen_addr'] = 'localhost:9236'
 ```
 
 To change a Gitaly setting in installations from source you can edit
-`/home/git/gitaly/config.toml`. Changes will be applied when you run 
+`/home/git/gitaly/config.toml`. Changes will be applied when you run
 `service gitlab restart`.
 
 ```toml
@@ -91,13 +91,13 @@ documentation on configuring Gitaly
 authentication](https://gitlab.com/gitlab-org/gitaly/blob/master/doc/configuration/README.md#authentication)
 .
 
-Gitaly must trigger some callbacks to GitLab via GitLab Shell. As a result, 
+Gitaly must trigger some callbacks to GitLab via GitLab Shell. As a result,
 the GitLab Shell secret must be the same between the other GitLab servers and
 the Gitaly server. The easiest way to accomplish this is to copy `/etc/gitlab/gitlab-secrets.json`
 from an existing GitLab server to the Gitaly server. Without this shared secret,
-Git operations in GitLab will result in an API error. 
+Git operations in GitLab will result in an API error.
 
-> **NOTE:** In most or all cases the storage paths below end in `/repositories` which is 
+> **NOTE:** In most or all cases the storage paths below end in `/repositories` which is
 different than `path` in `git_data_dirs` of Omnibus installations. Check the
 directory layout on your Gitaly server to be sure.
 
@@ -133,6 +133,11 @@ gitaly['storage'] = [
   { 'name' => 'default', 'path' => '/mnt/gitlab/default/repositories' },
   { 'name' => 'storage1', 'path' => '/mnt/gitlab/storage1/repositories' },
 ]
+
+# To use tls for gitaly you need to add
+gitaly['tls_listen_addr'] = "0.0.0.0:9999"
+gitaly['certificate_path'] = "path/to/cert.pem"
+gitaly['key_path'] = "path/to/key.pem"
 ```
 
 Source installations:
@@ -140,6 +145,11 @@ Source installations:
 ```toml
 # /home/git/gitaly/config.toml
 listen_addr = '0.0.0.0:8075'
+tls_listen_addr = '0.0.0.0:9999'
+
+[tls]
+certificate_path = /path/to/cert.pem
+key_path = /path/to/key.pem
 
 [auth]
 token = 'abc123secret'
@@ -205,6 +215,70 @@ Gitaly logs on your Gitaly server (`sudo gitlab-ctl tail gitaly` or
 coming in. One sure way to trigger a Gitaly request is to clone a
 repository from your GitLab server over HTTP.
 
+## TLS support
+
+Gitaly supports TLS credentials for GRPC authentication. To be able to communicate
+with a gitaly instance that listens for secure connections you will need to use `tls://` url
+scheme in the `gitaly_address` of the corresponding storage entry in the gitlab configuration.
+
+The admin needs to bring their own certificate as we do not provide that automatically.
+The certificate to be used needs to be installed on all gitaly nodes and on all client nodes that communicate with it following procedures described in [GitLab custom certificate configuration](https://docs.gitlab.com/omnibus/settings/ssl.html#install-custom-public-certificates)
+
+### Example TLS configuration
+
+### Omnibus installations:
+
+#### On client nodes:
+
+```ruby
+# /etc/gitlab/gitlab.rb
+git_data_dirs({
+  'default' => { 'path' => '/mnt/gitlab/default', 'gitaly_address' => 'tls://gitaly.internal:9999' },
+  'storage1' => { 'path' => '/mnt/gitlab/storage1', 'gitaly_address' => 'tls://gitaly.internal:9999' },
+})
+
+gitlab_rails['gitaly_token'] = 'abc123secret'
+```
+
+#### On gitaly server nodes:
+
+```ruby
+gitaly['tls_listen_addr'] = "0.0.0.0:9999"
+gitaly['certificate_path'] = "path/to/cert.pem"
+gitaly['key_path'] = "path/to/key.pem"
+```
+
+### Source installations:
+
+#### On client nodes:
+
+```yaml
+# /home/git/gitlab/config/gitlab.yml
+gitlab:
+  repositories:
+    storages:
+      default:
+        path: /mnt/gitlab/default/repositories
+        gitaly_address: tls://gitaly.internal:9999
+      storage1:
+        path: /mnt/gitlab/storage1/repositories
+        gitaly_address: tls://gitaly.internal:9999
+
+  gitaly:
+    token: 'abc123secret'
+```
+
+#### On gitaly server nodes:
+
+```toml
+# /home/git/gitaly/config.toml
+tls_listen_addr = '0.0.0.0:9999'
+
+[tls]
+certificate_path = '/path/to/cert.pem'
+key_path = '/path/to/key.pem'
+```
+
 ## Disabling or enabling the Gitaly service in a cluster environment
 
 If you are running Gitaly [as a remote

lib/gitlab/gitaly_client.rb

@@ -26,6 +26,7 @@ module Gitlab
       end
     end
 
+    PEM_REGEX = /\-+BEGIN CERTIFICATE\-+.+?\-+END CERTIFICATE\-+/m
     SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'
     MAXIMUM_GITALY_CALLS = 35
     CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze
@@ -50,11 +51,42 @@ module Gitlab
         @stubs[storage][name] ||= begin
           klass = stub_class(name)
           addr = stub_address(storage)
-          klass.new(addr, :this_channel_is_insecure)
+          creds = stub_creds(storage)
+          klass.new(addr, creds)
         end
       end
     end
 
+    def self.stub_cert_paths
+      cert_paths = Dir["#{OpenSSL::X509::DEFAULT_CERT_DIR}/*"]
+      cert_paths << OpenSSL::X509::DEFAULT_CERT_FILE if File.exist? OpenSSL::X509::DEFAULT_CERT_FILE
+      cert_paths
+    end
+
+    def self.stub_certs
+      return @certs if @certs
+
+      @certs = stub_cert_paths.flat_map do |cert_file|
+        File.read(cert_file).scan(PEM_REGEX).map do |cert|
+          begin
+            OpenSSL::X509::Certificate.new(cert).to_pem
+          rescue OpenSSL::OpenSSLError => e
+            Rails.logger.error "Could not load certificate #{cert_file} #{e}"
+            Gitlab::Sentry.track_exception(e, extra: { cert_file: cert_file })
+            nil
+          end
+        end.compact
+      end.uniq.join("\n")
+    end
+
+    def self.stub_creds(storage)
+      if URI(address(storage)).scheme == 'tls'
+        GRPC::Core::ChannelCredentials.new stub_certs
+      else
+        :this_channel_is_insecure
+      end
+    end
+
     def self.stub_class(name)
       if name == :health_check
         Grpc::Health::V1::Health::Stub
@@ -64,9 +96,7 @@ module Gitlab
     end
 
     def self.stub_address(storage)
-      addr = address(storage)
-      addr = addr.sub(%r{^tcp://}, '') if URI(addr).scheme == 'tcp'
-      addr
+      address(storage).sub(%r{^tcp://|^tls://}, '')
     end
 
     def self.clear_stubs!
@@ -88,8 +118,8 @@ module Gitlab
         raise "storage #{storage.inspect} is missing a gitaly_address"
       end
 
-      unless URI(address).scheme.in?(%w(tcp unix))
-        raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix'"
+      unless URI(address).scheme.in?(%w(tcp unix tls))
+        raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix' or 'tls'"
       end
 
       address

spec/lib/gitlab/gitaly_client_spec.rb

@@ -3,6 +3,20 @@ require 'spec_helper'
 # We stub Gitaly in `spec/support/gitaly.rb` for other tests. We don't want
 # those stubs while testing the GitalyClient itself.
 describe Gitlab::GitalyClient do
+  let(:sample_cert) { Rails.root.join('spec/fixtures/clusters/sample_cert.pem').to_s }
+
+  before do
+    allow(described_class)
+      .to receive(:stub_cert_paths)
+      .and_return([sample_cert])
+  end
+
+  def stub_repos_storages(address)
+    allow(Gitlab.config.repositories).to receive(:storages).and_return({
+      'default' => { 'gitaly_address' => address }
+    })
+  end
+
   describe '.stub_class' do
     it 'returns the gRPC health check stub' do
       expect(described_class.stub_class(:health_check)).to eq(::Grpc::Health::V1::Health::Stub)
@@ -15,12 +29,8 @@ describe Gitlab::GitalyClient do
 
   describe '.stub_address' do
     it 'returns the same result after being called multiple times' do
-      address = 'localhost:9876'
-      prefixed_address = "tcp://#{address}"
-
-      allow(Gitlab.config.repositories).to receive(:storages).and_return({
-        'default' => { 'gitaly_address' => prefixed_address }
-      })
+      address = 'tcp://localhost:9876'
+      stub_repos_storages address
 
       2.times do
         expect(described_class.stub_address('default')).to eq('localhost:9876')
@@ -28,6 +38,45 @@ describe Gitlab::GitalyClient do
     end
   end
 
+  describe '.stub_certs' do
+    it 'skips certificates if OpenSSLError is raised and report it' do
+      expect(Rails.logger).to receive(:error).at_least(:once)
+      expect(Gitlab::Sentry)
+        .to receive(:track_exception)
+        .with(
+          a_kind_of(OpenSSL::X509::CertificateError),
+          extra: { cert_file: a_kind_of(String) }).at_least(:once)
+
+      expect(OpenSSL::X509::Certificate)
+        .to receive(:new)
+        .and_raise(OpenSSL::X509::CertificateError).at_least(:once)
+
+      expect(described_class.stub_certs).to be_a(String)
+    end
+  end
+  describe '.stub_creds' do
+    it 'returns :this_channel_is_insecure if unix' do
+      address = 'unix:/tmp/gitaly.sock'
+      stub_repos_storages address
+
+      expect(described_class.stub_creds('default')).to eq(:this_channel_is_insecure)
+    end
+
+    it 'returns :this_channel_is_insecure if tcp' do
+      address = 'tcp://localhost:9876'
+      stub_repos_storages address
+
+      expect(described_class.stub_creds('default')).to eq(:this_channel_is_insecure)
+    end
+
+    it 'returns Credentials object if tls' do
+      address = 'tls://localhost:9876'
+      stub_repos_storages address
+
+      expect(described_class.stub_creds('default')).to be_a(GRPC::Core::ChannelCredentials)
+    end
+  end
+
   describe '.stub' do
     # Notice that this is referring to gRPC "stubs", not rspec stubs
     before do
@@ -37,9 +86,19 @@ describe Gitlab::GitalyClient do
     context 'when passed a UNIX socket address' do
       it 'passes the address as-is to GRPC' do
         address = 'unix:/tmp/gitaly.sock'
-        allow(Gitlab.config.repositories).to receive(:storages).and_return({
-          'default' => { 'gitaly_address' => address }
-        })
+        stub_repos_storages address
+
+        expect(Gitaly::CommitService::Stub).to receive(:new).with(address, any_args)
+
+        described_class.stub(:commit_service, 'default')
+      end
+    end
+
+    context 'when passed a TLS address' do
+      it 'strips tls:// prefix before passing it to GRPC::Core::Channel initializer' do
+        address = 'localhost:9876'
+        prefixed_address = "tls://#{address}"
+        stub_repos_storages prefixed_address
 
         expect(Gitaly::CommitService::Stub).to receive(:new).with(address, any_args)
 
@@ -51,10 +110,7 @@ describe Gitlab::GitalyClient do
       it 'strips tcp:// prefix before passing it to GRPC::Core::Channel initializer' do
         address = 'localhost:9876'
         prefixed_address = "tcp://#{address}"
-
-        allow(Gitlab.config.repositories).to receive(:storages).and_return({
-          'default' => { 'gitaly_address' => prefixed_address }
-        })
+        stub_repos_storages prefixed_address
 
         expect(Gitaly::CommitService::Stub).to receive(:new).with(address, any_args)