From b0902626cdb61ebc316dca12608927ff5da155dc Mon Sep 17 00:00:00 2001
From: peppelinux <giuseppe.demarco@unical.it>
Date: Sun, 21 Mar 2021 01:16:11 +0100
Subject: [PATCH 1/3] fix: assertion format check
---
src/saml2/response.py | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/src/saml2/response.py b/src/saml2/response.py
index 26963a04e..92bac1085 100644
--- a/src/saml2/response.py
+++ b/src/saml2/response.py
@@ -394,6 +394,23 @@ def issue_instant_ok(self):
issued_at = str_to_time(self.response.issue_instant)
return lower < issued_at < upper
+ def issuer_ok(self):
+ """ Check if the issuer have a valid Format, additional check may be implemented"""
+ if self.response.issuer and self.response.issuer.format:
+ if self.response.issuer.format != saml.NAMEID_FORMAT_ENTITY:
+ return False
+ return True
+
+ def assertion_ok(self):
+ """ Check assetions attributes, additional checks may be implemented """
+ valid = True
+ if hasattr(self.response, 'assertion'):
+ for ass in self.response.assertion:
+ if ass.version and ass.version != '2.0':
+ valid = False
+ break
+ return valid
+
def _verify(self):
if self.request_id and self.in_response_to and \
self.in_response_to != self.request_id:
@@ -416,7 +433,14 @@ def _verify(self):
logger.error("%s not in %s", self.response.destination, self.return_addrs)
return None
- valid = self.issue_instant_ok() and self.status_ok()
+ valid = all(
+ (
+ self.issue_instant_ok(),
+ self.issuer_ok(),
+ self.status_ok(),
+ self.assertion_ok()
+ )
+ )
return valid
def loads(self, xmldata, decode=True, origxml=None):
@@ -1116,7 +1140,7 @@ def session_info(self):
raise StatusInvalidAuthnResponseStatement(
"The Authn Response Statement is not valid"
)
-
+
def __str__(self):
return self.xmlstr
From 130834cd3676764f7e6f63aa48eadd02b9a7caad Mon Sep 17 00:00:00 2001
From: peppelinux <giuseppe.demarco@unical.it>
Date: Sun, 21 Mar 2021 02:13:24 +0100
Subject: [PATCH 2/3] Additional checks on assertion sub elements
---
src/saml2/response.py | 54 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 48 insertions(+), 6 deletions(-)
diff --git a/src/saml2/response.py b/src/saml2/response.py
index 92bac1085..b9ea71d2c 100644
--- a/src/saml2/response.py
+++ b/src/saml2/response.py
@@ -383,7 +383,7 @@ def status_ok(self):
logger.info(msg)
raise err_cls(msg)
- def issue_instant_ok(self):
+ def issue_instant_ok(self, issue_instant):
""" Check that the response was issued at a reasonable time """
upper = time_util.shift_time(time_util.time_in_a_while(days=1),
self.timeslack).timetuple()
@@ -391,7 +391,7 @@ def issue_instant_ok(self):
-self.timeslack).timetuple()
# print("issue_instant: %s" % self.response.issue_instant)
# print("%s < x < %s" % (lower, upper))
- issued_at = str_to_time(self.response.issue_instant)
+ issued_at = str_to_time(issue_instant)
return lower < issued_at < upper
def issuer_ok(self):
@@ -402,13 +402,55 @@ def issuer_ok(self):
return True
def assertion_ok(self):
- """ Check assetions attributes, additional checks may be implemented """
+ """ Check assertions attributes, additional checks may be implemented
+
+ Commented check may be too pedantic
+ """
valid = True
if hasattr(self.response, 'assertion'):
+ # breakpoint()
for ass in self.response.assertion:
+ # Version
if ass.version and ass.version != '2.0':
- valid = False
- break
+ raise VersionMismatch(f'{ass.version}')
+ # IssueInstant
+ # if hasattr(ass, 'issue_instant') and not self.issue_instant_ok(ass.issue_instant):
+ # breakpoint()
+ # raise Exception('Invalid Issue Instant')
+ # NameQualifier
+ # if not hasattr(ass.subject.name_id, 'name_qualifier') or \
+ # not ass.subject.name_id.name_qualifier:
+ # raise Exception('Not a valid subject.name_id.name_qualifier')
+ if hasattr(ass.subject.name_id, 'format') and not ass.subject.name_id.format:
+ raise Exception('Not a valid subject.name_id.format')
+ if hasattr(ass.subject.name_id, 'format'):
+ if ass.subject.name_id.format not in dict(saml.NAMEID_FORMATS_SAML2).values():
+ msg = 'Not a valid subject.name_id.format: {}'
+ raise Exception(msg.format(ass.subject.name_id.format))
+
+ # subject confirmation
+ for subject_confirmation in ass.subject.subject_confirmation:
+ if not hasattr(subject_confirmation, 'subject_confirmation_data') or \
+ not getattr(subject_confirmation, 'subject_confirmation_data', None):
+ msg = 'subject_confirmation_data not present'
+ raise Exception(msg)
+
+ if not subject_confirmation.subject_confirmation_data.in_response_to:
+ raise Exception('subject.subject_confirmation_data in response -> null data')
+
+ # TODO: match to the recipient
+ # if self.recipient != subject_confirmation.subject_confirmation_data.recipient:
+ # msg = 'subject_confirmation_data.recipient not valid: {}'
+ # raise Exception(msg.format(subject_confirmation_data.recipient))
+
+ if not hasattr(subject_confirmation.subject_confirmation_data, 'not_on_or_after') or \
+ not getattr(subject_confirmation.subject_confirmation_data, 'not_on_or_after', None):
+ raise Exception('subject.subject_confirmation_data not_on_or_after not valid')
+
+ if not hasattr(subject_confirmation.subject_confirmation_data, 'in_response_to') or \
+ not getattr(subject_confirmation.subject_confirmation_data, 'in_response_to', None):
+ raise Exception('subject.subject_confirmation_data in response to not valid')
+
return valid
def _verify(self):
@@ -435,7 +477,7 @@ def _verify(self):
valid = all(
(
- self.issue_instant_ok(),
+ self.issue_instant_ok(self.response.issue_instant),
self.issuer_ok(),
self.status_ok(),
self.assertion_ok()
From 56dd9655cdc53fc19e1e73202c51c821f4b127d5 Mon Sep 17 00:00:00 2001
From: peppelinux <giuseppe.demarco@unical.it>
Date: Sun, 21 Mar 2021 02:25:33 +0100
Subject: [PATCH 3/3] clean up
---
src/saml2/response.py | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/saml2/response.py b/src/saml2/response.py
index b9ea71d2c..968639434 100644
--- a/src/saml2/response.py
+++ b/src/saml2/response.py
@@ -421,9 +421,10 @@ def assertion_ok(self):
# if not hasattr(ass.subject.name_id, 'name_qualifier') or \
# not ass.subject.name_id.name_qualifier:
# raise Exception('Not a valid subject.name_id.name_qualifier')
- if hasattr(ass.subject.name_id, 'format') and not ass.subject.name_id.format:
- raise Exception('Not a valid subject.name_id.format')
if hasattr(ass.subject.name_id, 'format'):
+ if not ass.subject.name_id.format:
+ raise Exception('Not a valid subject.name_id.format')
+
if ass.subject.name_id.format not in dict(saml.NAMEID_FORMATS_SAML2).values():
msg = 'Not a valid subject.name_id.format: {}'
raise Exception(msg.format(ass.subject.name_id.format))
@@ -447,10 +448,6 @@ def assertion_ok(self):
not getattr(subject_confirmation.subject_confirmation_data, 'not_on_or_after', None):
raise Exception('subject.subject_confirmation_data not_on_or_after not valid')
- if not hasattr(subject_confirmation.subject_confirmation_data, 'in_response_to') or \
- not getattr(subject_confirmation.subject_confirmation_data, 'in_response_to', None):
- raise Exception('subject.subject_confirmation_data in response to not valid')
-
return valid
def _verify(self):