Skip to content

Commit f54b49d

Browse files
pitbulkpitbulk
authored
Merge pull request #743 from johnnyshields/v2.x-partial-nokogiri-upgrade
 V2.x - Migrate some inner code to Nokogiri
2 parents cfad250 + 0d10629 commit f54b49d

File tree

7 files changed

+126
-98
lines changed

7 files changed

+126
-98
lines changed

‎lib/ruby_saml/authrequest.rb‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ def create_xml_document(settings)
101101

102102
request_doc = RubySaml::XML::Document.new
103103
request_doc.context[:attribute_quote] = :quote
104-
request_doc.uuid = uuid
105104

106105
root = request_doc.add_element "samlp:AuthnRequest", { "xmlns:samlp" => "urn:oasis:names:tc:SAML:2.0:protocol", "xmlns:saml" => "urn:oasis:names:tc:SAML:2.0:assertion" }
107106
root.attributes['ID'] = uuid

‎lib/ruby_saml/logoutrequest.rb‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ def create_xml_document(settings)
9999

100100
request_doc = RubySaml::XML::Document.new
101101
request_doc.context[:attribute_quote] = :quote
102-
request_doc.uuid = uuid
103102

104103
root = request_doc.add_element "samlp:LogoutRequest", { "xmlns:samlp" => "urn:oasis:names:tc:SAML:2.0:protocol", "xmlns:saml" => "urn:oasis:names:tc:SAML:2.0:assertion" }
105104
root.attributes['ID'] = uuid

