Replace REXML with Nokogiri for consistency

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    saml_idp (0.21.5.pre.18f)
+    saml_idp (0.21.6.pre.18f)
       activesupport
       builder
       faraday
@@ -308,7 +308,6 @@ DEPENDENCIES
   pry-byebug
   rails (~> 7.1)
   rake
-  rexml
   rspec
   rubocop (= 1.62.0)
   rubocop-rails (= 2.9)

lib/saml_idp/version.rb

@@ -1,3 +1,3 @@
 module SamlIdp
-  VERSION = '0.21.5-18f'.freeze
+  VERSION = '0.21.6-18f'.freeze
 end

lib/saml_idp/xml_security.rb

@@ -22,16 +22,14 @@
 # Copyright 2007 Sun Microsystems Inc. All Rights Reserved
 # Portions Copyrighted 2007 Todd W Saxton.
 
-require 'rexml/document'
-require 'rexml/xpath'
 require 'openssl'
 require 'nokogiri'
 require 'digest/sha1'
 require 'digest/sha2'
 
 module SamlIdp
   module XMLSecurity
-    class SignedDocument < REXML::Document
+    class SignedDocument
       class ValidationError < StandardError
         attr_reader :error_code
 
@@ -44,11 +42,10 @@ def initialize(msg = nil, error_code = nil)
       C14N = 'http://www.w3.org/2001/10/xml-exc-c14n#'
       DSIG = 'http://www.w3.org/2000/09/xmldsig#'
 
-      attr_accessor :signed_element_id
+      attr_accessor :document
 
       def initialize(response)
-        super(response)
-        extract_signed_element_id
+        @document = Nokogiri.parse(response)
       end
 
       def validate(idp_cert_fingerprint, soft = true, options = {})
@@ -97,10 +94,10 @@ def signature_algorithm(options)
         if options[:get_params] && options[:get_params][:SigAlg]
           algorithm(options[:get_params][:SigAlg])
         else
-          ref_elem = REXML::XPath.first(self, '//ds:Reference', { 'ds' => DSIG })
+          ref_elem = document.at_xpath('//*:Reference')
           return nil unless ref_elem
 
-          algorithm(REXML::XPath.first(ref_elem, '//ds:DigestMethod', { 'ds' => DSIG }))
+          algorithm(ref_elem.at_xpath('//*:DigestMethod'))
         end
       end
 
@@ -114,7 +111,7 @@ def fingerprint_cert_sha1(cert)
       end
 
       def find_base64_cert(options)
-        cert_element = REXML::XPath.first(self, '//ds:X509Certificate', { 'ds' => DSIG })
+        cert_element = document.at_xpath('//*:X509Certificate')
         if cert_element
           return cert_element.text if cert_element.text.present?
 
@@ -139,7 +136,7 @@ def find_base64_cert(options)
       end
 
       def request?
-        root.name != 'Response'
+        document.root.name != 'Response'
       end
 
       # matches RubySaml::Utils
@@ -179,56 +176,26 @@ def validate_doc_embedded_signature(
       )
         # check for inclusive namespaces
         inclusive_namespaces = extract_inclusive_namespaces
-        document = Nokogiri.parse(to_s)
 
-        # create a working copy so we don't modify the original
-        @working_copy ||= REXML::Document.new(to_s).root
 
-        # store and remove signature node
-        @sig_element ||= begin
-          element = REXML::XPath.first(
-            @working_copy,
-            '//ds:Signature',
-            { 'ds' => DSIG }
-          )
-          element.remove
-        end
+        sig_element = document.at_xpath('//*:Signature')
+        sig_namespace_hash = sig_element.at_xpath('//*:SignedInfo')&.namespaces
+        noko_signed_info_element = sig_element.at_xpath('./*:SignedInfo')
 
