Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
8 changes: 8 additions & 0 deletions config/packages/ci/parameters.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
parameters:
encryption_keys:
default:
publicFile: /config/engine/engineblock.crt
privateFile: /config/engine/engineblock.pem
rollover:
publicFile: '%kernel.project_dir%/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/keys/rolled-over.crt'
privateFile: '%kernel.project_dir%/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/keys/rolled-over.key'
8 changes: 8 additions & 0 deletions config/packages/dev/parameters.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
parameters:
encryption_keys:
default:
publicFile: /config/engine/engineblock.crt
privateFile: /config/engine/engineblock.pem
rollover:
publicFile: '%kernel.project_dir%/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/keys/rolled-over.crt'
privateFile: '%kernel.project_dir%/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/keys/rolled-over.key'
3 changes: 3 additions & 0 deletions config/packages/parameters.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ parameters:
## The Signing / Encryption keys used for the SAML2 authentication and metadata
## When EngineBlock signs responses (when it acts as an Idp)
## or requests (when it acts as an SP) it uses these X.509 certs.
## During a key rollover, add the new key as 'rollover'. The default metadata will then
## contain both keys, allowing IdPs to accept responses signed with either key.
## Once all IdPs have updated their metadata, remove the old key or swap default/rollover.
encryption_keys:
default:
publicFile: /config/engine/engineblock.crt
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@
class EngineBlockIdentityProvider extends AbstractIdentityProvider
{
/**
* @var X509KeyPair
* @var array<X509KeyPair>
*/
private $keyPair;
private array $keyPairs;

/**
* @var UrlProvider
*/
Expand All @@ -46,20 +47,23 @@ class EngineBlockIdentityProvider extends AbstractIdentityProvider
public function __construct(
IdentityProviderEntityInterface $entity,
?string $keyId,
X509KeyPair $keyPair,
array $keyPairs,
UrlProvider $urlProvider
) {
parent::__construct($entity);
$this->keyId = $keyId;
$this->keyPair = $keyPair;
$this->keyPairs = $keyPairs;
$this->urlProvider = $urlProvider;
}

public function getCertificates(): array
{
return [
$this->keyPair->getCertificate(),
];
$certificates = [];
foreach ($this->keyPairs as $keyPair) {
$certificates[] = $keyPair->getCertificate();
}

return $certificates;
}

public function getSupportedNameIdFormats(): array
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,35 +32,42 @@
class EngineBlockServiceProvider extends AbstractServiceProvider
{
/**
* @var X509KeyPair
* @var array<X509KeyPair>
*/
private $keyPair;
private array $keyPairs;

/**
* @var AttributesMetadata
*/
private $attributes;

/**
* @var UrlProvider
*/
private $urlProvider;

public function __construct(
ServiceProviderEntityInterface $entity,
X509KeyPair $keyPair,
array $keyPairs,
AttributesMetadata $attributes,
UrlProvider $urlProvider
) {
parent::__construct($entity);

$this->keyPair = $keyPair;
$this->keyPairs = $keyPairs;
$this->attributes = $attributes;
$this->urlProvider = $urlProvider;
}


public function getCertificates(): array
{
return [$this->keyPair->getCertificate()];
$certificates = [];
foreach ($this->keyPairs as $keyPair) {
$certificates[] = $keyPair->getCertificate();
}

return $certificates;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ private function buildEngineBlockEntityFromEntity(IdentityProvider $entity, ?str
$this->engineBlockConfiguration
),
$keyId,
$this->keyPairFactory->buildFromIdentifier($keyId),
($keyId === KeyPairFactory::DEFAULT_KEY_PAIR_IDENTIFIER || $keyId === null)
? $this->keyPairFactory->buildAll()
: [$this->keyPairFactory->buildFromIdentifier($keyId)],
$this->urlProvider
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class ServiceProviderFactory
* @var EngineBlockConfiguration
*/
private $engineBlockConfiguration;

/**
* @var UrlProvider
*/
Expand Down Expand Up @@ -95,7 +96,9 @@ public function createEngineBlockEntityFrom(string $keyId): ServiceProviderEntit
new ServiceProviderEntity($entity),
$this->engineBlockConfiguration
),
$this->keyPairFactory->buildFromIdentifier($keyId),
($keyId === KeyPairFactory::DEFAULT_KEY_PAIR_IDENTIFIER || $keyId === null)
? $this->keyPairFactory->buildAll()
: [$this->keyPairFactory->buildFromIdentifier($keyId)],
$this->attributes,
$this->urlProvider
);
Expand Down
20 changes: 18 additions & 2 deletions src/OpenConext/EngineBlock/Metadata/X509/KeyPairFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class KeyPairFactory
{
const DEFAULT_KEY_PAIR_IDENTIFIER = 'default';

private $keyPairConfiguration = [];
private array $keyPairConfiguration = [];

/**
* @param array $keyPairConfiguration
Expand All @@ -42,7 +42,7 @@ public function __construct(array $keyPairConfiguration)
*
* @throws RuntimeException
*/
public function buildFromIdentifier(?string $identifier) : X509KeyPair
public function buildFromIdentifier(?string $identifier): X509KeyPair
{
if ($identifier === null) {
$identifier = self::DEFAULT_KEY_PAIR_IDENTIFIER;
Expand All @@ -57,4 +57,20 @@ public function buildFromIdentifier(?string $identifier) : X509KeyPair
}
throw new UnknownKeyIdException($identifier);
}

/**
* @return array<X509KeyPair>
*
* @throws RuntimeException
*/
public function buildAll(): array
{
$pairs = [];

foreach (array_keys($this->keyPairConfiguration) as $keyId) {
$pairs[] = $this->buildFromIdentifier((string)$keyId);
}

return $pairs;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,25 @@ Feature:

Scenario: The user can see all available metadata links
When I go to Engineblock URL "/"
Then I should see 8 links on the front page
Then I should see 12 links on the front page
And I should see text matching "The Public SAML Signing certificate of the OpenConext IdP"
And I should see URL "/authentication/idp/certificate"
# The default key should not get a separate URL next to the key-less url
And I should not see URL "/authentication/idp/certificate/key:default"
# The rollover key should get its own URL
And I should see URL "/authentication/idp/certificate/key:rollover"
And I should see text matching "The Public SAML metadata \(the entity descriptor\) of the OpenConext IdP Proxy"
And I should see URL "/authentication/idp/metadata"
And I should not see URL "/authentication/idp/metadata/key:default"
And I should see URL "/authentication/idp/metadata/key:rollover"
And I should see text matching "The Public SAML metadata \(the entities descriptor\) for all the OpenConext IdPs"
And I should see URL "/authentication/proxy/idps-metadata"
And I should not see URL "/authentication/proxy/idps-metadata/key:default"
And I should see URL "/authentication/proxy/idps-metadata/key:rollover"
And I should see text matching "The Public SAML metadata \(the entities descriptor\) of the OpenConext IdPs which"
And I should see URL "/authentication/proxy/idps-metadata?sp-entity-id=urn:example.org"
And I should not see URL "/authentication/proxy/idps-metadata/key:default?sp-entity-id=urn:example.org"
And I should see URL "/authentication/proxy/idps-metadata/key:rollover?sp-entity-id=urn:example.org"
# The test debug link is present on the page
And I should see text matching "Test authentication with an identity provider."
And I should see URL "/authentication/sp/debug"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,27 @@ Feature:
# Verify the used signing key is EB key
And the response should match xpath '//ds:Signature//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'

Scenario: The default SP metadata contains all configured keys (rollover support)
When I go to Engineblock URL "/authentication/sp/metadata"
# Verify the default (current) signing key is present
Then the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'
# Verify the rollover signing key is also present
And the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDhTCCAm2gAwIBAgIJALJlbT5u9cXzMA0GCSqGSIb3DQEBBQUAMFkxCzAJ")]'

Scenario: The default IdP metadata contains all configured keys (rollover support)
When I go to Engineblock URL "/authentication/idp/metadata"
# Verify the default (current) signing key is present
Then the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'
# Verify the rollover signing key is also present
And the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDhTCCAm2gAwIBAgIJALJlbT5u9cXzMA0GCSqGSIb3DQEBBQUAMFkxCzAJ")]'

Scenario: Requesting metadata with a specific key id returns only that key
When I go to Engineblock URL "/authentication/sp/metadata/key:rollover"
# Verify only the rollover key is present
Then the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDhTCCAm2gAwIBAgIJALJlbT5u9cXzMA0GCSqGSIb3DQEBBQUAMFkxCzAJ")]'
# Verify the default key is not present
And the response should not match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'

Scenario: A user can request the EngineBlock SP public certificate
When I go to Engineblock URL "/authentication/sp/certificate"
Then the response should contain '-----BEGIN CERTIFICATE-----'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function (...$parameters) use ($matcher) {
$attributesMock->method('getRequestedAttributes')
->willReturn($attributes);

$decorator = new EngineBlockServiceProvider($adapter, $keyPairMock, $attributesMock, $this->urlProvider);
$decorator = new EngineBlockServiceProvider($adapter, [$keyPairMock], $attributesMock, $this->urlProvider);

$supportedNameIdFormats = [
Constants::NAMEID_PERSISTENT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public function test_methods()
}
});

$decorator = new EngineBlockIdentityProvider($this->adapter, 'default', $this->keyPairMock, $this->urlProvider);
$decorator = new EngineBlockIdentityProvider($this->adapter, 'default', [$this->keyPairMock], $this->urlProvider);

$supportedNameIdFormats = [
Constants::NAMEID_PERSISTENT,
Expand All @@ -110,7 +110,7 @@ public function test_override_slo_service_if_child_slo_service_set()
->with('authentication_logout', false, null, null)
->willReturn('sloLocation');

$decorator = new EngineBlockIdentityProvider($this->adapter, 'default', $this->keyPairMock, $this->urlProvider);
$decorator = new EngineBlockIdentityProvider($this->adapter, 'default', [$this->keyPairMock], $this->urlProvider);

$this->assertEquals($decorator->getSingleLogoutService(), new Service('sloLocation', Constants::BINDING_HTTP_REDIRECT));
}
Expand All @@ -121,7 +121,7 @@ public function test_return_null_for_slo_service_if_child_has_no_slo_service_set
'singleLogoutService' => null,
]);

