refactor/soap-ssl-validator-openssl (#414)

* Remove xmlseclibs coupling by switching SOAP/Artifact validators to OpenSSL-based key extraction and updating tests

* Refactor signature verification to fail fast by removing redundant algorithm type guard

* Remove string-casts

* Fix syntax

---------

Co-authored-by: Tim van Dijen <tvdijen@gmail.com>

Author: ioigoume

composer.json

@@ -39,10 +39,11 @@
         "ext-intl": "*",
 
         "beste/clock": "~3.0",
+        "icanhazstring/composer-unused": "^0.9.6",
+        "maglnet/composer-require-checker": "^4.20",
         "mockery/mockery": "~1.6",
         "simplesamlphp/simplesamlphp": "^2.5",
-        "simplesamlphp/simplesamlphp-test-framework": "~2.0",
-        "icanhazstring/composer-unused": "^0.9.6"
+        "simplesamlphp/simplesamlphp-test-framework": "~2.0"
     },
     "suggest": {
         "ext-soap": "*"

src/Binding/HTTPArtifact.php

@@ -25,13 +25,18 @@
 use SimpleSAML\Store\StoreFactory;
 use SimpleSAML\Utils\HTTP;
 use SimpleSAML\XMLSecurity\Alg\Signature\SignatureAlgorithmFactory;
+use SimpleSAML\XMLSecurity\Key\PublicKey;
 use SimpleSAML\XMLSecurity\TestUtils\PEMCertificatesMock;
 
 use function array_key_exists;
 use function base64_decode;
 use function base64_encode;
 use function bin2hex;
+use function chunk_split;
+use function file_exists;
 use function hexdec;
+use function openssl_pkey_get_details;
+use function openssl_pkey_get_public;
 use function openssl_random_pseudo_bytes;
 use function pack;
 use function sha1;
@@ -76,7 +81,9 @@ public function getRedirectURL(AbstractMessage $message): string
         if ($issuer === null) {
             throw new Exception('Cannot get redirect URL, no Issuer set in the message.');
         }
-        $artifact = base64_encode("\x00\x04\x00\x00" . sha1($issuer->getContent(), true) . $generatedId);
+        $artifact = base64_encode(
+            "\x00\x04\x00\x00" . sha1($issuer->getContent()->getValue(), true) . $generatedId,
+        );
         $artifactData = $message->toXML();
         $artifactDataString = $artifactData->ownerDocument?->saveXML($artifactData);
 
@@ -96,7 +103,7 @@ public function getRedirectURL(AbstractMessage $message): string
         }
 
         $httpUtils = new HTTP();
-        return $httpUtils->addURLparameters($destination, $params);
+        return $httpUtils->addURLparameters($destination->getValue(), $params);
     }
 
 
@@ -184,6 +191,8 @@ public function receive(ServerRequestInterface $request): AbstractMessage
             throw new Exception('Received error from ArtifactResolutionService.');
         }
 
+        $artifactResponse = $this->verifyArtifactResponseSignature($artifactResponse, $idpMetadata);
+
         $samlResponse = $artifactResponse->getMessage();
         if ($samlResponse === null) {
             /* Empty ArtifactResponse - possibly because of Artifact replay? */
@@ -218,4 +227,74 @@ public function setSPMetadata(Configuration $sp): void
     {
         $this->spMetadata = $sp;
     }
+
+
+    /**
+     * Verify the ArtifactResponse signature using IdP metadata keys.
+     *
+     * Returns the verified ArtifactResponse instance.
+     *
+     * @throws \Exception When unsigned, when metadata has no signing keys, or when verification fails.
+     */
+    private function verifyArtifactResponseSignature(
+        ArtifactResponse $artifactResponse,
+        Configuration $idpMetadata,
+    ): ArtifactResponse {
+        if ($artifactResponse->isSigned() !== true) {
+            throw new Exception('ArtifactResponse must be signed.');
+        }
+
+        $keys = $idpMetadata->getPublicKeys('signing', true);
+        if (empty($keys)) {
+            throw new Exception('No signing keys found in IdP metadata.');
+        }
+
+        $signatureMethod = $artifactResponse
+            ->getSignature()
+            ->getSignedInfo()
+            ->getSignatureMethod()
+            ->getAlgorithm()
+            ->getValue();
+
+        $factory = new SignatureAlgorithmFactory();
+
+        $lastException = null;
+        foreach ($keys as $k) {
+            if (($k['type'] ?? null) !== 'X509Certificate') {
+                continue;
+            }
+
+            $pemCert = "-----BEGIN CERTIFICATE-----\n" .
+                chunk_split($k['X509Certificate'], 64) .
+                "-----END CERTIFICATE-----\n";
+
+            $opensslKey = openssl_pkey_get_public($pemCert);
+            if ($opensslKey === false) {
+                $lastException = new Exception('Unable to extract public key from X509 certificate.');
+                continue;
+            }
+
+            $keyInfo = openssl_pkey_get_details($opensslKey);
+            if ($keyInfo === false || !isset($keyInfo['key']) || !is_string($keyInfo['key'])) {
+                $lastException = new Exception('Unable to get public key details from X509 certificate.');
+                continue;
+            }
+
+            $pemPublicKey = $keyInfo['key'];
+
+            $file = Utils::getContainer()->getTempDir() . '/' . sha1($pemPublicKey) . '.pem';
+            if (!file_exists($file)) {
+                Utils::getContainer()->writeFile($file, $pemPublicKey);
+            }
+
+            try {
+                $verifier = $factory->getAlgorithm($signatureMethod, PublicKey::fromFile($file));
+                return $artifactResponse->verify($verifier);
+            } catch (Exception $e) {
+                $lastException = $e;
+            }
+        }
+
+        throw $lastException ?? new Exception('Unable to verify ArtifactResponse signature.');
+    }
 }

