Merge pull request #1791 from OpenConext/feature/add-expicit-signing-key-check

Add explicit IdP signing key feedback

Author: pablothedude

languages/messages.en.php

@@ -197,6 +197,8 @@
     'error_unknown_identity_provider_no_idp_name'   => 'Error - Unknown %organisationNoun%',
     'error_unknown_identity_provider_desc'     => '%idpName%, which you are trying to log in with, is unknown to %suiteName%.',
     'error_unknown_identity_provider_desc_no_idp_name'     => 'The %organisationNoun% you are trying to log in with is unknown to %suiteName%.',
+    'error_unknown_signing_key' => 'Error - unknown signing key',
+    'error_unknown_signing_key_desc' => 'The signing key used is not known to %suiteName%. This is possibly a configuration error.',
     'error_generic'                     => 'Error - An error occurred',
     'error_generic_desc'                => 'Logging in has failed and we don\'t know exactly why. Please try again first by going back to %spName% and logging in again. If this doesn\'t work, please contact the service desk of %idpName%.',
     'error_generic_desc_no_sp_name' => 'Logging in has failed and we don\'t know exactly why. Please try again first by going back to the service and logging in again. If this doesn\'t work, please contact the service desk of %idpName%.',

languages/messages.nl.php

@@ -195,6 +195,8 @@
     'error_unknown_identity_provider_no_idp_name'   => 'Error - Onbekende %organisationNoun%',
     'error_unknown_identity_provider_desc'     => '%idpName%, waarmee je probeert in te loggen, is onbekend bij %suiteName%.',
     'error_unknown_identity_provider_desc_no_idp_name'     => 'De %organisationNoun% waarmee je probeert in te loggen is onbekend bij %suiteName%.',
+    'error_unknown_signing_key' => 'Error - onbekende signing key',
+    'error_unknown_signing_key_desc' => 'De gebruikte signing key is niet bekend bij %suiteName%. Dit komt waarschijnlijk door een configuratiefout.',
     'error_generic'                     => 'Fout - Generieke foutmelding',
     'error_generic_desc'                => 'Inloggen is niet gelukt en we weten niet precies waarom. Probeer het eerst eens opnieuw door terug te gaan naar %spName% en opnieuw in te loggen. Lukt dit niet, neem dan contact op met de helpdesk van %idpName%.',
     'error_generic_desc_no_sp_name' => 'Inloggen is niet gelukt en we weten niet precies waarom. Probeer het eerst eens opnieuw door terug te gaan naar de dienst en opnieuw in te loggen. Lukt dit niet, neem dan contact op met de helpdesk van %idpName%.',

languages/messages.pt.php

@@ -193,6 +193,8 @@
     'error_unknown_identity_provider_no_idp_name'   => 'Erro - %organisationNoun% desconhecido',
     'error_unknown_identity_provider_desc'     => 'O %organisationNoun% a que pretende autenticar-se é desconhecido para a %suiteName%.',
     'error_unknown_identity_provider_desc_no_idp_name'     => 'O %organisationNoun% a que pretende autenticar-se é desconhecido para a %suiteName%.',
+    'error_unknown_signing_key' => 'Erro - chave de assinatura desconhecida',
+    'error_unknown_signing_key_desc' => 'A chave de assinatura utilizada não é conhecida por %suiteName%. Isto pode ser um erro de configuração.',
     'error_generic'                     => 'Erro - Ocorreu um erro',
     'error_generic_desc'                => 'A sua autenticação falhou e não sabemos exactamente porquê. Tente de novo e no caso de voltar a não funcionar, entre em contacto com o suporte da %idpName% para pedir ajuda.',
     'error_missing_required_fields'     => 'Erro - Campo necessário em falta',

library/EngineBlock/Corto/Exception/UnknownIdentityProviderSigningKey.php

@@ -0,0 +1,36 @@
+<?php
+
+/**
+ * Copyright 2010 SURFnet B.V.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class EngineBlock_Corto_Exception_UnknownIdentityProviderSigningKey extends EngineBlock_Exception
+{
+    /**
+     * @var string
+     */
+    private $entityId;
+
+    public function __construct($message, string $entityId, $severity = self::CODE_NOTICE, Exception $previous = null)
+    {
+        $this->entityId = $entityId;
+        parent::__construct($message, $severity, $previous);
+    }
+
+    public function getEntityId(): string
+    {
+        return $this->entityId;
+    }
+}

library/EngineBlock/Corto/Module/Bindings.php

@@ -342,6 +342,11 @@ public function receiveResponse($serviceEntityId, $expectedDestination)
         // check the IssueInstant against our own time to see if the SP's clock is getting out of sync
         $this->_checkIssueInstant( $sspResponse->getIssueInstant(), 'IdP', $idpEntityId );
 
+        // This is an initial check to validate when a signing key is in the
+        // response if it's the one that's expected by EB for that IdP,
+        //  so a decent error message can be shown if this is not the case.
+        $this->assertKnownIdPSigningKey($cortoIdpMetadata, $sspResponse);
+
         try {
             // 'Process' the response, verify the signature, verify the timings.
             if ($this->hasEncryptedAssertion($sspResponse)
@@ -886,4 +891,44 @@ private function hasEncryptedAssertion(Response $sspResponse)
         }
         return $hasEncryptedAssertion;
     }
