Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
"mockery/mockery": "~1.6",
"simplesamlphp/simplesamlphp": "^2.5",
"simplesamlphp/simplesamlphp-test-framework": "~1.11",
"icanhazstring/composer-unused": "^0.9.6"
"icanhazstring/composer-unused": "^0.9.6",
"maglnet/composer-require-checker": "^4.20"
},
"suggest": {
"ext-soap": "*"
Expand Down
80 changes: 78 additions & 2 deletions src/Binding/HTTPArtifact.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,20 @@
use SimpleSAML\SAML2\XML\samlp\ArtifactResponse;
use SimpleSAML\Store\StoreFactory;
use SimpleSAML\Utils\HTTP;
use SimpleSAML\XMLSchema\Type\AnyURIValue;
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;
Expand Down Expand Up @@ -76,7 +82,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((string)$issuer->getContent(), true) . $generatedId,
);
$artifactData = $message->toXML();
$artifactDataString = $artifactData->ownerDocument?->saveXML($artifactData);

Expand All @@ -96,7 +104,7 @@ public function getRedirectURL(AbstractMessage $message): string
}

$httpUtils = new HTTP();
return $httpUtils->addURLparameters($destination, $params);
return $httpUtils->addURLparameters((string)$destination, $params);
}


Expand Down Expand Up @@ -184,6 +192,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? */
Expand Down Expand Up @@ -218,4 +228,70 @@ 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.');
}

$alg = $artifactResponse->getSignature()->getSignedInfo()->getSignatureMethod()->getAlgorithm();
$signatureMethod = $alg instanceof AnyURIValue ? $alg->getValue() : (string) $alg;
Comment thread
ioigoume marked this conversation as resolved.
Outdated

$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.');
}
}
90 changes: 77 additions & 13 deletions src/SOAPClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
Expand All @@ -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.');
}


Expand Down
Loading
Loading