From 29188981201a617ee5aa255d429e29490781d4b1 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Oct 23 2015 19:38:40 +0000 Subject: Add per-SP option to verify SAML authn signatures If processing the request/response fails due to a missing signature the per-SP option is set as a hint for processing and the message re-processed. The hint cannot be set in advance because the providerId of the remote entity is embedded in the message so needs to be processed so we know who they are representing. https://fedorahosted.org/ipsilon/ticket/173 Signed-off-by: Rob Crittenden --- diff --git a/ipsilon/providers/saml2/admin.py b/ipsilon/providers/saml2/admin.py index c7a0289..31efcdd 100644 --- a/ipsilon/providers/saml2/admin.py +++ b/ipsilon/providers/saml2/admin.py @@ -227,7 +227,8 @@ class SPAdminPage(AdminPage): 'Allowed NameIDs', 'Attribute Mapping', 'Allowed Attributes', 'Description', 'Service Provider link', - 'Visible in Portal', 'Image File']: + 'Visible in Portal', 'Image File', + 'Verify Signatures']: if not self.user.is_admin: raise UnauthorizedUser( "Unauthorized to set %s" % key @@ -240,7 +241,8 @@ class SPAdminPage(AdminPage): value = new_db_values.get(name, False) # A value of None means remove from the data store if ((value is False or value == []) and - name != 'Visible in Portal'): + name not in ['Visible in Portal', + 'Verify Signatures']): continue if name == 'Name': if not self.sp.is_valid_name(value): @@ -277,6 +279,8 @@ class SPAdminPage(AdminPage): raise InvalidValueFormat( 'Invalid Image file format' ) + elif name == 'Verify Signatures': + self.sp.verify_signatures = value except InvalidValueFormat, e: message = str(e) diff --git a/ipsilon/providers/saml2/auth.py b/ipsilon/providers/saml2/auth.py index 5412240..8b5ca3b 100644 --- a/ipsilon/providers/saml2/auth.py +++ b/ipsilon/providers/saml2/auth.py @@ -69,12 +69,28 @@ class AuthenticateRequest(ProviderPageBase): self.saml2error(login, e.code, e.message) return self.reply(login) - def _parse_request(self, message): + def _parse_request(self, message, hint=None, final=False): login = self.cfg.idp.get_login_handler() try: + if hint: + login.setSignatureVerifyHint(hint) login.processAuthnRequestMsg(message) + except lasso.DsInvalidSigalgError as e: + if login.remoteProviderId and not final: + provider = ServiceProvider(self.cfg, login.remoteProviderId) + if not provider.verify_signatures: + self.error('Invalid or missing signature, setting hint.') + return self._parse_request( + message, + hint=provider.get_signature_hint(), + final=True + ) + msg = 'Invalid or missing signature algorithm %r [%r]' % ( + e, message + ) + raise InvalidRequest(msg) except (lasso.ProfileInvalidMsgError, lasso.ProfileMissingIssuerError), e: diff --git a/ipsilon/providers/saml2/logout.py b/ipsilon/providers/saml2/logout.py index cec1d03..6c06b2b 100644 --- a/ipsilon/providers/saml2/logout.py +++ b/ipsilon/providers/saml2/logout.py @@ -36,6 +36,11 @@ class LogoutRequest(ProviderPageBase): try: logout.processRequestMsg(message) + except lasso.DsInvalidSigalgError as e: + msg = 'Invalid or missing signature algorithm %r [%r]' % ( + e, message + ) + raise InvalidRequest(msg) except (lasso.ServerProviderNotFoundError, lasso.ProfileUnknownProviderError) as e: msg = 'Invalid SP [%s] (%r [%r])' % (logout.remoteProviderId, @@ -113,6 +118,11 @@ class LogoutRequest(ProviderPageBase): try: logout.processResponseMsg(message) + except lasso.ProfileCannotVerifySignatureError as e: + msg = 'Invalid or missing signature algorithm %r [%r]' % ( + e, message + ) + raise InvalidRequest(msg) except getattr(lasso, 'ProfileRequestDeniedError', lasso.LogoutRequestDeniedError): self.error('Logout request denied by %s' % @@ -162,6 +172,11 @@ class LogoutRequest(ProviderPageBase): try: logout.processRequestMsg(message) + except lasso.DsInvalidSigalgError as e: + msg = 'Invalid or missing signature algorithm %r [%r]' % ( + e, message + ) + raise InvalidRequest(msg) except (lasso.ServerProviderNotFoundError, lasso.ProfileUnknownProviderError) as e: msg = 'Invalid SP [%s] (%r [%r])' % (logout.remoteProviderId, diff --git a/ipsilon/providers/saml2/provider.py b/ipsilon/providers/saml2/provider.py index 6d46ad2..2104f5a 100644 --- a/ipsilon/providers/saml2/provider.py +++ b/ipsilon/providers/saml2/provider.py @@ -42,6 +42,8 @@ class ServiceProvider(ServiceProviderConfig): def __init__(self, config, provider_id): super(ServiceProvider, self).__init__() self.cfg = config + if provider_id is None: + raise InvalidProviderId('No provider id supplied') data = self.cfg.get_data(name='id', value=provider_id) if len(data) != 1: raise InvalidProviderId('multiple matches') @@ -109,6 +111,10 @@ class ServiceProvider(ServiceProviderConfig): 'Defines a list of allowed attributes, applied after mapping.' ' Setting this overrides the global values.', self.allowed_attributes), + pconfig.Condition( + 'Verify Signatures', + 'Verify Signatures for this SP (recommended).', + self.verify_signatures), ) @property @@ -233,6 +239,15 @@ class ServiceProvider(ServiceProviderConfig): value = temp.export_value() self._staging['allowed_attributes'] = value + @property + def verify_signatures(self): + value = self._properties.get('verify_signatures', True) + return value in [u'1', 'True', True] + + @verify_signatures.setter + def verify_signatures(self, value): + self._staging['verify_signatures'] = value + def save_properties(self): data = self.cfg.get_data(name='id', value=self.provider_id) if len(data) != 1: @@ -267,6 +282,24 @@ class ServiceProvider(ServiceProviderConfig): return nip.format raise NameIdNotAllowed(nip.format) + def get_signature_hint(self): + """ + Call this before any lasso process message methods to get + the hint on whether to enforce signatures or not. + """ + if self.verify_signatures: + hint = lasso.PROFILE_SIGNATURE_VERIFY_HINT_FORCE + else: + hint = lasso.PROFILE_SIGNATURE_VERIFY_HINT_IGNORE + + self.error( + 'Setting signature hint to "%s"' % + (hint == lasso.PROFILE_SIGNATURE_VERIFY_HINT_FORCE and + "force" or "ignore") + ) + + return hint + def permanently_delete(self): data = self.cfg.get_data(name='id', value=self.provider_id) if len(data) != 1: diff --git a/templates/admin/providers/saml2.html b/templates/admin/providers/saml2.html index 2ecb1df..454fef8 100644 --- a/templates/admin/providers/saml2.html +++ b/templates/admin/providers/saml2.html @@ -11,10 +11,14 @@ {% for p in providers %}
- {{ p.name }} + {{ p.name }}
- {{ p.provider_id }} + {% if not p.verify_signatures %} + {{ p.provider_id }} + {% else %} + {{ p.provider_id }} + {% endif %} Delete