diff --git a/config/module_oidc.php.dist b/config/module_oidc.php.dist index cc5f988c..b68d2491 100644 --- a/config/module_oidc.php.dist +++ b/config/module_oidc.php.dist @@ -108,6 +108,40 @@ $config = [ ModuleConfig::OPTION_TOKEN_REFRESH_TOKEN_TTL => 'P1M', // 1 month ModuleConfig::OPTION_TOKEN_ACCESS_TOKEN_TTL => 'PT1H', // 1 hour, + /** + * (optional) An encryption key used to encrypt / decrypt artifacts like + * authorization codes and refresh tokens. + * + * If this option is NOT set (the default), the SimpleSAMLphp secret salt + * (config option 'secretsalt' in the main config.php) is used as a string + * password. In that case, the underlying League OAuth2 library derives the + * actual encryption key from the password using a slow, CPU-intensive key + * derivation function (key stretching) on every encrypt / decrypt + * operation. This is intentionally slow to reduce vulnerability to brute + * force attacks against a (potentially weak) password. + * + * Alternatively, you can set this option to a strong, randomly generated + * \Defuse\Crypto\Key (serialized to its ASCII-safe string form). Such a key + * is already a strong encryption key, so the slow key derivation is skipped, + * which reduces encryption / decryption time. You can generate the value + * using the script shipped with the defuse/php-encryption library and copy + * the printed ASCII-safe string: + * + * vendor/bin/generate-defuse-key + * + * (Internally the module loads it back via + * \Defuse\Crypto\Key::loadFromAsciiSafeString().) + * + * IMPORTANT (caveat): the encryption key protects already-issued artifacts. + * Changing it (including switching from the secret salt to a Key, or + * rotating an existing Key) invalidates all outstanding encrypted artifacts, + * meaning previously issued authorization codes and refresh tokens will no + * longer be decryptable and will be rejected. Set or change this value only + * during a planned maintenance / migration window, and expect affected + * clients to re-authenticate / obtain new tokens. + */ +// ModuleConfig::OPTION_ENCRYPTION_KEY => 'def0000040c2...ascii-safe-key-string...', + /** * (optional) Timestamp Validation Leeway - additional time tolerance * allowed for timestamp validation. This is used when validating diff --git a/docker/ssp/module_oidc.php b/docker/ssp/module_oidc.php index e44f3049..9be5023e 100644 --- a/docker/ssp/module_oidc.php +++ b/docker/ssp/module_oidc.php @@ -21,6 +21,11 @@ ModuleConfig::OPTION_TOKEN_REFRESH_TOKEN_TTL => 'P1M', ModuleConfig::OPTION_TOKEN_ACCESS_TOKEN_TTL => 'PT1H', + // Exercise the Defuse\Crypto\Key encryption key path during conformance + // testing. Generated with `vendor/bin/generate-defuse-key`. + ModuleConfig::OPTION_ENCRYPTION_KEY => + 'def000009b5323e0e424b49f9d84ac93ce9cf22228bba26cc6c7bd3d12663df9b12fc69f7041b44f77ec7e14a98c8832cd7e2a6c72923a8babfde0df554e644afff6b94b', + ModuleConfig::OPTION_PROTOCOL_SIGNATURE_KEY_PAIRS => [ [ ModuleConfig::KEY_ALGORITHM => \SimpleSAML\OpenID\Algorithms\SignatureAlgorithmEnum::RS256, diff --git a/docs/3-oidc-configuration.md b/docs/3-oidc-configuration.md index f53daeb0..6142ea12 100644 --- a/docs/3-oidc-configuration.md +++ b/docs/3-oidc-configuration.md @@ -201,7 +201,7 @@ $config = [ 'description' => 'private scope', 'claim_name_prefix' => '', 'are_multiple_claim_values_allowed' => false, - 'attributes' => ['national_document_id'], + 'claims' => ['national_document_id'], ], ], ]; diff --git a/docs/6-oidc-upgrade.md b/docs/6-oidc-upgrade.md index 1f6821f8..a08f4329 100644 --- a/docs/6-oidc-upgrade.md +++ b/docs/6-oidc-upgrade.md @@ -61,6 +61,19 @@ in the OP discovery metadata via the `response_modes_supported` claim. (`query`, `fragment`, `form_post`) are allowed, so existing clients are unaffected. It can be narrowed, for example to `form_post` only, to protect against browser-swapping attacks (if supported by the client). +- The encryption key (used to encrypt / decrypt artifacts like authorization +codes and refresh tokens) can now optionally be set to a strong, pre-generated +`\Defuse\Crypto\Key`, instead of always deriving it from the SimpleSAMLphp +secret salt. By default, the behavior is unchanged: the secret +salt is used as a string password, from which the underlying League OAuth2 +library derives the key using a slow key-stretching function (key derivation) on +every operation. If you instead provide a `\Defuse\Crypto\Key` (serialized to its +ASCII-safe string form) via the new option below, that strong key is used +directly, skipping the slow derivation and improving encryption / decryption +performance. Note that changing the encryption key (including switching from the +secret salt to a Key, or rotating a Key) invalidates all outstanding encrypted +artifacts (existing authorization codes, refresh tokens, and PAR request URIs +will be rejected), so only set or change it during a planned maintenance window. See the [configuration guide](3-oidc-configuration.md#pushed-authorization-requests-par-and-request-objects) for details. @@ -99,6 +112,11 @@ response size in bytes when fetching a remote `request_uri` (default `102400`). SSRF/DoS allowlist of `request_uri` prefixes for OpenID Federation candidates; `[]` (default) denies all such fetches, a non-empty array allows matching prefixes, and `null` allows any (not recommended). +- `ModuleConfig::OPTION_ENCRYPTION_KEY` - optional, ASCII-safe string +representation of a `\Defuse\Crypto\Key` to use as the encryption key. If not +set (default), the SimpleSAMLphp secret salt is used as before. See the config +template for how to generate the key and for the important caveat about +invalidating already-issued artifacts when the key changes. - Several new options regarding experimental support for OpenID4VCI. Major impact changes: diff --git a/src/ModuleConfig.php b/src/ModuleConfig.php index 3409f144..c4362192 100644 --- a/src/ModuleConfig.php +++ b/src/ModuleConfig.php @@ -17,6 +17,8 @@ namespace SimpleSAML\Module\oidc; use DateInterval; +use Defuse\Crypto\Exception\CryptoException; +use Defuse\Crypto\Key; use SimpleSAML\Configuration; use SimpleSAML\Error\ConfigurationError; use SimpleSAML\Module\oidc\Bridges\SspBridge; @@ -53,6 +55,7 @@ class ModuleConfig final public const string OPTION_TOKEN_AUTHORIZATION_CODE_TTL = 'authCodeDuration'; final public const string OPTION_TOKEN_REFRESH_TOKEN_TTL = 'refreshTokenDuration'; final public const string OPTION_TOKEN_ACCESS_TOKEN_TTL = 'accessTokenDuration'; + final public const string OPTION_ENCRYPTION_KEY = 'encryption_key'; final public const string OPTION_AUTH_SOURCE = 'auth'; final public const string OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE = 'useridattr'; final public const string OPTION_AUTH_SAML_TO_OIDC_TRANSLATE_TABLE = 'translate'; @@ -558,11 +561,42 @@ public function getPrivateScopes(): array } /** - * @return string + * Get the encryption key used to encrypt / decrypt artifacts like + * authorization codes and refresh tokens. + * + * By default (option not set), this returns the SimpleSAMLphp secret salt + * as a string. The underlying League OAuth2 library then derives an + * encryption key from it using a slow, CPU-intensive key derivation + * function (key stretching) on every encrypt / decrypt operation. + * + * If the OPTION_ENCRYPTION_KEY option is set to an ASCII-safe string + * representation of a \Defuse\Crypto\Key, that strong key is used directly, + * which avoids the slow key derivation and is therefore faster. See the + * config template for details on how to generate such a key. + * + * @return \Defuse\Crypto\Key|string + * @throws \SimpleSAML\Error\ConfigurationError */ - public function getEncryptionKey(): string + public function getEncryptionKey(): Key|string { - return $this->sspBridge->utils()->config()->getSecretSalt(); + $encryptionKey = $this->config()->getOptionalString(self::OPTION_ENCRYPTION_KEY, null); + + if ($encryptionKey === null || $encryptionKey === '') { + return $this->sspBridge->utils()->config()->getSecretSalt(); + } + + try { + return Key::loadFromAsciiSafeString($encryptionKey); + } catch (CryptoException $exception) { + throw new ConfigurationError( + sprintf( + 'Invalid value for %s. Expected an ASCII-safe string representation of a ' . + '\Defuse\Crypto\Key. Error was: %s', + self::OPTION_ENCRYPTION_KEY, + $exception->getMessage(), + ), + ); + } } /** diff --git a/templates/config/protocol.twig b/templates/config/protocol.twig index 081f755b..79db95df 100644 --- a/templates/config/protocol.twig +++ b/templates/config/protocol.twig @@ -92,12 +92,21 @@

{{ 'Scopes'|trans }}

-

- {% for scope, claims in moduleConfig.getScopes %} - {{ scope }}{{ loop.last ? '' : ', ' }} - {# TODO v7 mivanci Add claims or extract scopes to sepparate page. #} - {% endfor %} -

+ {% for scope, scopeConfig in moduleConfig.getScopes %} +

+ {{ scope }} + {% if scopeConfig.description is defined %} + — {{ scopeConfig.description }} + {% endif %} + {% if scopeConfig.claims is defined and scopeConfig.claims is not empty %} +
+ {{ 'Claims'|trans }}: + {% for claim in scopeConfig.claims %} + {{ claim }}{{ loop.last ? '' : ', ' }} + {% endfor %} + {% endif %} +

+ {% endfor %}

{{ 'Cache'|trans }}

diff --git a/tests/unit/src/Controllers/AuthorizationControllerTest.php b/tests/unit/src/Controllers/AuthorizationControllerTest.php index ec0636f5..bf52819e 100644 --- a/tests/unit/src/Controllers/AuthorizationControllerTest.php +++ b/tests/unit/src/Controllers/AuthorizationControllerTest.php @@ -170,7 +170,6 @@ public function testReturnsResponseWhenInvoked(array $queryParameters): void ->method('getAuthorizationRequestFromState') ->willReturn($this->authorizationRequestMock); - // TODO mivanci Move to mock() method. $controller = new AuthorizationController( $this->authenticationServiceStub, $this->authorizationServerStub, diff --git a/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php b/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php index eeda4fe0..aaba8f9a 100644 --- a/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php +++ b/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php @@ -94,7 +94,6 @@ public function testCanGetConfigurationStatement(): void $this->moduleConfigMock->expects($this->once())->method('getFederationEnabled')->willReturn(true); $this->federationCacheMock->expects($this->once())->method('get')->willReturn(null); - // TODO v7 mivanci $this->markTestIncomplete('Move to simplesamlphp/openid library for building entity statements.'); } } diff --git a/tests/unit/src/ModuleConfigTest.php b/tests/unit/src/ModuleConfigTest.php index 163b9b78..0c396925 100644 --- a/tests/unit/src/ModuleConfigTest.php +++ b/tests/unit/src/ModuleConfigTest.php @@ -5,6 +5,7 @@ namespace SimpleSAML\Test\Module\oidc\unit; use DateInterval; +use Defuse\Crypto\Key; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -395,6 +396,28 @@ public function testCanGetEncryptionKey(): void $this->assertSame('secretSalt', $this->sut()->getEncryptionKey()); } + public function testCanGetEncryptionKeyAsDefuseKey(): void + { + $this->sspBridgeUtilsConfigMock->expects($this->never())->method('getSecretSalt'); + + $key = Key::createNewRandomKey(); + $this->overrides[ModuleConfig::OPTION_ENCRYPTION_KEY] = $key->saveToAsciiSafeString(); + + $encryptionKey = $this->sut()->getEncryptionKey(); + + $this->assertInstanceOf(Key::class, $encryptionKey); + $this->assertSame($key->saveToAsciiSafeString(), $encryptionKey->saveToAsciiSafeString()); + } + + public function testGetEncryptionKeyThrowsForInvalidDefuseKey(): void + { + $this->overrides[ModuleConfig::OPTION_ENCRYPTION_KEY] = 'not-a-valid-ascii-safe-key'; + + $this->expectException(ConfigurationError::class); + + $this->sut()->getEncryptionKey(); + } + public function testCanGetProtocolCacheConfiguration(): void { $this->assertNotEmpty($this->sut()->getProtocolCacheAdapterClass());