Library sends code_verifier=0 to token endpoint when session cookies are missing, breaking PKCE

[micolous]

OpenID-Connect-PHP depends on session cookies in order to get the code_verifier:

OpenID-Connect-PHP/src/OpenIDConnectClient.php

Lines 1858 to 1860 in f7c91b9

	protected function getCodeVerifier() {
	return $this->getSessionKey('openid_connect_code_verifier');
	}

OpenID-Connect-PHP/src/OpenIDConnectClient.php

Lines 1920 to 1927 in f7c91b9

	protected function getSessionKey(string $key) {
	$this->startSession();
	if (array_key_exists($key, $_SESSION)) {
	return $_SESSION[$key];
	}
	return false;
	}

However, if an application sets SameSite=Strict on session cookies (eg: ownCloud), and the IdP and application are on different domains (eg: when self-hosting your IdP on a separate top-level domain for security, or using a SaaS IdP like Microsoft Entra ID), then browsers won't send back any session cookies on the redirect back from the IdP.

If this session cookie is missing, getCodeVerifier() returns false. This is then turned into the string "0" by http_build_query() and passed to the IdP's token endpoint:

OpenID-Connect-PHP/src/OpenIDConnectClient.php

Lines 916 to 929 in f7c91b9

	$token_params = array_merge($token_params, [
	'client_id' => $this->clientID,
	'code_verifier' => $this->getCodeVerifier()
	]);
	}
	// Convert token params to string format
	$token_params = http_build_query($token_params, '', '&', $this->encType);
	if (null !== $authorizationHeader) {
	$headers[] = $authorizationHeader;
	}
	$this->tokenResponse = json_decode($this->fetchURL($token_endpoint, $token_params, $headers), false);

...which will then fail a PKCE check, and return an IdP-defined error, which may or may not be helpful.

This failure mode makes it look like an IdP error – but this library is sending an invalid request because it never handles the error case from its own functions.

The current work-around is to set SameSite=Lax or SameSite=None (as recommended by ownCloud: 1 2), but as this session cookie is shared with the rest of the application, this could have significant security impacts.

Better behaviour ideas

• getCodeVerifier() should throw an error if it fails to load the session, rather than returning false. Alternatively, requestTokens() could check the return value from getCodeVerifier() and return an error there.

  That way, it's a lot more obvious what's gone wrong, and the library won't send an invalid request to the IdP.

• The library creates its own separate session (and cookie), which is isolated from the rest of the app and uses SameSite=Lax (though this is difficult with PHP's built-in session manager, which is always global).

  This would allow the rest of an app to use SameSite=Strict, limit the scope of the Lax policy, and improve library usability.

• The library should not depend on session cookies to pass a code_verifier (previously flagged in Get error 503 Service unavailable when using OIDC owncloud/openidconnect#123 (comment)).

  For example, it could pass the code_verifier in an encrypted form (using a secret key known only to the application) in the callback URL. This blob should have appropriate mitigations against CSRF and ensure short-term validity.

  That way, an application can use SameSite=Strict with cross-domain auth.

Related issues

Previously noted in #400, and also in these ownCloud bugs:

• PKCE code verifier not specified owncloud/openidconnect#249
• PKCE error with cookie.samesite owncloud/openidconnect#219
• PKCE code error owncloud/openidconnect#218
• OIDC callback requests rely on SameSite: strict cookies which fails with cross domain IdP owncloud/openidconnect#134
• Login Succeeds, but Requesting Token causes "Error in OpenIdConnect:PKCE code verifier not specified" owncloud/openidconnect#284