diff --git a/CHANGELOG.md b/CHANGELOG.md index d6281f88..17972ad0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ * [#715](https://github.com/SAML-Toolkits/ruby-saml/pull/715) Fix typo in error when SPNameQualifier value does not match the SP entityID. * [#718](https://github.com/SAML-Toolkits/ruby-saml/pull/718/) Add support to retrieve from SAMLResponse the AuthnInstant and AuthnContextClassRef values * [#711](https://github.com/SAML-Toolkits/ruby-saml/pull/711) Standardize how RubySaml reads and formats certificate and private_key PEM values, including the `RubySaml::Util#format_cert` and `#format_private_key` methods. +* [#732](https://github.com/SAML-Toolkits/ruby-saml/pull/732) Return original certificate and private key objects from `RubySaml::Utils` build functions. +* [#732](https://github.com/SAML-Toolkits/ruby-saml/pull/732) Allow SP `certificate`, `certificate_new`, and `private_key` settings to be `OpenSSL::X509::Certificate` and `OpenSSL::PKey::PKey` objects respectively. ### 1.18.0 (???) * [#718](https://github.com/SAML-Toolkits/ruby-saml/pull/718) Add support to retrieve from SAMLResponse the AuthnInstant and AuthnContextClassRef values diff --git a/lib/ruby_saml/settings.rb b/lib/ruby_saml/settings.rb index 1d90e887..70623668 100644 --- a/lib/ruby_saml/settings.rb +++ b/lib/ruby_saml/settings.rb @@ -363,6 +363,24 @@ def compress_deprecation(old_param, new_param) '"HTTP-Redirect" will always be compressed, and "HTTP-POST" will always be uncompressed.' end + # Quick check if a certificate value is present as a string or OpenSSL::X509::Certificate. + # Does not check if the string can actually be parsed. + # + # @param cert [OpenSSL::X509::Certificate|String] The x509 certificate. + # @return [true|false] Whether the certificate value is present. + def cert?(cert) + !!cert && (cert.is_a?(OpenSSL::X509::Certificate) || !cert.empty?) + end + + # Quick check if a private key value is present as a string or OpenSSL::PKey::PKey. + # Does not check if the string can actually be parsed. + # + # @param private_key [OpenSSL::PKey::PKey|String] The private key. + # @return [true|false] Whether the private key value is present. + def private_key?(private_key) + !!private_key && (private_key.is_a?(OpenSSL::PKey::PKey) || !private_key.empty?) + end + # @return [Hash>>] # Build the SP certificates and private keys from the settings. Returns all # certificates and private keys, even if they are expired. @@ -373,11 +391,8 @@ def get_all_sp_certs # Validate certificate, certificate_new, private_key, and sp_cert_multi params. def validate_sp_certs_params! - multi = sp_cert_multi && !sp_cert_multi.empty? - cert = certificate && !certificate.empty? - cert_new = certificate_new && !certificate_new.empty? - pk = private_key && !private_key.empty? - if multi && (cert || cert_new || pk) + has_multi = sp_cert_multi && !sp_cert_multi.empty? + if has_multi && (cert?(certificate) || cert?(certificate_new) || private_key?(private_key)) raise ArgumentError.new("Cannot specify both sp_cert_multi and certificate, certificate_new, private_key parameters") end end diff --git a/lib/ruby_saml/utils.rb b/lib/ruby_saml/utils.rb index fa71174f..cc577d40 100644 --- a/lib/ruby_saml/utils.rb +++ b/lib/ruby_saml/utils.rb @@ -8,7 +8,7 @@ module RubySaml # SAML2 Auxiliary class # - module Utils + module Utils # rubocop:disable Metrics/ModuleLength extend self BINDINGS = { post: "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", @@ -37,7 +37,7 @@ module Utils # @param cert [OpenSSL::X509::Certificate|String] The x509 certificate. # @return [true|false] Whether the certificate is expired. def is_cert_expired(cert) - cert = build_cert_object(cert) if cert.is_a?(String) + cert = build_cert_object(cert) cert.not_after < Time.now end @@ -46,7 +46,7 @@ def is_cert_expired(cert) # @param cert [OpenSSL::X509::Certificate|String] The x509 certificate. # @return [true|false] Whether the certificate is currently active. def is_cert_active(cert) - cert = build_cert_object(cert) if cert.is_a?(String) + cert = build_cert_object(cert) now = Time.now cert.not_before <= now && cert.not_after >= now end @@ -119,6 +119,7 @@ def format_private_key(key, multi: false) # @param pem [String] The original certificate # @return [OpenSSL::X509::Certificate] The certificate object def build_cert_object(pem) + return pem if pem.is_a?(OpenSSL::X509::Certificate) return unless (pem = PemFormatter.format_cert(pem, multi: false)) OpenSSL::X509::Certificate.new(pem) @@ -129,6 +130,7 @@ def build_cert_object(pem) # @param pem [String] The original private key. # @return [OpenSSL::PKey::PKey] The private key object. def build_private_key_object(pem) + return pem if pem.is_a?(OpenSSL::PKey::PKey) return unless (pem = PemFormatter.format_private_key(pem, multi: false)) error = nil diff --git a/test/settings_test.rb b/test/settings_test.rb index 5f8a722a..10ae8296 100644 --- a/test/settings_test.rb +++ b/test/settings_test.rb @@ -515,6 +515,78 @@ class SettingsTest < Minitest::Test end end + it 'handles OpenSSL::X509::Certificate objects for single case' do + @settings.certificate = OpenSSL::X509::Certificate.new(cert_text1) + @settings.private_key = key_text1 + + actual = @settings.get_sp_certs + expected = [[cert_text1, key_text1]] + assert_equal [:signing, :encryption], actual.keys + assert_equal expected, actual[:signing].map { |ary| ary.map(&:to_pem) } + assert_equal expected, actual[:encryption].map { |ary| ary.map(&:to_pem) } + end + + it 'handles OpenSSL::X509::Certificate objects for single case with new cert' do + @settings.certificate = cert_text1 + @settings.certificate_new = OpenSSL::X509::Certificate.new(cert_text2) + @settings.private_key = key_text1 + + actual = @settings.get_sp_certs + expected = [[cert_text1, key_text1], [cert_text2, key_text1]] + assert_equal [:signing, :encryption], actual.keys + assert_equal expected, actual[:signing].map { |ary| ary.map(&:to_pem) } + assert_equal expected, actual[:encryption].map { |ary| ary.map(&:to_pem) } + end + + it 'handles OpenSSL::X509::Certificate objects for multi case' do + x509_certificate1 = OpenSSL::X509::Certificate.new(cert_text1) + x509_certificate2 = OpenSSL::X509::Certificate.new(cert_text2) + @settings.sp_cert_multi = { + signing: [{ certificate: x509_certificate1, private_key: key_text1 }, + { certificate: cert_text2, private_key: key_text1 }], + encryption: [{ certificate: x509_certificate2, private_key: key_text1 }, + { certificate: cert_text3, private_key: key_text2 }] + } + + actual = @settings.get_sp_certs + expected_signing = [[cert_text1, key_text1], [cert_text2, key_text1]] + expected_encryption = [[cert_text2, key_text1], [cert_text3, key_text2]] + assert_equal [:signing, :encryption], actual.keys + assert_equal expected_signing, actual[:signing].map { |ary| ary.map(&:to_pem) } + assert_equal expected_encryption, actual[:encryption].map { |ary| ary.map(&:to_pem) } + end + + + it 'handles OpenSSL::PKey::PKey objects for single case' do + @settings.certificate = cert_text1 + @settings.private_key = OpenSSL::PKey::RSA.new(key_text1) + + actual = @settings.get_sp_certs + expected = [[cert_text1, key_text1]] + assert_equal [:signing, :encryption], actual.keys + assert_equal expected, actual[:signing].map { |ary| ary.map(&:to_pem) } + assert_equal expected, actual[:encryption].map { |ary| ary.map(&:to_pem) } + end + + it 'handles OpenSSL::PKey::PKey objects for multi case' do + pkey2 = OpenSSL::PKey::RSA.new(key_text2) + pkey3 = CertificateHelper.generate_private_key(:dsa) + pkey4 = CertificateHelper.generate_private_key(:ecdsa) + @settings.sp_cert_multi = { + signing: [{ certificate: cert_text1, private_key: pkey3 }, + { certificate: cert_text2, private_key: pkey4 }], + encryption: [{ certificate: cert_text2, private_key: key_text1 }, + { certificate: cert_text3, private_key: pkey2 }] + } + + actual = @settings.get_sp_certs + expected_signing = [[cert_text1, pkey3.to_pem], [cert_text2, pkey4.to_pem]] + expected_encryption = [[cert_text2, key_text1], [cert_text3, key_text2]] + assert_equal [:signing, :encryption], actual.keys + assert_equal expected_signing, actual[:signing].map { |ary| ary.map(&:to_pem) } + assert_equal expected_encryption, actual[:encryption].map { |ary| ary.map(&:to_pem) } + end + it "sp_cert_multi allows sending only signing" do @settings.sp_cert_multi = { signing: [{ certificate: cert_text1, private_key: key_text1 }, @@ -920,5 +992,49 @@ class SettingsTest < Minitest::Test end end end + + describe '#cert?' do + it 'returns true for a valid OpenSSL::X509::Certificate object' do + cert = CertificateHelper.generate_cert + assert @settings.send(:cert?, cert) + end + + it 'returns true for a non-empty certificate string' do + cert_string = '-----BEGIN CERTIFICATE-----\nVALID_CERTIFICATE\n-----END CERTIFICATE-----' + assert @settings.send(:cert?, cert_string) + end + + it 'returns false for an empty certificate string' do + cert_string = '' + refute @settings.send(:cert?, cert_string) + end + + it 'returns false for nil' do + cert = nil + refute @settings.send(:cert?, cert) + end + end + + describe '#private_key?' do + it 'returns true for a valid OpenSSL::PKey::PKey object' do + private_key = CertificateHelper.generate_private_key + assert @settings.send(:private_key?, private_key) + end + + it 'returns true for a non-empty private key string' do + private_key_string = '-----BEGIN PRIVATE KEY-----\nVALID_PRIVATE_KEY\n-----END PRIVATE KEY-----' + assert @settings.send(:private_key?, private_key_string) + end + + it 'returns false for an empty private key string' do + private_key_string = '' + refute @settings.send(:private_key?, private_key_string) + end + + it 'returns false for nil' do + private_key = nil + refute @settings.send(:private_key?, private_key) + end + end end end diff --git a/test/utils_test.rb b/test/utils_test.rb index 4ea5cb8f..d85fab7f 100644 --- a/test/utils_test.rb +++ b/test/utils_test.rb @@ -156,6 +156,11 @@ def result(duration, reference = 0) end end + it 'returns the original certificate when an OpenSSL::X509::Certificate is given' do + certificate = OpenSSL::X509::Certificate.new + assert_same certificate, RubySaml::Utils.build_cert_object(certificate) + end + it 'returns nil for nil certificate string' do assert_nil RubySaml::Utils.build_cert_object(nil) end @@ -180,6 +185,13 @@ def result(duration, reference = 0) end end + [OpenSSL::PKey::RSA, OpenSSL::PKey::DSA, OpenSSL::PKey::EC].each do |key_class| + it 'returns the original private key when an instance of OpenSSL::PKey::PKey is given' do + private_key = key_class.new + assert_same private_key, RubySaml::Utils.build_private_key_object(private_key) + end + end + it 'returns nil for nil private key string' do assert_nil RubySaml::Utils.build_private_key_object(nil) end