Skip to content

Commit ecb99ef

Browse files
pitbulkpitbulk
committed
Improve Signature validation process
1 parent 53f6f57 commit ecb99ef

File tree

2 files changed

+64
-26
lines changed

2 files changed

+64
-26
lines changed

‎src/onelogin/saml2/response.py‎

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ def is_valid(self, request_data, request_id=None):
8484

8585
signed_elements = self.process_signed_elements()
8686

87+
has_signed_response = '{%s}Response' % OneLogin_Saml2_Constants.NS_SAMLP in signed_elements
88+
has_signed_assertion = '{%s}Assertion' % OneLogin_Saml2_Constants.NS_SAML in signed_elements
89+
8790
if self.__settings.is_strict():
8891
no_valid_xml_msg = 'Invalid SAML Response. Not match the saml-schema-protocol-2.0.xsd'
8992
res = OneLogin_Saml2_XML.validate_xml(self.document, 'saml-schema-protocol-2.0.xsd', self.__settings.is_debug_active())
@@ -196,33 +199,26 @@ def is_valid(self, request_data, request_id=None):
196199
if not any_subject_confirmation:
197200
raise Exception('A valid SubjectConfirmation was not found on this Response')
198201

199-
if security['wantAssertionsSigned'] and ('{%s}Assertion' % OneLogin_Saml2_Constants.NS_SAML) not in signed_elements:
202+
if security['wantAssertionsSigned'] and not not has_signed_assertion:
200203
raise Exception('The Assertion of the Response is not signed and the SP require it')
201204

202-
if security['wantMessagesSigned'] and ('{%s}Response' % OneLogin_Saml2_Constants.NS_SAMLP) not in signed_elements:
205+
if security['wantMessagesSigned'] and has_signed_response:
203206
raise Exception('The Message of the Response is not signed and the SP require it')
204207

205-
if len(signed_elements) > 0:
206-
if len(signed_elements) > 2:
207-
raise Exception('Too many Signatures found. SAML Response rejected')
208-
209-
cert = idp_data['x509cert']
210-
fingerprint = idp_data['certFingerprint']
211-
fingerprintalg = idp_data['certFingerprintAlgorithm']
208+
if not signed_elements or (not has_signed_response and not has_signed_assertion):
209+
raise Exception('No Signature found. SAML Response rejected')
210+
else:
211+
cert = idp_data.get('x509cert', None)
212+
fingerprint = idp_data.get('certFingerprint', None)
213+
fingerprintalg = idp_data.get('certFingerprintAlgorithm', None)
212214

213215
# If find a Signature on the Response, validates it checking the original response
214-
if '{%s}Response' % OneLogin_Saml2_Constants.NS_SAMLP in signed_elements:
215-
document_to_validate = self.document
216-
# Otherwise validates the assertion (decrypted assertion if was encrypted)
217-
else:
218-
if self.encrypted:
219-
document_to_validate = self.decrypted_document
220-
else:
221-
document_to_validate = self.document
222-
if not OneLogin_Saml2_Utils.validate_sign(document_to_validate, cert, fingerprint, fingerprintalg):
216+
if has_signed_response and not OneLogin_Saml2_Utils.validate_sign(self.document, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.RESPONSE_SIGNATURE_XPATH):
217+
raise Exception('Signature validation failed. SAML Response rejected')
218+
219+
document_check_assertion = self.decrypted_document if self.encrypted else self.document
220+
if has_signed_assertion and not OneLogin_Saml2_Utils.validate_sign(document_check_assertion, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.ASSERTION_SIGNATURE_XPATH):
223221
raise Exception('Signature validation failed. SAML Response rejected')
224-
else:
225-
raise Exception('No Signature found. SAML Response rejected')
226222

