Skip to content
Merged
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
34 changes: 34 additions & 0 deletions config/module_oidc.php.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions docker/ssp/module_oidc.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion docs/3-oidc-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
],
],
];
Expand Down
18 changes: 18 additions & 0 deletions docs/6-oidc-upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand Down
40 changes: 37 additions & 3 deletions src/ModuleConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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(),
),
);
}
}

/**
Expand Down
21 changes: 15 additions & 6 deletions templates/config/protocol.twig
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,21 @@


<h4>{{ 'Scopes'|trans }}</h4>
<p>
{% for scope, claims in moduleConfig.getScopes %}
{{ scope }}{{ loop.last ? '' : ', ' }}
{# TODO v7 mivanci Add claims or extract scopes to sepparate page. #}
{% endfor %}
</p>
{% for scope, scopeConfig in moduleConfig.getScopes %}
<p>
<strong>{{ scope }}</strong>
{% if scopeConfig.description is defined %}
&mdash; {{ scopeConfig.description }}
{% endif %}
{% if scopeConfig.claims is defined and scopeConfig.claims is not empty %}
<br>
{{ 'Claims'|trans }}:
{% for claim in scopeConfig.claims %}
{{ claim }}{{ loop.last ? '' : ', ' }}
{% endfor %}
{% endif %}
</p>
{% endfor %}

<h4>{{ 'Cache'|trans }}</h4>
<p>
Expand Down
1 change: 0 additions & 1 deletion tests/unit/src/Controllers/AuthorizationControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
}
23 changes: 23 additions & 0 deletions tests/unit/src/ModuleConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
Loading