Skip to content

Commit

Permalink
Support single-label-domains as valid. New security parameter allowSi…
Browse files Browse the repository at this point in the history
…ngleLabelDomains
  • Loading branch information
pitbulk committed Jan 9, 2021
1 parent e0ffaf8 commit f904996
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 9 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,10 @@ In addition to the required settings data (idp, sp), extra settings can be defin
// Provide the desired duration, for example PT518400S (6 days)
"metadataCacheDuration": null,

// If enabled, URLs with single-label-domains will
// be allowed and not rejected by the settings validator (Enable it under Docker/Kubernetes/testing env, not recommended on production)
"allowSingleLabelDomains": false,

// Algorithm that the toolkit will use on signing process. Options:
// 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'
// 'http://www.w3.org/2000/09/xmldsig#dsa-sha1'
Expand Down
1 change: 1 addition & 0 deletions demo-bottle/saml/advanced_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"wantNameId" : true,
"wantNameIdEncrypted": false,
"wantAssertionsEncrypted": false,
"allowSingleLabelDomains": false,
"signatureAlgorithm": "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
"digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256"
},
Expand Down
1 change: 1 addition & 0 deletions demo-django/saml/advanced_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"wantNameId" : true,
"wantNameIdEncrypted": false,
"wantAssertionsEncrypted": false,
"allowSingleLabelDomains": false,
"signatureAlgorithm": "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
"digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256"
},
Expand Down
1 change: 1 addition & 0 deletions demo-flask/saml/advanced_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"wantNameId" : true,
"wantNameIdEncrypted": false,
"wantAssertionsEncrypted": false,
"allowSingleLabelDomains": false,
"signatureAlgorithm": "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
"digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256"
},
Expand Down
1 change: 1 addition & 0 deletions demo_pyramid/demo_pyramid/saml/advanced_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"wantNameId" : true,
"wantNameIdEncrypted": false,
"wantAssertionsEncrypted": false,
"allowSingleLabelDomains": false,
"signatureAlgorithm": "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
"digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256"
},
Expand Down
35 changes: 28 additions & 7 deletions src/onelogin/saml2/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,38 @@
r'\[?[A-F0-9]*:[A-F0-9:]+\]?)' # ...or ipv6
r'(?::\d+)?' # optional port
r'(?:/?|[/?]\S+)$', re.IGNORECASE)
url_regex_single_label_domain = re.compile(
r'^(?:[a-z0-9\.\-]*)://' # scheme is validated separately
r'(?:(?:[A-Z0-9_](?:[A-Z0-9-_]{0,61}[A-Z0-9_])?\.)+(?:[A-Z]{2,6}\.?|[A-Z0-9-]{2,}\.?)|' # domain...
r'(?:[A-Z0-9_](?:[A-Z0-9-_]{0,61}[A-Z0-9_]))|' # single-label-domain
r'localhost|' # localhost...
r'\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}|' # ...or ipv4
r'\[?[A-F0-9]*:[A-F0-9:]+\]?)' # ...or ipv6
r'(?::\d+)?' # optional port
r'(?:/?|[/?]\S+)$', re.IGNORECASE)
url_schemes = ['http', 'https', 'ftp', 'ftps']


def validate_url(url):
def validate_url(url, allow_single_label_domain=False):
"""
Auxiliary method to validate an urllib
:param url: An url to be validated
:type url: string
:param allow_single_label_domain: In order to allow or not single label domain
:type url: bool
:returns: True if the url is valid
:rtype: bool
"""

scheme = url.split('://')[0].lower()
if scheme not in url_schemes:
return False
if not bool(url_regex.search(url)):
return False
if allow_single_label_domain:
if not bool(url_regex_single_label_domain.search(url)):
return False
else:
if not bool(url_regex.search(url)):
return False
return True


Expand Down Expand Up @@ -351,17 +366,18 @@ def check_idp_settings(self, settings):
if not settings.get('idp'):
errors.append('idp_not_found')
else:
allow_single_domain_urls = self._get_allow_single_label_domain(settings)
idp = settings['idp']
if not idp.get('entityId'):
errors.append('idp_entityId_not_found')

