Browse Source

Fixes #2914 - LDAP SSL verification not performed.

Dusan Vuckovic 1 year ago
parent
commit
7bbe2e33da

+ 1 - 0
.gitleaks.toml

@@ -10,6 +10,7 @@ paths = [
   '''^doc/developer_manual/cookbook/how-to-setup-smime-integration.md''',
   '''^log/''',
   '''^spec/factories/pgp_key\.rb''',
+  '''^spec/fixtures/files/ldap/.*\.crt''',
   '''^spec/fixtures/files/pgp/.*\.asc''',
   '''^spec/fixtures/files/smime/.*\.key''',
   '''^test/data/saml/zammad-client.json''',

+ 0 - 2
lib/certificate/apply_ssl_certificates.rb

@@ -13,8 +13,6 @@ class Certificate::ApplySSLCertificates
 
         all_certificates = SSLCertificate.all
 
-        return if !all_certificates.exists?
-
         # Only update the default store if there are changes with the stored SSL certificates.
         cache_key = all_certificates.cache_key_with_version
         return if @cache_key == cache_key

+ 1 - 0
lib/ldap.rb

@@ -197,6 +197,7 @@ class Ldap
 
     if @config[:ssl_verify]
       Certificate::ApplySSLCertificates.ensure_fresh_ssl_context
+      @encryption[:tls_options] = OpenSSL::SSL::SSLContext::DEFAULT_PARAMS
       return
     end
 

+ 15 - 0
spec/fixtures/files/ldap/ca.crt

@@ -0,0 +1,15 @@
+-----BEGIN CERTIFICATE-----
+MIICXDCCAeOgAwIBAgIUS/kNI8Am3Kzp40tgE9IdoKaj+ZswCgYIKoZIzj0EAwMw
+bjELMAkGA1UEBhMCREUxDzANBgNVBAgTBkJlcmxpbjEPMA0GA1UEBxMGQmVybGlu
+MRowGAYDVQQKExFaYW1tYWQgRm91bmRhdGlvbjELMAkGA1UECxMCQ0kxFDASBgNV
+BAMTC2V4YW1wbGUuY29tMB4XDTIzMDkwNjA4MzUwMFoXDTI4MDkwNDA4MzUwMFow
+bjELMAkGA1UEBhMCREUxDzANBgNVBAgTBkJlcmxpbjEPMA0GA1UEBxMGQmVybGlu
+MRowGAYDVQQKExFaYW1tYWQgRm91bmRhdGlvbjELMAkGA1UECxMCQ0kxFDASBgNV
+BAMTC2V4YW1wbGUuY29tMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEYJOiYlDD4+7J
+MB4zYITSDXkC0e18T+KUbEQdm9F/wWskNFiFqXqOnSslj2GfDe0u51tejq/ogv8z
+K/Q7JLKTVVMOo2h8LaoGw9qhPy38tXmGlnCNCIAzLXeuLMKKhrXKo0IwQDAOBgNV
+HQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUaO4UGDuWatMx
+nBH4bIJAtQxJ8D8wCgYIKoZIzj0EAwMDZwAwZAIwZqrPI0QlsegqlN8DBLquOcGO
+KkCDWdjV3gr8TF63k6K2Dg5kKGS8ooPLR4efiAA+AjBMHr4qBMAmYa3IV4mG+LSB
+3C+umJ/NU5uB8OfYPUclRuaIyvVRcjPBs404/mZPo34=
+-----END CERTIFICATE-----

+ 56 - 4
spec/integration/ldap_spec.rb

@@ -64,8 +64,15 @@ RSpec.describe 'Ldap import', integration: true, required_envs: %w[IMPORT_LDAP_E
     end
   end
 
+  shared_examples 'certificate verification error' do
+    it 'returns certificate verify failed error' do
+      expect(ImportJob.last.result[:error]).to match(%r{error: certificate verify failed \(self(-|\s)signed certificate in certificate chain\)})
+    end
+  end
+
   context 'when importing' do
     before do
+      before_hook if defined? before_hook
       Setting.set('ldap_integration', true)
       TCR.turned_off do
         ldap_source
@@ -76,15 +83,60 @@ RSpec.describe 'Ldap import', integration: true, required_envs: %w[IMPORT_LDAP_E
     include_examples 'ldap import'
 
     context 'with ssl' do
-      let(:ldap_source) { create(:ldap_source, :with_ssl) }
+      context 'with ssl verification' do
+        context 'with trusted certificate' do
+          let(:ldap_source)    { create(:ldap_source, :with_ssl_verified) }
+          let(:ca_certificate) { Rails.root.join('spec/fixtures/files/ldap/ca.crt').read }
+          let(:before_hook) do
+            import_ca_certificate
+          end
+
+          include_examples 'ldap import'
+        end
+
+        context 'without trusted certificate' do
+          let(:ldap_source) { create(:ldap_source, :with_ssl_verified) }
 
-      include_examples 'ldap import'
+          include_examples 'certificate verification error'
+        end
+      end
+
+      context 'without ssl verification' do
+        let(:ldap_source) { create(:ldap_source, :with_ssl) }
+
+        include_examples 'ldap import'
+      end
     end
 
     context 'with starttls' do
-      let(:ldap_source) { create(:ldap_source, :with_starttls) }
+      context 'with ssl verification' do
+        context 'with trusted certificate' do
+          let(:ldap_source)    { create(:ldap_source, :with_starttls_verified) }
+          let(:ca_certificate) { Rails.root.join('spec/fixtures/files/ldap/ca.crt').read }
+          let(:before_hook) do
+            import_ca_certificate
+          end
+
+          include_examples 'ldap import'
+        end
+
+        context 'without trusted certificate' do
+          let(:ldap_source) { create(:ldap_source, :with_ssl_verified) }
+
+          include_examples 'certificate verification error'
+        end
+      end
+
+      context 'without ssl verification' do
+        let(:ldap_source) { create(:ldap_source, :with_starttls) }
+
+        include_examples 'ldap import'
+      end
+    end
 
-      include_examples 'ldap import'
+    def import_ca_certificate
+      # Import CA certificate into the trust store.
+      SSLCertificate.create!(certificate: Rails.root.join('spec/fixtures/files/ldap/ca.crt').read)
     end
   end
 end

+ 6 - 6
spec/lib/certificate/apply_ssl_certificates_spec.rb

@@ -9,12 +9,6 @@ RSpec.describe Certificate::ApplySSLCertificates, :aggregate_failures, type: :mo
       OpenSSL::SSL::SSLContext::DEFAULT_CERT_STORE
     end
 
-    context 'without custom certificates present' do
-      it 'does not change the context' do
-        expect { described_class.ensure_fresh_ssl_context }.not_to change { current_store }
-      end
-    end
-
     context 'with a custom certificate present' do
       it 'changes the context' do
         create(:ssl_certificate, fixture: 'RootCA')
@@ -24,6 +18,12 @@ RSpec.describe Certificate::ApplySSLCertificates, :aggregate_failures, type: :mo
         expect { described_class.ensure_fresh_ssl_context }.to change { current_store }
       end
     end
+
+    context 'without custom certificates present' do
+      it 'changes the context' do
+        expect { described_class.ensure_fresh_ssl_context }.to change { current_store }
+      end
+    end
   end
 
   describe '.extract_metadata' do