Improvements

CHANGELOG.md

@@ -11,6 +11,7 @@
 * [#690](https://github.com/SAML-Toolkits/ruby-saml/pull/690) Remove deprecated `settings.security[:embed_sign]` parameter.
 * [#697](https://github.com/SAML-Toolkits/ruby-saml/pull/697) Add deprecation for various parameters in `RubySaml::Settings`.
 * [#709](https://github.com/SAML-Toolkits/ruby-saml/pull/709) Allow passing in `Net::HTTP` `:open_timeout`, `:read_timeout`, and `:max_retries` settings to `IdpMetadataParser#parse_remote`.
+* [#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.
 
 ### 1.17.0
 * [#687](https://github.com/SAML-Toolkits/ruby-saml/pull/687) Add CI coverage for Ruby 3.3 and Windows.

UPGRADING.md

@@ -50,7 +50,7 @@ settings.security[:digest_method] = RubySaml::XML::Document::SHA1
 settings.security[:signature_method] = RubySaml::XML::Document::RSA_SHA1
 ```
 
-### Removal of embed_sign Setting
+### Removal of embed_sign setting
 
 The deprecated `settings.security[:embed_sign]` parameter has been removed. If you were using it, please instead switch
 to using both the `settings.idp_sso_service_binding` and `settings.idp_slo_service_binding` parameters as show below.
@@ -68,7 +68,7 @@ settings.idp_slo_service_binding = :redirect
 
 For clarity, the default value of both parameters is `:redirect` if they are not set.
 
-### Deprecation of Compression Settings
+### Deprecation of compression settings
 
 The `settings.compress_request` and `settings.compress_response` parameters have been deprecated
 and are no longer functional. They will be removed in RubySaml 2.1.0. Please remove `compress_request`
@@ -80,7 +80,7 @@ The SAML SP request/response message compression behavior is now controlled auto
 "compression" is used to make redirect URLs which contain SAML messages be shorter. For POST messages,
 compression may be achieved by enabling `Content-Encoding: gzip` on your webserver.
 
-## Settings deprecations
+### Other settings deprecations
 
 The following parameters in `RubySaml::Settings` are deprecated and will be removed in RubySaml 2.1.0:
 
@@ -92,6 +92,35 @@ The following parameters in `RubySaml::Settings` are deprecated and will be remo
 - `#certificate_new` is deprecated and replaced by `#sp_cert_multi`. Refer to documentation as `#sp_cert_multi`
   has a different value type than `#certificate_new`.
 
+### 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
+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          |
+| 2 | Input contains `\r` character(s)                     | Strip out all `\r` character(s) and format as PEM       | Skip PEM formatting       |
+| 3 | PEM header other than `CERTIFICATE` or `PRIVATE KEY` | Format if header ends in `CERTIFICATE` or `PRIVATE KEY` | Skip PEM formatting       |
+| 4 | `#format_cert` given `PRIVATE KEY` (and vice-versa)  | Ignore PEMs of incorrect type                           | Return a bad PEM          |
+| 5 | Text outside header/footer values                    | Strip out text outside header/footer values             | Skip PEM formatting       |
+| 6 | Input non-ASCII characters                           | Ignore non-ASCII chars if they are outside the PEM      | Skip PEM formatting       |
+| 7 | `#format_cert` input contains mix of good/bad certs  | Return only good cert PEMs (joined with `\n`)           | Return good and bad certs |
+
+**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
+  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),
+  text present between the footer and header of two different certificates will also be
+  stripped out.
+- Case 7: If no valid certificates are found, the entire original string will be returned.
+
 ## Updating from 1.12.x to 1.13.0
 
 Version `1.13.0` adds `settings.idp_sso_service_binding` and `settings.idp_slo_service_binding`, and

lib/ruby_saml/pem_formatter.rb

@@ -1,68 +1,126 @@
 # frozen_string_literal: true
 
 module RubySaml
-  # Formats PEM-encoded X.509 certificates and private keys to
-  # canonical PEM format with 64-char lines and BEGIN/END headers.
+  # Formats PEM-encoded X.509 certificates and private keys to canonical
+  # RFC 7468 PEM format, including 64-char lines and BEGIN/END headers.
   #
   # @api private
   module PemFormatter
     extend self
 
-    # Formats one or many X.509 certificate(s) to canonical
-    # PEM format with 64-char lines and BEGIN/END headers.
+    # Formats X.509 certificate(s) to an array of strings in canonical
+    # RFC 7468 PEM format.
     #
-    # @param cert [String] The original certificate(s)
-    # @param multi [true|false] Whether to return multiple keys delimited by newline
-    # @return [String|nil] The formatted certificate(s)
+    # @param certs [String|Array<String>] String(s) containing
+    #   unformatted certificate(s).
+    # @return [Array<String>] The formatted certificate(s).
+    def format_cert_array(certs)
+      format_pem_array(certs, 'CERTIFICATE')
+    end
+
+    # Formats one or multiple X.509 certificate(s) to canonical
+    # RFC 7468 PEM format.
+    #
+    # @param cert [String] A string containing unformatted certificate(s).
+    # @param multi [true|false] Whether to return multiple certificates
+    #   delimited by newline. Default false.
+    # @return [String] The formatted certificate(s). Returns nil if the
+    #   input is blank.
     def format_cert(cert, multi: false)
-      detect_pems(cert, 'CERTIFICATE', multi: multi) do |pem|
-        format_cert_single(pem)
-      end
+      pem_array_to_string(format_cert_array(cert), multi: multi)
+    end
+
+    # Formats private keys(s) to canonical RFC 7468 PEM format.
+    #
+    # @param keys [String|Array<String>] String(s) containing unformatted
+    #   private keys(s).
+    # @return [Array<String>] The formatted private keys(s).
+    def format_private_key_array(keys)
+      format_pem_array(keys, 'PRIVATE KEY', %w[RSA ECDSA EC DSA])
     end
 
-    # Formats one or many private key(s) to canonical PEM format
-    # with 64-char lines and BEGIN/END headers.
+    # Formats one or multiple private key(s) to canonical RFC 7468
+    # PEM format.
     #
-    # @param key [String] The original private key(s)
-    # @param multi [true|false] Whether to return multiple keys delimited by newline
-    # @return [String|nil] The formatted private key(s)
+    # @param key [String] A string containing unformatted private keys(s).
+    # @param multi [true|false] Whether to return multiple keys
+    #   delimited by newline. Default false.
+    # @return [String|nil] The formatted private key(s). Returns
+    #   nil if the input is blank.
     def format_private_key(key, multi: false)
-      detect_pems(key, '(?:RSA|DSA|EC|ECDSA) PRIVATE KEY', multi: multi) do |pem|
-        format_private_key_single(pem)
-      end
+      pem_array_to_string(format_private_key_array(key), multi: multi)
     end
 
     private
 
-    def detect_pems(str, label, multi: false, &block)
-      return if str.nil? || str.empty?
-      return str unless str.ascii_only?
-      return if str.match?(/\A\s*\z/)
+    def format_pem_array(str, label, known_prefixes = nil)
+      return [] unless str
+
+      # Normalize array input using '?' char as a delimiter
+      str = str.is_a?(Array) ? str.map { |s| encode_utf8(s) }.join('?') : encode_utf8(str)
+      str.strip!
+      return [] if str.empty?
+
+      # Find and format PEMs matching the desired label
+      pems = str.scan(pem_scan_regexp(label)).map { |pem| format_pem(pem, label, known_prefixes) }
 
-      pems = str.scan(/-{5}\s*BEGIN #{label}\s*-{5}.*?-{5}\s*END #{label}\s*-{5}?/m).map(&block)
+      # If no PEMs matched, remove non-matching PEMs then format the remaining string
+      if pems.empty?
+        str.gsub!(pem_scan_regexp, '')
+        str.strip!
+        pems = format_pem(str, label, known_prefixes).scan(pem_scan_regexp(label)) unless str.empty?
+      end
+
+      pems
+    end
 
-      # Try to format the original string if no pems were found
-      return yield(str) if pems.empty?
+    def pem_array_to_string(pems, multi: false)
+      return if pems.empty?
+      return pems unless pems.is_a?(Array)
 
       multi ? pems.join("\n") : pems.first
     end
 
-    def format_cert_single(cert)
-      format_pem(cert, 'CERTIFICATE')
+    # Given a PEM, a label such as "PRIVATE KEY", and a list of known prefixes
+    # such as "RSA", "DSA", etc., returns the formatted PEM preserving the known
+    # prefix if possible.
+    def format_pem(pem, label, known_prefixes = nil)
+      prefix = detect_label_prefix(pem, label, known_prefixes)
+      label = "#{prefix} #{label}" if prefix
+      "-----BEGIN #{label}-----\n#{format_pem_body(pem)}\n-----END #{label}-----"
+    end
+
+    # Given a PEM, a label such as "PRIVATE KEY", and a list of known prefixes
+    # such as "RSA", "DSA", etc., detects and returns the known prefix if it exists.
+    def detect_label_prefix(pem, label, known_prefixes)
+      return unless known_prefixes && !known_prefixes.empty?
+
+      pem.match(/(#{Array(known_prefixes).join('|')})\s+#{label.gsub(' ', '\s+')}/)&.[](1)
+    end
+
+    # Given a PEM, strips all whitespace and the BEGIN/END lines,
+    # then splits the body into 64-character lines.
+    def format_pem_body(pem)
+      pem.gsub(/\s|#{pem_scan_header}/, '').scan(/.{1,64}/).join("\n")
+    end
+
+    # 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*'
+      /#{pem_scan_header('BEGIN', label)}#{base64}#{pem_scan_header('END', label)}/m
     end
 
-    def format_private_key_single(key)
-      algo = key.match(/((?:RSA|ECDSA|EC|DSA) )PRIVATE KEY/)&.[](1)
-      label = "#{algo}PRIVATE KEY"
-      format_pem(key, label)
+    # Returns a regexp component string to match PEM headers.
+    def pem_scan_header(marker = nil, label = nil)
+      marker ||= '(BEGIN|END)'
+      label ||= '[A-Z\d]+'
+      "-{5}\\s*#{marker}\\s(?:[A-Z\\d\\s]*\\s)?#{label.gsub(' ', '\s+')}\\s*-{5}"
     end
 
-    # Strips all whitespace and the BEGIN/END lines,
-    # then splits the string into 64-character lines,
-    # and re-applies BEGIN/END labels
-    def format_pem(str, label)
-      str = str.gsub(/\s|-{5}\s*(BEGIN|END) [A-Z\d\s]+-{5}/, '').scan(/.{1,64}/).join("\n")
-      "-----BEGIN #{label}-----\n#{str}\n-----END #{label}-----"
+    # Encode to UTF-8 using '?' as a delimiter so that non-ASCII chars
+    # appearing inside a PEM will cause the PEM to be considered invalid.
+    def encode_utf8(str)
+      str.encode('UTF-8', invalid: :replace, undef: :replace, replace: '?')
     end
   end
 end

lib/ruby_saml/utils.rb

@@ -37,8 +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 = OpenSSL::X509::Certificate.new(cert) if cert.is_a?(String)
-
+      cert = build_cert_object(cert) if cert.is_a?(String)
       cert.not_after < Time.now
     end
 
@@ -47,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 = OpenSSL::X509::Certificate.new(cert) if cert.is_a?(String)
+      cert = build_cert_object(cert) if cert.is_a?(String)
       now = Time.now
       cert.not_before <= now && cert.not_after >= now
     end
@@ -87,42 +86,50 @@ def parse_duration(duration, timestamp=Time.now.utc)
       datetime.to_time.utc.to_i + (durHours * 3600) + (durMinutes * 60) + durSeconds
     end
 
-    # Formats one or many X.509 certificate(s) to canonical
-    # PEM format with 64-char lines and BEGIN/END headers.
+    # Formats one or multiple X.509 certificate(s) to canonical RFC 7468 PEM format.
+    #
+    # @note Unlike `PemFormatter#format_cert`, this method returns the original
+    # input string if the input cannot be parsed.
     #
-    # @param cert [String] The original certificate(s)
-    # @param multi [true|false] Whether to return multiple keys delimited by newline
-    # @return [String|nil] The formatted certificate(s)
+    # @param cert [String] The original certificate(s).
+    # @param multi [true|false] Whether to return multiple keys delimited by newline.
+    #   Default true for compatibility with legacy behavior (i.e. to parse cert chains).
+    # @return [String] The formatted certificate(s). For legacy compatibility reasons,
+    #   this method returns the original string if the input cannot be parsed.
     def format_cert(cert, multi: true)
-      PemFormatter.format_cert(cert, multi: multi)
+      PemFormatter.format_cert(cert, multi: multi) || cert
     end
 
-    # Formats one or many private key(s) to canonical PEM format
-    # with 64-char lines and BEGIN/END headers.
+    # Formats one or multiple private key(s) to canonical RFC 7468 PEM format.
+    #
+    # @note Unlike `PemFormatter#format_private_key`, this method returns the
+    # original input string if the input cannot be parsed.
     #
     # @param key [String] The original private key(s)
-    # @param multi [true|false] Whether to return multiple keys delimited by newline
-    # @return [String|nil] The formatted private key(s)
+    # @param multi [true|false] Whether to return multiple keys delimited by newline.
+    #   Default false for compatibility with legacy behavior.
+    # @return [String] The formatted private key(s). For legacy compatibility reasons,
+    #   this method returns the original string if the input cannot be parsed.
     def format_private_key(key, multi: false)
-      PemFormatter.format_private_key(key, multi: multi)
+      PemFormatter.format_private_key(key, multi: multi) || key
     end
 
     # Given a certificate string, return an OpenSSL::X509::Certificate object.
     #
-    # @param cert [String] The original certificate
+    # @param pem [String] The original certificate
     # @return [OpenSSL::X509::Certificate] The certificate object
-    def build_cert_object(cert)
-      return unless (pem = format_cert(cert, multi: false))
+    def build_cert_object(pem)
+      return unless (pem = PemFormatter.format_cert(pem, multi: false))
 
       OpenSSL::X509::Certificate.new(pem)
     end
 
     # Given a private key string, return an OpenSSL::PKey::RSA object.
     #
-    # @param cert [String] The original private key
-    # @return [OpenSSL::PKey::RSA] The private key object
-    def build_private_key_object(private_key)
-      return unless (pem = format_private_key(private_key, multi: false))
+    # @param pem [String] The original private key.
+    # @return [OpenSSL::PKey::RSA] 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)
     end