-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove c_nonce from the token response #39
Comments
Imported from AB/Connect bitbucket - Original Commenter: josephheenanI was tempted to make a comment onto https://bitbucket.org/openid/connect/pull-requests/535/adding-security-considerations-on-the but this issue is probably a more appropriate place. It’s interesting to note that, if we made the assumption that dpop is in use, and that the (optional) dpop nonces are being used, then an optimal solution would be removing c_nonce from the token response and instead use the dpop nonce as the nonce for the VCI key proof. Doing that is probably also kind of a layering violation though. On the original point though, Torsten mentioned on today’s call that returning c_nonce from the token response is a nice optimisation that avoids a round trip at the resource server. I wanted to observe that when you’re using nonces with DPoP the first nonce for the resource server has to be obtained from the resource server, similarly causing an extra round trip, and I don’t remember anyone complaining much about the additional round trip for that. (I’m sure Brian will correct me if they have.) |
Imported from AB/Connect bitbucket - Original Commenter: authlete-takaThe current spec provides implementers with two choices.
Removing the option to return Even if we can agree not to issue |
Imported from AB/Connect bitbucket - Original Commenter: b_d_cThe wallet implementation needs to be able to handle an |
Imported from AB/Connect bitbucket - Original Commenter: pedro-felix
|
Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda
that’s editorial error. I will do a PR removing references to |
Imported from AB/Connect bitbucket - Original Commenter: KristinaYasudaHaving read the arguments, I am increasingly in favor of removing the |
Imported from AB/Connect bitbucket - Original Commenter: tlodderstedtI would like to hear more feedback from implementers (both wallets and issuers) before we decide. |
Imported from AB/Connect bitbucket - Original Commenter: authlete-takaI tried writing a virtual wallet-side code for “c_nonce” handling. (Its error handling is not perfect, though.) // Make a token request and receive a token response.
token_response = make_token_request();
// The access token issued from the token endpoint.
access_token = token_response.access_token;
// The token response MAY contain "c_nonce".
c_nonce = token_response.c_nonce;
// Repeat credential requests until a credential or a transaction ID is issued.
while (true)
{
// Make a credential request and receive a credential response.
// It is assumed that the "make_credential_request" method here
// internally generates a key proof with the given "c_nonce" and
// includes the key proof in the credential request.
credential_response = make_credential_request(access_token, c_nonce);
// If either a credential or a transaction ID has been issued.
if (credential_response.credential != null ||
credential_response.transaction_id != null)
{
// Stop repeating credential requests.
break;
}
// If the reason of the error is "invalid_proof" and the value
// of "c_nonce" in the credential response is different from
// the one used in the credential request, it is worth trying
// to make a credential request again with the new "c_nonce".
if (credential_response.error == invalid_proof &&
credential_response.c_nonce != c_nonce)
{
// Make a credential request again with the new "c_nonce".
c_nonce = credential_response.c_nonce;
continue;
}
// Give up.
break;
} The virtual code above indicates that whether the token response contains the My conclusion is that it is acceptable to allow authorization server implementations to issue A credential issuer validates an access token that has been issued by an authorization server. Therefore, there exists a relationship between a credential issuer and an authorization server to some degree. If such a relationship is assumed for acess tokens, it should not be a major concern to allow a similar relationship to be established between a credential issuer and an authorization server for |
Could c_nonce be returned by Token Endpoint as Access Token claim? So Credential Issuer can use/trust this initial nonce? |
so that Issuer can use c_nonce from the access token in the credential error response when telling the wallet which c_nonce to use? that is an implementation detail, i don't think anything in the spec prohibits it, but the spec will be silent about this. |
Token Endpoint returns Access Token that is signed by Authorization Server - so the claims can be trusted by Credential Issuer and therefore the c_nonce claim could be trusted and the extra roundtrip to ask the first nonce from Credential Issuer can be avoided. You are correct the the spec allows any extra claims needed to access the Protected Resource (Credential Endpoint). |
Access Token is opaque to the Wallet, it is consumed by the Issuer. the wallet cannot look into the AT and extract any claims. |
@Sakurann
Either way c_nonce could be used as AT claim? |
@peppelinux What kind of approach are you considering for c_nonce in Italy's implementation? I see you use opaque AT and therefore token introspection request. |
Ciao @aarmam token introspection is used in the SPID CIE OIDC Profile, while for the wallet solution we didn't have put this on the requirements of the credential issuer. In SPID CIE OIDC the access token is a signed JWT. For the wallet solution we don't have specified yet the format of the AT and I'll back to you soon with a PR here and a comment in this thread |
@aarmam my bad, it is already defined here the AT is a DPoP token, then a signed JWT, in the specs you find the claims within in |
This means it is a opaque AT and token introspection request must be used to get information about AT. What kind of approach are you considering for c_nonce? Right now I see it is returned by the token endpoint, that means the Authorization Server will request Credential Issuer nonce endpoint? Have you considered the solution I proposed to include c_nonce as AT claim instead, so that Credential Issuer can trust the Authorization Server generated c_nonce? Or you havent considered this as issue at all because Authorization Server and Credential Issuer are the same entities in Italy's implementation? |
AT is a signed jwt, then it is not opaque. For the wallet solution we don't use introspection endpoint Formally the credential endpoint is an RS, no matter if It is within the same node or not. We have a concrete proposal with the nonce endpoint as well, if confirmed we'll have It in a PR. The sole concern Is that we want to issue a single nonce and that's good for auth code flow but not for pre-authz code flow (we don't use, but It would be better using a solution usable even in other implementation profiles). Please reach us on GitHub Italia eudi-wallet-it-docs for any related to the italian implementation, and please insist if anything is missing and needs better answers. Thank you for the interest |
@aarmam I forgot to reply on your proposal! OAuth 2.0 doesn't require AT in JWT format, then the flow must be designed as the AT was opaque. Thus, the (un)security of the AT as a bearer token, having the c_nonce within It would reduce the security of the token endpoint, where the distinguished AT and c_nonce mades something you have (bearer) and something you know (c_nonce) |
does anyone remember why we chose c_nonce as opposed to putting access token hash into the proof? if access token is sender-constrained, wouldn't that be stronger than c_nonce, so maybe we can drop the c_nonce option altogether..? |
OAuth 2.0 doesn't mandate the access token be sender constrained. issuing different c_nonce in the credential response allows to reuse the same access token, with different c_nonces, to obtain more tha a single credential without using the batch endpoint |
Annoyingly those two alternatives have slightly different security properties, but it's rather hard to reason about because of all the optionality: If the access token was sender-constrained and the token and credential endpoint require DPoP nonces then it's definitely at least as strong as c_nonce (but actually probably stronger). If instead dpop nonces aren't in use at all and the access token was long lived, then that's probably weaker (in particular it's vulnerable to the pre-generation attacks that dpop nonces prevent). I was a bit ambivalent on whether we should remove c_nonce from the token endpoint or not, but after today's working group meeting I think I'm more in favour of removing c_nonce at the token endpoint from the spec. The only advantage of c_nonce from the token endpoint that, if supported by the AS, it avoids one http call, i.e. it's potentially a performance gain, albeit I believe probably a fairly minor one. In theory, the AS returning c_nonce at the token endpoint is completely optional and hence it doesn't impose a burden on authorization servers and they're free to implement or not implement at the token endpoint (as they can always return a c_nonce at the credential endpoint instead). And some authorization servers just won't implement it, because it introduces a coupling between the AS & the RS that's not always easy to do. In practice this does complicate the spec (which is complicated enough even without this part), and worse it was mentioned on today's working group call that some wallet implementors end up assuming that they will get a c_nonce from the token endpoint and hence it's causing interoperability issues with authorization servers that don't issue one. |
I tried to mention some of the potential mismatched expectations and issues that'll likely result in this #199 (comment) on the rejected PR to Remove c_nonce* from the token response but I didn't anticipate wallets being unable to interoperate due to not implementing mandatory nonce handling at the credential endpoint. Which is not a good thing. |
By doing interop with wallet providers I've noticed that there is a bit of confusion regarding the proof / no-proof flows and how the nonces are communicated to the wallet. We found that some wallets and issuers thought that the nonce should be by default returned from the token endpoint leading to some discussion whether the nonce in the error response is a valid way to start the flow or just a fallback crutch to enable ASs that can't serve the nonce through the token response. This reminded me of 'the early' days where some implementers wanted to save http calls and started moving I like optimizations and less round trips, but I would keep it simple for the beginning so we have one way of doing things that is stable and can evolve, and later we can optimize. I just give this as a feedback to highlight that we might need additional clarifications regarding this topic as it is causing some confusion on the implementer side leading to interoperability issues that stem from misunderstanding the spec text. That being said, I do find it a bit unnatural to force errors in order to get a nonce, but I prefer that compared to multiple optional paths (although my opinion regarding this is not strong). I think a piece of text that clearly states what is mandatory and what is an optimization, how the first credential request is sent, is it with a proof with null nonce, or without proof at all is required so we loose the either/or but make it into a must/if you really want to. I vaguely remember that a "nonce generation" endpoint was once discussed but don't know where that conversation ended up. Also, not a cryptographer here, but can we have a no-negotiation simple nonce value that is deterministic but resistant to fight replays and prevent same values like hash of the token value + a nonce that is a timestamp that is not in the past but not more than x seconds in the future or something similar? Non really proposing the last one, just curious, although I see that in this case the issuer would have to ensure that same nonce values are not used. |
👍 In case proofs are required, it is clear that the credential endpoint must provide For this reason, By keeping only the mandatory requirement ( Finally, perhaps credential endpoint should always return a |
My comments here #331 (comment) |
It has been a year, and I find myself reflecting more on the design than on the solution itself. To enhance clarity and maintain alignment with OAuth 2.0 principles, it's important to consider the specific roles and functionalities of different endpoints. The c_nonce parameter, designed specifically for the credential endpoint (RS) , should not be handled in the token endpoint response, even if optional, which serves a broader purpose in token issuance. The token endpoint only returns tokens and metadata about the tokens, not parameters optionally required for RS endpoints. OAuth 2.0 architecture is structured around the concept of specialized endpoints to ensure scalability and clear separation of features and scopes. Imposing properties on an endpoint to accept parameters intended for another can complicate the design and potentially conflict with the foundational specifications of OAuth 2.0, as well as creating difficulties in traditional OAuth 2.0 implementations. Masking a breaking change with optionality, and at the same time saying that this optionality is vital for the security of the solution, can consequently represent two things, one or the other or both at the same time:
therefore I support the removal of the c_nonce issuance in the token endpoint response. About the way we secure the RS we're working on several others issues and therefore I try to not blat too much this one. |
I support removing |
Well, if there is such strong resistance to including Who manages the correspondences between Even if it is difficult to implement including If the same logic for removing |
Dear @TakahikoKawasaki
Having to work in a use case with a legacy authorization server, I ended up exposing from the credential issuer a protected endpoint that would return to the wallet the I have the feeling that this is broader though. For every state-full credential offer (with
|
Imported from AB/Connect bitbucket: https://bitbucket.org/openid/connect/issues/1956
Original Reporter: b_d_c
The option to return c_nonce with token response should be dropped to simplify the spec and implementations. It’s a lot of complexity and optionality for the potential avoidance of just one HTTP request/response direct from wallet/client to credential issuer.
The text was updated successfully, but these errors were encountered: