openid-certification / oidctest

THE CERTIFICATION TEST SUITE HAS BEEN MIGRATED TO A NEW SERVICE https://www.certificatinon.openid.net
Other
49 stars 15 forks source link

rp-backchannel-rpinitlogout-lt-wrong-alg: spec interpretation #207

Open zandbelt opened 4 years ago

zandbelt commented 4 years ago

Presumably rp-backchannel-rpinitlogout-lt-wrong-alg refers to https://openid.net/specs/openid-connect-backchannel-1_0.html#Validation

The test suite seems to assume that the text about the signature means that the same algorithm that was used to sign the ID token must be used to sign the logout token. I don't read it that way, and even if it would say/mean that, I don't think it provides any benefit.

But one can interpret the text wrt. encryption and signing of ID tokens in various ways when different algorithms have been published by the OP.

I believe it would be fine to use any algorithm published by the OP, since the OP publishes it, I don't see a security issue with that. The only thing that would be important to me is that the logout token is encrypted if the ID token was encrypted.

@selfissued please clarify and advise.

panva commented 4 years ago

I agree with Roland here, the OP should use the same algorithms and producing rules to Logout Token as it does for ID Token.

This saves us a number of client metadata.

Relevant parts:

Validate the Logout Token signature in the same way that an ID Token signature is validated, with the following refinements.

"in the same way that an ID Token signature" part of ID Token validation is checking it is signed with the expected/registered algorithm.

A Logout Token MUST be signed and MAY also be encrypted. The same keys are used to sign and encrypt Logout Tokens as are used for ID Tokens.

The only weird quirk is the one condition under which an ID Token can be signed with none, in that case (code only OP) none would be used for an ID Token and an RP should not be using backchannel_logout_uri.

zandbelt commented 4 years ago

For me the "using the same algorithms and procedures" means that any algo/rule that the OP can use to produce an ID token, can also be used to produce a logout token. It does not mean that exactly the same algo/key has to be used for the logout token, any valid combination will do.

So: you are saying that if the OP publishes both RSA and EC key material, and the ID token was signed using RSA, the logout token cannot use the EC keys, even though they are valid keys for the OP. What security benefit does that bring?

I could even argue that if there's a very long session, the RSA algo may be deprecated during that time and a different algorithm would bring increased security for the logout token...

panva commented 4 years ago

What's the purpose of id_token_signed_response_alg which defaults to RS256 and is always set to an algorithm value then?

A client expects a single pre-agreed algorithm to be used and in many cases leaving the OP to decide what alg to sign with would lead to the client not being able to cope.

Not to mention that leaving any algorithm exposed via metadata to be used by the RP for validation is super dangerous when it comes to the possibly exposed "none" algorithm.

zandbelt commented 4 years ago

I agree that dealing with id_token_signed_response_alg is a requirement for clients that use dynamic client registration; but what about clients that use static registration?

panva commented 4 years ago

I understand that, and have seen software that does that. But at the same time, strictly reading the specification, there's no language in Core 1.0 saying the RP is free to validate any ID Token algorithm exposed by the OP.

The alg value SHOULD be the default of RS256 or the algorithm sent by the Client in the id_token_signed_response_alg parameter during Registration.

If registration is dynamic or static, there is a configured value on the OP somewhere that the RP should be aware of and check that value to be used as the alg.

panva commented 4 years ago

Don't get me wrong, i get the benefit of having less hassle when changing the OP side registered algorithm (not having to update the client deployment)... if an errata to 1.0 clarified this, i'd be okay with it - it would however be a bigger undertaking since it would have to deal with both static and dynamic registration.

zandbelt commented 4 years ago

I guess I would then indeed like more text about static registration following dynamic registration, or even more specific text that says the client must only accept one "known and shared" algorithm; I'm not sure how that relates to crypto agility though... (taking into very account long lived sessions)

selfissued commented 4 years ago

I agree that if an algorithm is valid for an ID Token, it should be valid for a Logout Token. If multiple algorithms are published in the metadata, it's fair game to use any of them, even if the ID Token and Logout Token algorithms don't match.

As a thought experiment, it's fine for one ID Token to use algorithm A and then for the next one to use algorithm B, provided both are in the metadata. That's no different than an ID Token using A and a Logout Token using B.

panva commented 4 years ago

As a thought experiment, it's fine for one ID Token to use algorithm A and then for the next one to use algorithm B, provided both are in the metadata.

I disagree that AS can choose a different alg than the one the client indicated it wants used or, if none was indicated use whatever used profile’s default.

There is client software out there that is e.g. only capable of accepting one algorithm. If the AS was able to change algs as it pleases, such software would not be usable.

selfissued commented 4 years ago

I had thought I remembered that id_token_signed_response_alg was a set, whereas looking it up, it's actually a single algorithm. So you're right that the OP doesn't have a way to switch signing algorithms. So then the Logout Token signing algorithm really does have to be the same as the ID Token signing algorithm.

zandbelt commented 4 years ago

which still leaves clients using static registration without that type of guidance

panva commented 4 years ago

Why would static be treated any differently?

rohe commented 4 years ago

:-) Did I open a can of worms or .. :-)

This is what my RP library does regarding id_token_signed_response_alg.

To begin with it compares the set of algs it has available (implementation support and configuration) with what the OP says it supports (assume dynamic discovery here). If what the OP supports is a subset of what the RP can handle the RP doesn’t set id_token_signed_response_alg (which basically means as Filip has pointed out that it choses the default=RS256) in the client registration request.

But, the RP also is not picky on which alg the OP choses to use (as long as it’s within the set the RP supports). It will happily verifies an ID Token signed using ES256 by the OP if the OP publishes an EC key that can be used for doing the verification.

Is that wrong ? I would say no.

So, what outcomes are ‘correct’ when running this specific test?

— Roland Can anything be sadder than work left unfinished? Yes, work never begun. -Christina Rossetti, poet (5 Dec 1830-1894)

zandbelt commented 4 years ago

IMHO we've basically had that discussion from the start and came to the conclusion that the existence of id_token_signed_response_alg in the registration metadata implies that one and only one type of signature can and must be used. I would like this to be more explicit so it also applies to scenario's where no client metadata is registered. As Filip suggests, static should not be treated different but there should be more explicit guidance around it since so we don't force developers to read the dynamic registration spec and to use induction to apply it to their static scenario's.

rohe commented 4 years ago

On 12 Mar 2020, at 09:37, Hans Zandbelt notifications@github.com wrote:

IMHO we've basically had that discussion from the start and came to the conclusion that the existence of id_token_signed_response_alg in the registration metadata implies that one and only one type of signature can and must be used.

I don’t know what’s most common. Is it to register an id_token_signed_response_alg or not. But as Filip pointed out not explicitly registering an id_token_signed_response_alg is the same as registering id_token_signed_response_alg=“RS256”.

Something some of us may think is unfortunate but that’s beside the point. It’s how it is.

I would like this to be more explicit so it also applies to scenario's where no client metadata is registered. As Filip suggests, static should not be treated different but there should be more explicit guidance around it since so we don't force developers to read the dynamic registration spec and to use induction to apply it to their static scenario’s.

Static registration with no id_token_signed_response_alg registered MUST be treated the same as dynamic client registration without registered id_token_signed_response_alg.

— Roland Can anything be sadder than work left unfinished? Yes, work never begun. -Christina Rossetti, poet (5 Dec 1830-1894)