[READY] v2.0: Support DSA and ECDSA signing keys (#705)

* v2.0 Support EC/DSA crypto

Author: johnnyshields

.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