src/SOAPClient.php

@@ -6,7 +6,7 @@
 
 use DOMDocument;
 use Exception;
-use RobRichards\XMLSecLibs\XMLSecurityKey;
+use OpenSSLAsymmetricKey;
 use SimpleSAML\Configuration;
 use SimpleSAML\SAML2\Compat\ContainerSingleton;
 use SimpleSAML\SAML2\XML\samlp\AbstractMessage;
@@ -24,12 +24,17 @@
 
 use function chunk_split;
 use function file_exists;
+use function is_object;
+use function is_string;
+use function method_exists;
 use function openssl_pkey_get_details;
 use function openssl_pkey_get_public;
+use function property_exists;
 use function sha1;
 use function sprintf;
 use function stream_context_create;
 use function stream_context_get_options;
+use function trim;
 
 /**
  * Implementation of the SAML 2.0 SOAP binding.
@@ -251,7 +256,7 @@ private static function addSSLValidator(AbstractMessage $msg, $context): void
             return;
         }
 
-        if (!isset($keyInfo['key'])) {
+        if (!isset($keyInfo['key']) || !is_string($keyInfo['key'])) {
             $container->getLogger()->warning('Missing key in public key details.');
             return;
         }
@@ -263,29 +268,88 @@ private static function addSSLValidator(AbstractMessage $msg, $context): void
     /**
      * Validate a SOAP message against the certificate on the SSL connection.
      *
-     * @param string $data The public key that was used on the connection.
-     * @param \RobRichards\XMLSecLibs\XMLSecurityKey $key The key we should validate the certificate against.
+     * @param string $data The public key (PEM) that was used on the connection.
+     * @param mixed $key The key we should validate the certificate against.
      * @throws \Exception
      */
-    public static function validateSSL(string $data, XMLSecurityKey $key): void
+    public static function validateSSL(string $data, mixed $key): void
     {
         $container = ContainerSingleton::getInstance();
 
-        $keyInfo = openssl_pkey_get_details($key->key);
+        $pem = self::extractPublicKeyPem($key);
 
-        if ($keyInfo === false) {
-            throw new Exception('Unable to get key details from XMLSecurityKey.');
+        if (trim($pem) !== trim($data)) {
+            throw new Exception('Key on SSL connection did not match key we validated against.');
         }
 
-        if (!isset($keyInfo['key'])) {
-            throw new Exception('Missing key in public key details.');
+        $container->getLogger()->debug('Message validated based on SSL certificate.');
+    }
+
+
+    /**
+     * Extract a PEM-encoded public key from different key representations.
+     *
+     * This avoids coupling to a specific XML security backend.
+     *
+     * @param mixed $key
+     * @throws \Exception
+     */
+    private static function extractPublicKeyPem(mixed $key): string
+    {
+        // If the validating key is already PEM, normalize it by re-loading through OpenSSL.
+        if (is_string($key)) {
+            $opensslKey = openssl_pkey_get_public($key);
+            if ($opensslKey === false) {
+                throw new Exception('Unable to load validating public key from PEM string.');
+            }
+
+            $keyInfo = openssl_pkey_get_details($opensslKey);
+            if ($keyInfo === false || !isset($keyInfo['key']) || !is_string($keyInfo['key'])) {
+                throw new Exception('Unable to get key details from validating PEM key.');
+            }
+
+            return $keyInfo['key'];
         }
 
-        if (trim($keyInfo['key']) !== trim($data)) {
-            throw new Exception('Key on SSL connection did not match key we validated against.');
+        // Some key implementations may expose PEM via a method.
+        if (is_object($key)) {
+            foreach (['getPublicKeyPem', 'getPem', 'toPEM', 'toPem'] as $method) {
+                if (method_exists($key, $method)) {
+                    /** @var mixed $pem */
+                    $pem = $key->{$method}();
+                    if (is_string($pem) && $pem !== '') {
+                        return self::extractPublicKeyPem($pem);
+                    }
+                }
+            }
+
+            // Common compatibility case: an object wraps an OpenSSL key or PEM in a public "key" property.
+            if (property_exists($key, 'key')) {
+                /** @var mixed $inner */
+                $inner = $key->key;
+
+                if (is_string($inner)) {
+                    return self::extractPublicKeyPem($inner);
+                }
+
+                if ($inner instanceof OpenSSLAsymmetricKey) {
+                    $keyInfo = openssl_pkey_get_details($inner);
+                    if ($keyInfo !== false && isset($keyInfo['key']) && is_string($keyInfo['key'])) {
+                        return $keyInfo['key'];
+                    }
+                }
+            }
         }
 
-        $container->getLogger()->debug('Message validated based on SSL certificate.');
+        // Last attempt: OpenSSL might accept the value directly (OpenSSLAsymmetricKey).
+        if ($key instanceof OpenSSLAsymmetricKey) {
+            $keyInfo = openssl_pkey_get_details($key);
+            if ($keyInfo !== false && isset($keyInfo['key']) && is_string($keyInfo['key'])) {
+                return $keyInfo['key'];
+            }
+        }
+
+        throw new Exception('Unable to extract public key PEM from validating key.');
     }