Merge remote-tracking branch 'origin/v1.0.0-saml_lib' into feature/up…

…date_slo_endpoint_parser

.github/workflows/ci.yml

@@ -10,34 +10,50 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        ruby: ['2.5', '2.6', '2.7', '3.0', '3.1', '3.2']
-        gemfile: [rails_5.2.gemfile, rails_6.1.gemfile, rails_7.0.gemfile, rails_dev.gemfile]
+        ruby: ['2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3']
+        gemfile: [rails_5.2.gemfile, rails_6.1.gemfile, rails_7.0.gemfile, rails_7.1.gemfile, rails_7.2.gemfile, rails_dev.gemfile]
         exclude:
+          # Ruby 3.2 is min version for Rails 8
+          - ruby: '2.5'
+            gemfile: rails_dev.gemfile
+          - ruby: '2.6'
+            gemfile: rails_dev.gemfile
+          - ruby: '2.7'
+            gemfile: rails_dev.gemfile
+          - ruby: '3.0'
+            gemfile: rails_dev.gemfile
+          - ruby: '3.1'
+            gemfile: rails_dev.gemfile
           - ruby: '2.5'
             gemfile: rails_7.0.gemfile
           - ruby: '2.5'
-            gemfile: rails_dev.gemfile
+            gemfile: rails_7.1.gemfile
           - ruby: '2.6'
             gemfile: rails_7.0.gemfile
           - ruby: '2.6'
-            gemfile: rails_dev.gemfile
+            gemfile: rails_7.1.gemfile
+          # Ruby 3.1 is min version for Rails 7.2
+          - ruby: '2.5'
+            gemfile: rails_7.2.gemfile
+          - ruby: '2.6'
+            gemfile: rails_7.2.gemfile
           - ruby: '2.7'
-            gemfile: rails_dev.gemfile
+            gemfile: rails_7.2.gemfile
           - ruby: '3.0'
-            gemfile: rails_5.2.gemfile
+            gemfile: rails_7.2.gemfile
           - ruby: '3.0'
-            gemfile: rails_dev.gemfile
+            gemfile: rails_5.2.gemfile
           - ruby: '3.1'
             gemfile: rails_5.2.gemfile
           - ruby: '3.2'
             gemfile: rails_5.2.gemfile
-          - ruby: '3.2'
-            gemfile: rails_6.1.gemfile
+          - ruby: '3.3'
+            gemfile: rails_5.2.gemfile
     runs-on: ubuntu-latest
     env:
       BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }}
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: ${{ matrix.ruby }}

.ruby-version

@@ -1 +1 @@
-3.1.2
+3.3.5

Appraisals

@@ -13,6 +13,11 @@ appraise 'rails-7.0' do
   gem 'activeresource', '~> 6.0.0'
 end
 
+appraise 'rails-7.1' do
+  gem 'rails', '~> 7.1.0'
+  gem 'activeresource', '~> 6.0.0'
+end
+
 appraise 'rails-dev' do
   gem 'rails', :github => 'rails/rails', :branch => 'main'
   gem 'activeresource', :github => 'rails/activeresource', :branch => 'main'

README.md

@@ -24,9 +24,13 @@ Add this to your Gemfile:
 
 Include `SamlIdp::Controller` and see the examples that use rails. It should be straightforward for you.
 
-Basically you call `decode_request(params[:SAMLRequest])` on an incoming request and then use the value
-`saml_acs_url` to determine the source for which you need to authenticate a user. How you authenticate
-a user is entirely up to you.
+Basically, you call `decode_request(params[:SAMLRequest])` on an incoming request and then use the value 
+`saml_acs_url` to determine the source for which you need to authenticate a user. 
+If the signature (`Signature`) and signing algorithm (`SigAlg`) are provided as external parameters in the request,
+you can pass those parameters as `decode_request(params[:SAMLRequest], params[:Signature], params[:SigAlg], params[:RelayState])`.
+Then, you can verify the request signature with the `valid?` method.
+
+How you authenticate a user is entirely up to you.
 
 Once a user has successfully authenticated on your system send the Service Provider a SAMLResponse by
 posting to `saml_acs_url` the parameter `SAMLResponse` with the return value from a call to
