stevenmaguire / oauth2-keycloak

Keycloak Provider for OAuth 2.0 Client
MIT License
204 stars 151 forks source link

CVE in Firebase PHP-JWT before 6.0.0 #50

Closed jimmym715 closed 1 year ago

jimmym715 commented 2 years ago

CVE-2021-46743 identifies the following issue:

"In Firebase PHP-JWT before 6.0.0, an algorithm-confusion issue (e.g., RS256 / HS256) exists via the kid (aka Key ID) header, when multiple types of keys are loaded in a key ring. This allows an attacker to forge tokens that validate under the incorrect key. NOTE: this provides a straightforward way to use the PHP-JWT library unsafely, but might not be considered a vulnerability in the library itself."

Breaking changes in v6.0.0 of firebase/php-jwt appear to be relevant to code in /src/Provider/Keycloak.php

JustinBack commented 1 year ago

Second this, getting alerted all the time.

Is there any workaround right now or a fork available?

senyahnoj commented 1 year ago

We aren't using the optional encryption parts of the provider so we forked and simply removed the bits which used firebase/php-jwt and the library itself.

'encryptionAlgorithm' => 'RS256', // optional 'encryptionKeyPath' => '../key.pem' // optional 'encryptionKey' => 'contents_of_key_or_certificate' // optional

https://github.com/senyahnoj/oauth2-keycloak

JustinBack commented 1 year ago

Cheers 🎉! ended up making a fork to make it compatible with KC20 as well!

senyahnoj commented 1 year ago

Would you mind sharing a link to your fork?

Devise03 commented 1 year ago

Hi, SecurityAdvisories has been posted an update related to this issue a few days ago. It is not possible to install oauth2-keycloak because of firebase/jwt < 6 conflict. Could you please update it? Here's a link to the issue: https://github.com/Roave/SecurityAdvisories/issues/106

leonboot commented 1 year ago

It seems fixing this is not just a matter of adding ^6.0 of firebase/php-jwt to the requirements in composer.json. Version 6.0.0 introduces a breaking change to mitigate the CVE by not allowing the second parameter to be a string; it needs to be a Key object or an array of Keys or strings, the latter also requiring a $kid to be provided.

The Key object seems to have been introduced in version 5.5.0, so changing the call signature in https://github.com/stevenmaguire/oauth2-keycloak/blob/master/src/Provider/Keycloak.php#L101 will drop support for all firebase/php-jwt versions before 5.5.0

fverry commented 1 year ago

The following pull request https://github.com/stevenmaguire/oauth2-keycloak/pull/64 provides the expected upgrade. JWT requirement changes from ~4.0|~5.0 to ~6.0.

fverry commented 1 year ago

Edit: last version here https://github.com/stevenmaguire/oauth2-keycloak/issues/50#issuecomment-1463908564

If you need to use the https://github.com/stevenmaguire/oauth2-keycloak/pull/64 in composer without waiting for the pull request to be merged, you need to edit your composer.json to reflect the following changes.

{
    "require": {
        "stevenmaguire/oauth2-keycloak": "dev-master"
    },
    "repositories": {
        "oauth2-keycloak": {
            "type": "vcs",
            "url": "https://github.com/muffe/oauth2-keycloak"
        }
    }
}

If you don't want to edit manually your composer.json file, you can run the following commands in your shell :

composer config repositories.oauth2-keycloak vcs https://github.com/muffe/oauth2-keycloak
composer require stevenmaguire/oauth2-keycloak:dev-master

:warning: This is not a recommanded way of doing things.

:dev-master means that composer will look at the master branch of the muffe/oauth2-keycloak package.

It is recommanded by composer documentation that the hotfix is hosted on its own branch, eg. hotfix-for-jwt-6, and thus called with composer require stevenmaguire/oauth2-keycloak:dev-hotfix-for-jwt-6.

This requires @muffe to create a hotfix-for-jwt-6 branch to host its hotfix.

muffe commented 1 year ago

Just created hotfix-for-jwt-6 from the current master.

fverry commented 1 year ago

Thank you very much.

You can now use in your terminal the following commands :

composer config repositories.oauth2-keycloak vcs https://github.com/muffe/oauth2-keycloak
composer require stevenmaguire/oauth2-keycloak:dev-hotfix-for-jwt-6
Polidoor commented 1 year ago

Hello, I have an error on your version : Parse error: syntax error, unexpected ')' in .../vendor/stevenmaguire/oauth2-keycloak/src/Provider/Keycloak.php on line 103

fverry commented 1 year ago

The issue is that there is a trailing comma in the JWT::decode() call at https://github.com/muffe/oauth2-keycloak/blob/hotfix-for-jwt-6/src/Provider/Keycloak.php#L102

The trailing comma was implemented starting PHP 7.3 https://wiki.php.net/rfc/trailing-comma-function-calls#implementation

@muffe https://packagist.org/packages/league/oauth2-client#2.0.0 requirements allows PHP >=5.6 : either remove the comma or add a php requirement to your https://github.com/muffe/oauth2-keycloak/blob/hotfix-for-jwt-6/composer.json

muffe commented 1 year ago

Fixing that as we speak, pushing the changes to the hotfix branch and the master

mstefan21 commented 1 year ago

Released new version 4.0.0 with allow JWT version 6:*.