From 6d60227cbd70f143e981e8ba71c9acc646f56b84 Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Mon, 13 Oct 2014 16:33:01 +0100 Subject: [PATCH 1/3] Fix #53: empty passphrase breaks signing provider If configuration defines the passphrase for the key as either null or an empty string, the test `isset($config['key_pass'])` returns false. Since `key_pass` is optional, this commit removes it from the test and instead provides a default value if it's not set. --- .../DependencyInjection/Security/Factory/SamlSpFactory.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/AerialShip/SamlSPBundle/DependencyInjection/Security/Factory/SamlSpFactory.php b/src/AerialShip/SamlSPBundle/DependencyInjection/Security/Factory/SamlSpFactory.php index e7648fb..9aae9ba 100644 --- a/src/AerialShip/SamlSPBundle/DependencyInjection/Security/Factory/SamlSpFactory.php +++ b/src/AerialShip/SamlSPBundle/DependencyInjection/Security/Factory/SamlSpFactory.php @@ -174,13 +174,12 @@ protected function createSPSigningProvider(ContainerBuilder $container, $id, $na if (isset($config['id'])) { $container->setAlias($serviceID, $config['id']); } else if (isset($config['cert_file']) && - isset($config['key_file']) && - isset($config['key_pass']) + isset($config['key_file']) ) { $service = new DefinitionDecorator('aerial_ship_saml_sp.sp_signing.file'); $service->replaceArgument(1, $config['cert_file']); $service->replaceArgument(2, $config['key_file']); - $service->replaceArgument(3, $config['key_pass']); + $service->replaceArgument(3, array_key_exists('key_pass', $config) ? $config['key_pass'] : null); $container->setDefinition($serviceID, $service); } else { $service = new DefinitionDecorator('aerial_ship_saml_sp.sp_signing.null'); From eeb539ef027d125549330d35480a1a5dc7f64cd1 Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Mon, 13 Oct 2014 17:24:39 +0100 Subject: [PATCH 2/3] Sign AuthNRequest Note: I have no idea if this explodes when signing is disabled or not configured. --- src/AerialShip/SamlSPBundle/Bridge/Authenticate.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/AerialShip/SamlSPBundle/Bridge/Authenticate.php b/src/AerialShip/SamlSPBundle/Bridge/Authenticate.php index b3ab53f..4a14f3e 100644 --- a/src/AerialShip/SamlSPBundle/Bridge/Authenticate.php +++ b/src/AerialShip/SamlSPBundle/Bridge/Authenticate.php @@ -84,6 +84,8 @@ public function manage(Request $request) $builder = new AuthnRequestBuilder($spED, $idpED, $spMeta); $message = $builder->build(); + $message->sign($serviceInfo->getSpSigningProvider()->getCertificate(), $serviceInfo->getSpSigningProvider()->getPrivateKey()); + $binding = $this->bindingManager->instantiate($spMeta->getAuthnRequestBinding()); $bindingResponse = $binding->send($message); From c10140b7c3556ff5e0a311b4f4562100445847f0 Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Mon, 13 Oct 2014 17:33:03 +0100 Subject: [PATCH 3/3] Don't sign unless the signing provider is enabled --- src/AerialShip/SamlSPBundle/Bridge/Authenticate.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/AerialShip/SamlSPBundle/Bridge/Authenticate.php b/src/AerialShip/SamlSPBundle/Bridge/Authenticate.php index 4a14f3e..275f968 100644 --- a/src/AerialShip/SamlSPBundle/Bridge/Authenticate.php +++ b/src/AerialShip/SamlSPBundle/Bridge/Authenticate.php @@ -84,7 +84,9 @@ public function manage(Request $request) $builder = new AuthnRequestBuilder($spED, $idpED, $spMeta); $message = $builder->build(); - $message->sign($serviceInfo->getSpSigningProvider()->getCertificate(), $serviceInfo->getSpSigningProvider()->getPrivateKey()); + if ($serviceInfo->getSpSigningProvider()->isEnabled()) { + $message->sign($serviceInfo->getSpSigningProvider()->getCertificate(), $serviceInfo->getSpSigningProvider()->getPrivateKey()); + } $binding = $this->bindingManager->instantiate($spMeta->getAuthnRequestBinding());