Fixes zammad#4558 - SAML request signing/encrypting.

Co-authored-by: Tobias Schäfer <[email protected]>
Co-authored-by: Florian Liebe <[email protected]>

.gitlab/ci/__includes__/services.yml

@@ -44,9 +44,12 @@
 
 .variables_auth:
   variables:
-    KEYCLOAK_BASE_URL: http://ci-service-auth:8080
-    KEYCLOAK_ADMIN: admin
-    KEYCLOAK_ADMIN_PASSWORD: admin
+    KEYCLOAK_BASE_URL: 'http://ci-service-auth:8080'
+    KEYCLOAK_CREATE_ADMIN_USER: 'true'
+    KEYCLOAK_ADMIN_USER: 'admin'
+    KEYCLOAK_ADMIN_PASSWORD: 'admin'
+    KEYCLOAK_DATABASE_VENDOR: 'dev-file'
+    KEYCLOAK_EXTRA_ARGS: '--import-realm'
 
 .variables_s3:
   variables:
@@ -98,7 +101,7 @@
     alias: ci-service-proxy
 
   auth:
-    name: $CI_REGISTRY/docker/zammad-auth:stable
+    name: $CI_REGISTRY/docker/zammad-auth:develop
     alias: ci-service-auth
 
   mattermost:

Gemfile

@@ -255,6 +255,10 @@ group :development, :test do
 
   # self-signed localhost certificates for puma / capybara
   gem 'localhost'
+
+  # Keycloak admin tool for setting up SAML auth tests
+  gem 'httparty'
+  gem 'keycloak-admin', git: 'https://github.com/tschaefer/ruby-keycloak-admin/', branch: 'develop', require: false
 end
 
 # To permanently extend Zammad with additional gems, you can specify them in Gemfile.local.

Gemfile.lock

@@ -1,3 +1,10 @@
+GIT
+  remote: https://github.com/tschaefer/ruby-keycloak-admin/
+  revision: 766702927ae9f2316f4468f8774dc7c018419b16
+  branch: develop
+  specs:
+    keycloak-admin (20.0.3)
+
 GIT
   remote: https://github.com/zammad-deps/autodiscover
   revision: ee9b53dfa797ce6d4f970b82beea7fbdd2df56bb
@@ -275,6 +282,9 @@ GEM
     http-parser (1.2.3)
       ffi-compiler (>= 1.0, < 2.0)
     http_parser.rb (0.6.0)
+    httparty (0.21.0)
+      mini_mime (>= 1.0.0)
+      multi_xml (>= 0.5.2)
     httpclient (2.8.3)
     i18n (1.14.1)
       concurrent-ruby (~> 1.0)
@@ -732,9 +742,11 @@ DEPENDENCIES
   graphql-batch
   hiredis
   htmlentities
+  httparty
   icalendar
   icalendar-recurrence
   json
+  keycloak-admin!
   koala
   listen
   localhost

app/models/setting.rb

@@ -5,6 +5,7 @@ class Setting < ApplicationModel
   store         :state_current
   store         :state_initial
   store         :preferences
+  before_validation :execute_validations
   before_create :state_check, :set_initial
   after_create  :reset_change_id, :reset_cache, :check_broadcast
   before_update :state_check
@@ -216,4 +217,18 @@ def check_refresh
 
     AppVersion.set(true, AppVersion::MSG_CONFIG_CHANGED)
   end
+
+  def execute_validations
+    return if preferences.blank? || preferences[:validations].blank?
+
+    preferences[:validations].each do |validation_module|
+      validation_result = validation_module.constantize.new(self).run
+
+      next if validation_result[:success]
+
+      errors.add(:base, :invalid, message: validation_result[:message])
+
+      return false
+    end
+  end
 end

app/models/setting/validation.rb

@@ -0,0 +1,20 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Setting::Validation
+  attr_reader :record, :value
+
+  def initialize(record)
+    @record = record
+    @value  = record.state_current.fetch('value', {})
+  end
+
+  private
+
+  def result_success
+    { success: true }
+  end
+
+  def result_failed(msg)
+    { success: false, message: msg }
+  end
+end

