saml_idp: cleanup, fix XML signing

passbook/saml_idp/base.py

@@ -51,7 +51,7 @@ class Processor:
     _saml_response = None
     _session_index = None
     _subject = None
-    _subject_format = 'urn:oasis:names:tc:SAML:2.0:nameid-format:email'
+    _subject_format = 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'
     _system_params = {
         'ISSUER': CONFIG.y('saml_idp.issuer'),
     }
@@ -84,8 +84,7 @@ class Processor:
             'AUTH_INSTANT': get_time_string(),
             'ISSUE_INSTANT': get_time_string(),
             'NOT_BEFORE': get_time_string(-1 * HOURS),  # TODO: Make these settings.
-            'NOT_ON_OR_AFTER': get_time_string(int(CONFIG.y('saml_idp.assertion_valid_for'))
+            'NOT_ON_OR_AFTER': get_time_string(86400 * MINUTES),
-                                               * MINUTES),
             'SESSION_INDEX': self._session_index,
             'SESSION_NOT_ON_OR_AFTER': get_time_string(8 * HOURS),
             'SP_NAME_QUALIFIER': self._audience,
@@ -226,7 +225,7 @@ class Processor:
         self._saml_response = sp_config
         self._session_index = sp_config
         self._subject = sp_config
-        self._subject_format = 'urn:oasis:names:tc:SAML:2.0:nameid-format:email'
+        self._subject_format = 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'
         self._system_params = {
             'ISSUER': CONFIG.y('saml_idp.issuer'),
         }

passbook/saml_idp/models.py

@@ -40,8 +40,12 @@ class SAMLProvider(Provider):
 
     def link_download_metadata(self):
         """Get link to download XML metadata for admin interface"""
-        return reverse('passbook_saml_idp:metadata_xml',
+        # pylint: disable=no-member
-                       kwargs={'provider_id': self.pk})
+        if self.application:
+            # pylint: disable=no-member
+            return reverse('passbook_saml_idp:metadata_xml',
+                           kwargs={'application': self.application.slug})
+        return None
 
     class Meta:
 

passbook/saml_idp/templates/saml/xml/assertions/generic.xml

@@ -5,14 +5,14 @@
     <saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">{{ ISSUER }}</saml:Issuer>
     {% include 'saml/xml/signature.xml' %}
     {{ SUBJECT_STATEMENT }}
-    <saml:Conditions NotBefore="2014-07-17T01:01:18Z" NotOnOrAfter="2024-01-18T06:21:48Z">
+    <saml:Conditions NotBefore="{{ NOT_BEFORE }}" NotOnOrAfter="{{ NOT_ON_OR_AFTER }}">
       <saml:AudienceRestriction>
         <saml:Audience>{{ AUDIENCE }}</saml:Audience>
       </saml:AudienceRestriction>
     </saml:Conditions>
-    <saml:AuthnStatement AuthnInstant="2014-07-17T01:01:48Z" SessionNotOnOrAfter="2024-07-17T09:01:48Z" SessionIndex="_be9967abd904ddcae3c0eb4189adbe3f71e327cf93">
+    <saml:AuthnStatement AuthnInstant="{{ NOT_BEFORE }}" SessionIndex="{{ ASSERTION_ID }}">
       <saml:AuthnContext>
-        <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:Password</saml:AuthnContextClassRef>
+        <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml:AuthnContextClassRef>
       </saml:AuthnContext>
     </saml:AuthnStatement>
     {{ ATTRIBUTE_STATEMENT }}

passbook/saml_idp/templates/saml/xml/attributes.xml

@@ -1,7 +1,7 @@
 <saml:AttributeStatement>
     {% for attr in attributes %}
-        <saml:Attribute FriendlyName="{{ attr.FriendlyName }}" Name="{{ attr.Name }}" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
+        <saml:Attribute {% if attr.FriendlyName %}FriendlyName="{{ attr.FriendlyName }}" {% endif %}Name="{{ attr.Name }}">
-           <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">{{ attr.Value }}</saml:AttributeValue>
+           <saml:AttributeValue>{{ attr.Value }}</saml:AttributeValue>
         </saml:Attribute>
     {% endfor %}
 </saml:AttributeStatement>

