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

8 files changed, 109 insertions, 3 deletions

diff --git a/app/models/auth_source.rb b/app/models/auth_source.rb
index c28a33fb2..8b745f33f 100644
--- 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 8e380d456..c3939e8d5 100644
--- 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 d52e9790b..dc3f41095 100644
--- 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 13be99421..57b11e22e 100644
--- 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 99ebbf72f..6bf07ff34 100644
--- 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
index 000000000..b36564c10
--- /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 c8a7df1fa..0239e0343 100644
--- 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 32feda25b..f6eefed02 100644
--- 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)