[READY] v2.0: Support DSA and ECDSA signing keys

.rubocop_todo.yml

@@ -204,7 +204,12 @@ Metrics/CyclomaticComplexity:
 # Offense count: 58
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
-  Max: 63
+  Max: 80
+
+# Offense count: 1
+# Configuration parameters: CountComments, CountAsOne.
+Metrics/ModuleLength:
+  Max: 300
 
 # Offense count: 1
 # Configuration parameters: CountComments, CountAsOne.

README.md

@@ -176,7 +176,7 @@ In the above there are a few assumptions, one being that `response.nameid` is an
 This is all handled with how you specify the settings that are in play via the `saml_settings` method.
 That could be implemented along the lines of this:
 
-```
+```ruby
 response = RubySaml::Response.new(params[:SAMLResponse])
 response.settings = saml_settings
 ```
@@ -759,6 +759,11 @@ Note the following:
   inactive/expired certificates. This avoids validation errors when the IdP reads the SP
   metadata.
 
+#### Key Algorithm Support
+
+Ruby SAML supports RSA, DSA, and ECDSA keys for both SP and IdP certificates.
+JRuby cannot support ECDSA due to a [known issue](https://github.com/jruby/jruby-openssl/issues/257).
+
 #### Audience Validation
 
 A service provider should only consider a SAML response valid if the IdP includes an <AudienceRestriction>

UPGRADING.md

@@ -8,7 +8,15 @@
 
 Before attempting to upgrade to `2.0.0`:
 - Upgrade your project to minimum Ruby 3.0, JRuby 9.4, or TruffleRuby 22.
-- Upgrade RubySaml to `1.17.x`. Note that RubySaml `1.17.x` is compatible with up to Ruby 3.3.
+- Upgrade RubySaml to `1.17.x`.
+- In RubySaml `1.17.x`, if you were using the SHA-1 default behavior, change your settings to use SHA-256 as per below:
+
+```ruby
+# Set this in RubySaml 1.17.x, can be removed when upgrading to 2.0.0
+settings.idp_cert_fingerprint_algorithm = XMLSecurity::Document::SHA256
+settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
+settings.security[:digest_method] = XMLSecurity::Document::SHA256
+```
 
 ### Root "OneLogin" namespace changed to "RubySaml"
 
@@ -38,16 +46,17 @@ For security reasons, RubySaml version `2.0.0` uses SHA-256 as its default hashi
 instead of the now-obsolete SHA-1. This affects:
 - The default signature and digest algorithms used when generating SP metadata.
 - The default signature algorithm used when generating SP messages such as AuthnRequests.
-- The default fingerprint of IdP metadata (`:idp_cert_fingerprint` as generated by `RubySaml::IdpMetadataParser`)
+- The `:idp_cert_fingerprint` of IdP metadata as generated by `RubySaml::IdpMetadataParser`.
 
-To preserve the old insecure SHA-1 behavior *(not recommended)*, you may set `RubySaml::Settings` as follows:
+If you see any signature or fingerprint mismatch errors after upgrading to RubySaml `2.0.0`,
+this change is likely the reason. To preserve the old insecure SHA-1 behavior *(not recommended)*,
+you may set `RubySaml::Settings` as follows:
 
 ```ruby
 # Preserve RubySaml 1.x insecure SHA-1 behavior
-settings = RubySaml::Settings.new
-settings.idp_cert_fingerprint_algorithm = RubySaml::XML::Document::SHA1
-settings.security[:digest_method] = RubySaml::XML::Document::SHA1
-settings.security[:signature_method] = RubySaml::XML::Document::RSA_SHA1
+settings.idp_cert_fingerprint_algorithm = RubySaml::XML::Crypto::SHA1
+settings.security[:digest_method] = RubySaml::XML::Crypto::SHA1
+settings.security[:signature_method] = RubySaml::XML::Crypto::RSA_SHA1
 ```
 
 ### Removal of embed_sign setting
@@ -94,12 +103,14 @@ The following parameters in `RubySaml::Settings` are deprecated and will be remo
 
 ### Minor changes to Util#format_cert and #format_private_key
 
-Version 2.0.0 standardizes how RubySaml reads and formats certificate and private key
-PEM strings. In general, version 2.0.0 is more permissive than 1.x, and the changes
+
+Version `2.0.0` standardizes how RubySaml reads and formats certificate and private key
+PEM strings. In general, version `2.0.0` is more permissive than `1.x`, and the changes
 are not anticipated to affect most users. Please note the change affects parameters
 such `#idp_cert` and `#certificate`, as well as the `RubySaml::Util#format_cert`
 and `#format_private_key` methods. Specifically:
 