passbook/saml_idp/templates/saml/xml/metadata.xml

@@ -16,7 +16,7 @@
       </ds:KeyInfo>
     </md:KeyDescriptor>
     <md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="{{ slo_url }}"/>
-    <md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:email</md:NameIDFormat>
+    <md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:persistent</md:NameIDFormat>
     <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="{{ sso_url }}"/>
   </md:IDPSSODescriptor>
 {% comment %}

passbook/saml_idp/templates/saml/xml/response.xml

@@ -1,4 +1,5 @@
 <samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
+                xmlns:ds="http://www.w3.org/2000/09/xmldsig#"
                 Destination="{{ ACS_URL }}"
                 ID="{{ RESPONSE_ID }}"
                 {{ IN_RESPONSE_TO|safe }}

passbook/saml_idp/templates/saml/xml/subject.xml

@@ -1,5 +1,5 @@
 <saml:Subject>
-    <saml:NameID Format="{{ SUBJECT_FORMAT }}" SPNameQualifier="{{ SP_NAME_QUALIFIER }}">
+    <saml:NameID Format="{{ SUBJECT_FORMAT }}">
     {{ SUBJECT }}
     </saml:NameID>
     <saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">

passbook/saml_idp/urls.py

@@ -6,8 +6,8 @@ from passbook.saml_idp import views
 urlpatterns = [
     path('login/<slug:application>/',
          views.LoginBeginView.as_view(), name="saml_login_begin"),
-    path('login/<slug:application>/idp_init/',
+    path('login/<slug:application>/initiate/',
-         views.LoginInitView.as_view(), name="saml_login_init"),
+         views.InitiateLoginView.as_view(), name="saml_login_init"),
     path('login/<slug:application>/process/',
          views.LoginProcessView.as_view(), name='saml_login_process'),
     path('logout/', views.LogoutView.as_view(), name="saml_logout"),

passbook/saml_idp/views.py

@@ -1,7 +1,6 @@
 """passbook SAML IDP Views"""
 from logging import getLogger
 
-from django.conf import settings
 from django.contrib.auth import logout
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.core.exceptions import ValidationError
@@ -10,14 +9,6 @@ from django.http import HttpResponse, HttpResponseBadRequest
 from django.shortcuts import get_object_or_404, redirect, render, reverse
 from django.utils.datastructures import MultiValueDictKeyError
 from django.views import View
-from saml2 import BINDING_HTTP_POST
-from saml2.authn_context import PASSWORD, AuthnBroker, authn_context_class_ref
-from saml2.config import IdPConfig
-from saml2.ident import NameID
-from saml2.metadata import entity_descriptor
-from saml2.s_utils import UnknownPrincipal, UnsupportedBinding
-from saml2.saml import NAMEID_FORMAT_EMAILADDRESS, NAMEID_FORMAT_UNSPECIFIED
-from saml2.server import Server
 from signxml.util import strip_pem_header
 
 from passbook.core.models import Application
@@ -50,11 +41,13 @@ def render_xml(request, template, ctx):
 
 
 class ProviderMixin:
+    """Mixin class for Views using a provider instance"""
 
     _provider = None
 
     @property
     def provider(self):
+        """Get provider instance"""
         if not self._provider:
             application = get_object_or_404(Application, slug=self.kwargs['application'])
             self._provider = get_object_or_404(SAMLProvider, pk=application.provider_id)
@@ -123,7 +116,7 @@ class LoginProcessView(ProviderMixin, View):
                 saml_response=request.POST.get('SAMLResponse'),
                 relay_state=request.POST.get('RelayState'))
         try:
-            full_res = _generate_response(request, provider)
+            full_res = _generate_response(request, self.provider)
             return full_res
         except exceptions.CannotHandleAssertion as exc:
             LOGGER.debug(exc)
@@ -166,33 +159,17 @@ class SLOLogout(CSRFExemptMixin, LoginRequiredMixin, View):
         logout(request)
         return render(request, 'saml/idp/logged_out.html')
 