227223
return True
228224
except Exception as err:
@@ -288,7 +284,7 @@ def get_issuers(self):
288284
"""
289285
issuers = set()
290286

291-
message_issuer_nodes = self.__query('/samlp:Response/saml:Issuer')
287+
message_issuer_nodes = OneLogin_Saml2_XML.query(self.document, '/samlp:Response/saml:Issuer')
292288
if len(message_issuer_nodes) == 1:
293289
issuers.add(message_issuer_nodes[0].text)
294290
else:
@@ -466,8 +462,41 @@ def process_signed_elements(self):
466462
verified_seis.append(sei)
467463

468464
signed_elements.append(signed_element)
465+
466+
if signed_elements:
467+
if not self.validate_signed_elements(signed_elements):
468+
raise Exception('Found an unexpected Signature Element. SAML Response rejected')
469469
return signed_elements
470470

471+
def validate_signed_elements(self, signed_elements):
472+
"""
473+
Verifies that the document has the expected signed nodes.
474+
"""
475+
if len(signed_elements) > 2:
476+
return False
477+
478+
response_tag = '{%s}Response' % OneLogin_Saml2_Constants.NS_SAMLP
479+
assertion_tag = '{%s}Assertion' % OneLogin_Saml2_Constants.NS_SAML
480+
481+
if (response_tag in signed_elements and signed_elements.count(response_tag) > 1) or \
482+
(assertion_tag in signed_elements and signed_elements.count(assertion_tag) > 1) or \
483+
(response_tag not in signed_elements and assertion_tag not in signed_elements):
484+
return False
485+
486+
# Check that the signed elements found here, are the ones that will be verified
487+
# by OneLogin_Saml2_Utils.validate_sign
488+
if response_tag in signed_elements:
489+
expected_signature_nodes = OneLogin_Saml2_XML.query(self.document, OneLogin_Saml2_Utils.RESPONSE_SIGNATURE_XPATH)
490+
if len(expected_signature_nodes) != 1:
491+
raise Exception('Unexpected number of Response signatures found. SAML Response rejected.')
492+
493+
if assertion_tag in signed_elements:
494+
expected_signature_nodes = self.__query(OneLogin_Saml2_Utils.ASSERTION_SIGNATURE_XPATH)
495+
if len(expected_signature_nodes) != 1:
496+
raise Exception('Unexpected number of Assertion signatures found. SAML Response rejected.')
497+
498+
return True
499+
471500
def validate_timestamps(self):
472501
"""
473502
Verifies that the document is valid according to Conditions Element

‎src/onelogin/saml2/utils.py‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ class OneLogin_Saml2_Utils(object):
4040
urls, add sign, encrypt, decrypt, sign validation, handle xml ...
4141
4242
"""
43+
44+
RESPONSE_SIGNATURE_XPATH = '/samlp:Response/ds:Signature'
45+
ASSERTION_SIGNATURE_XPATH = '/samlp:Response/saml:Assertion/ds:Signature'
46+
4347
@staticmethod
4448
def escape_url(url, lowercase_urlencoding=False):
4549
"""
@@ -715,7 +719,7 @@ def add_sign(xml, key, cert, debug=False, sign_algorithm=OneLogin_Saml2_Constant
715719
return OneLogin_Saml2_XML.to_string(elem)
716720

717721
@staticmethod
718-
def validate_sign(xml, cert=None, fingerprint=None, fingerprintalg='sha1', validatecert=False, debug=False):
722+
def validate_sign(xml, cert=None, fingerprint=None, fingerprintalg='sha1', validatecert=False, debug=False, xpath=None):
719723
"""
720724
Validates a signature (Message or Assertion).
721725
@@ -736,6 +740,9 @@ def validate_sign(xml, cert=None, fingerprint=None, fingerprintalg='sha1', valid
736740
737741
:param debug: Activate the xmlsec debug
738742
:type: bool
743+
744+
:param xpath: The xpath of the signed element
745+
:type: string
739746
"""
740747
try:
741748
if xml is None or xml == '':
@@ -745,11 +752,13 @@ def validate_sign(xml, cert=None, fingerprint=None, fingerprintalg='sha1', valid
745752
xmlsec.enable_debug_trace(debug)
746753
xmlsec.tree.add_ids(elem, ["ID"])
747754

748-
signature_nodes = OneLogin_Saml2_XML.query(elem, '/samlp:Response/ds:Signature')
755+
if xpath:
756+
signature_nodes = OneLogin_Saml2_XML.query(elem, xpath)
757+
else:
758+
signature_nodes = OneLogin_Saml2_XML.query(elem, OneLogin_Saml2_Utils.RESPONSE_SIGNATURE_XPATH)
749759

750-
if not len(signature_nodes) > 0:
751-
signature_nodes += OneLogin_Saml2_XML.query(elem, '/samlp:Response/ds:Signature')
752-
signature_nodes += OneLogin_Saml2_XML.query(elem, '/samlp:Response/saml:Assertion/ds:Signature')
760+
if len(signature_nodes) == 0:
761+
signature_nodes = OneLogin_Saml2_XML.query(elem, OneLogin_Saml2_Utils.ASSERTION_SIGNATURE_XPATH)
753762

754763
if len(signature_nodes) == 1:
755764
signature_node = signature_nodes[0]

0 commit comments

Comments
 (0)