Fix Http-Redirect Signature: SignAlg and RelayState issue

Author: pitbulk

README.md

@@ -341,7 +341,7 @@ The settings related to sign are stored in the `security` attribute of the setti
   settings.security[:logout_responses_signed] = true     # Enable or not signature on Logout Response
 
   settings.security[:digest_method]    = XMLSecurity::Document::SHA1
-  settings.security[:signature_method] = XMLSecurity::Document::SHA1
+  settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
 
   settings.security[:embed_sign]        = false                # Embeded signature or HTTP GET parameter Signature
 ```

lib/onelogin/ruby-saml/authrequest.rb

@@ -28,6 +28,8 @@ def create(settings, params = {})
 
       def create_params(settings, params={})
         params = {} if params.nil?
+        # Some ruby-saml versions uses :RelayState others use 'RelayState'
+        relay_state = params[:RelayState] || params['RelayState']
 
         request_doc = create_authentication_xml_doc(settings)
         request_doc.context[:attribute_quote] = :quote if settings.double_quote_xml_attribute_values
@@ -42,9 +44,9 @@ def create_params(settings, params={})
         request_params = {"SAMLRequest" => base64_request}
 
         if settings.security[:authn_requests_signed] && !settings.security[:embed_sign] && settings.private_key
-          params['SigAlg']    = XMLSecurity::Document::RSA_SHA1
+          params['SigAlg']    = settings.security[:signature_method]
           url_string          = "SAMLRequest=#{CGI.escape(base64_request)}"
-          url_string         += "&RelayState=#{CGI.escape(params['RelayState'])}" if params['RelayState']
+          url_string         += "&RelayState=#{CGI.escape(relay_state)}" if relay_state
           url_string         += "&SigAlg=#{CGI.escape(params['SigAlg'])}"
           private_key         = settings.get_sp_key()
           signature           = private_key.sign(XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method]).new, url_string)

lib/onelogin/ruby-saml/logoutrequest.rb

@@ -26,6 +26,8 @@ def create(settings, params={})
 
       def create_params(settings, params={})
         params = {} if params.nil?
+        # Some ruby-saml versions uses :RelayState others use 'RelayState'
+        relay_state = params[:RelayState] || params['RelayState']
 
         request_doc = create_logout_request_xml_doc(settings)
         request_doc.context[:attribute_quote] = :quote if settings.double_quote_xml_attribute_values
@@ -40,9 +42,9 @@ def create_params(settings, params={})
         request_params = {"SAMLRequest" => base64_request}
 
         if settings.security[:logout_requests_signed] && !settings.security[:embed_sign] && settings.private_key
-          params['SigAlg']    = XMLSecurity::Document::RSA_SHA1
+          params['SigAlg']    = settings.security[:signature_method]
           url_string          = "SAMLRequest=#{CGI.escape(base64_request)}"
-          url_string         += "&RelayState=#{CGI.escape(params['RelayState'])}" if params['RelayState']
+          url_string         += "&RelayState=#{CGI.escape(relay_state)}" if relay_state
           url_string         += "&SigAlg=#{CGI.escape(params['SigAlg'])}"
           private_key         = settings.get_sp_key()
           signature           = private_key.sign(XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method]).new, url_string)

lib/onelogin/ruby-saml/slo_logoutresponse.rb

@@ -27,6 +27,8 @@ def create(settings, request_id = nil, logout_message = nil, params = {})
 
       def create_params(settings, request_id = nil, logout_message = nil, params = {})
         params = {} if params.nil?
+        # Some ruby-saml versions uses :RelayState others use 'RelayState'
+        relay_state = params[:RelayState] || params['RelayState']
 
         response_doc = create_logout_response_xml_doc(settings, request_id, logout_message)
         response_doc.context[:attribute_quote] = :quote if settings.double_quote_xml_attribute_values
@@ -41,9 +43,9 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {})
         response_params = {"SAMLResponse" => base64_response}
 
         if settings.security[:logout_responses_signed] && !settings.security[:embed_sign] && settings.private_key
-          params['SigAlg']    = XMLSecurity::Document::RSA_SHA1
+          params['SigAlg']    = settings.security[:signature_method]
           url_string          = "SAMLResponse=#{CGI.escape(base64_response)}"
-          url_string         += "&RelayState=#{CGI.escape(params['RelayState'])}" if params['RelayState']
+          url_string         += "&RelayState=#{CGI.escape(relay_state)}" if relay_state
           url_string         += "&SigAlg=#{CGI.escape(params['SigAlg'])}"
           private_key         = settings.get_sp_key()
           signature           = private_key.sign(XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method]).new, url_string)

lib/xml_security.rb

@@ -56,9 +56,10 @@ def algorithm(element)
       algorithm = element
       if algorithm.is_a?(REXML::Element)
         algorithm = element.attribute("Algorithm").value
-        algorithm = algorithm && algorithm =~ /sha(.*?)$/i && $1.to_i
       end
 
+      algorithm = algorithm && algorithm =~ /(rsa-)?sha(.*?)$/i && $2.to_i
+
       case algorithm
       when 256 then OpenSSL::Digest::SHA256
       when 384 then OpenSSL::Digest::SHA384

test/logoutrequest_test.rb

@@ -148,11 +148,11 @@ class RequestTest < Minitest::Test
         assert params['Signature']
         assert params['SigAlg'] == XMLSecurity::Document::RSA_SHA1
 
-        # signature_method only affects the embedeed signature
+        # if signature_method changes, the SigAlg also changes
         settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
         params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings)
         assert params['Signature']
-        assert params['SigAlg'] == XMLSecurity::Document::RSA_SHA1
+        assert params['SigAlg'] == XMLSecurity::Document::RSA_SHA256
       end
     end
 

test/request_test.rb

@@ -197,11 +197,11 @@ class RequestTest < Minitest::Test
         assert params['Signature']
         assert params['SigAlg'] == XMLSecurity::Document::RSA_SHA1
 
-        # signature_method only affects the embedeed signature
-        settings.security[:signature_method] = XMLSecurity::Document::SHA256
+        # if signature_method changes, the SigAlg also changes
+        settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
         params = OneLogin::RubySaml::Authrequest.new.create_params(settings)
         assert params['Signature']
-        assert params['SigAlg'] == XMLSecurity::Document::RSA_SHA1
+        assert params['SigAlg'] == XMLSecurity::Document::RSA_SHA256
       end
     end
 

test/slo_logoutresponse_test.rb

@@ -124,11 +124,11 @@ class SloLogoutresponseTest < Minitest::Test
         assert params['Signature']
         assert params['SigAlg'] == XMLSecurity::Document::RSA_SHA1
 
-        # signature_method only affects the embedeed signature
-        settings.security[:signature_method] = XMLSecurity::Document::SHA256
+        # if signature_method changes, the SigAlg also changes
+        settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
         params = OneLogin::RubySaml::SloLogoutresponse.new.create_params(settings, request.id, "Custom Logout Message")
         assert params['Signature']
-        assert params['SigAlg'] == XMLSecurity::Document::RSA_SHA1
+        assert params['SigAlg'] == XMLSecurity::Document::RSA_SHA256
       end
     end
   end