‎lib/ruby_saml/slo_logoutresponse.rb‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ def create_xml_document(settings, request_id = nil, logout_message = nil, status
110110

111111
response_doc = RubySaml::XML::Document.new
112112
response_doc.context[:attribute_quote] = :quote
113-
response_doc.uuid = uuid
114113

115114
destination = settings.idp_slo_response_service_url || settings.idp_slo_service_url
116115

‎lib/ruby_saml/xml/crypto.rb‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ def hash_algorithm(element)
8585
private
8686

8787
def get_algorithm_attr(element)
88-
if element.is_a?(REXML::Element)
88+
if element.is_a?(Nokogiri::XML::Element)
89+
element['Algorithm']
90+
elsif element.is_a?(REXML::Element)
8991
element.attribute('Algorithm').value
9092
elsif element
9193
element

‎lib/ruby_saml/xml/document.rb‎

Lines changed: 91 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,6 @@ class Document < BaseDocument
2727
SHA512 = RubySaml::XML::Crypto::SHA512
2828
ENVELOPED_SIG = RubySaml::XML::Crypto::ENVELOPED_SIG
2929

30-
attr_writer :uuid
31-
32-
def uuid
33-
@uuid ||= document.root&.attributes&.[]('ID')
34-
end
35-
3630
# <Signature>
3731
# <SignedInfo>
3832
# <CanonicalizationMethod />
@@ -48,70 +42,114 @@ def uuid
4842
# <KeyInfo />
4943
# <Object />
5044
# </Signature>
51-
def sign_document(private_key, certificate, signature_method = RSA_SHA256, digest_method = SHA256)
45+
def sign_document(private_key, certificate, signature_method = RubySaml::XML::Crypto::RSA_SHA256, digest_method = RubySaml::XML::Crypto::SHA256)
46+
signature_element = build_signature_element(private_key, certificate, signature_method, digest_method)
47+
signature_element = convert_nokogiri_to_rexml(signature_element)
48+
issuer_element = elements['//saml:Issuer']
49+
if issuer_element
50+
root.insert_after(issuer_element, signature_element)
51+
elsif (first_child = root.children[0])
52+
root.insert_before(first_child, signature_element)
53+
else
54+
root.add_element(signature_element)
55+
end
56+
end
57+
58+
private
59+
60+
def build_signature_element(private_key, certificate, signature_method, digest_method)
61+
# Parse the original document
5262
noko = Nokogiri::XML(to_s) do |config|
5363
config.options = RubySaml::XML::BaseDocument::NOKOGIRI_OPTIONS
5464
end
5565

56-
signature_element = REXML::Element.new('ds:Signature').add_namespace('ds', RubySaml::XML::Crypto::DSIG)
57-
signed_info_element = signature_element.add_element('ds:SignedInfo')
58-
signed_info_element.add_element('ds:CanonicalizationMethod', {'Algorithm' => RubySaml::XML::Crypto::C14N})
59-
signed_info_element.add_element('ds:SignatureMethod', {'Algorithm'=>signature_method})
66+
# Build the Signature element
67+
signature_element = Nokogiri::XML::Builder.new do |xml|
68+
xml['ds'].Signature('xmlns:ds' => RubySaml::XML::Crypto::DSIG) do
69+
xml['ds'].SignedInfo do
70+
xml['ds'].CanonicalizationMethod(Algorithm: RubySaml::XML::Crypto::C14N)
71+
xml['ds'].SignatureMethod(Algorithm: signature_method)
72+
xml['ds'].Reference(URI: "##{noko.root.attr('ID')}") do
73+
xml['ds'].Transforms do
74+
xml['ds'].Transform(Algorithm: RubySaml::XML::Crypto::ENVELOPED_SIG)
75+
xml['ds'].Transform(Algorithm: RubySaml::XML::Crypto::C14N) do
76+
xml['ec'].InclusiveNamespaces(
77+
'xmlns:ec' => RubySaml::XML::Crypto::C14N,
78+
PrefixList: INC_PREFIX_LIST
79+
)
80+
end
81+
end
82+
xml['ds'].DigestMethod(Algorithm: digest_method)
83+
xml['ds'].DigestValue(digest_value(noko, digest_method))
84+
end
85+
end
86+
xml['ds'].SignatureValue # Value is added below
87+
xml['ds'].KeyInfo do
88+
xml['ds'].X509Data do
89+
xml['ds'].X509Certificate(certificate_value(certificate))
90+
end
91+
end
92+
end
93+
end.doc.root
94+
95+
# Set the signature value
96+
signed_info_element = signature_element.at_xpath('//ds:SignedInfo', 'ds' => RubySaml::XML::Crypto::DSIG)
97+
sig_value_element = signature_element.at_xpath('//ds:SignatureValue', 'ds' => RubySaml::XML::Crypto::DSIG)
98+
sig_value_element.content = signature_value(signed_info_element, private_key, signature_method)
99+
100+
signature_element
101+
end
60102

61-
# Add Reference
62-
reference_element = signed_info_element.add_element('ds:Reference', {'URI' => "##{uuid}"})
103+
def digest_value(document, digest_method)
104+
inclusive_namespaces = INC_PREFIX_LIST.split
105+
canon_algorithm = RubySaml::XML::Crypto.canon_algorithm(RubySaml::XML::Crypto::C14N)
106+
hash_algorithm = RubySaml::XML::Crypto.hash_algorithm(digest_method)
63107

64-
# Add Transforms
65-
transforms_element = reference_element.add_element('ds:Transforms')
66-
transforms_element.add_element('ds:Transform', {'Algorithm' => RubySaml::XML::Crypto::ENVELOPED_SIG})
67-
c14element = transforms_element.add_element('ds:Transform', {'Algorithm' => RubySaml::XML::Crypto::C14N})
68-
c14element.add_element('ec:InclusiveNamespaces', {'xmlns:ec' => RubySaml::XML::Crypto::C14N, 'PrefixList' => INC_PREFIX_LIST})
108+
canon_doc = document.canonicalize(canon_algorithm, inclusive_namespaces)
109+
Base64.encode64(hash_algorithm.digest(canon_doc)).strip
110+
end
69111

70-
digest_method_element = reference_element.add_element('ds:DigestMethod', {'Algorithm' => digest_method})
71-
inclusive_namespaces = INC_PREFIX_LIST.split
72-
canon_doc = noko.canonicalize(RubySaml::XML::Crypto.canon_algorithm(RubySaml::XML::Crypto::C14N), inclusive_namespaces)
73-
reference_element.add_element('ds:DigestValue').text = compute_digest(canon_doc, RubySaml::XML::Crypto.hash_algorithm(digest_method_element))
112+
def signature_value(signed_info_element, private_key, signature_method)
113+
canon_algorithm = RubySaml::XML::Crypto.canon_algorithm(RubySaml::XML::Crypto::C14N)
114+
hash_algorithm = RubySaml::XML::Crypto.hash_algorithm(signature_method).new
74115

75-
# add SignatureValue
76-
noko_sig_element = Nokogiri::XML(signature_element.to_s) do |config|
77-
config.options = RubySaml::XML::BaseDocument::NOKOGIRI_OPTIONS
78-
end
116+
canon_string = signed_info_element.canonicalize(canon_algorithm)
117+
Base64.encode64(private_key.sign(hash_algorithm, canon_string)).delete("\n")
118+
end
79119

80-
noko_signed_info_element = noko_sig_element.at_xpath('//ds:Signature/ds:SignedInfo', 'ds' => RubySaml::XML::Crypto::DSIG)
81-
canon_string = noko_signed_info_element.canonicalize(RubySaml::XML::Crypto.canon_algorithm(RubySaml::XML::Crypto::C14N))
120+
def certificate_value(certificate)
121+
certificate = OpenSSL::X509::Certificate.new(certificate) if certificate.is_a?(String)
122+
Base64.encode64(certificate.to_der).delete("\n")
123+
end
82124

83-
signature = compute_signature(private_key, RubySaml::XML::Crypto.hash_algorithm(signature_method).new, canon_string)
84-
signature_element.add_element('ds:SignatureValue').text = signature
125+
# TODO: This is a shim method which will be removed when the
126+
# full Nokogiri conversion is complete
127+
def convert_nokogiri_to_rexml(noko_element, parent_namespaces = Set.new)
128+
rexml_element = REXML::Element.new("#{"#{noko_element.namespace.prefix}:" if noko_element.namespace}#{noko_element.name}")
85129

86-
# add KeyInfo
87-
key_info_element = signature_element.add_element('ds:KeyInfo')
88-
x509_element = key_info_element.add_element('ds:X509Data')
89-
x509_cert_element = x509_element.add_element('ds:X509Certificate')
90-
if certificate.is_a?(String)
91-
certificate = OpenSSL::X509::Certificate.new(certificate)
130+
if noko_element.namespace && !parent_namespaces.include?(noko_element.namespace)
131+
rexml_element.add_namespace(noko_element.namespace.prefix, noko_element.namespace.href)
92132
end
93-
x509_cert_element.text = Base64.encode64(certificate.to_der).gsub(/\n/, '')
94133

95-
# add the signature
96-
issuer_element = elements['//saml:Issuer']
97-
if issuer_element
98-
root.insert_after(issuer_element, signature_element)
99-
elsif (first_child = root.children[0])
100-
root.insert_before(first_child, signature_element)
101-
else
102-
root.add_element(signature_element)
134+
noko_element.attributes.each do |name, value|
135+
rexml_element.add_attribute(name, value)
103136
end
104-
end
105137

106-
private
138+
# Copy text content (if any)
139+
if noko_element.text?
140+
rexml_element.text = noko_element.text
141+
end
107142

108-
def compute_signature(private_key, signature_hash_algorithm, document)
109-
Base64.encode64(private_key.sign(signature_hash_algorithm, document)).gsub(/\n/, '')
110-
end
143+
# Recursively copy child elements
144+
noko_element.children.each do |child|
145+
if child.element?
146+
rexml_element.add_element(convert_nokogiri_to_rexml(child, parent_namespaces << noko_element.namespace))
147+
elsif child.text?
148+
rexml_element.add_text(child.text)
149+
end
150+
end
111151

112-
def compute_digest(document, digest_algorithm)
113-
digest = digest_algorithm.digest(document)
114-
Base64.encode64(digest).strip
152+
rexml_element
115153
end
116154
end
117155
end

‎lib/ruby_saml/xml/signed_document.rb‎

Lines changed: 30 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ module XML
99
class SignedDocument < BaseDocument
1010
include RubySaml::ErrorHandling
1111

12-
attr_writer :signed_element_id
13-
1412
def initialize(response, errors = [])
1513
super(response)
1614
@errors = errors
@@ -55,6 +53,7 @@ def validate_document(idp_cert_fingerprint, soft = true, options = {})
5553
else
5654
return append_error('Certificate element missing in response (ds:X509Certificate) and not cert provided at settings', soft)
5755
end
56+
5857
validate_signature(base64_cert, soft)
5958
end
6059

@@ -82,56 +81,50 @@ def validate_document_with_cert(idp_cert, soft = true)
8281
else
8382
base64_cert = Base64.encode64(idp_cert.to_pem)
8483
end
84+
8585
validate_signature(base64_cert, true)
8686
end
8787

8888
def validate_signature(base64_cert, soft = true)
89-
document = Nokogiri::XML(to_s) do |config|
89+
noko = Nokogiri::XML(to_s) do |config|
9090
config.options = RubySaml::XML::BaseDocument::NOKOGIRI_OPTIONS
9191
end
9292

93-
# create a rexml document
94-
@working_copy ||= REXML::Document.new(to_s).root
95-
9693
# get signature node
97-
sig_element = REXML::XPath.first(
98-
@working_copy,
99-
'//ds:Signature',
100-
{ 'ds' => RubySaml::XML::Crypto::DSIG }
101-
)
94+
sig_element = noko.at_xpath(
95+
'//ds:Signature',
96+
{ 'ds' => RubySaml::XML::Crypto::DSIG }
97+
)
10298

10399
# signature method
104-
sig_alg_value = REXML::XPath.first(
105-
sig_element,
100+
sig_alg_value = sig_element.at_xpath(
106101
'./ds:SignedInfo/ds:SignatureMethod',
107102
{ 'ds' => RubySaml::XML::Crypto::DSIG }
108103
)
109104
signature_hash_algorithm = RubySaml::XML::Crypto.hash_algorithm(sig_alg_value)
110105

111106
# get signature
112-
base64_signature = REXML::XPath.first(
113-
sig_element,
107+
base64_signature = sig_element.at_xpath(
114108
'./ds:SignatureValue',
115-
{ 'ds' => RubySaml::XML::Crypto::DSIG}
116-
)
117-
signature = Base64.decode64(RubySaml::Utils.element_text(base64_signature))
109+
{ 'ds' => RubySaml::XML::Crypto::DSIG }
110+
).content
111+
signature = Base64.decode64(base64_signature)
118112

119113
# canonicalization method
120-
canon_algorithm = RubySaml::XML::Crypto.canon_algorithm(REXML::XPath.first(
121-
sig_element,
114+
canon_method_node = sig_element.at_xpath(
122115
'./ds:SignedInfo/ds:CanonicalizationMethod',
123-
'ds' => RubySaml::XML::Crypto::DSIG
124-
))
116+
{ 'ds' => RubySaml::XML::Crypto::DSIG }
117+
)
118+
canon_algorithm = RubySaml::XML::Crypto.canon_algorithm(canon_method_node)
125119

126-
noko_sig_element = document.at_xpath('//ds:Signature', 'ds' => RubySaml::XML::Crypto::DSIG)
120+
noko_sig_element = noko.at_xpath('//ds:Signature', 'ds' => RubySaml::XML::Crypto::DSIG)
127121
noko_signed_info_element = noko_sig_element.at_xpath('./ds:SignedInfo', 'ds' => RubySaml::XML::Crypto::DSIG)
128122

129123
canon_string = noko_signed_info_element.canonicalize(canon_algorithm)
130124
noko_sig_element.remove
131125

132126
# get signed info
133-
signed_info_element = REXML::XPath.first(
134-
sig_element,
127+
signed_info_element = sig_element.at_xpath(
135128
'./ds:SignedInfo',
136129
{ 'ds' => RubySaml::XML::Crypto::DSIG }
137130
)
@@ -140,13 +133,12 @@ def validate_signature(base64_cert, soft = true)
140133
inclusive_namespaces = extract_inclusive_namespaces
141134

142135
# check digests
143-
ref = REXML::XPath.first(
144-
signed_info_element,
136+
ref = signed_info_element.at_xpath(
145137
'./ds:Reference',
146138
{ 'ds' => RubySaml::XML::Crypto::DSIG }
147139
)
148140

149-
reference_nodes = document.xpath('//*[@ID=$id]', nil, { 'id' => extract_signed_element_id })
141+
reference_nodes = noko.xpath('//*[@ID=$id]', nil, { 'id' => extract_signed_element_id })
150142

151143
# ensure no elements with same ID to prevent signature wrapping attack.
152144
if reference_nodes.length > 1
@@ -155,28 +147,27 @@ def validate_signature(base64_cert, soft = true)
155147

156148
hashed_element = reference_nodes[0]
157149

158-
canon_algorithm = RubySaml::XML::Crypto.canon_algorithm(REXML::XPath.first(
159-
signed_info_element,
150+
canon_method_node = signed_info_element.at_xpath(
160151
'./ds:CanonicalizationMethod',
161152
{ 'ds' => RubySaml::XML::Crypto::DSIG }
162-
))
163-
153+
)
154+
canon_algorithm = RubySaml::XML::Crypto.canon_algorithm(canon_method_node)
164155
canon_algorithm = process_transforms(ref, canon_algorithm)
165156

166157
canon_hashed_element = hashed_element.canonicalize(canon_algorithm, inclusive_namespaces)
167158

168-
digest_algorithm = RubySaml::XML::Crypto.hash_algorithm(REXML::XPath.first(
169-
ref,
159+
digest_method_node = ref.at_xpath(
170160
'./ds:DigestMethod',
171161
{ 'ds' => RubySaml::XML::Crypto::DSIG }
172-
))
162+
)
163+
digest_algorithm = RubySaml::XML::Crypto.hash_algorithm(digest_method_node)
164+
173165
hash = digest_algorithm.digest(canon_hashed_element)
174-
encoded_digest_value = REXML::XPath.first(
175-
ref,
166+
encoded_digest_value = ref.at_xpath(
176167
'./ds:DigestValue',
177168
{ 'ds' => RubySaml::XML::Crypto::DSIG }
178-
)
179-
digest_value = Base64.decode64(RubySaml::Utils.element_text(encoded_digest_value))
169+
).content
170+
digest_value = Base64.decode64(encoded_digest_value)
180171

181172
unless digests_match?(hash, digest_value)
182173
return append_error('Digest mismatch', soft)

‎test/utils_test.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,11 +375,11 @@ def result(duration, reference = 0)
375375
end
376376

377377
it 'successfully decrypts with the first private key' do
378-
assert_match(%r{\A<saml:Assertion}, RubySaml::Utils.decrypt_multi(encrypted, [private_key]))
378+
assert_match(/\A<saml:Assertion/, RubySaml::Utils.decrypt_multi(encrypted, [private_key]))
379379
end
380380

381381
it 'successfully decrypts with a subsequent private key' do
382-
assert_match(%r{\A<saml:Assertion}, RubySaml::Utils.decrypt_multi(encrypted, [invalid_key1, private_key]))
382+
assert_match(/\A<saml:Assertion/, RubySaml::Utils.decrypt_multi(encrypted, [invalid_key1, private_key]))
383383
end
384384

385385
it 'raises an error when there is only one key and it fails to decrypt' do

0 commit comments

Comments
 (0)