Support self-signed LDAPS connections (#29606).

Patch by Gregor Schmidt.

git-svn-id: http://svn.redmine.org/redmine/trunk@17505 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/models/auth_source.rb b/app/models/auth_source.rb
index c28a33fb27e6c7804d75f7a6cf55657b76815a49..8b745f33fba444b2f9bf540ac0b6ff227e8b07ba 100644 (file)
--- a/app/models/auth_source.rb
+++ b/app/models/auth_source.rb
@@ -43,6 +43,7 @@ class AuthSource < ActiveRecord::Base
     'attr_mail',
     'onthefly_register',
     'tls',
+    'verify_peer',
     'filter',
     'timeout'
 

diff --git a/app/models/auth_source_ldap.rb b/app/models/auth_source_ldap.rb
index 8e380d456ea44c699f93d8a9a3070aa7ff77a3d7..c3939e8d5f0fce4d719674719b6fc32d63d8c085 100644 (file)
--- a/app/models/auth_source_ldap.rb
+++ b/app/models/auth_source_ldap.rb
@@ -37,6 +37,14 @@ class AuthSourceLdap < AuthSource
 
   before_validation :strip_ldap_attributes
 
+  safe_attributes 'ldap_mode'
+
+  LDAP_MODES = [
+    :ldap,
+    :ldaps_verify_none,
+    :ldaps_verify_peer
+  ]
+
   def initialize(attributes=nil, *args)
     super
     self.port = 389 if self.port == 0
@@ -101,6 +109,31 @@ class AuthSourceLdap < AuthSource
     raise AuthSourceException.new(e.message)
   end
 
+  def ldap_mode
+    case
+    when tls && verify_peer
+      :ldaps_verify_peer
+    when tls && !verify_peer
+      :ldaps_verify_none
+    else
+      :ldap
+    end
+  end
+
+  def ldap_mode=(ldap_mode)
+    case ldap_mode.try(:to_sym)
+    when :ldaps_verify_peer
+      self.tls = true
+      self.verify_peer = true
+    when :ldaps_verify_none
+      self.tls = true
+      self.verify_peer = false
+    else
+      self.tls = false
+      self.verify_peer = false
+    end
+  end
+
   private
 
   def with_timeout(&block)
@@ -143,9 +176,18 @@ class AuthSourceLdap < AuthSource
 
   def initialize_ldap_con(ldap_user, ldap_password)
     options = { :host => self.host,
-                :port => self.port,
-                :encryption => (self.tls ? :simple_tls : nil)
+                :port => self.port
               }
+    if tls
+      options[:encryption] = {
+        :method => :simple_tls,
+        # Always provide non-empty tls_options, to make sure, that all
+        # OpenSSL::SSL::SSLContext::DEFAULT_PARAMS as well as the default cert
+        # store are used.
+        :tls_options => { :verify_mode => verify_peer? ? OpenSSL::SSL::VERIFY_PEER : OpenSSL::SSL::VERIFY_NONE }
+      }
+    end
+
     options.merge!(:auth => { :method => :simple, :username => ldap_user, :password => ldap_password }) unless ldap_user.blank? && ldap_password.blank?
     Net::LDAP.new options
   end

diff --git a/app/views/auth_sources/_form_auth_source_ldap.html.erb b/app/views/auth_sources/_form_auth_source_ldap.html.erb
index d52e9790bdf9425e307880b2bf7bccab7a93f0d8..dc3f4109590fc9e0d6c5194c35e9464108b4ffc6 100644 (file)
--- a/app/views/auth_sources/_form_auth_source_ldap.html.erb
+++ b/app/views/auth_sources/_form_auth_source_ldap.html.erb
@@ -3,7 +3,11 @@
 <div class="box tabular">
   <p><%= f.text_field :name, :required => true %></p>
   <p><%= f.text_field :host, :required => true %></p>
-  <p><%= f.text_field :port, :required => true, :size => 6 %> <%= f.check_box :tls, :no_label => true %> LDAPS</p>
+  <p>
+    <%= f.text_field :port, :required => true, :size => 6 %>
+    <%= f.select :ldap_mode, AuthSourceLdap::LDAP_MODES.map { |m| [l("label_#{m}"), m] }, :no_label => true %>
+    <em class="info ldaps_warning"><%= l("label_ldaps_warning") %></em>
+  </p>
   <p><%= f.text_field :account %></p>
   <p><%= f.password_field :account_password, :label => :field_password,
            :name => 'dummy_password',

diff --git a/config/locales/de.yml b/config/locales/de.yml
index 13be9942165a25e17fac0a6ec01dd7a50142a404..57b11e22eda1ca294ecad5394bb26414ad7bdfaa 100644 (file)
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -608,7 +608,11 @@ de:
   label_latest_compatible_version: Letzte kompatible Version
   label_latest_revision: Aktuellste Revision
   label_latest_revision_plural: Aktuellste Revisionen
+  label_ldap: LDAP
   label_ldap_authentication: LDAP-Authentifizierung
+  label_ldaps_verify_none: LDAPS (ohne Zertifikatsprüfung)
+  label_ldaps_verify_peer: LDAPS
+  label_ldaps_warning: Es wird empfohlen, eine verschlüsselte LDAPS-Verbindung mit Zertifikatsprüfung zu verwenden, um Manipulationen während der Authentifizierung zu verhindern.
   label_less_or_equal: "<="
   label_less_than_ago: vor weniger als
   label_link: Link

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 99ebbf72fe8116f4e5af5e365f55173fdadba8fb..6bf07ff34df7c58f9af6d09c5dd19db251ae70b6 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -892,7 +892,11 @@ en:
   label_general: General
   label_scm: SCM
   label_plugins: Plugins
+  label_ldap: LDAP
   label_ldap_authentication: LDAP authentication
+  label_ldaps_verify_none: LDAPS (without certificate check)
+  label_ldaps_verify_peer: LDAPS
+  label_ldaps_warning: It is recommended to use an encrypted LDAPS connection with certificate check to prevent any manipulation during the authentication process.
   label_downloads_abbr: D/L
   label_optional_description: Optional description
   label_add_another_file: Add another file

diff --git a/db/migrate/20180913072918_add_verify_peer_to_auth_sources.rb b/db/migrate/20180913072918_add_verify_peer_to_auth_sources.rb
new file mode 100644 (file)
index 0000000..b36564c
--- /dev/null
+++ b/db/migrate/20180913072918_add_verify_peer_to_auth_sources.rb
@@ -0,0 +1,7 @@
+class AddVerifyPeerToAuthSources < ActiveRecord::Migration[5.2]
+  def change
+    change_table :auth_sources do |t|
+      t.boolean :verify_peer, default: true, null: false
+    end
+  end
+end

diff --git a/public/javascripts/application.js b/public/javascripts/application.js
index c8a7df1fab1adf42a0a125aede88482d67c1c41a..0239e034349a9973782b00155f853f1fe1d5c4d1 100644 (file)
--- a/public/javascripts/application.js
+++ b/public/javascripts/application.js
@@ -857,6 +857,12 @@ function keepAnchorOnSignIn(form){
   return true;
 }
 
+$(function ($) {
+  $('#auth_source_ldap_mode').change(function () {
+    $('.ldaps_warning').toggle($(this).val() != 'ldaps_verify_peer');
+  }).change();
+});
+
 $(document).ready(setupAjaxIndicator);
 $(document).ready(hideOnLoad);
 $(document).ready(addFormObserversForDoubleSubmit);

diff --git a/test/unit/auth_source_ldap_test.rb b/test/unit/auth_source_ldap_test.rb
index 32feda25b904cef745cad6285a2c5cfe6b8feace..f6eefed0268a7aff5286ecff7c27267a9b94ea8c 100644 (file)
--- a/test/unit/auth_source_ldap_test.rb
+++ b/test/unit/auth_source_ldap_test.rb
@@ -40,6 +40,8 @@ class AuthSourceLdapTest < ActiveSupport::TestCase
     assert_nil auth_source.attr_mail
     assert_equal false, auth_source.onthefly_register
     assert_equal false, auth_source.tls
+    assert_equal true, auth_source.verify_peer
+    assert_equal :ldap, auth_source.ldap_mode
     assert_nil auth_source.filter
     assert_nil auth_source.timeout
   end
@@ -77,6 +79,42 @@ class AuthSourceLdapTest < ActiveSupport::TestCase
     assert a.valid?
   end
 
+  test 'ldap_mode setter sets tls and verify_peer' do
+    a = AuthSourceLdap.new
+
+    a.ldap_mode = 'ldaps_verify_peer'
+    assert a.tls
+    assert a.verify_peer
+
+    a.ldap_mode = 'ldaps_verify_none'
+    assert a.tls
+    assert !a.verify_peer
+
+    a.ldap_mode = 'ldap'
+    assert !a.tls
+    assert !a.verify_peer
+  end
+
+  test 'ldap_mode getter reads from tls and verify_peer' do
+    a = AuthSourceLdap.new
+
+    a.tls = true
+    a.verify_peer = true
+    assert_equal :ldaps_verify_peer, a.ldap_mode
+
+    a.tls = true
+    a.verify_peer = false
+    assert_equal :ldaps_verify_none, a.ldap_mode
+
+    a.tls = false
+    a.verify_peer = false
+    assert_equal :ldap, a.ldap_mode
+
+    a.tls = false
+    a.verify_peer = true
+    assert_equal :ldap, a.ldap_mode
+  end
+
   if ldap_configured?
     test '#authenticate with a valid LDAP user should return the user attributes' do
       auth = AuthSourceLdap.find(1)