app/models/setting/validation/saml/required_attributes.rb

@@ -0,0 +1,23 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Setting::Validation::Saml::RequiredAttributes < Setting::Validation
+
+  REQUIRED_ATTRIBUTES = %i[idp_sso_target_url idp_slo_service_url idp_cert name_identifier_format].freeze
+
+  def run
+    return result_success if value.blank? || value.deep_symbolize_keys.keys.eql?([:display_name])
+
+    msg = check_prerequisites
+    return result_failed(msg) if !msg.nil?
+
+    result_success
+  end
+
+  private
+
+  def check_prerequisites
+    return "One of the required attributes #{REQUIRED_ATTRIBUTES.map { |e| "'#{e}'" }.join(', ')} is missing." if REQUIRED_ATTRIBUTES.any? { |key| !value.key?(key) || value[key].blank? }
+
+    nil
+  end
+end

app/models/setting/validation/saml/security.rb

@@ -0,0 +1,94 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Setting::Validation::Saml::Security < Setting::Validation
+
+  def run
+    return result_success if value.blank? || disabled_security?
+
+    %w[check_security_prerequisites check_private_key].each do |method|
+      msg = send(method)
+      next if msg.nil?
+
+      return result_failed(msg)
+    end
+
+    cert = read_certificate
+    return result_failed(__('The certificate could not be parsed.')) if cert.nil?
+
+    msg = check_certificate(cert)
+
+    {
+      success: msg.nil?,
+      message: msg,
+    }
+  end
+
+  private
+
+  def disabled_security?
+    value.fetch('security', 'off').eql?('off')
+  end
+
+  def check_security_prerequisites
+    return __('No certificate found.') if certificate_pem.blank?
+    return __('No private key found.') if private_key_pem.blank?
+
+    nil
+  end
+
+  def check_private_key
+    begin
+      private_key = OpenSSL::PKey.read(private_key_pem, private_key_secret)
+
+      return __('The type of the private key is wrong.') if !private_key.class.name.end_with?('RSA')
+      return __('The length of the private key is too short.') if private_key.n.num_bits < 2048
+    rescue => e
+      return e.message
+    end
+
+    nil
+  end
+
+  def read_certificate
+    begin
+      cert = Certificate::X509.new(certificate_pem)
+    rescue
+      return nil
+    end
+
+    cert
+  end
+
+  def check_certificate(cert)
+    return __('The certificate is not usable due to being a CA certificate.') if cert.ca?
+    return __('The certificate is not usable (e.g. expired).') if !cert.usable?
+    return __('The certificate is not usable for signing and encryption.') if !cert.signature? || !cert.encryption?
+
+    msg = check_cert_key_match(cert)
+    return msg if !msg.nil?
+
+    nil
+  end
+
+  def check_cert_key_match(cert)
+    begin
+      return __('The certificate does not match the given private key.') if !cert.key_match?(private_key_pem, private_key_secret)
+    rescue => e
+      return e.message
+    end
+
+    nil
+  end
+
+  def certificate_pem
+    @certificate_pem ||= value.fetch('certificate', '')
+  end
+
+  def private_key_pem
+    @private_key_pem ||= value.fetch('private_key', '')
+  end
+
+  def private_key_secret
+    @private_key_secret ||= value.fetch('private_key_secret', '')
+  end
+end

app/models/setting/validation/saml/tls.rb

@@ -0,0 +1,35 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Setting::Validation::Saml::TLS < Setting::Validation
+
+  def run
+    return result_success if value.blank?
+
+    msg = check_tls_verification
+    return result_failed(msg) if !msg.nil?
+
+    result_success
+  end
+
+  private
+
+  def check_tls_verification
+    return nil if !value[:ssl_verify]
+
+    url = value[:idp_sso_target_url]
+    return nil if !url.starts_with?('https://')
+
+    resp = UserAgent.get(
+      url,
+      {},
+      {
+        verify_ssl: true,
+        log:        { facility: 'SAML' }
+      }
+    )
+
+    return nil if resp.error.empty? || !resp.error.starts_with?('#<OpenSSL::SSL::SSLError')
+
+    __('The verification of the TLS connection failed. Please check the IDP certificate.')
+  end
+end

