[Security] Support RSA algorithm signature for OIDC tokens

[louismariegaborit]

Q	A
Branch?	7.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues
License	MIT

Add support for RSA signature algorithm for OidcTokenHandler.
Amazon Cognito uses RS256 algorithm for its tokens.

[Spomky]

Hi,

Is it possible to put this PR on hold?
The reason is that algorithms from the web-token/ suite will be grouped and, very soon, with web-token/jwt-library you will have all at once (including encryption to help #50441)

[Spomky]

Hi @louismariegaborit,

Can you test with "web-token/jwt-library": "3.3.0@dev" adn "ext-openssl": "*"?
Both web-token/jwt-signature-algorithm-ecdsa and web-token/jwt-signature-algorithm-rsa dependencies can be removed.

Also, because the new web-token/jwt-library contains more than ESxxx and RSxxx algorithms, I am wondering if this PR could be renamed to [Security] Support any signature algorithms for OIDC tokens and allow developers picking the algorthims they need.

Note: web-token/jwt-library will be tagged in a couple of days.

[louismariegaborit]

Hi @Spomky

This seems ok. I removed also web-token/jwt-checker.
For the ext-openssl, I added it in the require-dev but it's in suggest like in your repository that you want that be added ?

I will look so that developers picked the algorithms they need.

I will update the PR title when we validate the work.

[louismariegaborit]

@Spomky I did a try. WDYT ?
I don't know if I can delete old algorithm services without deprecations. And if I keep the services with deprecations, could you tell me how do it ?

[Spomky]

This seems ok. I removed also web-token/jwt-checker.
For the ext-openssl, I added it in the require-dev but it's in suggest like in your repository that you want that be added ?

Indeed, most of jwt-* packages are now under jwt-library.
OK for ext-openssl in the required-dev section. I'll make sure the algorithms complain if it's missing.

---

@vincentchalamon do you have any recommendation to have a better algorithm support architecture.
What could be interesting is to allow multiple algorithms

# config/packages/security.yaml
security:
    firewalls:
        main:
            access_token:
                token_handler:
                    oidc:
                        # Algorithms used to sign the JWS
                        algorithms:
                            - 'ES256'
                            - 'RS256'
                            - 'PS256'
                        # A JSON-encoded JWK
                        key: '{"kty":"...","k":"..."}'

From my understanding, it will require:

• If the configuraition key algorithm is present: to normaliwe the current configuration fro; ["algorithm" => $alg] to ["algorithms" => [$alg]]
• Change the OidcTokenHandler first argument to accept an algorithm manager object instead a single algorithm
• Use the algorithm manager factory to create the algorithm manager according to the configuration

Any ideas on this?

[louismariegaborit]

@vincentchalamon do you have any recommendation to have a better algorithm support architecture. What could be interesting is to allow multiple algorithms

# config/packages/security.yaml
security:
    firewalls:
        main:
            access_token:
                token_handler:
                    oidc:
                        # Algorithms used to sign the JWS
                        algorithms:
                            - 'ES256'
                            - 'RS256'
                            - 'PS256'
                        # A JSON-encoded JWK
                        key: '{"kty":"...","k":"..."}'

From my understanding, it will require:

• If the configuraition key algorithm is present: to normaliwe the current configuration fro; ["algorithm" => $alg] to ["algorithms" => [$alg]]
• Change the OidcTokenHandler first argument to accept an algorithm manager object instead a single algorithm
• Use the algorithm manager factory to create the algorithm manager according to the configuration

Any ideas on this?

@Spomky I started this work in another PR (#51665).
I still need to support an array for the algorithms.
I think, we must decide what is the responsability of each PR.

[vincentchalamon]

@Spomky on an OIDC server (e.g.: Keycloak), is it possible to allow multiple algorithms on a single realm?

If true, multiple algorithms configuration could be interesting, indeed. I'm just wondering if the key configuration (JWK) shouldn't be associated to the corresponding algorithm, as I think they're linked (cf. JWA).

If false, I don't think we should allow multiple algorithms configuration as multiple realms are not supported.

[Spomky]

The CI seems to be fixed. Errors are not related to the changes.
fabbot is asking me to apply an empty patch. Not sure what is causing this error.
Any advice for the next step?

[nicolas-grekas]

LGTM with minor CS fixes

Some tests are broken due to this change

[chalasr]

Can you look at failing tests?

[Spomky]

Hi @chalasr,

It seems to be fine now.
I corrected one of the tests without the @group legacy. Other errors are not related to this PR.

[chalasr]

Thank you @louismariegaborit and @Spomky.