-class IdPMixin(ProviderMixin):
-
-    provider = None
-
-    def dispatch(self, request, application):
-
-    def get_identity(self, provider, user):
-        """ Create Identity dict (using SP-specific mapping)
-        """
-        sp_mapping = {'username': 'username'}
-        # return provider.processor.create_identity(user, sp_mapping)
-        return {
-            out_attr: getattr(user, user_attr)
-            for user_attr, out_attr in sp_mapping.items()
-            if hasattr(user, user_attr)
-        }
-
 
 class DescriptorDownloadView(ProviderMixin, View):
     """Replies with the XML Metadata IDSSODescriptor."""
 
     def get(self, request, application):
         """Replies with the XML Metadata IDSSODescriptor."""
-        super().dispatch(request, application)
         entity_id = CONFIG.y('saml_idp.issuer')
         slo_url = request.build_absolute_uri(reverse('passbook_saml_idp:saml_logout'))
-        sso_url = request.build_absolute_uri(reverse('passbook_saml_idp:saml_login_begin'))
+        sso_url = request.build_absolute_uri(reverse('passbook_saml_idp:saml_login_begin', kwargs={
+            'application': application
+        }))
         pubkey = strip_pem_header(self.provider.signing_cert.replace('\r', '')).replace('\n', '')
         ctx = {
             'entity_id': entity_id,
@@ -207,19 +184,11 @@ class DescriptorDownloadView(ProviderMixin, View):
         return response
 
 
-class LoginInitView(IdPMixin, LoginRequiredMixin, View):
+class InitiateLoginView(ProviderMixin, LoginRequiredMixin, View):
+    """IdP-initiated Login"""
 
     def dispatch(self, request, application):
         """Initiates an IdP-initiated link to a simple SP resource/target URL."""
         super().dispatch(request, application)
-
+        self.provider.processor.init_deep_link(request, '')
-        # # linkdict = dict(metadata.get_links(sp_config))
+        return _generate_response(request, self.provider)
-        # # pattern = linkdict[resource]
-        # # is_simple_link = ('/' not in resource)
-        # # if is_simple_link:
-        # #     simple_target = kwargs['target']
-        # #     url = pattern % simple_target
-        # # else:
-        # #     url = pattern % kwargs
-        # provider.processor.init_deep_link(request, 'deep url')
-        # return _generate_response(request, provider)

passbook/saml_idp/xml_render.py

@@ -83,5 +83,5 @@ def get_response_xml(parameters, saml_provider: 'SAMLProvider', assertion_id='')
 
     signed = sign_with_signxml(
         saml_provider.signing_key, raw_response, saml_provider.signing_cert,
-        reference_uri=assertion_id).decode("utf-8")
+        reference_uri=assertion_id)
     return signed

passbook/saml_idp/xml_signing.py

@@ -3,9 +3,8 @@ from logging import getLogger
 
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import serialization
-from defusedxml import ElementTree
 from lxml import etree  # nosec
-from signxml import XMLSigner
+from signxml import XMLSigner, XMLVerifier
 
 from passbook.lib.utils.template import render_to_string
 
@@ -17,12 +16,13 @@ def sign_with_signxml(private_key, data, cert, reference_uri=None):
     key = serialization.load_pem_private_key(
         str.encode('\n'.join([x.strip() for x in private_key.split('\n')])),
         password=None, backend=default_backend())
-    # LXML is used here because defusedxml causes issues with serialization
+    # defused XML is not used here because it messes up XML namespaces
-    # data is trusted so no issues
+    # Data is trusted, so lxml is ok
     root = etree.fromstring(data) # nosec
     signer = XMLSigner(c14n_algorithm='http://www.w3.org/2001/10/xml-exc-c14n#')
-    signed = signer.sign(root, key=key, cert=cert, reference_uri=reference_uri)
+    signed = signer.sign(root, key=key, cert=[cert], reference_uri=reference_uri)
-    return ElementTree.tostring(signed)
+    XMLVerifier().verify(signed, x509_cert=cert)
+    return etree.tostring(signed).decode('utf-8') # nosec
 
 
 def get_signature_xml():