Add new LDAP option for certificate verification

app/models/ldap_auth_source.rb

@@ -29,7 +29,11 @@
 require 'net/ldap'
 
 class LdapAuthSource < AuthSource
-  enum tls_mode: %w[plain_ldap simple_tls start_tls], _default: 'start_tls'
+  enum tls_mode: {
+    plain_ldap: 0,
+    simple_tls: 1,
+    start_tls: 2
+  }.freeze, _default: :start_tls
   validates :tls_mode, inclusion: { in: tls_modes.keys }
 
   validates_presence_of :host, :port, :attr_login
@@ -39,9 +43,10 @@ class LdapAuthSource < AuthSource
   validates_numericality_of :port, only_integer: true
 
   validate :validate_filter_string
+  validate :validate_tls_certificate_string, if: -> { tls_certificate_string.present? }
 
-  before_validation :strip_ldap_attributes
   after_initialize :set_default_port
+  before_validation :strip_ldap_attributes
 
   def authenticate(login, password)
     return nil if login.blank? || password.blank?
@@ -147,6 +152,10 @@ class LdapAuthSource < AuthSource
   end
 
   def initialize_ldap_con(ldap_user, ldap_password)
+    unless plain_ldap? || verify_peer?
+      Rails.logger.info { "SSL connection to LDAP host #{host} is set up to skip certificate verification." }
+    end
+
     options = { host: host,
                 port: port,
                 force_no_page: OpenProject::Configuration.ldap_force_no_page,
@@ -159,14 +168,44 @@ class LdapAuthSource < AuthSource
   end
 
   def ldap_encryption
-    return nil if tls_mode.to_s == 'plain_ldap'
+    return nil if plain_ldap?
 
     {
       method: tls_mode.to_sym,
-      tls_options: Setting.ldap_tls_options.with_indifferent_access
+      tls_options:,
     }
   end
 
+  def cert_store
+    @cert_store ||= OpenSSL::X509::Store.new.tap do |store|
+      store.set_default_paths
+      provided_certs = Array(read_ldap_certificates)
+      provided_certs.each { |cert| store.add_cert cert }
+    end
+  end
+
+  def tls_options
+    {
+      verify_mode: tls_verify_mode,
+      cert_store:
+    }.compact
+  end
+
+  def read_ldap_certificates
+    return if tls_certificate_string.blank?
+
+    # Using load will allow multiple PEM certificates to be passed
+    OpenSSL::X509::Certificate.load(tls_certificate_string)
+  end
+
+  def tls_verify_mode
+    if verify_peer?
+      OpenSSL::SSL::VERIFY_PEER
+    else
+      OpenSSL::SSL::VERIFY_NONE
+    end
+  end
+
   # Check if a DN (user record) authenticates with the password
   def authenticate_dn(dn, password)
     if dn.present? && password.present?
@@ -215,4 +254,10 @@ class LdapAuthSource < AuthSource
   rescue Net::LDAP::FilterSyntaxInvalidError
     errors.add :filter_string, :invalid
   end
+
+  def validate_tls_certificate_string
+    read_ldap_certificates
+  rescue OpenSSL::X509::CertificateError => e
+    errors.add :tls_certificate_string, :invalid_certificate, additional_message: e.message
+  end
 end

app/models/permitted_params.rb

@@ -439,6 +439,8 @@ class PermittedParams
           attr_lastname
           attr_mail
           attr_admin
+          verify_peer
+          tls_certificate_string
         ),
         forum: %i(
           name

app/views/ldap_auth_sources/_form.html.erb

@@ -62,7 +62,7 @@ See COPYRIGHT and LICENSE files for more details.
     <legend class="form--fieldset-legend"><%= t('ldap_auth_sources.connection_encryption') %></legend>
     <p>
       <%= t 'ldap_auth_sources.tls_mode.section_more_info_link_html',
-            link: OpenProject::Static::Links[:ldap_encryption_documentation][:href]  %>
+            link: OpenProject::Static::Links[:ldap_encryption_documentation][:href] %>
     </p>
     <div class="form--field">
       <%= f.radio_button :tls_mode,
@@ -74,10 +74,10 @@ See COPYRIGHT and LICENSE files for more details.
       </p>
     </div>
     <div class="form--field">
-        <%= f.radio_button :tls_mode,
-                          'simple_tls',
-                          label: t('ldap_auth_sources.tls_mode.simple_tls'),
-                          container_class: '-wide' %>
+      <%= f.radio_button :tls_mode,
+                         'simple_tls',
+                         label: t('ldap_auth_sources.tls_mode.simple_tls'),
+                         container_class: '-wide' %>
       <p class="form--field-instructions">
         <%= t('ldap_auth_sources.tls_mode.simple_tls_description') %>
       </p>
@@ -92,12 +92,33 @@ See COPYRIGHT and LICENSE files for more details.
       </p>
     </div>
   </fieldset>
+  <fieldset class="form--fieldset">
+    <legend class="form--fieldset-legend"><%= t('ldap_auth_sources.connection_encryption') %></legend>
+    <div class="form--field">
+      <%= f.check_box :verify_peer,
+                      label: t('ldap_auth_sources.tls_options.verify_peer'),
+                      container_class: '-wide' %>
+      <p class="form--field-instructions">
+        <%= t('ldap_auth_sources.tls_options.verify_peer_description_html') %>
+      </p>
+    </div>
+    <div class="form--field">
+      <%= f.text_area :tls_certificate_string,
+                      label: AuthSource.human_attribute_name(:tls_certificate_string),
+                      placeholder: "-----BEGIN CERTIFICATE-----\n ..... \n-----END CERTIFICATE-----",
+                      rows: 10,
+                      container_class: '-wide' %>
+      <div class="form--field-instructions">
+        <p><%= t('ldap_auth_sources.tls_options.tls_certificate_description') %></p>
+      </div>
+    </div>
+  </fieldset>
   <fieldset class="form--fieldset">
     <legend class="form--fieldset-legend"><%= t('ldap_auth_sources.system_account') %></legend>
     <p><%= t 'ldap_auth_sources.system_account_legend' %></p>
 
     <div class="form--field">
-      <%= f.text_field 'account', container_class: '-middle'  %>
+      <%= f.text_field 'account', container_class: '-middle' %>
       <span class="form--field-instructions">
         <%= t('ldap_auth_sources.attribute_texts.system_user_dn_html') %>
       </span>
@@ -105,7 +126,7 @@ See COPYRIGHT and LICENSE files for more details.
     <div class="form--field">
       <%= f.password_field 'account_password',
                            label: AuthSource.human_attribute_name(:password),
-                           placeholder: ((@auth_source.new_record? || @auth_source.account_password.blank?) ? '' : ('●'*15)),
+                           placeholder: ((@auth_source.new_record? || @auth_source.account_password.blank?) ? '' : ('●' * 15)),
                            autocomplete: 'off',
                            container_class: '-middle' %>
       <span class="form--field-instructions">
@@ -116,7 +137,7 @@ See COPYRIGHT and LICENSE files for more details.
 </section>
 
 <fieldset class="form--fieldset">
-  <legend class="form--fieldset-legend"><%=t('ldap_auth_sources.ldap_details')%></legend>
+  <legend class="form--fieldset-legend"><%= t('ldap_auth_sources.ldap_details') %></legend>
   <div class="form--field">
     <%= f.text_field :base_dn, size: 60, container_class: '-wide' %>
     <span class="form--field-instructions">
@@ -142,7 +163,7 @@ See COPYRIGHT and LICENSE files for more details.
 </fieldset>
 
 <fieldset class="form--fieldset">
-  <legend class="form--fieldset-legend"><%=t('ldap_auth_sources.user_settings')%></legend>
+  <legend class="form--fieldset-legend"><%= t('ldap_auth_sources.user_settings') %></legend>
   <p><%= t 'ldap_auth_sources.user_settings_legend' %></p>
 
   <div class="form--field -required">
@@ -151,7 +172,7 @@ See COPYRIGHT and LICENSE files for more details.
                      required: true,
                      size: 20,
                      placeholder: 'uid',
-                     container_class: '-middle'  %>
+                     container_class: '-middle' %>
     <span class="form--field-instructions">
       <%= t('ldap_auth_sources.attribute_texts.login_map') %>
     </span>
@@ -161,7 +182,7 @@ See COPYRIGHT and LICENSE files for more details.
                      label: AuthSource.human_attribute_name(:firstname),
                      size: 20,
                      placeholder: 'givenName',
-                     container_class: '-middle'  %>
+                     container_class: '-middle' %>
     <span class="form--field-instructions">
       <%= t('ldap_auth_sources.attribute_texts.generic_map', attribute: AuthSource.human_attribute_name(:firstname)) %>
     </span>
@@ -171,7 +192,7 @@ See COPYRIGHT and LICENSE files for more details.
                      label: AuthSource.human_attribute_name(:lastname),
                      size: 20,
                      placeholder: 'sn',
-                     container_class: '-middle'  %>
+                     container_class: '-middle' %>
     <span class="form--field-instructions">
       <%= t('ldap_auth_sources.attribute_texts.generic_map', attribute: AuthSource.human_attribute_name(:lastname)) %>
     </span>
@@ -181,13 +202,13 @@ See COPYRIGHT and LICENSE files for more details.
                      label: AuthSource.human_attribute_name(:mail),
                      size: 20,
                      placeholder: 'mail',
-                     container_class: '-middle'  %>
+                     container_class: '-middle' %>
     <span class="form--field-instructions">
       <%= t('ldap_auth_sources.attribute_texts.generic_map', attribute: AuthSource.human_attribute_name(:mail)) %>
     </span>
   </div>
   <div class="form--field">
-    <%= f.text_field 'attr_admin', label: AuthSource.human_attribute_name(:admin), size: 20, container_class: '-middle'  %>
+    <%= f.text_field 'attr_admin', label: AuthSource.human_attribute_name(:admin), size: 20, container_class: '-middle' %>
     <span class="form--field-instructions">
       <%= t('ldap_auth_sources.attribute_texts.admin_map_html') %>
     </span>

config/locales/en.yml

@@ -153,7 +153,7 @@ en:
         Leave this unchecked to only allow existing accounts in OpenProject to authenticate through LDAP!
 
     connection_encryption: 'Connection encryption'
-    encryption_details: 'Encryption details'
+    encryption_details: 'LDAPS / STARTTLS options'
     system_account: 'System account'
     system_account_legend: |
       OpenProject requires read-only access through a system account to lookup users and groups in your LDAP tree.
@@ -175,9 +175,12 @@ en:
         For more information, visit <a href="%{link}">the Net::LDAP documentation</a>.
     tls_options:
       verify_peer: "Verify SSL certificate"
-      verify_peer_description: "Enables strict SSL verification of the certificate trusted chain."
-      verify_hostname: "Verify certificate hostname"
-      verify_hostname_description: "Verifies the SSL certificate hostname"
+      verify_peer_description_html: >
+        Enables strict SSL verification of the certificate trusted chain.
+        <br/>
+        <strong>Warning:</strong> Unchecking this option disables SSL verification of the LDAP server certificate.
+        This exposes your connection to Man in the Middle attacks.
+      tls_certificate_description: "If the LDAP server certificate is not in the trust sources of this system, you can add it manually here. Enter a PEM X509 certifiate string."
   forums:
     show:
       no_results_title_text: There are currently no posts for the forum.
@@ -488,6 +491,7 @@ en:
         host: "Host"
         onthefly: "Automatic user creation"
         port: "Port"
+        tls_certificate_string: "LDAP server SSL certificate"
       changeset:
         repository: "Repository"
       comment:
@@ -697,6 +701,11 @@ en:
           is not providing a "Secure Context". Either use HTTPS or a loopback address, such as localhost.
         wrong_length: "is the wrong length (should be %{count} characters)."
       models:
+        auth_source:
+          attributes:
+            tls_certificate_string:
+              invalid_certificate: "The provides SSL certificate is invalid: %{additional_message}"
+              format: "%{message}"
         attachment:
           attributes:
             content_type:

db/migrate/20221115082403_add_ldap_tls_options.rb

@@ -0,0 +1,18 @@
+class AddLdapTlsOptions < ActiveRecord::Migration[7.0]
+  def change
+    change_table :auth_sources, bulk: true do |t|
+      t.boolean :verify_peer, default: true, null: false
+      t.text :tls_certificate_string
+    end
+
+    reversible do |dir|
+      dir.up do
+        # Current LDAP library default is to not verify the certificate
+        LdapAuthSource.reset_column_information
+        ldap_settings = (Setting.ldap_tls_options || {}).with_indifferent_access
+        verify_peer = ldap_settings[:verify_mode] == OpenSSL::SSL::VERIFY_PEER
+        LdapAuthSource.update_all(verify_peer:)
+      end
+    end
+  end
+end