-        # TODO: Should we be discovering/assigning the sig_namespace_hash differently?
-        sig_ns_elem = REXML::XPath.first(
-          @sig_element,
-          '//ds:SignedInfo',
-          { 'ds' => DSIG }
-        )
-        sig_namespace_hash = { 'ds' => DSIG } if sig_ns_elem
-
-        signed_info_element = REXML::XPath.first(
-          @sig_element,
-          '//ds:SignedInfo',
-          sig_namespace_hash
-        )
-
-        noko_sig_element = document.at_xpath('//ds:Signature', 'ds' => DSIG)
-        noko_signed_info_element = noko_sig_element.at_xpath('./ds:SignedInfo', 'ds' => DSIG)
-        canon_algorithm = canon_algorithm REXML::XPath.first(
-          @sig_element,
-          '//ds:CanonicalizationMethod',
-          sig_namespace_hash
+        canon_algorithm = canon_algorithm(
+          sig_element.at_xpath('//*:CanonicalizationMethod')
         )
 
         canon_string = noko_signed_info_element.canonicalize(canon_algorithm)
-        noko_sig_element.remove
-
         # check digests
-        REXML::XPath.each(@sig_element, '//ds:Reference', sig_namespace_hash) do |ref|
-          uri = ref.attributes.get_attribute('URI').value
+        sig_element.xpath('//*:Reference').each do |ref|
+          uri = ref.attribute('URI').value
 
-          hashed_element = document.at_xpath("//*[@ID='#{uri[1..-1]}']")
-          canon_algorithm = canon_algorithm REXML::XPath.first(
-            ref,
-            '//ds:CanonicalizationMethod',
-            sig_namespace_hash
-          )
+          hashed_element = document.dup.at_xpath("//*[@ID='#{uri[1..-1]}']")
+          # removing the Signature node and children to get digest
+          hashed_element.at_xpath('//*:Signature').remove
+
+          canon_algorithm = canon_algorithm(ref.at_xpath('//*:CanonicalizationMethod'))
 
           canon_hashed_element = hashed_element.canonicalize(
             canon_algorithm,
@@ -237,16 +204,12 @@ def validate_doc_embedded_signature(
 
           digest_algorithm = digest_method_algorithm(
             ref,
-            sig_namespace_hash,
             digest_method_fix_enabled
           )
 
           hash = digest_algorithm.digest(canon_hashed_element)
-          digest_value = Base64.decode64(REXML::XPath.first(
-            ref,
-            '//ds:DigestValue',
-            sig_namespace_hash
-          ).text)
+
+          digest_value = Base64.decode64(ref.at_xpath('//*:DigestValue').text)
 
           unless digests_match?(hash, digest_value)
             return soft ? false : (raise ValidationError.new(
@@ -255,28 +218,21 @@ def validate_doc_embedded_signature(
           end
         end
 
-        base64_signature = REXML::XPath.first(
-          @sig_element,
-          '//ds:SignatureValue',
-          sig_namespace_hash
-        ).text
-
+        base64_signature = sig_element.at_xpath('//*:SignatureValue').text
         signature = Base64.decode64(base64_signature)
-        sig_alg = REXML::XPath.first(
-          signed_info_element,
-          '//ds:SignatureMethod',
-          sig_namespace_hash
-        )
+
+        sig_alg = sig_element.at_xpath('//*:SignatureMethod')
 
         log '***** validate_doc_embedded_signature: verify_signature:'
         verify_signature(base64_cert, sig_alg, signature, canon_string, soft)
       end
 
-      def digest_method_algorithm(ref, hash, digest_method_fix_enabled)
-        if digest_method_fix_enabled
-          algorithm(REXML::XPath.first(ref, '//ds:DigestMethod', hash))
+      def digest_method_algorithm(ref, digest_method_fix_enabled)
+        digest_method = ref.at_xpath('//*:DigestMethod')
+        if digest_method_fix_enabled || digest_method.namespace.prefix.present?
+          algorithm(digest_method)
         else
-          algorithm(REXML::XPath.first(ref, '//ds:DigestMethod'))
+          algorithm(nil)
         end
       end
 
@@ -298,14 +254,10 @@ def digests_match?(hash, digest_value)
       end
 
       def extract_signed_element_id
-        reference_element = REXML::XPath.first(
-          self,
-          '//ds:Signature/ds:SignedInfo/ds:Reference',
-          { 'ds' => DSIG }
-        )
+        reference_element = document.at_xpath('//*:Signature/*:SignedInfo/*:Reference')
         return if reference_element.nil?
 
-        self.signed_element_id = reference_element.attribute('URI').value[1..-1]
+        @signed_element_id = reference_element.attribute('URI').value[1..-1]
       end
 
       def canon_algorithm(element)
@@ -324,7 +276,7 @@ def canon_algorithm(element)
 
       def algorithm(element)
         algorithm = element
-        algorithm = element.attribute('Algorithm').value if algorithm.is_a?(REXML::Element)
+        algorithm = element.attribute('Algorithm').value if algorithm.is_a?(Nokogiri::XML::Element)
         log "~~~~~~ Algorithm: #{algorithm}"
         algorithm = algorithm && algorithm =~ /(rsa-)?sha(.*?)$/i && ::Regexp.last_match(2).to_i
         case algorithm
@@ -344,12 +296,9 @@ def algorithm(element)
       end
 
       def extract_inclusive_namespaces
-        if element = REXML::XPath.first(self, '//ec:InclusiveNamespaces', { 'ec' => C14N })
-          prefix_list = element.attributes.get_attribute('PrefixList').value
-          prefix_list.split(' ')
-        else
-          []
-        end
+        document.at_xpath(
+          "//ec:InclusiveNamespaces", { 'ec' =>  C14N }
+        )&.attr('PrefixList')&.split(' ') || []
       end
 
       def log(msg, level: :debug)

saml_idp.gemspec

@@ -34,7 +34,6 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'pry-byebug'
   s.add_development_dependency('rails', '~> 7.1')
   s.add_development_dependency 'rake'
-  s.add_development_dependency('rexml')
   s.add_development_dependency 'rspec'
   s.add_development_dependency 'rubocop', '1.62.0'
   s.add_development_dependency 'rubocop-rails', '2.9'

spec/lib/saml_idp/controller_spec.rb

@@ -91,10 +91,9 @@ def head(status, options = {}); end
     it 'signs a SAML Response if requested' do
       saml_response_encoded = encode_response(principal, signed_response_message: true)
       saml_response_text = Base64.decode64(saml_response_encoded)
-      saml_response = REXML::Document.new(saml_response_text)
-      response_id = REXML::XPath.match(saml_response, '//samlp:Response').first.attributes['ID']
-      signature_ref = REXML::XPath.match(saml_response,
-                                         '//ds:Reference').first.attributes['URI'][1..-1]
+      saml_response = Nokogiri.parse(saml_response_text)
+      response_id = saml_response.at_xpath('//*:Response').attributes['ID'].value
+      signature_ref = saml_response.at_xpath('//*:Reference').attributes['URI'].value[1..-1]
 
       expect(signature_ref).to eq response_id
     end

spec/lib/saml_idp/logout_response_builder_spec.rb

@@ -39,10 +39,8 @@ module SamlIdp
     end
 
     it 'includes the response_id in the signature' do
-      signed = subject.signed
-      doc = REXML::Document.new signed
-      signature = REXML::XPath.first(doc, '//ds:Signature',
-                                     { 'ds' => SamlIdp::XMLSecurity::SignedDocument::DSIG })
+      doc = Nokogiri.parse subject.signed
+      signature = doc.at_xpath('//*:Signature')
       expect(signature.to_s).to include(response_id)
     end
   end

spec/support/saml_request_macros.rb

@@ -94,11 +94,4 @@ def invalid_saml_settings(values: {'issuer': ''}, signed: false)
 
     settings
   end
-
-  def print_pretty_xml(xml_string)
-    doc = REXML::Document.new xml_string
-    outbuf = ''
-    doc.write(outbuf, 1)
-    puts outbuf
-  end
 end