Fix verification of SAML message signatures

Old behaviour is preserved if the correct (raw URI-encoded) parts
are not provided.

Author: jeffparsons

lib/onelogin/ruby-saml/slo_logoutrequest.rb

@@ -229,18 +229,48 @@ def validate_signature
         return true unless options.has_key? :get_params
         return true unless options[:get_params].has_key? 'Signature'
 
+        # SAML specifies that the signature should be derived from a concatenation
+        # of URI-encoded values _as sent by the IDP_:
+        #
+        # > Further, note that URL-encoding is not canonical; that is, there are multiple legal encodings for a given
+        # > value. The relying party MUST therefore perform the verification step using the original URL-encoded
+        # > values it received on the query string. It is not sufficient to re-encode the parameters after they have been
+        # > processed by software because the resulting encoding may not match the signer's encoding.
+        #
+        # <http://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf>
+        #
+        # If we don't have the original parts (for backward compatibility) required to correctly verify the signature,
+        # then fabricate them by re-encoding the parsed URI parameters, and hope that we're lucky enough to use
+        # the exact same URI-encoding as the IDP. (This is not the case if the IDP is ADFS!)
+        options[:raw_get_params] ||= {}
+        if options[:raw_get_params]['SAMLRequest'].nil? && !options[:get_params]['SAMLRequest'].nil?
+          options[:raw_get_params]['SAMLRequest'] = CGI.escape(options[:get_params]['SAMLRequest'])
+        end
+        if options[:raw_get_params]['RelayState'].nil? && !options[:get_params]['RelayState'].nil?
+          options[:raw_get_params]['RelayState'] = CGI.escape(options[:get_params]['RelayState'])
+        end
+        if options[:raw_get_params]['SigAlg'].nil? && !options[:get_params]['SigAlg'].nil?
+          options[:raw_get_params]['SigAlg'] = CGI.escape(options[:get_params]['SigAlg'])
+        end
+
+        # If we only received the raw version of SigAlg,
+        # then parse it back into the decoded params hash for convenience.
+        if options[:get_params]['SigAlg'].nil? && !options[:raw_get_params]['SigAlg'].nil?
+          options[:get_params]['SigAlg'] = CGI.unescape(options[:raw_get_params]['SigAlg'])
+        end
+
         idp_cert = settings.get_idp_cert
         idp_certs = settings.get_idp_cert_multi
 
         if idp_cert.nil? && (idp_certs.nil? || idp_certs[:signing].empty?)
           return options.has_key? :relax_signature_validation
         end
 
-        query_string = OneLogin::RubySaml::Utils.build_query(
-          :type        => 'SAMLRequest',
-          :data        => options[:get_params]['SAMLRequest'],
-          :relay_state => options[:get_params]['RelayState'],
-          :sig_alg     => options[:get_params]['SigAlg']
+        query_string = OneLogin::RubySaml::Utils.build_query_from_raw_parts(
+          :type            => 'SAMLRequest',
+          :raw_data        => options[:raw_get_params]['SAMLRequest'],
+          :raw_relay_state => options[:raw_get_params]['RelayState'],
+          :raw_sig_alg     => options[:raw_get_params]['SigAlg']
         )
 
         if idp_certs.nil? || idp_certs[:signing].empty?

lib/onelogin/ruby-saml/utils.rb

@@ -67,6 +67,24 @@ def self.build_query(params)
         url_string << "&SigAlg=#{CGI.escape(sig_alg)}"
       end
 
+      # Reconstruct a canonical query string from raw URI-encoded parts, to be used in verifying a signature
+      # sent by an IDP.
+      #
+      # @param params [Hash] Parameters to build the Query String
+      # @option params [String] :type 'SAMLRequest' or 'SAMLResponse'
+      # @option params [String] :raw_data URI-encoded, base64 encoded SAMLRequest or SAMLResponse, as sent by IDP
+      # @option params [String] :raw_relay_state URI-encoded RelayState parameter, as sent by IDP
+      # @option params [String] :raw_sig_alg URI-encoded SigAlg parameter, as sent by IDP
+      # @return [String] The Query String
+      #
+      def self.build_query_from_raw_parts(params)
+        type, raw_data, raw_relay_state, raw_sig_alg = [:type, :raw_data, :raw_relay_state, :raw_sig_alg].map { |k| params[k]}
+
+        url_string = "#{type}=#{raw_data}"
+        url_string << "&RelayState=#{raw_relay_state}" if raw_relay_state
+        url_string << "&SigAlg=#{raw_sig_alg}"
+      end
+
       # Validate the Signature parameter sent on the HTTP-Redirect binding
       # @param params [Hash] Parameters to be used in the validation process
       # @option params [OpenSSL::X509::Certificate] cert The Identity provider public certtificate