+
+    /**
+     * This is an initial check to validate when a signing key is in the
+     * response, if it's the one that's expected by EB for that IdP,
+     * so a decent error message can be shown if this is not the case.
+     */
+    private function assertKnownIdPSigningKey(IdentityProvider $metadata, Response $sspResponse): void {
+
+        // If no key is in the response we don't need to validate if it's known because the
+        // validation if the signature is present is the responsibility of the SamlResponseValidator.
+        if (count($sspResponse->getCertificates()) === 0) {
+            return;
+        }
+
+        if (count($sspResponse->getCertificates()) > 1) {
+            $this->_logger->notice(
+                sprintf(
+                    'Tried to verify a message from issuer "%s", but requests with multiple signing keys are not supported.',
+                    $metadata->entityId
+                )
+            );
+
+            throw new EngineBlock_Corto_Module_Bindings_Exception(sprintf('Multiple signing keys supplied for issuer: %s', $metadata->entityId));
+        }
+
+        foreach ($metadata->certificates as $metaDataCertificate) {
+            if ($metaDataCertificate->toCertData() === $sspResponse->getCertificates()[0]) {
+                return;
+            }
+        }
+
+        $this->_logger->notice(
+            sprintf(
+                'Tried to verify a message from issuer "%s", but the expected signing key does not match.',
+                $metadata->entityId
+            )
+        );
+
+        throw new EngineBlock_Corto_Exception_UnknownIdentityProviderSigningKey('Unknown signing key supplied for issuer', $metadata->entityId);
+    }
 }

src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php

@@ -201,6 +201,19 @@ public function unknownIdentityProviderAction(Request $request)
         return new Response($body, 404);
     }
 
+    /**
+     * @param Request $request
+     * @return Response
+     * @throws \EngineBlock_Exception
+     */
+    public function unknownSigningKeyAction(Request $request)
+    {
+        return new Response(
+            $this->twig->render('@theme/Authentication/View/Feedback/unknown-signing-key.html.twig'),
+            400
+        );
+    }
+
     /**
      * @return Response
      * @throws \EngineBlock_Exception

src/OpenConext/EngineBlockBundle/EventListener/RedirectToFeedbackPageExceptionListener.php

@@ -28,6 +28,7 @@
 use EngineBlock_Corto_Exception_MissingRequiredFields;
 use EngineBlock_Corto_Exception_PEPNoAccess;
 use EngineBlock_Corto_Exception_ReceivedErrorStatusCode;
+use EngineBlock_Corto_Exception_UnknownIdentityProviderSigningKey;
 use EngineBlock_Corto_Exception_UnknownPreselectedIdp;
 use EngineBlock_Corto_Exception_InvalidAttributeValue;
 use EngineBlock_Corto_Exception_UserCancelledStepupCallout;
@@ -171,6 +172,9 @@ public function onKernelException(GetResponseForExceptionEvent $event)
                 'entity-id'   => $exception->getEntityId(),
                 'destination' => $exception->getDestination()
             ];
+        } elseif ($exception instanceof EngineBlock_Corto_Exception_UnknownIdentityProviderSigningKey) {
+            $message         = $exception->getMessage();
+            $redirectToRoute = 'authentication_feedback_unknown_signing_key';
         } elseif ($exception instanceof EngineBlock_Exception_UnknownRequesterIdInAuthnRequest) {
             $message         = 'Encountered unknown RequesterID for the Service Provider (transparant proxying)';
             $redirectToRoute = 'authentication_feedback_unknown_requesterid_in_authnrequest';

src/OpenConext/EngineBlockBundle/Resources/config/routing/feedback.yml

@@ -48,6 +48,11 @@ authentication_feedback_unknown_identity_provider:
     methods:    [GET]
     defaults:   { _controller: engineblock.controller.authentication.feedback:unknownIdentityProviderAction }
 
+authentication_feedback_unknown_signing_key:
+    path:       '/authentication/feedback/unknown-signing-key'
+    methods:    [GET]
+    defaults:   { _controller: engineblock.controller.authentication.feedback:unknownSigningKeyAction }
+
 authentication_feedback_missing_required_fields:
     path:       '/authentication/feedback/missing-required-fields'
     methods:    [GET]

theme/base/templates/modules/Authentication/View/Feedback/unknown-signing-key.html.twig

@@ -0,0 +1,12 @@
+{% extends '@theme/Default/View/Error/error.html.twig' %}
+
+{% set pageTitle = 'error_unknown_signing_key'|trans %}
+{% block pageTitle %}
+    {{ 'error_unknown_signing_key'|trans }}
+{% endblock %}
+{% block title %}{{ parent() }} - {{ pageTitle }} {% endblock %}
+{% block pageHeading %}{{ pageTitle }}{% endblock %}
+
+{% block errorMessage %}
+    {{ 'error_unknown_signing_key_desc'|trans }}
+{% endblock %}

theme/openconext/templates/modules/Authentication/View/Feedback/unknown-signing-key.html.twig

@@ -0,0 +1,12 @@
+{% extends '@theme/Default/View/Error/error.html.twig' %}
+
+{% set pageTitle = 'error_unknown_signing_key'|trans %}
+{% block pageTitle %}
+    {{ 'error_unknown_signing_key'|trans }}
+{% endblock %}
+{% block title %}{{ parent() }} - {{ pageTitle }} {% endblock %}
+{% block pageHeading %}{{ pageTitle }}{% endblock %}
+
+{% block errorMessage %}
+    {{ 'error_unknown_signing_key_desc'|trans }}
+{% endblock %}