$decorator = new EngineBlockIdentityProvider($this->adapter, 'default', $this->keyPairMock, $this->urlProvider);
$decorator = new EngineBlockIdentityProvider($this->adapter, 'default', [$this->keyPairMock], $this->urlProvider);

$this->assertEquals($decorator->getSingleLogoutService(), null);
}
Expand All @@ -145,7 +145,7 @@ function (...$parameters) use ($matcher) {
} // we would expect the fourth paremeter to be null and not 'entity-id' becasue we are EB
);

$decorator = new EngineBlockIdentityProvider($this->adapter, 'default', $this->keyPairMock, $this->urlProvider);
$decorator = new EngineBlockIdentityProvider($this->adapter, 'default', [$this->keyPairMock], $this->urlProvider);

$this->assertEquals([new Service('ssoLocation', Constants::BINDING_HTTP_REDIRECT)], $decorator->getSingleSignOnServices());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ public function test_builds_key_pair_from_specified_identifier()
$this->assertEquals('file://' . __DIR__ . '/test.pem.key', $defaultKeyPair->getPrivateKey()->getFilePath());
}

public function test_build_all_returns_all_configured_key_pairs()
{
$keyPairs = $this->factory->buildAll();

$this->assertCount(2, $keyPairs);
$this->assertContainsOnlyInstancesOf(X509KeyPair::class, $keyPairs);
$this->assertEquals(file_get_contents('file://' . __DIR__ . '/test.pem.crt'), $keyPairs[0]->getCertificate()->toPem());
$this->assertEquals(file_get_contents('file://' . __DIR__ . '/test2.pem.crt'), $keyPairs[1]->getCertificate()->toPem());
}

public function test_it_raises_exception_when_requesting_empty_key_pair_identifier()
{
$this->expectException(InvalidArgumentException::class);
Expand Down