config/initializers/inflections.rb

@@ -27,4 +27,5 @@
   inflect.acronym 'PGP'
   inflect.acronym 'SMIME'
   inflect.acronym 'SSL'
+  inflect.acronym 'TLS'
 end

db/migrate/20231011055535_saml_sign_encrypt.rb

@@ -0,0 +1,105 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class SamlSignEncrypt < ActiveRecord::Migration[7.0]
+  def change
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    saml_setting = Setting.find_by(name: 'auth_saml_credentials')
+    return if !saml_setting
+
+    required_attributes(saml_setting)
+    fingerprint_help(saml_setting)
+    add_validations(saml_setting)
+    sign_and_encrypt_attributes(saml_setting)
+    check_ssl_verify(saml_setting)
+
+    saml_setting.save!
+  end
+
+  private
+
+  def required_attributes(saml_setting)
+    [1, 2, 3, 5].each do |idx|
+      saml_setting.options[:form][idx][:required] = true
+    end
+
+    true
+  end
+
+  def fingerprint_help(saml_setting)
+    saml_setting.options[:form][4][:help] = 'Please note that this attribute is deprecated within one of the next versions of Zammad. Use the IDP certificate instead.'
+
+    true
+  end
+
+  def add_validations(saml_setting)
+    saml_setting.preferences[:validations] = [
+      'Setting::Validation::Saml::RequiredAttributes',
+      'Setting::Validation::Saml::TLS',
+      'Setting::Validation::Saml::Security',
+    ]
+
+    true
+  end
+
+  def sign_and_encrypt_attributes(saml_setting)
+    saml_setting.options[:form].insert(-2, {
+                                         display: 'SSL verification',
+                                         null:    true,
+                                         name:    'ssl_verify',
+                                         tag:     'boolean',
+                                         options: {
+                                           true  => 'yes',
+                                           false => 'no',
+                                         },
+                                         default: true,
+                                         help:    'Turning off SSL verification is a security risk and should be used only temporary. Use this option at your own risk!',
+                                       },
+                                       {
+                                         display: 'Signing & Encrypting',
+                                         null:    true,
+                                         name:    'security',
+                                         tag:     'select',
+                                         options: {
+                                           'off'     => 'None',
+                                           'on'      => 'Signing & Encrypting',
+                                           'sign'    => 'Only Signing',
+                                           'encrypt' => 'Only Encrypting',
+                                         },
+                                       },
+                                       {
+                                         display:     'Certificate (PEM)',
+                                         null:        true,
+                                         name:        'certificate',
+                                         tag:         'textarea',
+                                         placeholder: '-----BEGIN CERTIFICATE-----\n...-----END CERTIFICATE-----',
+                                       },
+                                       {
+                                         display:     'Private key (PEM)',
+                                         null:        true,
+                                         name:        'private_key',
+                                         tag:         'textarea',
+                                         placeholder: '-----BEGIN RSA PRIVATE KEY-----\n...-----END RSA PRIVATE KEY-----', # gitleaks:allow
+                                       },
+                                       {
+                                         display:     'Private key secret',
+                                         null:        true,
+                                         name:        'private_key_secret',
+                                         tag:         'input',
+                                         type:        'password',
+                                         single:      true,
+                                         placeholder: '',
+                                       })
+
+    true
+  end
+
+  def check_ssl_verify(_saml_setting)
+    if Setting.get('auth_saml_credentials').present? && Setting.get('auth_saml')
+      Setting.set('auth_saml_credentials', Setting.get('auth_saml_credentials').merge(ssl_verify: false))
+    end
+
+    true
+  end
+end