From 2cc2ea85f243a4feff8493d2734b9296f3a99a23 Mon Sep 17 00:00:00 2001 From: nfebe Date: Thu, 12 Mar 2026 15:31:18 +0100 Subject: [PATCH] fix: Validate JWT key length for HS256 compatibility The HS256 algorithm requires a minimum key of 32 bytes per RFC 7518. firebase/php-jwt v7 (used by user_oidc) enforces this, while v6 (used by GSS) does not, causing silent JWT decode failures during OIDC logout delegation from slave to master. Added key length validation with clear error logging in both Master and Slave, and updated README to document the requirement. Signed-off-by: nfebe --- README.md | 15 +++++++++------ lib/GlobalSiteSelector.php | 22 +++++++++++++--------- lib/Master.php | 10 ++++++++++ lib/Slave.php | 10 ++++++++++ tests/unit/lib/GlobalSiteSelectorTest.php | 21 +++++++++++++++++++++ 5 files changed, 63 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index b407b553..65febe73 100644 --- a/README.md +++ b/README.md @@ -21,9 +21,11 @@ To use the Global Site Connector you need to add some config parameters to the c Config.php parameters to operate the server in master mode: ```` -// can be chosen freely, you just have to make sure the master and -// all slaves have the same key. Also make sure to choose a strong shared secret. -'gss.jwt.key' => 'random-key', +// Shared secret used to sign JWT tokens between master and slave nodes. +// IMPORTANT: Must be at least 32 characters long (required by HS256 per RFC 7518). +// Must be identical on master and all slave nodes. +// Example: use `openssl rand -base64 32` to generate a strong key. +'gss.jwt.key' => 'random-key-at-least-32-characters', // operation mode 'gss.mode' => 'master', @@ -50,9 +52,10 @@ Config.php parameters to operate the server in master mode: Config parameters to operate the server in slave mode: ```` -// can be chosen freely, you just have to make sure the master and -// all slaves have the same key. Also make sure to choose a strong shared secret. -'gss.jwt.key' => 'random-key', +// Shared secret used to sign JWT tokens between master and slave nodes. +// IMPORTANT: Must be at least 32 characters long (required by HS256 per RFC 7518). +// Must be identical on master and all slave nodes. +'gss.jwt.key' => 'random-key-at-least-32-characters', // operation mode 'gss.mode' => 'slave', diff --git a/lib/GlobalSiteSelector.php b/lib/GlobalSiteSelector.php index d1af66f5..3fada414 100644 --- a/lib/GlobalSiteSelector.php +++ b/lib/GlobalSiteSelector.php @@ -23,15 +23,11 @@ class GlobalSiteSelector { public const MASTER = 'master'; public const SLAVE = 'slave'; - /** @var IConfig */ - private $config; + public const MIN_JWT_KEY_LENGTH = 32; - /** - * GlobalSiteSelector constructor. - * - * @param IConfig $config - */ - public function __construct(IConfig $config) { + public function __construct( + private IConfig $config, + ) { $this->config = $config; } @@ -64,10 +60,18 @@ public function isSlave(): bool { * @return string */ public function getJwtKey(): string { - // TODO: returns exception if non-existant return $this->config->getSystemValueString('gss.jwt.key', ''); } + /** + * Validate that the JWT key meets minimum length requirements. + * HS256 requires a key of at least 256 bits (32 bytes) per RFC 7518 ยง3.2. + */ + public function isJwtKeyValid(): bool { + $key = $this->getJwtKey(); + return $key !== '' && strlen($key) >= self::MIN_JWT_KEY_LENGTH; + } + /** * get the URL of the global site selector master * diff --git a/lib/Master.php b/lib/Master.php index 4d65e83f..f6f5c380 100644 --- a/lib/Master.php +++ b/lib/Master.php @@ -310,6 +310,16 @@ protected function redirectUser($uid, $password, $location, array $options = []) * @return string */ protected function createJwt($uid, $password, $options) { + if (!$this->gss->isJwtKeyValid()) { + $this->logger->error( + 'gss.jwt.key is too short: HS256 requires at least ' + . GlobalSiteSelector::MIN_JWT_KEY_LENGTH . ' characters (per RFC 7518). ' + . 'Current key length: ' . strlen($this->gss->getJwtKey()) . '. ' + . 'Please update gss.jwt.key in config.php on all nodes.', + ['app' => Application::APP_ID] + ); + } + $token = [ 'uid' => $uid, 'password' => $this->crypto->encrypt($password, $this->gss->getJwtKey()), diff --git a/lib/Slave.php b/lib/Slave.php index 2b068943..4a8aff67 100644 --- a/lib/Slave.php +++ b/lib/Slave.php @@ -257,6 +257,16 @@ protected function checkConfiguration(): bool { return false; } + if (!$this->gss->isJwtKeyValid()) { + $this->logger->error( + 'gss.jwt.key is too short: HS256 requires at least ' + . GlobalSiteSelector::MIN_JWT_KEY_LENGTH . ' characters (per RFC 7518). ' + . 'Current key length: ' . strlen($this->authKey) . '. ' + . 'Please update gss.jwt.key in config.php on all nodes.', + ['app' => Application::APP_ID] + ); + } + return true; } diff --git a/tests/unit/lib/GlobalSiteSelectorTest.php b/tests/unit/lib/GlobalSiteSelectorTest.php index ee75f4ac..1d44ccb4 100644 --- a/tests/unit/lib/GlobalSiteSelectorTest.php +++ b/tests/unit/lib/GlobalSiteSelectorTest.php @@ -60,4 +60,25 @@ public function testGetLookupServerUrl() { $this->assertSame('result', $result); } + + public function testIsJwtKeyValidWithShortKey() { + $this->config->method('getSystemValueString') + ->with('gss.jwt.key', '')->willReturn('short-key'); + + $this->assertFalse($this->gss->isJwtKeyValid()); + } + + public function testIsJwtKeyValidWithEmptyKey() { + $this->config->method('getSystemValueString') + ->with('gss.jwt.key', '')->willReturn(''); + + $this->assertFalse($this->gss->isJwtKeyValid()); + } + + public function testIsJwtKeyValidWithValidKey() { + $this->config->method('getSystemValueString') + ->with('gss.jwt.key', '')->willReturn('this-key-is-at-least-32-characters-long!'); + + $this->assertTrue($this->gss->isJwtKeyValid()); + } }