[READY] v2.0 - Consistently format cert/private key PEMs (#711)

* Standardize how RubySaml reads and formats certificate and private_key PEM values, including the `RubySaml::Util#format_cert` and `#format_private_key` methods. Introduces new `RubySaml::PemFormatter` module.

* Small change to regex

Author: johnnyshields

.rubocop.yml

@@ -16,6 +16,9 @@ AllCops:
     - 'tmp/**/*'
     - 'vendor/**/*'
 
+Style/ModuleFunction:
+  EnforcedStyle: extend_self
+
 Style/NumericPredicate:
   EnforcedStyle: comparison
 

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2024-07-10 16:10:44 UTC using RuboCop version 1.64.1.
+# on 2024-07-11 13:04:30 UTC using RuboCop version 1.64.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -127,7 +127,7 @@ Layout/SpaceAroundOperators:
     - 'lib/ruby_saml/xml/document.rb'
     - 'lib/ruby_saml/xml/signed_document.rb'
 
-# Offense count: 5
+# Offense count: 3
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBraces, SpaceBeforeBlockParameters.
 # SupportedStyles: space, no_space
@@ -180,7 +180,7 @@ Lint/UselessAssignment:
   Exclude:
     - 'lib/ruby_saml/slo_logoutrequest.rb'
 
-# Offense count: 42
+# Offense count: 41
 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
 Metrics/AbcSize:
   Max: 100
@@ -191,21 +191,26 @@ Metrics/AbcSize:
 Metrics/BlockLength:
   Max: 27
 
-# Offense count: 9
+# Offense count: 8
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
   Max: 652
 
-# Offense count: 25
+# Offense count: 26
 # Configuration parameters: AllowedMethods, AllowedPatterns.
 Metrics/CyclomaticComplexity:
   Max: 21
 
-# Offense count: 59
+# Offense count: 58
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
   Max: 63
 
+# Offense count: 1
+# Configuration parameters: CountComments, CountAsOne.
+Metrics/ModuleLength:
+  Max: 244
+
 # Offense count: 2
 # Configuration parameters: Max, CountKeywordArgs.
 Metrics/ParameterLists:
@@ -279,22 +284,20 @@ Performance/RedundantEqualityComparisonBlock:
   Exclude:
     - 'lib/ruby_saml/settings.rb'
 
-# Offense count: 5
+# Offense count: 3
 # This cop supports unsafe autocorrection (--autocorrect-all).
 Performance/StringInclude:
   Exclude:
     - 'lib/ruby_saml/authrequest.rb'
     - 'lib/ruby_saml/logoutrequest.rb'
     - 'lib/ruby_saml/slo_logoutresponse.rb'
-    - 'lib/ruby_saml/utils.rb'
 
-# Offense count: 8
+# Offense count: 4
 # This cop supports safe autocorrection (--autocorrect).
 Performance/StringReplacement:
   Exclude:
     - 'lib/ruby_saml/metadata.rb'
     - 'lib/ruby_saml/saml_message.rb'
-    - 'lib/ruby_saml/utils.rb'
     - 'lib/ruby_saml/xml/document.rb'
 
 # Offense count: 48
@@ -409,14 +412,6 @@ Style/IfUnlessModifier:
     - 'lib/ruby_saml/xml/document.rb'
     - 'lib/ruby_saml/xml/signed_document.rb'
 
-# Offense count: 1
-# This cop supports unsafe autocorrection (--autocorrect-all).
-# Configuration parameters: EnforcedStyle, Autocorrect.
-# SupportedStyles: module_function, extend_self, forbidden
-Style/ModuleFunction:
-  Exclude:
-    - 'lib/ruby_saml/logging.rb'
-
 # Offense count: 16
 # Configuration parameters: AllowedMethods.
 # AllowedMethods: respond_to_missing?
@@ -432,18 +427,11 @@ Style/OptionalBooleanParameter:
     - 'lib/ruby_saml/utils.rb'
     - 'lib/ruby_saml/xml/signed_document.rb'
 
-# Offense count: 1
-# This cop supports safe autocorrection (--autocorrect).
-Style/RedundantBegin:
-  Exclude:
-    - 'lib/ruby_saml/utils.rb'
-
-# Offense count: 8
+# Offense count: 3
 # This cop supports safe autocorrection (--autocorrect).
 Style/RedundantRegexpArgument:
   Exclude:
     - 'lib/ruby_saml/saml_message.rb'
-    - 'lib/ruby_saml/utils.rb'
     - 'lib/ruby_saml/xml/document.rb'
 
 # Offense count: 3
@@ -472,7 +460,7 @@ Style/StringConcatenation:
     - 'lib/ruby_saml/saml_message.rb'
     - 'lib/ruby_saml/slo_logoutrequest.rb'
 
-# Offense count: 351
+# Offense count: 339
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline.
 # SupportedStyles: single_quotes, double_quotes
@@ -509,7 +497,7 @@ Style/SymbolArray:
   Exclude:
     - 'lib/ruby_saml/settings.rb'
 
-# Offense count: 92
+# Offense count: 104
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
 # URISchemes: http, https

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.rb

@@ -16,6 +16,7 @@
 require 'ruby_saml/validation_error'
 require 'ruby_saml/metadata'
 require 'ruby_saml/idp_metadata_parser'
+require 'ruby_saml/pem_formatter'
 require 'ruby_saml/utils'
 require 'ruby_saml/version'
 

lib/ruby_saml/pem_formatter.rb

@@ -0,0 +1,126 @@
+# frozen_string_literal: true
+
+module RubySaml
+  # 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 X.509 certificate(s) to an array of strings in canonical
+    # RFC 7468 PEM format.
+    #
+    # @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)
+      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 multiple private key(s) to canonical RFC 7468
+    # PEM format.
+    #
+    # @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)
+      pem_array_to_string(format_private_key_array(key), multi: multi)
+    end
+
+    private
+
+    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) }
+
+      # 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
+
+    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
+
+    # 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
+
+    # 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
+
+    # 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