@@ -74,6 +78,11 @@ KEY DATA
 -----END RSA PRIVATE KEY-----
 CERT
 
+  # x509_certificate, secret_key, and password may also be set from within a proc, for example:
+  # config.x509_certificate = -> { File.read("cert.pem") }
+  # config.secret_key = -> { SecretKeyFinder.key_for(id: 1) }
+  # config.password = -> { "password" }
+
   # config.password = "secret_key_password"
   # config.algorithm = :sha256                                    # Default: sha1 only for development.
   # config.organization_name = "Your Organization"

gemfiles/rails_7.1.gemfile

@@ -0,0 +1,8 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "rails", "~> 7.1.0"
+gem "activeresource", "~> 6.0.0"
+
+gemspec path: "../"

gemfiles/rails_7.2.gemfile

@@ -0,0 +1,8 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "rails", "~> 7.2.0"
+gem 'activeresource', '~> 6.1', '>= 6.1.3'
+
+gemspec path: "../"

lib/saml_idp.rb

@@ -9,7 +9,7 @@ module SamlIdp
   require 'saml_idp/metadata_builder'
   require 'saml_idp/version'
   require 'saml_idp/fingerprint'
-  require 'saml_idp/engine' if defined?(::Rails)
+  require 'saml_idp/engine' if defined?(::Rails::Engine)
 
   def self.config
     @config ||= SamlIdp::Configurator.new
@@ -70,9 +70,9 @@ def signed?
         !!xpath("//ds:Signature", ds: signature_namespace).first
       end
 
-      def valid_signature?(fingerprint)
+      def valid_signature?(certificate, fingerprint)
         signed? &&
-          signed_document.validate(fingerprint, :soft)
+          signed_document.validate(certificate, fingerprint, :soft)
       end
 
       def signed_document

lib/saml_idp/configurator.rb

@@ -25,8 +25,8 @@ class Configurator
     attr_accessor :logger
 
     def initialize
-      self.x509_certificate = Default::X509_CERTIFICATE
-      self.secret_key = Default::SECRET_KEY
+      self.x509_certificate = -> { Default::X509_CERTIFICATE }
+      self.secret_key = -> { Default::SECRET_KEY }
       self.algorithm = :sha1
       self.reference_id_generator = ->() { SecureRandom.uuid }
       self.service_provider = OpenStruct.new
@@ -35,7 +35,7 @@ def initialize
       self.service_provider.persisted_metadata_getter = ->(id, service_provider) {  }
       self.session_expiry = 0
       self.attributes = {}
-      self.logger = defined?(::Rails) ? Rails.logger : ->(msg) { puts msg }
+      self.logger = (defined?(::Rails) && Rails.respond_to?(:logger)) ? Rails.logger : ->(msg) { puts msg }
     end
 
     # formats

lib/saml_idp/controller.rb

@@ -33,15 +33,21 @@ def acs_url
     end
 
     def validate_saml_request(raw_saml_request = params[:SAMLRequest])
-      decode_request(raw_saml_request)
+      decode_request(raw_saml_request, params[:Signature], params[:SigAlg], params[:RelayState])
       return true if valid_saml_request?
 
       head :forbidden if defined?(::Rails)
       false
     end
 
-    def decode_request(raw_saml_request)
-      @saml_request = Request.from_deflated_request(raw_saml_request)
+    def decode_request(raw_saml_request, signature, sig_algorithm, relay_state)
+      @saml_request = Request.from_deflated_request(
+        raw_saml_request,
+        saml_request: raw_saml_request,
+        signature: signature,
+        sig_algorithm: sig_algorithm,
+        relay_state: relay_state
+      )
     end
 
     def authn_context_classref
