Skip to content

Commit f683f5c

Browse files
pitbulkpitbulk
committed
Fix Http-Redirect Signature: SignAlg and RelayState issue and algorithm calculator #215
Now Signature algorithm 256, 384, 512 are supported on HTTP-Redirect Signatures (Tested with real IdPs). It was fixed a bug of mixing SHA1 and RSA_SHA1. algorithm and test were fixed. We found a problem when creating the Signature, after some research we discovered that in some environments params['RelayState'] and params[:RelayState] are differents so Signatures were created without the RelayState parameter when keep sending a RelayState to the IdP, so the validation always failed. We added a compatibility line to fix that issue.
2 parents 4927043 + 967e392 commit f683f5c

File tree

7 files changed

+147
-62
lines changed

7 files changed

+147
-62
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ The settings related to sign are stored in the `security` attribute of the setti
342342
settings.security[:metadata_signed] = true # Enable or not signature on Metadata
343343
344344
settings.security[:digest_method] = XMLSecurity::Document::SHA1
345-
settings.security[:signature_method] = XMLSecurity::Document::SHA1
345+
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
346346
347347
# Embeded signature or HTTP GET parameter signature
348348
# Note that metadata signature is always embedded regardless of this value.

‎lib/onelogin/ruby-saml/authrequest.rb‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ def create(settings, params = {})
2727
end
2828

2929
def create_params(settings, params={})
30-
params = {} if params.nil?
30+
# The method expects :RelayState but sometimes we get 'RelayState' instead.
31+
# Based on the HashWithIndifferentAccess value in Rails we could experience
32+
# conflicts so this line will solve them.
33+
relay_state = params[:RelayState] || params['RelayState']
3134

3235
request_doc = create_authentication_xml_doc(settings)
3336
request_doc.context[:attribute_quote] = :quote if settings.double_quote_xml_attribute_values
@@ -42,10 +45,10 @@ def create_params(settings, params={})
4245
request_params = {"SAMLRequest" => base64_request}
4346

4447
if settings.security[:authn_requests_signed] && !settings.security[:embed_sign] && settings.private_key
45-
params['SigAlg'] = XMLSecurity::Document::RSA_SHA1
48+
params['SigAlg'] = settings.security[:signature_method]
4649
url_string = "SAMLRequest=#{CGI.escape(base64_request)}"
47-
url_string += "&RelayState=#{CGI.escape(params['RelayState'])}" if params['RelayState']
48-
url_string += "&SigAlg=#{CGI.escape(params['SigAlg'])}"
50+
url_string << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
51+
url_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
4952
private_key = settings.get_sp_key()
5053
signature = private_key.sign(XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method]).new, url_string)
5154
params['Signature'] = encode(signature)

‎lib/onelogin/ruby-saml/logoutrequest.rb‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ def create(settings, params={})
2525
end
2626

2727
def create_params(settings, params={})
28-
params = {} if params.nil?
28+
# The method expects :RelayState but sometimes we get 'RelayState' instead.
29+
# Based on the HashWithIndifferentAccess value in Rails we could experience
30+
# conflicts so this line will solve them.
31+
relay_state = params[:RelayState] || params['RelayState']
2932

3033
request_doc = create_logout_request_xml_doc(settings)
3134
request_doc.context[:attribute_quote] = :quote if settings.double_quote_xml_attribute_values
@@ -40,10 +43,10 @@ def create_params(settings, params={})
4043
request_params = {"SAMLRequest" => base64_request}
4144

4245
if settings.security[:logout_requests_signed] && !settings.security[:embed_sign] && settings.private_key
43-
params['SigAlg'] = XMLSecurity::Document::RSA_SHA1
46+
params['SigAlg'] = settings.security[:signature_method]
4447
url_string = "SAMLRequest=#{CGI.escape(base64_request)}"
45-
url_string += "&RelayState=#{CGI.escape(params['RelayState'])}" if params['RelayState']
46-
url_string += "&SigAlg=#{CGI.escape(params['SigAlg'])}"
48+
url_string << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
49+
url_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
4750
private_key = settings.get_sp_key()
4851
signature = private_key.sign(XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method]).new, url_string)
4952
params['Signature'] = encode(signature)

‎lib/onelogin/ruby-saml/slo_logoutresponse.rb‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ def create(settings, request_id = nil, logout_message = nil, params = {})
2626
end
2727

2828
def create_params(settings, request_id = nil, logout_message = nil, params = {})
29-
params = {} if params.nil?
29+
# The method expects :RelayState but sometimes we get 'RelayState' instead.
30+
# Based on the HashWithIndifferentAccess value in Rails we could experience
31+
# conflicts so this line will solve them.
32+
relay_state = params[:RelayState] || params['RelayState']
3033

3134
response_doc = create_logout_response_xml_doc(settings, request_id, logout_message)
3235
response_doc.context[:attribute_quote] = :quote if settings.double_quote_xml_attribute_values
@@ -41,10 +44,10 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {})
4144
response_params = {"SAMLResponse" => base64_response}
4245

