Set `Net::LDAP` encryption properly

2 files changed, 117 insertions, 19 deletions

diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb
index c531100fbc4..383e0a09e42 100644
--- a/lib/gitlab/ldap/config.rb
+++ b/lib/gitlab/ldap/config.rb
@@ -2,6 +2,16 @@
 module Gitlab
   module LDAP
     class Config
+      NET_LDAP_ENCRYPTION_METHOD = {
+        :simple_tls => :simple_tls,
+        :start_tls => :start_tls,
+        :plain => nil,
+
+        # Deprecated. Better to pass-through the actual `Net::LDAP` encryption type.
+        :ssl => :simple_tls,
+        :tls => :start_tls,
+      }
+
       attr_accessor :provider, :options
 
       def self.enabled?
@@ -39,7 +49,7 @@ module Gitlab
 
       def adapter_options
         opts = base_options.merge(
-          encryption: encryption
+          encryption: encryption_options
         )
 
         opts.merge!(auth_options) if has_auth?
@@ -157,14 +167,22 @@ module Gitlab
         base_config.servers.values.find { |server| server['provider_name'] == provider }
       end
 
-      def encryption
-        case options['encryption'].to_s
-        when 'ssl'
-          :simple_tls
-        when 'tls'
-          :start_tls
+      def encryption_options
+        method = translate_method(options['encryption'])
+        options = { method: method }
+        options.merge!(tls_options: tls_options(method)) if method
+        options
+      end
+
+      def translate_method(method_from_config)
+        NET_LDAP_ENCRYPTION_METHOD[method_from_config.to_sym]
+      end
+
+      def tls_options(method)
+        if method && options['verify_certificates']
+          OpenSSL::SSL::SSLContext::DEFAULT_PARAMS
         else
-          nil
+          { verify_mode: OpenSSL::SSL::VERIFY_NONE }
         end
       end
 
diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb
index e75e1e3ea2f..bbd4da58252 100644
--- a/spec/lib/gitlab/ldap/config_spec.rb
+++ b/spec/lib/gitlab/ldap/config_spec.rb
@@ -5,7 +5,7 @@ describe Gitlab::LDAP::Config, lib: true do
 
   let(:config) { Gitlab::LDAP::Config.new('ldapmain') }
 
-  describe '#initalize' do
+  describe '#initialize' do
     it 'requires a provider' do
       expect{ Gitlab::LDAP::Config.new }.to raise_error ArgumentError
     end
@@ -32,31 +32,111 @@ describe Gitlab::LDAP::Config, lib: true do
       expect(config.adapter_options).to eq(
         host: 'ldap.example.com',
         port: 386,
-        encryption: nil
+        encryption: { method: nil }
       )
     end
 
     it 'includes authentication options when auth is configured' do
       stub_ldap_config(
         options: {
-          'host'       => 'ldap.example.com',
-          'port'       => 686,
-          'encryption' => 'ssl',
-          'bind_dn'    => 'uid=admin,dc=example,dc=com',
-          'password'   => 'super_secret'
+          'host'                => 'ldap.example.com',
+          'port'                => 686,
+          'encryption'          => 'simple_tls',
+          'verify_certificates' => true,
+          'bind_dn'             => 'uid=admin,dc=example,dc=com',
+          'password'            => 'super_secret'
         }
       )
 
-      expect(config.adapter_options).to eq(
-        host: 'ldap.example.com',
-        port: 686,
-        encryption: :simple_tls,
+      expect(config.adapter_options).to include({
         auth: {
           method: :simple,
           username: 'uid=admin,dc=example,dc=com',
           password: 'super_secret'
         }
+      })
+    end
+
+    it 'sets encryption method to simple_tls when configured as simple_tls' do
+      stub_ldap_config(
+        options: {
+          'host'                => 'ldap.example.com',
+          'port'                => 686,
+          'encryption'          => 'simple_tls'
+        }
+      )
+
+      expect(config.adapter_options[:encryption]).to include({ method: :simple_tls })
+    end
+
+    it 'sets encryption method to simple_tls when configured as ssl, for backwards compatibility' do
+      stub_ldap_config(
+        options: {
+          'host'                => 'ldap.example.com',
+          'port'                => 686,
+          'encryption'          => 'ssl'
+        }
+      )
+
+      expect(config.adapter_options[:encryption]).to include({ method: :simple_tls })
+    end
+
+    it 'sets encryption method to start_tls when configured as start_tls' do
+      stub_ldap_config(
+        options: {
+          'host'                => 'ldap.example.com',
+          'port'                => 686,
+          'encryption'          => 'start_tls'
+        }
+      )
+
+      expect(config.adapter_options[:encryption]).to include({ method: :start_tls })
+    end
+
+    it 'sets encryption method to start_tls when configured as tls, for backwards compatibility' do
+      stub_ldap_config(
+        options: {
+          'host'                => 'ldap.example.com',
+          'port'                => 686,
+          'encryption'          => 'tls'
+        }
       )
+
+      expect(config.adapter_options[:encryption]).to include({ method: :start_tls })
+    end
+
+    context 'when verify_certificates is enabled' do
+      it 'sets tls_options to OpenSSL defaults' do
+        stub_ldap_config(
+          options: {
+            'host'                => 'ldap.example.com',
+            'port'                => 686,
+            'encryption'          => 'simple_tls',
+            'verify_certificates' => true
+          }
+        )
+
+        expect(config.adapter_options[:encryption]).to include({ tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS })
+      end
+    end
+
+    context 'when verify_certificates is disabled' do
+      it 'sets verify_mode to OpenSSL VERIFY_NONE' do
+        stub_ldap_config(
+          options: {
+            'host'                => 'ldap.example.com',
+            'port'                => 686,
+            'encryption'          => 'simple_tls',
+            'verify_certificates' => false
+          }
+        )
+
+        expect(config.adapter_options[:encryption]).to include({
+          tls_options: {
+            verify_mode: OpenSSL::SSL::VERIFY_NONE
+          }
+        })
+      end
     end
   end