From c5fd6d94adaaecd0f1333be5f22686c86a3c05cf Mon Sep 17 00:00:00 2001 From: Gregor Schmidt Date: Thu, 13 Sep 2018 12:47:06 +0200 Subject: [PATCH] Allow unchecked LDAPS TLS connections --- app/models/auth_source.rb | 1 + app/models/auth_source_ldap.rb | 46 ++++++++++++++++++- .../_form_auth_source_ldap.html.erb | 6 ++- config/locales/de.yml | 4 ++ config/locales/en.yml | 4 ++ ...3072918_add_verify_peer_to_auth_sources.rb | 7 +++ public/javascripts/application.js | 6 +++ test/unit/auth_source_ldap_test.rb | 38 +++++++++++++++ 8 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20180913072918_add_verify_peer_to_auth_sources.rb 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 @@

<%= f.text_field :name, :required => true %>

<%= f.text_field :host, :required => true %>

-

<%= f.text_field :port, :required => true, :size => 6 %> <%= f.check_box :tls, :no_label => true %> LDAPS

+

+ <%= f.text_field :port, :required => true, :size => 6 %> + <%= f.select :ldap_mode, AuthSourceLdap::LDAP_MODES.map { |m| [l("label_#{m}"), m] }, :no_label => true %> + <%= l("label_ldaps_warning") %> +

<%= f.text_field :account %>

<%= 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..08342f12a 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 mit Zertifikatsprüfung + label_ldaps_warning: Es wird empfohlen, eine verschlüsselte LDAPS mit Zertifikatsprüfung zu verwenden, um Manipulationen an der Authorisierung 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 d8f09431f..750ead65f 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 with certificate check + label_ldaps_warning: It is recommended to use an encrypted LDAPS connection with certificate check to hamper any manipulation of the authorization 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) -- 2.18.0