4346
if settings.security[:logout_responses_signed] && !settings.security[:embed_sign] && settings.private_key
44-
params['SigAlg'] = XMLSecurity::Document::RSA_SHA1
47+
params['SigAlg'] = settings.security[:signature_method]
4548
url_string = "SAMLResponse=#{CGI.escape(base64_response)}"
46-
url_string += "&RelayState=#{CGI.escape(params['RelayState'])}" if params['RelayState']
47-
url_string += "&SigAlg=#{CGI.escape(params['SigAlg'])}"
49+
url_string << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
50+
url_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
4851
private_key = settings.get_sp_key()
4952
signature = private_key.sign(XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method]).new, url_string)
5053
params['Signature'] = encode(signature)

‎test/logoutrequest_test.rb‎

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -132,29 +132,53 @@ class RequestTest < Minitest::Test
132132
end
133133
end
134134

135-
describe "when the settings indicate to sign the logout request" do
136-
it "create a signature parameter" do
137-
settings = OneLogin::RubySaml::Settings.new
138-
settings.compress_request = false
139-
settings.idp_slo_target_url = "http://example.com?field=value"
140-
settings.name_identifier_value = "f00f00"
141-
settings.security[:logout_requests_signed] = true
142-
settings.security[:embed_sign] = false
135+
describe "#create_params when the settings indicate to sign the logout request" do
136+
def setup
137+
@settings = OneLogin::RubySaml::Settings.new
138+
@settings.compress_request = false
139+
@settings.idp_sso_target_url = "http://example.com?field=value"
140+
@settings.name_identifier_value = "f00f00"
141+
@settings.security[:logout_requests_signed] = true
142+
@settings.security[:embed_sign] = false
143+
@settings.certificate = ruby_saml_cert_text
144+
@settings.private_key = ruby_saml_key_text
145+
@cert = OpenSSL::X509::Certificate.new(ruby_saml_cert_text)
146+
end
147+
148+
it "create a signature parameter with RSA_SHA1 and validate it" do
143149
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
144-
settings.certificate = ruby_saml_cert_text
145-
settings.private_key = ruby_saml_key_text
146150

147-
params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings)
151+
params = OneLogin::RubySaml::Logoutrequest.new.create_params(@settings, :RelayState => 'http://example.com')
152+
assert params['SAMLRequest']
153+
assert params[:RelayState]
148154
assert params['Signature']
149155
assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA1
150156

151-
# signature_method only affects the embedeed signature
152-
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
153-
params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings)
157+
query_string = "SAMLRequest=#{CGI.escape(params['SAMLRequest'])}"
158+
query_string << "&RelayState=#{CGI.escape(params[:RelayState])}"
159+
query_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
160+
161+
signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
162+
assert_equal signature_algorithm, OpenSSL::Digest::SHA1
163+
assert @cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
164+
end
165+
166+
it "create a signature parameter with RSA_SHA256 and validate it" do
167+
@settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
168+
169+
params = OneLogin::RubySaml::Logoutrequest.new.create_params(@settings, :RelayState => 'http://example.com')
154170
assert params['Signature']
155-
assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA1
171+
assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA256
172+
173+
query_string = "SAMLRequest=#{CGI.escape(params['SAMLRequest'])}"
174+
query_string << "&RelayState=#{CGI.escape(params[:RelayState])}"
175+
query_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
176+
177+
signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
178+
assert_equal signature_algorithm, OpenSSL::Digest::SHA256
179+
assert @cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
156180
end
157-
end
158181

182+
end
159183
end
160184
end

‎test/request_test.rb‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class RequestTest < Minitest::Test
145145
end
146146
end
147147

148-
describe "when the settings indicate to sign (embedded) request" do
148+
describe "#create_params when the settings indicate to sign (embebed) the request" do
149149
it "create a signed request" do
150150
settings = OneLogin::RubySaml::Settings.new
151151
settings.compress_request = false
@@ -181,27 +181,51 @@ class RequestTest < Minitest::Test
181181
end
182182
end
183183

184-
describe "when the settings indicate to sign the request" do
185-
it "create a signature parameter" do
186-
settings = OneLogin::RubySaml::Settings.new
187-
settings.compress_request = false
188-
settings.idp_sso_target_url = "http://example.com?field=value"
189-
settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST-SimpleSign"
190-
settings.security[:authn_requests_signed] = true
191-
settings.security[:embed_sign] = false
192-
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
193-
settings.certificate = ruby_saml_cert_text
194-
settings.private_key = ruby_saml_key_text
184+
describe "#create_params when the settings indicate to sign the request" do
185+
def setup
186+
@settings = OneLogin::RubySaml::Settings.new
187+
@settings.compress_request = false
188+
@settings.idp_sso_target_url = "http://example.com?field=value"
189+
@settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST-SimpleSign"
190+
@settings.security[:authn_requests_signed] = true
191+
@settings.security[:embed_sign] = false
192+
@settings.certificate = ruby_saml_cert_text
193+
@settings.private_key = ruby_saml_key_text
194+
@cert = OpenSSL::X509::Certificate.new(ruby_saml_cert_text)
195+
end
196+
197+
it "create a signature parameter with RSA_SHA1 and validate it" do
198+
@settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
195199

