Skip to content

Commit

Permalink
v2.0 Support EC/DSA crypto
Browse files Browse the repository at this point in the history
  • Loading branch information
johnnyshields committed Jul 13, 2024
1 parent 113f39a commit f9cbf62
Show file tree
Hide file tree
Showing 33 changed files with 1,990 additions and 1,337 deletions.
4 changes: 2 additions & 2 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +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: 244
Max: 300

# Offense count: 2
# Configuration parameters: Max, CountKeywordArgs.
Expand Down
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down Expand Up @@ -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>
Expand Down
31 changes: 20 additions & 11 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -94,13 +103,13 @@ 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 |
| # | Input value | RubySaml 2.0.0 | RubySaml 1.17.x |
|---|------------------------------------------------------|---------------------------------------------------------|---------------------------|
| 1 | Input contains a bad (e.g. non-base64) PEM | Skip PEM formatting | Return a bad PEM |
| 2 | Input contains `\r` character(s) | Strip out all `\r` character(s) and format as PEM | Skip PEM formatting |
Expand All @@ -113,7 +122,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),
Expand Down
6 changes: 3 additions & 3 deletions lib/ruby_saml/authrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_saml/idp_metadata_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/ruby_saml/logoutrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_saml/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_saml/pem_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def format_pem_body(pem)

# Returns a regexp which can be used to loosely match unformatted PEM(s) in a string.
def pem_scan_regexp(label = nil)
base64 = '[A-Za-z\d+/\s]*[A-Za-z\d+]+[A-Za-z\d+/\s]*=?\s*=?\s*'
base64 = '[A-Za-z\d+/\s]*[A-Za-z\d+][A-Za-z\d+/\s]*=?\s*=?\s*'
/#{pem_scan_header('BEGIN', label)}#{base64}#{pem_scan_header('END', label)}/m
end

Expand Down
4 changes: 2 additions & 2 deletions lib/ruby_saml/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -861,8 +861,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)
Expand Down
6 changes: 1 addition & 5 deletions lib/ruby_saml/saml_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 40 additions & 3 deletions lib/ruby_saml/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/ruby_saml/slo_logoutresponse.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
38 changes: 30 additions & 8 deletions lib/ruby_saml/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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
1 change: 1 addition & 0 deletions lib/ruby_saml/xml.rb
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
Loading

0 comments on commit f9cbf62

Please sign in to comment.