@@ -81,8 +87,8 @@ def encode_authn_response(principal, opts = {})
         session_expiry,
         name_id_formats_opts,
         asserted_attributes_opts,
-        signed_assertion_opts,
         signed_message_opts,
+        signed_assertion_opts,
         compress_opts
       ).build
     end

lib/saml_idp/incoming_metadata.rb

@@ -63,6 +63,15 @@ def contact_person
     end
     hashable :contact_person
 
+    def unspecified_certificate
+      xpath(
+        "//md:SPSSODescriptor/md:KeyDescriptor[not(@use)]/ds:KeyInfo/ds:X509Data/ds:X509Certificate",
+        ds: signature_namespace,
+        md: metadata_namespace
+      ).first.try(:content).to_s
+    end
+    hashable :unspecified_certificate
+
     def signing_certificate
       xpath(
         "//md:SPSSODescriptor/md:KeyDescriptor[@use='signing']/ds:KeyInfo/ds:X509Data/ds:X509Certificate",

lib/saml_idp/metadata_builder.rb

@@ -152,7 +152,8 @@ def raw_algorithm
     private :raw_algorithm
 
     def x509_certificate
-      SamlIdp.config.x509_certificate
+      certificate = SamlIdp.config.x509_certificate.is_a?(Proc) ? SamlIdp.config.x509_certificate.call : SamlIdp.config.x509_certificate
+      certificate
       .to_s
       .gsub(/-----BEGIN CERTIFICATE-----/,"")
       .gsub(/-----END CERTIFICATE-----/,"")

lib/saml_idp/request.rb

@@ -3,7 +3,9 @@
 require 'logger'
 module SamlIdp
   class Request
-    def self.from_deflated_request(raw)
+    attr_accessor :errors
+
+    def self.from_deflated_request(raw, external_attributes = {})
       if raw
         decoded = Base64.decode64(raw)
         zstream = Zlib::Inflate.new(-Zlib::MAX_WBITS)
@@ -18,18 +20,23 @@ def self.from_deflated_request(raw)
       else
         inflated = ""
       end
-      new(inflated)
+      new(inflated, external_attributes)
     end
 
-    attr_accessor :raw_xml
+    attr_accessor :raw_xml, :saml_request, :signature, :sig_algorithm, :relay_state
 
     delegate :config, to: :SamlIdp
     private :config
     delegate :xpath, to: :document
     private :xpath
 
-    def initialize(raw_xml = "")
+    def initialize(raw_xml = "", external_attributes = {})
       self.raw_xml = raw_xml
+      self.saml_request = external_attributes[:saml_request]
+      self.relay_state = external_attributes[:relay_state]
+      self.sig_algorithm = external_attributes[:sig_algorithm]
+      self.signature = external_attributes[:signature]
+      self.errors = []
     end
 
     def logout_request?
@@ -85,30 +92,53 @@ def log(msg)
       end
     end
 
-    def valid?
+    def collect_errors(error_type)
+      errors.push(error_type)
+    end
+
+    def valid?(external_attributes = {})
       unless service_provider?
         log "Unable to find service provider for issuer #{issuer}"
+        collect_errors(:sp_not_found)
         return false
       end
 
       unless (authn_request? ^ logout_request?)
         log "One and only one of authnrequest and logout request is required. authnrequest: #{authn_request?} logout_request: #{logout_request?} "
+        collect_errors(:unaccepted_request)
         return false
       end
 
-      unless valid_signature?
-        log "Signature is invalid in #{raw_xml}"
+      if (logout_request? || validate_auth_request_signature?) && (service_provider.cert.to_s.empty? || !!service_provider.fingerprint.to_s.empty?)
+        log "Verifying request signature is required. But certificate and fingerprint was empty."
+        collect_errors(:empty_certificate)
+        return false
+      end
+
+      # XML embedded signature
+      if signature.nil? && !valid_signature?
+        log "Requested document signature is invalid in #{raw_xml}"
+        collect_errors(:invalid_embedded_signature)
+        return false
+      end
+
+      # URI query signature
+      if signature.present? && !valid_external_signature?
+        log "Requested URI signature is invalid in #{raw_xml}"
+        collect_errors(:invalid_external_signature)
         return false
       end
 
       if response_url.nil?
         log "Unable to find response url for #{issuer}: #{raw_xml}"
+        collect_errors(:empty_response_url)
         return false
       end
 
       if !service_provider.acceptable_response_hosts.include?(response_host)
         log "#{service_provider.acceptable_response_hosts} compare to #{response_host}"
         log "No acceptable AssertionConsumerServiceURL, either configure them via config.service_provider.response_hosts or match to your metadata_url host"
+        collect_errors(:not_allowed_host)
         return false
       end
 
@@ -117,15 +147,39 @@ def valid?
 
     def valid_signature?
       # Force signatures for logout requests because there is no other protection against a cross-site DoS.
-      # Validate signature when metadata specify AuthnRequest should be signed
-      metadata = service_provider.current_metadata
-      if logout_request? || authn_request? && metadata.respond_to?(:sign_authn_request?) && metadata.sign_authn_request?
-        document.valid_signature?(service_provider.fingerprint)
+      if logout_request? || authn_request? && validate_auth_request_signature?
+        document.valid_signature?(service_provider.cert, service_provider.fingerprint)
       else
         true
       end
     end
 
+    def valid_external_signature?
+      return true if authn_request? && !validate_auth_request_signature?
+
+      cert = OpenSSL::X509::Certificate.new(service_provider.cert)
+
+      sha_version = sig_algorithm =~ /sha(.*?)$/i && $1.to_i
+      raw_signature = Base64.decode64(signature)
+
+      signature_algorithm = case sha_version
+      when 256 then OpenSSL::Digest::SHA256
+      when 384 then OpenSSL::Digest::SHA384
+      when 512 then OpenSSL::Digest::SHA512
+      else
+        OpenSSL::Digest::SHA1
+      end
+
+      result = cert.public_key.verify(signature_algorithm.new, raw_signature, query_request_string)
+      # Match all percent-encoded sequences (e.g., %20, %2B) and convert them to lowercase
+      # Upper case is recommended for consistency but some services such as MS Entra Id not follows it
+      # https://datatracker.ietf.org/doc/html/rfc3986#section-2.1
+      result || cert.public_key.verify(signature_algorithm.new, raw_signature, query_request_string.gsub(/%[A-F0-9]{2}/) { |match| match.downcase })
+    rescue OpenSSL::X509::CertificateError => e
+      log e.message
+      collect_errors(:cert_format_error)
+    end
+
     def service_provider?
       service_provider && service_provider.valid?
     end
@@ -148,6 +202,13 @@ def session_index
       @_session_index ||= xpath("//samlp:SessionIndex", samlp: samlp).first.try(:content)
     end
 
+    def query_request_string
+      url_string = "SAMLRequest=#{CGI.escape(saml_request)}"
+      url_string << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
+      url_string << "&SigAlg=#{CGI.escape(sig_algorithm)}"
+    end
+    private :query_request_string
+
     def response_host
       uri = URI(response_url)
       if uri
@@ -197,5 +258,14 @@ def service_provider_finder
       config.service_provider.finder
     end
     private :service_provider_finder
+
+    def validate_auth_request_signature?
+      # Validate signature when metadata specify AuthnRequest should be signed
+      metadata = service_provider.current_metadata
+      sign_authn_request = metadata.respond_to?(:sign_authn_request?) && metadata.sign_authn_request?
+      sign_authn_request = service_provider.sign_authn_request unless service_provider.sign_authn_request.nil? 
+      sign_authn_request
+    end
+    private :validate_auth_request_signature?
   end
 end