196-
params = OneLogin::RubySaml::Authrequest.new.create_params(settings)
200+
params = OneLogin::RubySaml::Authrequest.new.create_params(@settings, :RelayState => 'http://example.com')
201+
assert params['SAMLRequest']
202+
assert params[:RelayState]
197203
assert params['Signature']
198204
assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA1
199205

200-
# signature_method only affects the embedeed signature
201-
settings.security[:signature_method] = XMLSecurity::Document::SHA256
202-
params = OneLogin::RubySaml::Authrequest.new.create_params(settings)
206+
query_string = "SAMLRequest=#{CGI.escape(params['SAMLRequest'])}"
207+
query_string << "&RelayState=#{CGI.escape(params[:RelayState])}"
208+
query_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
209+
210+
signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
211+
assert_equal signature_algorithm, OpenSSL::Digest::SHA1
212+
assert @cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
213+
end
214+
215+
it "create a signature parameter with RSA_SHA256 and validate it" do
216+
@settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
217+
218+
params = OneLogin::RubySaml::Authrequest.new.create_params(@settings, :RelayState => 'http://example.com')
203219
assert params['Signature']
204-
assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA1
220+
assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA256
221+
222+
query_string = "SAMLRequest=#{CGI.escape(params['SAMLRequest'])}"
223+
query_string << "&RelayState=#{CGI.escape(params[:RelayState])}"
224+
query_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
225+
226+
signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
227+
assert_equal signature_algorithm, OpenSSL::Digest::SHA256
228+
assert @cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
205229
end
206230
end
207231

‎test/slo_logoutresponse_test.rb‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -107,29 +107,57 @@ class SloLogoutresponseTest < Minitest::Test
107107
end
108108
end
109109

110-
describe "when the settings indicate to sign the logout response" do
111-
it "create a signature parameter" do
112-
settings = OneLogin::RubySaml::Settings.new
113-
settings.compress_response = false
114-
settings.idp_slo_target_url = "http://example.com?field=value"
115-
settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST-SimpleSign"
116-
settings.security[:logout_responses_signed] = true
117-
settings.security[:embed_sign] = false
110+
describe "#create_params when the settings indicate to sign the logout response" do
111+
def setup
112+
@settings = OneLogin::RubySaml::Settings.new
113+
@settings.compress_response = false
114+
@settings.idp_slo_target_url = "http://example.com?field=value"
115+
@settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST-SimpleSign"
116+
@settings.security[:logout_responses_signed] = true
117+
@settings.security[:embed_sign] = false
118+
@settings.certificate = ruby_saml_cert_text
119+
@settings.private_key = ruby_saml_key_text
120+
@cert = OpenSSL::X509::Certificate.new(ruby_saml_cert_text)
121+
@request = OneLogin::RubySaml::SloLogoutrequest.new(logout_request_document)
122+
end
123+
124+
it "create a signature parameter with RSA_SHA1 and validate it" do
118125
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
119-
settings.certificate = ruby_saml_cert_text
120-
settings.private_key = ruby_saml_key_text
121126

122-
request = OneLogin::RubySaml::SloLogoutrequest.new(logout_request_document)
123-
params = OneLogin::RubySaml::SloLogoutresponse.new.create_params(settings, request.id, "Custom Logout Message")
127+
params = OneLogin::RubySaml::SloLogoutresponse.new.create_params(@settings, @request.id, "Custom Logout Message", :RelayState => 'http://example.com')
128+
assert params['SAMLResponse']
129+
assert params[:RelayState]
124130
assert params['Signature']
125131
assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA1
126132

127-
# signature_method only affects the embedeed signature
128-
settings.security[:signature_method] = XMLSecurity::Document::SHA256
129-
params = OneLogin::RubySaml::SloLogoutresponse.new.create_params(settings, request.id, "Custom Logout Message")
133+
query_string = "SAMLResponse=#{CGI.escape(params['SAMLResponse'])}"
134+
query_string << "&RelayState=#{CGI.escape(params[:RelayState])}"
135+
query_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
136+
137+
signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
138+
assert_equal signature_algorithm, OpenSSL::Digest::SHA1
139+
assert @cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
140+
end
141+
142+
it "create a signature parameter with RSA_SHA256 and validate it" do
143+
@settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
144+
145+
params = OneLogin::RubySaml::SloLogoutresponse.new.create_params(@settings, @request.id, "Custom Logout Message", :RelayState => 'http://example.com')
146+
assert params['SAMLResponse']
147+
assert params[:RelayState]
130148
assert params['Signature']
131-
assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA1
149+
150+
assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA256
151+
152+
query_string = "SAMLResponse=#{CGI.escape(params['SAMLResponse'])}"
153+
query_string << "&RelayState=#{CGI.escape(params[:RelayState])}"
154+
query_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
155+
156+
signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
157+
assert_equal signature_algorithm, OpenSSL::Digest::SHA256
158+
assert @cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
132159
end
160+
133161
end
134162
end
135163
end

0 commit comments

Comments
 (0)