From 8ee2e489c3570d459674abfd8c92eb8aef527799 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Freitag?= <mail@franek.fr>
Date: Thu, 7 Feb 2019 15:01:28 +0100
Subject: [PATCH] Make metadata validateFingerprint algorithm configurable
Some metadata publishers offer SHA256 fingerprints to validate the
fingerprint of the certificate used to sign metadata. Allow users to
configure the fingerprinting algorithm to facilitate validation.
For backward compatibility, the default algorithm remains SHA1.
---
docs/simplesamlphp-automated_metadata.md | 4 +
docs/simplesamlphp-changelog.md | 5 +
lib/SimpleSAML/Metadata/SAMLParser.php | 38 ++++++-
lib/SimpleSAML/Metadata/Sources/MDQ.php | 11 +-
modules/metarefresh/bin/metarefresh.php | 16 +++
.../config-templates/config-metarefresh.php | 1 +
modules/metarefresh/lib/MetaLoader.php | 6 +-
.../SimpleSAML/Metadata/SAMLParserTest.php | 104 +++++++++++++++++-
.../metarefresh/lib/MetaLoaderTest.php | 27 ++++-
9 files changed, 205 insertions(+), 7 deletions(-)
diff --git a/docs/simplesamlphp-automated_metadata.md b/docs/simplesamlphp-automated_metadata.md
index cc40de2ce..b06b116f7 100644
--- a/docs/simplesamlphp-automated_metadata.md
+++ b/docs/simplesamlphp-automated_metadata.md
@@ -162,6 +162,10 @@ Each metadata source has the following options:
don't need this option if you don't want to validate the signature
on the metadata.
+`validateFingerprintAlgorithm`
+: Algorithm used to compute the signing certificate's fingerprint. Defaults to
+ `XMLSecurityDSig::SHA1`.
+
`template`
: This is an array which will be combined with the metadata fetched to
generate the final metadata array.
diff --git a/docs/simplesamlphp-changelog.md b/docs/simplesamlphp-changelog.md
index c57556975..02e4ce002 100644
--- a/docs/simplesamlphp-changelog.md
+++ b/docs/simplesamlphp-changelog.md
@@ -48,6 +48,11 @@ Released TBD
* Allow `core:PHP` to manipulate the entire state array.
* IdP initiated login: add compatibility with Shibboleth parameters.
+### metarefresh
+ * The algorithm to compute the fingerprint of the certificate that signed
+ metadata can be specified with the new `validateFingerprintAlgorithm`
+ configuration option.
+
### multiauth
* Added a `preselect` configuration option to skip authsource selection (#1005).
diff --git a/lib/SimpleSAML/Metadata/SAMLParser.php b/lib/SimpleSAML/Metadata/SAMLParser.php
index 961971758..1a86ae37b 100644
--- a/lib/SimpleSAML/Metadata/SAMLParser.php
+++ b/lib/SimpleSAML/Metadata/SAMLParser.php
@@ -2,6 +2,7 @@
namespace SimpleSAML\Metadata;
+use RobRichards\XMLSecLibs\XMLSecurityDSig;
use RobRichards\XMLSecLibs\XMLSecurityKey;
/**
@@ -1450,16 +1451,48 @@ class SAMLParser
}
+ private function computeFingerprint($algorithm, $data)
+ {
+ switch ($algorithm) {
+ case XMLSecurityDSig::SHA1:
+ $algo = 'SHA1';
+ break;
+ case XMLSecurityDSig::SHA256:
+ $algo = 'SHA256';
+ break;
+ case XMLSecurityDSig::SHA384:
+ $algo = 'SHA384';
+ break;
+ case XMLSecurityDSig::SHA512:
+ $algo = 'SHA512';
+ break;
+ default:
+ $known_opts = implode(", ", [
+ XMLSecurityDSig::SHA1,
+ XMLSecurityDSig::SHA256,
+ XMLSecurityDSig::SHA384,
+ XMLSecurityDSig::SHA512,
+ ]);
+ throw new \UnexpectedValueException(
+ "Unsupported hashing function {$algorithm}. " .
+ "Known options: [{$known_opts}]"
+ );
+ }
+ return hash($algo, $data);
+ }
+
+
/**
* This function checks if this EntityDescriptor was signed with a certificate with the
* given fingerprint.
*
* @param string $fingerprint Fingerprint of the certificate which should have been used to sign this
* EntityDescriptor.
+ * @param string $algorithm Algorithm used to compute the fingerprint of the signing certicate.
*
* @return boolean True if it was signed with the certificate with the given fingerprint, false otherwise.
*/
- public function validateFingerprint($fingerprint)
+ public function validateFingerprint($fingerprint, $algorithm)
{
assert(is_string($fingerprint));
@@ -1468,7 +1501,8 @@ class SAMLParser
$candidates = [];
foreach ($this->validators as $validator) {
foreach ($validator->getValidatingCertificates() as $cert) {
- $fp = strtolower(sha1(base64_decode($cert)));
+ $decoded_cert = base64_decode($cert);
+ $fp = $this->computeFingerprint($algorithm, $decoded_cert);
$candidates[] = $fp;
if ($fp === $fingerprint) {
return true;
diff --git a/lib/SimpleSAML/Metadata/Sources/MDQ.php b/lib/SimpleSAML/Metadata/Sources/MDQ.php
index 413846427..7c73449fe 100644
--- a/lib/SimpleSAML/Metadata/Sources/MDQ.php
+++ b/lib/SimpleSAML/Metadata/Sources/MDQ.php
@@ -2,6 +2,7 @@
namespace SimpleSAML\Metadata\Sources;
+use RobRichards\XMLSecLibs\XMLSecurityDSig;
use SimpleSAML\Logger;
use SimpleSAML\Utils\HTTP;
@@ -78,6 +79,11 @@ class MDQ extends \SimpleSAML\Metadata\MetaDataStorageSource
} else {
$this->validateFingerprint = null;
}
+ if (isset($config['validateFingerprintAlgorithm'])) {
+ $this->validateFingerprintAlgorithm = $config['validateFingerprintAlgorithm'];
+ } else {
+ $this->validateFingerprintAlgorithm = XMLSecurityDSig::SHA1;
+ }
if (array_key_exists('cachedir', $config)) {
$globalConfig = \SimpleSAML\Configuration::getInstance();
@@ -315,7 +321,10 @@ class MDQ extends \SimpleSAML\Metadata\MetaDataStorageSource
Logger::debug(__CLASS__.': completed parsing of ['.$mdq_url.']');
if ($this->validateFingerprint !== null) {
- if (!$entity->validateFingerprint($this->validateFingerprint)) {
+ if (!$entity->validateFingerprint(
+ $this->validateFingerprint,
+ $this->validateFingerprintAlgorithm
+ )) {
throw new \Exception(__CLASS__.': error, could not verify signature for entity: '.$index.'".');
}
}
diff --git a/modules/metarefresh/bin/metarefresh.php b/modules/metarefresh/bin/metarefresh.php
index b02fb75a6..a7c34aeff 100755
--- a/modules/metarefresh/bin/metarefresh.php
+++ b/modules/metarefresh/bin/metarefresh.php
@@ -5,6 +5,7 @@
* This script can be used to generate metadata for SimpleSAMLphp
* based on an XML metadata file.
*/
+use RobRichards\XMLSecLibs\XMLSecurityDSig;
// This is the base directory of the SimpleSAMLphp installation
@@ -44,6 +45,11 @@ $certificates = null;
*/
$validateFingerprint = null;
+/* $validateFingerprintAlgorithm is the algorithm to use to compute the fingerprint of the
+ * certificate that signed the metadata.
+ */
+$validateFingerprintAlgorithm = null;
+
// This variable contains the files we will parse
$files = [];
@@ -97,6 +103,9 @@ foreach ($argv as $a) {
}
$validateFingerprint = $v;
break;
+ case '--validate-fingerprint-algorithm':
+ $validateFingerprintAlgorithm = $v;
+ break;
case '--help':
printHelp();
exit(0);
@@ -134,6 +143,9 @@ foreach ($files as $f) {
if (isset($validateFingerprint)) {
$source['validateFingerprint'] = $validateFingerprint;
}
+ if (isset($validateFingerprintAlgorithm)) {
+ $source['validateFingerprintAlgorithm'] = $validateFingerprintAlgorithm;
+ }
$metaloader->loadSource($source);
}
@@ -167,6 +179,10 @@ function printHelp()
echo ' Check the signature of the metadata,'."\n";
echo ' and check the fingerprint of the'."\n";
echo ' certificate against <FINGERPRINT>.'."\n";
+ echo ' --validate-fingerprint-algorithm=<ALGORITHM>'."\n";
+ echo ' Use <ALGORITHM> to validate fingerprint of'."\n";
+ echo ' the certificate that signed the metadata.'."\n";
+ echo ' Default: '.XMLSecurityDSig::SHA1.".\n";
echo ' -h, --help Print this help.'."\n";
echo ' -o=<DIR>, --out-dir=<DIR> Write the output to this directory. The'."\n";
echo ' default directory is metadata-generated/.'."\n";
diff --git a/modules/metarefresh/config-templates/config-metarefresh.php b/modules/metarefresh/config-templates/config-metarefresh.php
index c8009d4c2..333b75dee 100644
--- a/modules/metarefresh/config-templates/config-metarefresh.php
+++ b/modules/metarefresh/config-templates/config-metarefresh.php
@@ -44,6 +44,7 @@ $config = [
'rollover.crt',
],
'validateFingerprint' => '59:1D:4B:46:70:46:3E:ED:A9:1F:CC:81:6D:C0:AF:2A:09:2A:A8:01',
+ #'validateFingerprintAlgorithm' => RobRichards\XMLSecLibs\XMLSecurityDSig::SHA1,
'template' => [
'tags' => ['kalmar'],
'authproc' => [
diff --git a/modules/metarefresh/lib/MetaLoader.php b/modules/metarefresh/lib/MetaLoader.php
index e8007803d..a100aeec3 100644
--- a/modules/metarefresh/lib/MetaLoader.php
+++ b/modules/metarefresh/lib/MetaLoader.php
@@ -2,6 +2,7 @@
namespace SimpleSAML\Module\metarefresh;
+use RobRichards\XMLSecLibs\XMLSecurityDSig;
use SimpleSAML\Logger;
/**
@@ -154,7 +155,10 @@ class MetaLoader
if (array_key_exists('validateFingerprint', $source) && $source['validateFingerprint'] !== null) {
if (!array_key_exists('certificates', $source) || $source['certificates'] == null) {
- if (!$entity->validateFingerprint($source['validateFingerprint'])) {
+ $algo = isset($source['validateFingerprintAlgorithm'])
+ ? $source['validateFingerprintAlgorithm']
+ : XMLSecurityDSig::SHA1;
+ if (!$entity->validateFingerprint($source['validateFingerprint'], $algo)) {
Logger::info(
'Skipping "'.$entity->getEntityId().'" - could not verify signature using fingerprint.'."\n"
);
diff --git a/tests/lib/SimpleSAML/Metadata/SAMLParserTest.php b/tests/lib/SimpleSAML/Metadata/SAMLParserTest.php
index a443d0e31..94c25ba9d 100644
--- a/tests/lib/SimpleSAML/Metadata/SAMLParserTest.php
+++ b/tests/lib/SimpleSAML/Metadata/SAMLParserTest.php
@@ -2,12 +2,17 @@
namespace SimpleSAML\Test\Metadata;
+require_once(__DIR__.'/../../../SigningTestCase.php');
+
use PHPUnit\Framework\TestCase;
+use RobRichards\XMLSecLibs\XMLSecurityDSig;
+use \SimpleSAML\XML\Signer;
+use \SimpleSAML\Metadata\SAMLParser;
/**
* Test SAML parsing
*/
-class SAMLParserTest extends TestCase
+class SAMLParserTest extends \SimpleSAML\Test\SigningTestCase
{
/**
* Test Registration Info is parsed
@@ -133,4 +138,101 @@ XML
$this->assertEquals($expected_a, $metadata['attributes']);
$this->assertEquals($expected_r, $metadata['attributes.required']);
}
+
+
+ public function makeTestDocument()
+ {
+ $doc = new \DOMDocument();
+ $doc->loadXML(
+ <<<XML
+<?xml version="1.0"?>
+<EntitiesDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata">
+ <EntityDescriptor entityID="theEntityID">
+ <SPSSODescriptor protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol"/>
+ </EntityDescriptor>
+</EntitiesDescriptor>
+XML
+ );
+
+ $entities_root = $doc->getElementsByTagName('EntitiesDescriptor')->item(0);
+ $signer = new Signer([]);
+ $signer->loadPrivateKey($this->good_private_key_file, null, true);
+ $signer->loadCertificate($this->good_certificate_file, true);
+ $signer->sign($entities_root, $entities_root);
+
+ return $doc;
+ }
+
+ public function _testValidateFingerprint($algo, $expected_fingerprint)
+ {
+ $doc = $this->makeTestDocument();
+ $entities = \SimpleSAML\Metadata\SAMLParser::parseDescriptorsElement($doc->documentElement);
+ foreach ($entities as $entity) {
+ $this->assertTrue(
+ $entity->validateFingerprint($expected_fingerprint, $algo)
+ );
+ }
+ }
+
+
+ public function testValidateFingerprintSHA1()
+ {
+ $this->_testValidateFingerprint(
+ XMLSecurityDSig::SHA1,
+ 'A7:FB:75:22:57:88:A1:B0:D0:29:0A:4B:D1:EA:0C:01:F8:98:44:A0'
+ );
+ }
+
+
+ public function testValidateFingerprintSHA256()
+ {
+ $this->_testValidateFingerprint(
+ XMLSecurityDSig::SHA256,
+ '3E:04:6B:2C:13:B5:02:FB:FC:93:66:EE:6C:A3:D1:BB:B8:9E:D8:38:03' .
+ ':96:C5:C0:EC:95:D5:C9:F6:C1:D5:FC'
+ );
+ }
+
+
+ public function testValidateFingerprintSHA384()
+ {
+ $this->_testValidateFingerprint(
+ XMLSecurityDSig::SHA384,
+ '38:87:CC:59:54:CF:ED:FC:71:B6:21:F3:8A:52:76:EF:30:C8:8C:A0:38' .
+ ':48:77:87:58:14:A0:B3:55:EF:48:9C:B4:B3:44:1F:B7:BB:FC:28:65' .
+ ':6E:93:83:52:C2:8E:A6'
+ );
+ }
+
+
+ public function testValidateFingerprintSHA512()
+ {
+ $this->_testValidateFingerprint(
+ XMLSecurityDSig::SHA512,
+ '72:6C:51:01:A1:E9:76:D8:61:C4:B2:4F:AC:0B:64:7D:0D:4E:B7:DC:B3' .
+ ':4A:92:23:51:A6:DC:A5:A1:9A:A5:DD:43:F5:05:6A:B7:7D:83:1F:B6:' .
+ 'CC:68:54:54:54:37:1B:EC:E1:22:5A:48:C6:BC:67:4B:A6:78:EE:E0:C6:8C:59'
+ );
+ }
+
+
+ public function testValidateFingerprintUnknownAlgorithmThrows()
+ {
+ $doc = $this->makeTestDocument();
+ $entities = \SimpleSAML\Metadata\SAMLParser::parseDescriptorsElement($doc->documentElement);
+ foreach ($entities as $entity) {
+ try {
+ $entity->validateFingerprint('unused', 'invalid_algorithm');
+ } catch (\UnexpectedValueException $e) {
+ $this->assertEquals(
+ 'Unsupported hashing function invalid_algorithm. Known options: [' .
+ 'http://www.w3.org/2000/09/xmldsig#sha1, ' .
+ 'http://www.w3.org/2001/04/xmlenc#sha256, ' .
+ 'http://www.w3.org/2001/04/xmldsig-more#sha384, ' .
+ 'http://www.w3.org/2001/04/xmlenc#sha512]',
+ $e->getMessage()
+ );
+ }
+ }
+ }
}
diff --git a/tests/modules/metarefresh/lib/MetaLoaderTest.php b/tests/modules/metarefresh/lib/MetaLoaderTest.php
index c0cfe5b68..d70d415f2 100644
--- a/tests/modules/metarefresh/lib/MetaLoaderTest.php
+++ b/tests/modules/metarefresh/lib/MetaLoaderTest.php
@@ -3,6 +3,7 @@
namespace SimpleSAML\Test\Module\metarefresh;
use PHPUnit\Framework\TestCase;
+use RobRichards\XMLSecLibs\XMLSecurityDSig;
use \SimpleSAML\Configuration;
class MetaLoaderTest extends TestCase
@@ -83,9 +84,31 @@ class MetaLoaderTest extends TestCase
);
}
- public function testSignatureVerificationFingerprintPass()
+ public function testSignatureVerificationFingerprintDefaultsToSHA1()
{
- $this->metaloader->loadSource(array_merge($this->source, [ 'validateFingerprint' => '85:11:00:FF:34:55:BC:20:C0:20:5D:46:9B:2F:23:8F:41:09:68:F2' ]));
+ $this->metaloader->loadSource(
+ array_merge(
+ $this->source,
+ [
+ 'validateFingerprint' => '85:11:00:FF:34:55:BC:20:C0:20:5D:46:9B:2F:23:8F:41:09:68:F2',
+ ]
+ )
+ );
+ $this->metaloader->dumpMetadataStdOut();
+ $this->expectOutputRegex('/UTEbMBkGA1UECgwSRXhhbXBsZSBVbml2ZXJzaXR5MRgwFgYDVQQDDA9pZHAuZXhh/');
+ }
+
+ public function testSignatureVerificationFingerprintSHA256()
+ {
+ $this->metaloader->loadSource(
+ array_merge(
+ $this->source,
+ [
+ 'validateFingerprint' => '36:64:49:4E:F4:4C:59:9F:5B:8F:FE:75:7E:B2:0C:1A:3A:27:AD:AF:11:B0:6D:EC:DF:38:B6:66:C8:C4:C6:84',
+ 'validateFingerprintAlgorithm' => XMLSecurityDSig::SHA256,
+ ]
+ )
+ );
$this->metaloader->dumpMetadataStdOut();
$this->expectOutputRegex('/UTEbMBkGA1UECgwSRXhhbXBsZSBVbml2ZXJzaXR5MRgwFgYDVQQDDA9pZHAuZXhh/');
}
--
GitLab