Skip to content

Commit eed8c67

Browse files
ioigoumetvdijen
authored andcommitted
HTTP-Artifact: verify signatures using IdP metadata signing keys (fix TODO in verifier setup) (#419)
1 parent ebfa794 commit eed8c67

2 files changed

Lines changed: 419 additions & 7 deletions

File tree

src/Binding/HTTPArtifact.php

Lines changed: 133 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
use SimpleSAML\SAML2\XML\samlp\ArtifactResponse;
2525
use SimpleSAML\Store\StoreFactory;
2626
use SimpleSAML\Utils\HTTP;
27-
use SimpleSAML\XMLSecurity\XMLSecurityKey;
27+
use SimpleSAML\XMLSecurity\Alg\Signature\SignatureAlgorithmFactory;
28+
use SimpleSAML\XMLSecurity\Key\PublicKey;
2829

2930
use function array_key_exists;
3031
use function base64_decode;
@@ -205,7 +206,11 @@ public function receive(ServerRequestInterface $request): AbstractMessage
205206
$this->setRelayState($query['RelayState']);
206207
}
207208

208-
return $samlResponse;
209+
if (!$samlResponse->isSigned()) {
210+
return $samlResponse;
211+
}
212+
213+
return $this->verifyMessageSignature($samlResponse, $idpMetadata);
209214
}
210215

211216

@@ -221,15 +226,136 @@ public function setSPMetadata(Configuration $sp): void
221226

222227

223228
/**
224-
* A validator which returns true if the ArtifactResponse was signed with the given key
229+
* Verify the signature on a signed SAML message using IdP metadata keys.
230+
*
231+
* Returns the verified message instance.
232+
*
233+
* @throws \Exception When metadata has no signing keys, or when verification fails.
234+
*/
235+
private function verifyMessageSignature(AbstractMessage $message, Configuration $idpMetadata): AbstractMessage
236+
{
237+
$container = ContainerSingleton::getInstance();
238+
$blacklist = $container->getBlacklistedEncryptionAlgorithms();
239+
240+
// getPublicKeys(..., $required = true) throws if no signing cert/key material is present in metadata,
241+
// so $keys is guaranteed non-empty here (no additional empty($keys) guard needed).
242+
$keys = $idpMetadata->getPublicKeys('signing', true);
243+
244+
$signatureMethod = $message
245+
->getSignature()
246+
->getSignedInfo()
247+
->getSignatureMethod()
248+
->getAlgorithm()
249+
->getValue();
250+
251+
$factory = new SignatureAlgorithmFactory($blacklist);
252+
253+
$lastException = null;
254+
foreach ($keys as $k) {
255+
if (($k['type'] ?? null) !== 'X509Certificate') {
256+
continue;
257+
}
258+
259+
$pemCert = "-----BEGIN CERTIFICATE-----\n" .
260+
chunk_split($k['X509Certificate'], 64) .
261+
"-----END CERTIFICATE-----\n";
262+
263+
$opensslKey = openssl_pkey_get_public($pemCert);
264+
if ($opensslKey === false) {
265+
$lastException = new Exception('Unable to extract public key from X509 certificate.');
266+
continue;
267+
}
268+
269+
$keyInfo = openssl_pkey_get_details($opensslKey);
270+
if ($keyInfo === false || !isset($keyInfo['key']) || !is_string($keyInfo['key'])) {
271+
$lastException = new Exception('Unable to get public key details from X509 certificate.');
272+
continue;
273+
}
274+
275+
$pemPublicKey = $keyInfo['key'];
276+
277+
$file = Utils::getContainer()->getTempDir() . '/' . sha1($pemPublicKey) . '.pem';
278+
if (!file_exists($file)) {
279+
Utils::getContainer()->writeFile($file, $pemPublicKey);
280+
}
281+
282+
try {
283+
$verifier = $factory->getAlgorithm($signatureMethod, PublicKey::fromFile($file));
284+
return $message->verify($verifier);
285+
} catch (Exception $e) {
286+
$lastException = $e;
287+
}
288+
}
289+
290+
throw $lastException ?? new Exception('Unable to verify message signature.');
291+
}
292+
293+
294+
/**
295+
* Verify the ArtifactResponse signature using IdP metadata keys.
225296
*
226297
* @param \SimpleSAML\SAML2\XML\samlp\ArtifactResponse $message
227298
* @param \SimpleSAML\XMLSecurity\XMLSecurityKey $key
228299
* @return bool
229300
*/
230-
public static function validateSignature(ArtifactResponse $message, XMLSecurityKey $key): bool
231-
{
232-
// @todo verify if this works and/or needs to do anything more. Ref. HTTPRedirect binding
233-
return $message->validate($key);
301+
private function verifyArtifactResponseSignature(
302+
ArtifactResponse $artifactResponse,
303+
Configuration $idpMetadata,
304+
): ArtifactResponse {
305+
if ($artifactResponse->isSigned() !== true) {
306+
throw new Exception('ArtifactResponse must be signed.');
307+
}
308+
309+
// getPublicKeys(..., $required = true) throws if no signing cert/key material is present in metadata,
310+
// so $keys is guaranteed non-empty here (no additional empty($keys) guard needed).
311+
$keys = $idpMetadata->getPublicKeys('signing', true);
312+
313+
$signatureMethod = $artifactResponse
314+
->getSignature()
315+
->getSignedInfo()
316+
->getSignatureMethod()
317+
->getAlgorithm()
318+
->getValue();
319+
320+
$factory = new SignatureAlgorithmFactory();
321+
322+
$lastException = null;
323+
foreach ($keys as $k) {
324+
if (($k['type'] ?? null) !== 'X509Certificate') {
325+
continue;
326+
}
327+
328+
$pemCert = "-----BEGIN CERTIFICATE-----\n" .
329+
chunk_split($k['X509Certificate'], 64) .
330+
"-----END CERTIFICATE-----\n";
331+
332+
$opensslKey = openssl_pkey_get_public($pemCert);
333+
if ($opensslKey === false) {
334+
$lastException = new Exception('Unable to extract public key from X509 certificate.');
335+
continue;
336+
}
337+
338+
$keyInfo = openssl_pkey_get_details($opensslKey);
339+
if ($keyInfo === false || !isset($keyInfo['key']) || !is_string($keyInfo['key'])) {
340+
$lastException = new Exception('Unable to get public key details from X509 certificate.');
341+
continue;
342+
}
343+
344+
$pemPublicKey = $keyInfo['key'];
345+
346+
$file = Utils::getContainer()->getTempDir() . '/' . sha1($pemPublicKey) . '.pem';
347+
if (!file_exists($file)) {
348+
Utils::getContainer()->writeFile($file, $pemPublicKey);
349+
}
350+
351+
try {
352+
$verifier = $factory->getAlgorithm($signatureMethod, PublicKey::fromFile($file));
353+
return $artifactResponse->verify($verifier);
354+
} catch (Exception $e) {
355+
$lastException = $e;
356+
}
357+
}
358+
359+
throw $lastException ?? new Exception('Unable to verify ArtifactResponse signature.');
234360
}
235361
}

0 commit comments

Comments
 (0)