+
 | # | Input value                                          | RubySaml 2.0.0                                          | RubySaml 1.x              |
 |---|------------------------------------------------------|---------------------------------------------------------|---------------------------|
 | 1 | Input contains a bad (e.g. non-base64) PEM           | Skip PEM formatting                                     | Return a bad PEM          |
@@ -113,7 +124,7 @@ and `#format_private_key` methods. Specifically:
 **Notes**
 - Case 3: For example, `-----BEGIN TRUSTED X509 CERTIFICATE-----` is now
   considered a valid header as an input, but it will be formatted to
-  `-----BEGIN CERTIFICATE-----` in the output. As a special case, in both 2.0.0
+  `-----BEGIN CERTIFICATE-----` in the output. As a special case, in both `2.0.0`
   and 1.x, if `RSA PRIVATE KEY` is present in the input string, the `RSA` prefix will
   be preserved in the output.
 - Case 5: When formatting multiple certificates in one string (i.e. a certificate chain),

lib/ruby_saml/authrequest.rb

@@ -77,14 +77,14 @@ def create_params(settings, params={})
       sp_signing_key = settings.get_sp_signing_key
 
       if binding_redirect && settings.security[:authn_requests_signed] && sp_signing_key
-        params['SigAlg'] = settings.security[:signature_method]
+        params['SigAlg'] = settings.get_sp_signature_method
         url_string = RubySaml::Utils.build_query(
           type: 'SAMLRequest',
           data: base64_request,
           relay_state: relay_state,
           sig_alg: params['SigAlg']
         )
-        sign_algorithm = RubySaml::XML::BaseDocument.new.algorithm(settings.security[:signature_method])
+        sign_algorithm = RubySaml::XML::Crypto.hash_algorithm(settings.get_sp_signature_method)
         signature = sp_signing_key.sign(sign_algorithm.new, url_string)
         params['Signature'] = encode(signature)
       end
@@ -185,7 +185,7 @@ def create_xml_document(settings)
     def sign_document(document, settings)
       cert, private_key = settings.get_sp_signing_pair
       if settings.idp_sso_service_binding == Utils::BINDINGS[:post] && settings.security[:authn_requests_signed] && private_key && cert
-        document.sign_document(private_key, cert, settings.security[:signature_method], settings.security[:digest_method])
+        document.sign_document(private_key, cert, settings.get_sp_signature_method, settings.get_sp_digest_method)
       end
 
       document

lib/ruby_saml/idp_metadata_parser.rb

