authorization code and refresh token must be holder of key bound
Currently FAPI RW has this clause:
shall only issue authorization code, access token, and refresh token that are holder of key bound;
This has caused some misunderstanding, e.g. https://github.com/ConsumerDataStandardsAustralia/infosec/issues/31
However there is an issue with the clause:
- both the code and the refresh token are used at the token endpoint
- we require the client to authenticate to that endpoint using either MTLS or private_key_jwt
- however we state that only OAUTB or MTLS must be used as holder of key mechanisms
- therefore strictly speaking we can only support private_key_jwt at the token endpoint if it is used with OAUTB or MTLS.
I don't think this makes sense and I suggest we remove the requirement that the refresh token and authorization code are holder of key bound.
The requirement that the client has to use asymmetric crypto to authenticate at the token endpoint has the same effect - and in practice is what I believe that people have interpreted the spec to mean.
Comments (24)
-
-
reporter There is also this clause in the security considerations:
8.3.2 Client credential and authorization code phishing at token endpoint
In this attack, the client developer is social engineered into believing that the token endpoint has changed to the URL that is controlled by the attacker. As the result, the client sends the code and the client secret to the attacker, which will be replayed subsequently.
When the FAPI client uses MTLS or OAUTB, the authorization code is bound to the TLS channel, any phished client credentials and authorization codes submitted to the token endpoint cannot be used since the authorization code is bound to a particular TLS channel.
We need to adjust this clause as we don't support client_secret in FAPI RW. However it does highlight an issue with
private_key_jwt
in that the requirement foraud
in OIDC for the private_key_jwt assertion is that:The Audience SHOULD be the URL of the Authorization Server's Token Endpoint.
If thisSHOULD
isn't implemented then private_key_jwt is open to the attack described. The access token issued will be holder of key bound, so at that point the attacker ends up with a valid token that they can't use, but this still isn't ideal.I suggest that we add the requirement to FAPI that the audience
SHALL
be the AS token endpoint. -
Our assumption is that HoK is token binding / TLS-type concern different from client auth.
Isn't
grant_type
code id_token token
also problematic for HoK? -
Isn't grant_type code id_token token also problematic for HoK?
It is, yes - the WG has agreed to remove code id_token token in the next version of FAPI; see https://bitbucket.org/openid/fapi/pull-requests/84/removed-response-type-code-token-id_token/diff
-
The Audience SHOULD be the URL of the Authorization Server's Token Endpoint. If this SHOULD isn't implemented then private_key_jwt is open to the attack described.
The preceding sentence in the spec is:
The Authorization Server MUST verify that it is an intended audience for the token
so I am struggling to imagine a case where the attack proposed is possible.
Nevertheless:
I suggest that we add the requirement to FAPI that the audience SHALL be the AS token endpoint.
That seems sensible to me, regardless of any other reason it is also helpful for interoperability.
-
- changed status to open
A couple of points.
- We should probably change all instance of "Holder of key" with "Sender Constrained"
- With this particular issue, 5.2.2-6 "shall support OAUTB or MTLS as a holder of key mechanism;" is not constraining the "holder of key" just to MTLS and OAUTB. The support needs to include them but other mechanisms can also be used including private_key_jwt. 5.2.2-6 is a statement that is expressing the MTI (mandatory to implement.) If it were a constraint, then I would not have put it that way. Rather, I would have defined the term "holder of key" with the defintion text "MTLS or OAUTB" and would not have written 5.2.2-6.
-
While this gets to the core of the problem with token (etc.) binding, it proposes the wrong solution. The problem is the bearer token, which will not go away until a different token type is chosen. Changing the language actually exposes the problem for all to see.
-
reporter Thanks @Nat that is a helpful explanation. I agree with changing from "Holder of Key" to "Sender Constrained". Perhaps we can discuss this on the next atlantic call.
-
While this is true, you must understand that the "sender" is NOT DEFINED. So what value is sender constrained? But at least now we are getting to the core problem. The account holder is not considered important in FAPI.
-
I suggest to distinguish sender constraining for access tokens, refresh tokens, and authorization codes. This document should refer to https://tools.ietf.org/html/draft-ietf-oauth-security-topics for the details.
-
reporter we discussed on the list and agreed as Torsten mentions above to separate out the clause to make it clearer for implementers.
-
If this has already been discussed, I'm sorry.
Well, FAPI Part 2, 8.3.2. (Implementer's Draft 2) says as follows:
When the FAPI client uses [MTLS] or [OAUTB], the authorization code is bound to the TLS channel, any phished client credentials and authorization codes submitted to the token endpoint cannot be used since the authorization code is bound to a particular TLS channel.
I have to interpret this in the same way as ajmcmiddlin described in https://github.com/ConsumerDataStandardsAustralia/infosec/issues/31 . That is, in order to bind an authorization code bound to the TLS channel by MTLS, a client certificate needs to be pre-installed into a user agent and a client application has to make the user agent use the client certificate when the user agent communicates with the authorization endpoint.
lukepopp said "unless client certificates are installed in each browser which is obviously not practical." in https://github.com/ConsumerDataStandardsAustralia/infosec/issues/31 . However, regardless of whether it is practical or not, what Part 2, 8.3.2. implies is that a client certificate needs to be installed if MTLS is chosen as HoK mechanism.
If my understanding is wrong, please correct me. Otherwise, if we agree that client certificates should be installed into user agents, it should be explicitly written in the spec. If not, (1) the description of Part2-8.3.2 should be modified and (2) "authorization code" should be removed from Part2-5.2.2-5.
-
The clause you referred to is not practical (and I assume has never been tested in real life). Authorization codes are bound to the respective client's id, which means the proof of legit ownership is provided by authenticating the client using its credential (whether this is a simple client secret or a private key of sort). Moreover, the Security BCP in the meantime requires the client to use PKCE in conjunction with codes, so the code is also bound to the respective PKCE challenge, i.e. the requester of the code exchange needs to include the respective PKCE in the request.
Conclusion: codes are sender constraint in several ways, but the there is no practical relevance of the requirement cited above. I suggest to replace this statement by something along the lines I just stated.
-
Torsten, thank you for your comment.
Because client authentication is not performed at the authorization endpoint regardless of whether the client type is confidential or public, client authentication is irrelevant to "HoK for authorization codes". On the other hand, regarding PKCE, you made me realize that technically PKCE can be regarded as HoK mechanism.
I now feel that the simplest way to solve confusion and inconsistency/contradiction would be to state in the spec like "PKCE shall be used as HoK mechanism for authorization codes while MTLS or OAUTB shall be used as HoK mechanism for access tokens and refresh tokens."
-
@tlodderstedt_yes_com, a pull request?
-
-
assigned issue to
-
assigned issue to
-
I believe we agreed to change
shall only issue authorization code, access token, and refresh token that are holder of key bound;
to
shall only issue access tokens that are holder of key bound;
There’s still an open discussion about whether ‘holder of key bound’ is the right phrase but that at least isn’t causing as much confusion.
-
reporter -
assigned issue to
I’ll prepare a PR for this one, and then let Torsten look at wording overall
-
assigned issue to
-
-
assigned issue to
Taking this one as I don’t believe Dave has had time yet.
-
assigned issue to
-
being address by Torsten in https://bitbucket.org/openid/fapi/pull-requests/178
-
reporter - changed status to resolved
PR merged
-
- changed component to Part 2: Advanced
-
- changed component to FAPI 1 – Part 2: Advanced
-
- changed component to FAPI 1: Advanced
- Log in to comment
Agree with removing the requirement that the refresh token and authorization code are holder of key bound. FWIW, I've always winced a bit at the phrasing that refresh tokens and authorization codes shall be holder of key bound but convinced myself that it's okay due to, as you mention, the client authentication at the token endpoint and the binding of the code & RT to the client. I hadn't followed the logic all the way to private_key_jwt being effectively forbidden but that does seem to be an unintended effect of the text.