if not idp.get('singleSignOnService', {}).get('url'):
errors.append('idp_sso_not_found')
elif not validate_url(idp['singleSignOnService']['url']):
elif not validate_url(idp['singleSignOnService']['url'], allow_single_domain_urls):
errors.append('idp_sso_url_invalid')

slo_url = idp.get('singleLogoutService', {}).get('url')
if slo_url and not validate_url(slo_url):
if slo_url and not validate_url(slo_url, allow_single_domain_urls):
errors.append('idp_slo_url_invalid')

if 'security' in settings:
Expand Down Expand Up @@ -408,6 +424,7 @@ def check_sp_settings(self, settings):
if not settings.get('sp'):
errors.append('sp_not_found')
else:
allow_single_domain_urls = self._get_allow_single_label_domain(settings)
# check_sp_certs uses self.__sp so I add it
old_sp = self.__sp
self.__sp = settings['sp']
Expand All @@ -420,7 +437,7 @@ def check_sp_settings(self, settings):

if not sp.get('assertionConsumerService', {}).get('url'):
errors.append('sp_acs_not_found')
elif not validate_url(sp['assertionConsumerService']['url']):
elif not validate_url(sp['assertionConsumerService']['url'], allow_single_domain_urls):
errors.append('sp_acs_url_invalid')

if sp.get('attributeConsumingService'):
Expand Down Expand Up @@ -449,7 +466,7 @@ def check_sp_settings(self, settings):
errors.append('sp_attributeConsumingService_serviceDescription_type_invalid')

slo_url = sp.get('singleLogoutService', {}).get('url')
if slo_url and not validate_url(slo_url):
if slo_url and not validate_url(slo_url, allow_single_domain_urls):
errors.append('sp_sls_url_invalid')

if 'signMetadata' in security and isinstance(security['signMetadata'], dict):
Expand Down Expand Up @@ -840,3 +857,7 @@ def is_debug_active(self):
:rtype: boolean
"""
return self.__debug

def _get_allow_single_label_domain(self, settings):
security = settings.get('security', {})
return 'allowSingleLabelDomains' in security.keys() and security['allowSingleLabelDomains']
2 changes: 0 additions & 2 deletions tests/src/OneLogin/saml2_tests/response_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,6 @@ def testDoesNotAllowSignatureWrappingAttack(self):
self.assertFalse(response.is_valid(self.get_request_data()))
self.assertEqual('[email protected]', response.get_nameid())


def testDoesNotAllowSignatureWrappingAttack2(self):
# Signature Wraping attack 2
settings = OneLogin_Saml2_Settings(self.loadSettingsJSON())
Expand All @@ -724,7 +723,6 @@ def testDoesNotAllowSignatureWrappingAttack2(self):
self.assertFalse(response.is_valid(self.get_request_data()))
self.assertEquals("SAML Response must contain 1 assertion", response.get_error())


def testNodeTextAttack(self):
"""
Tests the get_nameid and get_attributes methods of the OneLogin_Saml2_Response
Expand Down
7 changes: 7 additions & 0 deletions tests/src/OneLogin/saml2_tests/settings_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ def testLoadSettingsFromDict(self):
with self.assertRaisesRegexp(Exception, 'Invalid dict settings: idp_sso_url_invalid'):
OneLogin_Saml2_Settings(settings_info)

settings_info['idp']['singleSignOnService']['url'] = 'http://single-label-domain'
settings_info['security'] = {}
settings_info['security']['allowSingleLabelDomains'] = True
settings = OneLogin_Saml2_Settings(settings_info)
self.assertEqual(len(settings.get_errors()), 0)

del settings_info['security']
del settings_info['sp']
del settings_info['idp']
with self.assertRaisesRegexp(Exception, 'Invalid dict settings: idp_not_found,sp_not_found'):
Expand Down

0 comments on commit f904996

Please sign in to comment.