@@ -398,7 +398,7 @@ def fingerprint(certificate, fingerprint_algorithm = RubySaml::XML::Document::SH
 
           cert = OpenSSL::X509::Certificate.new(Base64.decode64(certificate))
 
-          fingerprint_alg = RubySaml::XML::BaseDocument.new.algorithm(fingerprint_algorithm).new
+          fingerprint_alg = RubySaml::XML::Crypto.hash_algorithm(fingerprint_algorithm).new
           fingerprint_alg.hexdigest(cert.to_der).upcase.scan(/../).join(":")
         end
       end

lib/ruby_saml/logoutrequest.rb

@@ -75,14 +75,14 @@ def create_params(settings, params={})
       sp_signing_key = settings.get_sp_signing_key
 
       if binding_redirect && settings.security[:logout_requests_signed] && sp_signing_key
-        params['SigAlg'] = settings.security[:signature_method]
+        params['SigAlg'] = settings.get_sp_signature_method
         url_string = RubySaml::Utils.build_query(
           type: 'SAMLRequest',
           data: base64_request,
           relay_state: relay_state,
           sig_alg: params['SigAlg']
         )
-        sign_algorithm = RubySaml::XML::BaseDocument.new.algorithm(settings.security[:signature_method])
+        sign_algorithm = RubySaml::XML::Crypto.hash_algorithm(settings.get_sp_signature_method)
         signature = settings.get_sp_signing_key.sign(sign_algorithm.new, url_string)
         params['Signature'] = encode(signature)
       end
@@ -144,7 +144,7 @@ def sign_document(document, settings)
       # embed signature
       cert, private_key = settings.get_sp_signing_pair
       if settings.idp_slo_service_binding == Utils::BINDINGS[:post] && settings.security[:logout_requests_signed] && private_key && cert
-        document.sign_document(private_key, cert, settings.security[:signature_method], settings.security[:digest_method])
+        document.sign_document(private_key, cert, settings.get_sp_signature_method, settings.get_sp_digest_method)
       end
 
       document

lib/ruby_saml/metadata.rb

@@ -142,7 +142,7 @@ def embed_signature(meta_doc, settings)
       cert, private_key = settings.get_sp_signing_pair
       return unless private_key && cert
 
-      meta_doc.sign_document(private_key, cert, settings.security[:signature_method], settings.security[:digest_method])
+      meta_doc.sign_document(private_key, cert, settings.get_sp_signature_method, settings.get_sp_digest_method)
     end
 
     def output_xml(meta_doc, pretty_print)

lib/ruby_saml/response.rb

@@ -882,8 +882,8 @@ def validate_signature
 
         if fingerprint && doc.validate_document(fingerprint, @soft, opts)
           if settings.security[:check_idp_cert_expiration] && RubySaml::Utils.is_cert_expired(idp_cert)
-              error_msg = "IdP x509 certificate expired"
-              return append_error(error_msg)
+            error_msg = "IdP x509 certificate expired"
+            return append_error(error_msg)
           end
         else
           return append_error(error_msg)

lib/ruby_saml/saml_message.rb

@@ -133,11 +133,7 @@ def decode(string)
     # @return [String] The encoded string
     #
     def encode(string)
-      if Base64.respond_to?(:strict_encode64)
-        Base64.strict_encode64(string)
-      else
-        Base64.encode64(string).gsub(/\n/, "")
-      end
+      Base64.strict_encode64(string)
     end
 
     # Check if a string is base64 encoded

lib/ruby_saml/settings.rb

@@ -126,7 +126,7 @@ def get_fingerprint
       idp_cert_fingerprint || begin
         idp_cert = get_idp_cert
         if idp_cert
-          fingerprint_alg = RubySaml::XML::BaseDocument.new.algorithm(idp_cert_fingerprint_algorithm).new
+          fingerprint_alg = RubySaml::XML::Crypto.hash_algorithm(idp_cert_fingerprint_algorithm).new
           fingerprint_alg.hexdigest(idp_cert.to_der).upcase.scan(/../).join(":")
         end
       end
@@ -159,7 +159,7 @@ def get_idp_cert_multi
       certs
     end
 
-    # @return [Hash<Symbol, Array<Array<OpenSSL::X509::Certificate, OpenSSL::PKey::RSA>>>]
+    # @return [Hash<Symbol, Array<Array<OpenSSL::X509::Certificate, OpenSSL::PKey::PKey>>>]
     #   Build the SP certificates and private keys from the settings. If
     #   check_sp_cert_expiration is true, only returns certificates and private keys
     #   that are not expired.
@@ -179,7 +179,7 @@ def get_sp_certs
       active_certs.freeze
     end
 
-    # @return [Array<OpenSSL::X509::Certificate, OpenSSL::PKey::RSA>]
+    # @return [Array<OpenSSL::X509::Certificate, OpenSSL::PKey::PKey>]
     #   The SP signing certificate and private key.
     def get_sp_signing_pair
       get_sp_certs[:signing].first
@@ -267,6 +267,43 @@ def get_binding(value)
       end
     end
 
+    # @return [String] The XML Signature Algorithm attribute.
+    #
+    # This method is intentionally hacky for backwards compatibility of the
+    # settings.security[:signature_method] parameter. Previously, this parameter
+    # could have a value such as "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
+    # which assumes the public key type RSA. To add support for DSA and ECDSA, we will now
+    # ignore the "rsa-" prefix and only use the "sha256" hash algorithm component.
+    def get_sp_signature_method
+      sig_alg = security[:signature_method] || 'sha256'
+      key_alg_fallback, hash_alg = sig_alg.to_s.match(/(?:\A|(rsa|ecdsa|ec|dsa)?[# _-])(sha\d+)\z/i)&.[](1..2)
+      key_alg_real = case get_sp_signing_key
+                     when OpenSSL::PKey::RSA then 'RSA'
+                     when OpenSSL::PKey::DSA then 'DSA'
+                     when OpenSSL::PKey::EC  then 'ECDSA'
+                     end
+      key_alg = key_alg_real || key_alg_fallback || 'RSA'
+      key_alg = 'ECDSA' if key_alg.casecmp('EC') == 0
+
+      begin
+        RubySaml::XML::Crypto.const_get("#{key_alg}_#{hash_alg}".upcase)
+      rescue NameError
+        raise ArgumentError.new("Unsupported signature method#{" for #{key_alg_real} key" if key_alg_real}: #{sig_alg}")
+      end
+    end
+
+    # @return [String] The XML Signature Digest attribute.
+    def get_sp_digest_method
+      digest_alg = security[:digest_method] || 'sha1' # TODO: change to sha256 by default
+      alg = digest_alg.to_s.match(/(?:\A|#)(sha\d+)\z/i)[1]
+
+      begin
+        RubySaml::XML::Crypto.const_get(alg.upcase)
+      rescue NameError
+        raise ArgumentError.new("Unsupported digest method: #{digest_alg}")
+      end
+    end
+
     # @deprecated Will be removed in v2.1.0
     def certificate_new
       certificate_new_deprecation

lib/ruby_saml/slo_logoutresponse.rb

@@ -84,14 +84,14 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {},
       sp_signing_key = settings.get_sp_signing_key
 
       if binding_redirect && settings.security[:logout_responses_signed] && sp_signing_key
-        params['SigAlg'] = settings.security[:signature_method]
+        params['SigAlg'] = settings.get_sp_signature_method
         url_string = RubySaml::Utils.build_query(
           type: 'SAMLResponse',
           data: base64_response,
           relay_state: relay_state,
           sig_alg: params['SigAlg']
         )
-        sign_algorithm = RubySaml::XML::BaseDocument.new.algorithm(settings.security[:signature_method])
+        sign_algorithm = RubySaml::XML::Crypto.hash_algorithm(settings.get_sp_signature_method)
         signature = sp_signing_key.sign(sign_algorithm.new, url_string)
         params['Signature'] = encode(signature)
       end
@@ -155,7 +155,7 @@ def sign_document(document, settings)
       # embed signature
       cert, private_key = settings.get_sp_signing_pair
       if settings.idp_slo_service_binding == Utils::BINDINGS[:post] && private_key && cert
-        document.sign_document(private_key, cert, settings.security[:signature_method], settings.security[:digest_method])
+        document.sign_document(private_key, cert, settings.get_sp_signature_method, settings.get_sp_digest_method)
       end
 
       document

lib/ruby_saml/utils.rb

@@ -124,14 +124,21 @@ def build_cert_object(pem)
       OpenSSL::X509::Certificate.new(pem)
     end
 
-    # Given a private key string, return an OpenSSL::PKey::RSA object.
+    # Given a private key string, return an OpenSSL::PKey::PKey object.
     #
     # @param pem [String] The original private key.
-    # @return [OpenSSL::PKey::RSA] The private key object.
+    # @return [OpenSSL::PKey::PKey] The private key object.
     def build_private_key_object(pem)
       return unless (pem = PemFormatter.format_private_key(pem, multi: false))
 
-      OpenSSL::PKey::RSA.new(pem)
+      error = nil
+      private_key_classes(pem).each do |key_class|
+        return key_class.new(pem)
+      rescue OpenSSL::PKey::PKeyError => e
+        error ||= e
+      end
+
+      raise error
     end
 
     # Build the Query String signature that will be used in the HTTP-Redirect binding
@@ -212,8 +219,8 @@ def escape_request_param(param, lowercase_url_encoding)
     # @return [Boolean] True if the Signature is valid, False otherwise
     #
     def verify_signature(params)
-      cert, sig_alg, signature, query_string = %i[cert sig_alg signature query_string].map { |k| params[k]}
-      signature_algorithm = RubySaml::XML::BaseDocument.new.algorithm(sig_alg)
+      cert, sig_alg, signature, query_string = params.values_at(:cert, :sig_alg, :signature, :query_string)
+      signature_algorithm = RubySaml::XML::Crypto.hash_algorithm(sig_alg)
       cert.public_key.verify(signature_algorithm.new, Base64.decode64(signature), query_string)
     end
 
@@ -243,11 +250,15 @@ def status_error_msg(error_msg, raw_status_code = nil, status_message = nil)
     # Obtains the decrypted string from an Encrypted node element in XML,
     # given multiple private keys to try.
     # @param encrypted_node [REXML::Element] The Encrypted element
-    # @param private_keys [Array<OpenSSL::PKey::RSA>] The Service provider private key
+    # @param private_keys [Array<OpenSSL::PKey::RSA>] The SP private key
     # @return [String] The decrypted data
     def decrypt_multi(encrypted_node, private_keys)
       raise ArgumentError.new('private_keys must be specified') if !private_keys || private_keys.empty?
 
+      if private_keys.none?(OpenSSL::PKey::RSA)
+        raise ArgumentError.new('private_keys must be OpenSSL::PKey::RSA keys')
+      end
+
       error = nil
       private_keys.each do |key|
         return decrypt_data(encrypted_node, key)
@@ -260,7 +271,7 @@ def decrypt_multi(encrypted_node, private_keys)
 
     # Obtains the decrypted string from an Encrypted node element in XML
     # @param encrypted_node [REXML::Element] The Encrypted element
-    # @param private_key [OpenSSL::PKey::RSA] The Service provider private key
+    # @param private_key [OpenSSL::PKey::RSA] The SP private key
     # @return [String] The decrypted data
     def decrypt_data(encrypted_node, private_key)
       encrypt_data = REXML::XPath.first(
@@ -286,7 +297,7 @@ def decrypt_data(encrypted_node, private_key)
 
     # Obtains the symmetric key from the EncryptedData element
     # @param encrypt_data [REXML::Element]     The EncryptedData element
-    # @param private_key [OpenSSL::PKey::RSA] The Service provider private key
+    # @param private_key [OpenSSL::PKey::RSA] The SP private key
     # @return [String] The symmetric key
     def retrieve_symmetric_key(encrypt_data, private_key)
       encrypted_key = REXML::XPath.first(
@@ -410,5 +421,16 @@ def original_uri_match?(destination_url, settings_url)
     def element_text(element)
       element.texts.map(&:value).join if element
     end
+
+    # Given a private key PEM string, return an array of OpenSSL::PKey::PKey classes
+    # that can be used to parse it, with the most likely match first.
+    def private_key_classes(pem)
+      priority = case pem.match(/(RSA|ECDSA|EC|DSA) PRIVATE KEY/)&.[](1)
+                 when 'RSA' then OpenSSL::PKey::RSA
+                 when 'DSA' then OpenSSL::PKey::DSA
+                 when 'ECDSA', 'EC' then OpenSSL::PKey::EC
+                 end
+      Array(priority) | [OpenSSL::PKey::RSA, OpenSSL::PKey::DSA, OpenSSL::PKey::EC]
+    end
   end
 end

lib/ruby_saml/xml.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require 'ruby_saml/xml/crypto'
 require 'ruby_saml/xml/base_document'
 require 'ruby_saml/xml/document'
 require 'ruby_saml/xml/signed_document'