make clear that nonce is always required for Hybrid flows
Assuming that that nonce
is always required for Hybrid flows no matter where the id_token
is returned from, also following:
https://github.com/rohe/oidctest/issues/111#issuecomment-426450954
In section 3.3.2.11. ID Token for the Core spec, https://openid.net/specs/openid-connect-core-1_0.html#HybridIDToken, it describes the ID token in the Hybrid flow, which says
When using the Hybrid Flow, these additional requirements for the following ID Token Claims apply to an ID Token returned from the Authorization Endpoint:
Since an ID Token may also be returned from the Token Endpoint, that sentence seems to be too restrictive and the last part "returned from the Authorization Endpoint" should be removed.
FWIW: this may be a left-over from
3.3.2.12. ID Token Validation
where ID token validation is discussed for an id_token
returned from the Authorization Endpoint, as opposed to:
3.3.3.7. ID Token Validation
where ID token validation is discussed for an id_token
returned from the Token Endpoint.
OTOH: if section 3.3.2.11. is only about ID tokens returned from the Authorization Endpoint and it is supposed to be the counterpart of
3.3.3.6. ID Token
where validation of the contents of an ID Token returned from the Token Endpoint is discussed, then the following should be added to the latter:
Use of the nonce Claim is REQUIRED for this flow.
otherwise it is not clear that nonce
is always required in Hybrid flows no matter where the ID token is returned from.
Comments (25)
-
-
Brian - it's not an assumption that nonce is required for all Hybrid response types. There's clear normative text requiring that.
Section 3.3.2.11. ID Token (in the Hybrid Flow section 3.3) at https://openid.net/specs/openid-connect-core-1_0.html#HybridIDToken says:
The contents of the ID Token are as described in Section 2. When using the Hybrid Flow, these additional requirements for the following ID Token Claims apply to an ID Token returned from the Authorization Endpoint:
nonce Use of the nonce Claim is REQUIRED for this flow.
This has been explicitly discussed by the working group in the past. 3.3.2.11 is clear that the request must contain a nonce and the nonce must be present in the issued ID Token for all hybrid flows. The fact that the ID Token is returned from the Token Endpoint doesn't invalidate this requirement.
In fact, this is parallel to the requirement in the code flow that if a nonce is present in the request, a nonce value MUST be present in the issued ID Token (which is also returned from the Token Endpoint). This is in 3.1.3.7, which says:
If a nonce value was sent in the Authentication Request, a nonce Claim MUST be present and its value checked to verify that it is the same value as the one that was sent in the Authentication Request. The Client SHOULD check the nonce value for replay attacks. The precise method for detecting replay attacks is Client specific.
Section 2 (ID Token) also makes it clear that a requested nonce must be acted upon:
nonce String value used to associate a Client session with an ID Token, and to mitigate replay attacks. The value is passed through unmodified from the Authentication Request to the ID Token. If present in the ID Token, Clients MUST verify that the nonce Claim Value is equal to the value of the nonce parameter sent in the Authentication Request. If present in the Authentication Request, Authorization Servers MUST include a nonce Claim in the ID Token with the Claim Value being the nonce value sent in the Authentication Request. Authorization Servers SHOULD perform no other processing on nonce values used. The nonce value is a case sensitive string.
With respect to breaking changes, the PingFederate Summer 2015 Release passed all these tests. Apparently a change somehow made it into more recent code that broke spec compliance in this area. Many product groups run regression tests against the conformance suite during development. If Ping had been doing that, this would have been caught at the time the breaking change was made to the code base that previously passed all the tests. I'm not telling you how to run your engineering processes, but many other development teams have found this to be useful.
-
reporter I've traced back the changes to the test suite here: https://github.com/rohe/oidctest/issues/111#issuecomment-426766270 which ultimately was done as the outcome of: https://bitbucket.org/openid/connect/issues/972/nonce-requirement-in-hybrid-auth-request
-
reporter I still think that the sentence "additional requirements for the following ID Token Claims apply to an ID Token returned from the Authorization Endpoint:" is very confusing and should get rid of the part that says "returned from the Authorization Endpoint:".
Just to clarify for quick readers: this discussion is not about what to do when a nonce is present, it is about when to reject when a nonce is not present.
-
I believe that this is a duplicate of https://bitbucket.org/openid/connect/issues/972/nonce-requirement-in-hybrid-auth-request - which was already discussed by the working group and resolved. I'll leave it open for now so it can be discussed at an upcoming call (or possible at the workshop later this month) but given this issue has already been discussed by the working group and a resolution agreed to, I believe that after discussion, the default action should be to close this issue as a duplicate.
-
reporter Once more, MHO it is not a duplicate since the text is (still) not clear: it should get rid of the "returned from the Authorization Endpoint:" part.
-
Providing more context, issue
#972was fixed on July 29, 2015 with the commit https://bitbucket.org/openid/connect/commits/0eed40d6c278. This applied the fix agreed to by the working group. You can also see that John Bradley wrote at the time that this was not a breaking change.You can see this fix in context in the latest Core draft at https://openid.net/specs/openid-connect-core-1_0-24.html#HybridAuthRequest (which is currently equivalent to the editor's draft at https://openid.bitbucket.io/connect/openid-connect-core-1_0.html#HybridAuthRequest).
Hans, Section 3.3 on Hybrid Authentication contains two ID Token subsections 3.3.2.11 and 3.3.3.6, respectively about ID Tokens returned from the Authentication Endpoint (if any) and ID Tokens returned from the Token Endpoint. So the right fix is not to delete the "returned from the Authorization Endpoint" clause in 3.3.2.11.
The fix I'd recommend is to add a note to 3.3.2.11 reminding readers that an ID Token is not returned from the Authentication Endpoint for the code+token and response_type and therefore that subsection (and 3.3.2.12 about ID Token Validation) only applies to the code+id_token and code+id_token+token response types for the Hybrid Flow. If we want to be truly pedantic, we can also add a note to 3.3.3.6 saying that the ID Token rules there and in 3.3.3.7 apply to all Hybrid response types.
Those reminders should eliminate any remaining possibility of confusion. I'll assign this bug to myself and add it to the errata milestone. Reviews of the proposed fix are welcomed.
In retrospect, while we were trying to be concise when writing the Authentication sections, and therefore a lot of subsections referenced others and said "do what it says there", it seems that being a bit more verbose in these cases could have eliminated potential confusion. We should consider that lesson when reviewing normative text in the future.
-
-
assigned issue to
-
assigned issue to
-
reporter By intent it was a breaking change, by syntax it still did not have the desired effect because of the lingering "returned from the Authorization Endpoint:"
The former is enough to sharpen the spec, but moreover: if one needs 5 paragraphs to explain that "always add a nonce in Hybrid flows" is obvious from the spec it probably makes sense to add something like "always add a nonce in Hybrid flows" to that very spec.
-
reporter And to your point specifically, John tried to make it more clear that
nonce
is required in Hybrid flows that return anid_token
from the Authorization Endpoint, as can be seen in: https://bitbucket.org/openid/connect/issues/972/nonce-requirement-in-hybrid-auth-request#comment-20507768The intent was for it to be required if the id_token is returned from the Authorization endpoint.
which is what the change actually effectuated, nothing more.
-
We don't need 5 paragraphs. The existing fix that's been in place since July 2015 where the nonce requirement is made explicit in https://openid.bitbucket.io/connect/openid-connect-core-1_0.html#HybridAuthRequest does the trick without any additional text being added. We can add more if people still think confusion is possible.
I think there's enough record in the tracker now to facilitate working group discussion. Therefore, I'm going to stop replying to these comments in real time because there's other work I also have to get done today - some of it for OpenID. ;-)
-
Brian - it's not an assumption that nonce is required for all Hybrid response types. There's clear normative text requiring that.
Section 3.3.2.11. ID Token (in the Hybrid Flow section 3.3) at https://openid.net/specs/openid-connect-core-1_0.html#HybridIDToken says:
The contents of the ID Token are as described in Section 2. When using the Hybrid Flow, these additional requirements for the following ID Token Claims apply to an ID Token returned from the Authorization Endpoint:
nonce Use of the nonce Claim is REQUIRED for this flow.
In that very text you cite, the requirement for nonce is clearly qualified to apply to an ID Token returned from the Authorization Endpoint. That's the case both by the text right before the list item with nonce as well as the section in which it appears - 3.3.2. and subsections are about the Authorization Endpoint in the hybrid flow.
Honestly, when looking at this within the spec as whole, I don't see how you can reach the interpretation that you've landed on. But you have and I'll have to concede you that interpretation. And we disagree. That disagreement and the confusion and disagreement by others in this ticket and the several related items should more than demonstrate that there are two interpretations that can reasonably be reached from the spec as written. Changing the spec via errata to the more restrictive of the two interpretations is unquestionably a breaking change to implementations built against the more tolerant interpretation. While on the other hand, changing the spec via errata to the more tolerant of the two interpretations doesn't break anything. Furthermore, because nonce is intended to protect ID tokens returned in the front channel, there's no necessary reason to go with the restrictive interpretation.
With respect to breaking changes, the PingFederate Summer 2015 Release passed all these tests. Apparently a change somehow made it into more recent code that broke spec compliance in this area.
As Hans points out, the breaking change was the new test being introduced to the test suite in 2017. The PingFederate behavior with respect to this hasn't changed since I wrote it in 2013 - it has always allowed an authentication request with a
response_type
ofcode token
and nononce
. Which again underscores the fact that requiringnonce
forcode token
is indeed a breaking change. -
Brian, I hear you but I still believe the spec was and is clear. The definition of "nonce" in Section 2 says:
If present in the Authentication Request, Authorization Servers MUST include a nonce Claim in the ID Token with the Claim Value being the nonce value sent in the Authentication Request.
That applies to all response types.
Also, 3.3.3.6 https://openid.net/specs/openid-connect-core-1_0.html#HybridIDToken2 - which is about ID Tokens returned from the Token Endpoint says that its validation steps are the same as those in 3.3.2.11 - where nonce processing is explicitly called out.
If it were only the text in subsections of 3.3 that specify the nonce requirement, I'd agree that that different people could reasonably interpret it differently, but the text in Section 2 is completely unambiguous.
Finally, I'm surprised that you're surprised that nonce processing is required for ID Tokens returned from the Token Endpoint. Your code supports that for response_type=code, as evidenced by passing the test OP-nonce-code (ID Token has nonce when requested for code flow). I'm surprised that you special-cased code+token to not do the same for the ID Token returned from the Token Endpoint by that flow, when your code works for the same situation with response_type=code.
Anyway, I'm sure this is now a topic for a future call. Again, there was a working group decision on this very topic in 2015 when it was raised in issue
#972. I still believe that the 2015 decision was the correct decision and that this issue is a duplicate (albeit, pointing out that the text should probably be further clarified to prevent future confusions). -
Indeed a topic for a future call. My availability for the next call(s) is a bit uncertain however due to family obligations. So that future may have to be a bit out in the future.
One point of clarification, however, as your last message suggests that we might have some miscommunication here (impossible I know!). Our code correctly handles the
nonce
when it's present in the request - the value is passed through unmodified from the Authentication Request to the ID Token. That's true for all requests including those withresponse_type=code+token
. That's not the issue. The issue at hand is that we don't reject aresponse_type=code+token
request that doesn't have anonce
parameter. -
reporter Agree with Brian, there's no discussion on what to do when a nonce is present., which is what the text @mbj refers to is about. Repeating myself:
this discussion is not about what to do when a nonce is present, it is about when to reject when a nonce is not present.
-
Repeating Hans:
"this discussion is not about what to do when a nonce is present, it is about when to reject when a nonce is not present."
In the Core section 3.3.2.1 defines the Authentication Request for the hybrid flow. That section defers to Authentication Requests as defined in Section 3.1.2.1 (excepting parameters defined right in that section) where
nonce
is OPTIONAL. The only requirement fornonce
in the request has to be inferred back from the claim being required in the resulting ID token. And, as previously stated, that requirement is qualified as only applying to ID tokens returned from the Authorization Endpoint. The logical net of this is thatnonce
is required for requests with response types that containid_token
and optional otherwise.As such, I propose that the following text be added to section 3.3.2.1 as a list item below response_type.
nonce
- REQUIRED if the
response_type
of the request iscode id_token
orcode id_token token
and OPTIONAL when theresponse_type
of the request iscode token
. It is a string value used to associate a Client session with an ID Token, and to mitigate replay attacks. The value is passed through unmodified from the Authentication Request to the ID Token. Sufficient entropy MUST be present in the nonce values used to prevent attackers from guessing values. For implementation notes, see Section 15.5.2.
- REQUIRED if the
-
If this is up for re-debate since the Errata set 2 isn't published yet, I agree with Brian,
nonce
presence in a hybrid authentication request shouldn't be REQUIRED forcode token
, onlycode id_token
andcode id_token token
-
- changed status to open
-
+1 for Brian's proposal
Here is my understanding of what the spec defines...
-
If the nonce is in the request, then it MUST be in the returned id_tokens (regardless of front-channel or /token endpoint).
-
Nonce MUST be specified for any response_type that returns an id_token in the front-channel ("code id_token", "code id_token token")
-
Nonce is NOT required in the request for response_type of "code"
The situation in question then, is response_type="code token". In this case an id_token is NOT returned via the front-channel and therefore I believe that nonce is NOT required for this response_type.
-
-
reporter @gffletch thanks for jumping in; however I would like to point out that both Brian's and your arguments should actually be applied against https://bitbucket.org/openid/connect/issues/972/nonce-requirement-in-hybrid-auth-request which should be re-opened IMHO.
The outcome of
#972was that nonce should be required for all hybrid flows, which apparently does not have consensus (include me in disagreeing...).This ticket is about the work that was done to reflect the outcome of
#972in the spec: it is (still) not clear that nonce is always required.However, if
#972is re-opened and gets a different outcome, this ticket may be discarded. -
That's a fair procedural point @zandbelt. This ticket just happens to be where the discussion is taking place because it's new and open and closely related while
#972seems to have slipped past unnoticed by a number of people.#972should be re-opened and then resolved (with the different and correct outcome) by applying the proposed text from my previous comment rather than the overarching and breaking change introduced by 58ba615e7f43. This ticket can then be marked as "invalid" and closed with no action on the spec documents. And following from that, https://github.com/rohe/oidctest/issues/111 should be re-opened and resolved (probably) with the change you suggest in the 5th comment https://github.com/rohe/oidctest/issues/111#issuecomment-423645478 -
Just noting that this was discussed on the 29-Oct-18 call with some consensus there that was generally leaning towards what I'd proposed in the previous comment, which was going to be written up for review before proceeding. From the call notes:
#1052 make clear that nonce is always required for Hybrid flows Mike and Brian spoke to this A nonce request parameter may not be strictly necessary for the code+token response type Both agreed that reasonable people could interpret this differently for code+token Brian made the case that we shouldn't introduce breaking changes via errata Mike pointed out that this change would probably require removing this particular test from the certification suite for code+token Mike will write up this possibility in the issue for review
-
Issue
#972has been reopened and pull request #4 submitted with proposed minor text updates to properly address the issue.This issue
#1052should be marked as "invalid" and/or "closed" with no action on the core document.@zandbelt and/or @Rohe and/or @ ? I'd request that https://github.com/rohe/oidctest/issues/111 (and maybe also https://github.com/openid-certification/oidctest/issues/14) be reopened and resolved to properly reflect the expected outcome here and in
#972. As @mbj said in the 29-Oct-18 call, the change would probably require removing this particular test from the certification suite for code+token. Also, however, so as to allow for historical implementations based on the prior erroneous conclusions, the should probably not be a certification test that expects specific behavior based on the presence or lack-thereof of nonce in code+token (other than checking that when nonce is provided in the authn request, it is included in the resulting ID token). -
reporter - changed status to resolved
As suggested in the Jan 7 call meeting notes: I agree that this ticket can be closed (technically not because it is a duplicate but because the text that this ticket is about will be obsoleted by re-addressing
#972) -
reporter - changed status to closed
- Log in to comment
I firmly believe that that assumption ("that that nonce is always required for Hybrid flows no matter where the id_token is returned from") is not correct.
Things would have been simpler, if Connect had just made
nonce
mandatory for all authentication requests. But that's not the case.The
nonce
parameter and claim are used to protect against replay of an ID token. Direct replay or injection of an ID token is only possible when returned from the Authorization Endpoint because that flows through the browser via some sort of redirection. When an ID token is returned from the Token Endpoint, it's over a direct HTTPS connection between client and server and there's no opportunity for replay or injection of an ID token. There is opportunity there for replay or injection of the authorization code but there are other protections for that including one time use of the code and redirect_uri checking on the access token request. That was the logic underlyingnonce
being required for ID tokens when returned from the Authorization Endpoint and optional for ID tokens when returned from the Token Endpoint (and in turn when it is required on the authentication request). That's why nonce is required for OIDC implicit flow (ID token returned from the Authorization Endpoint) and optional for OIDC code flow (ID token returned from the Token Endpoint). Consistent with that, for hybrid it depends on theresponse_type
withcode id_token
orcode id_token token
requiring nonce while it's optional forcode token
.Both Connect Core 3.3.2.11 and 3.3.2.12 are about the hybrid flow where ID tokens when returned from the Authorization Endpoint. Which is what the text "apply to an ID Token returned from the Authorization Endpoint" and "the contents of an ID Token returned from the Authorization Endpoint" says. Whereas 3.3.3.6 and 3.3.3.7 are about ID tokens when returned from the Token Endpoint.
Changing Core so that nonce is always required for Hybrid flows no matter where the id_token is returned from would be a breaking change, which really isn't okay for an errata.
Per spec, our AS/OP implementation only requires
nonce
on authentication requests that have aresponse_type
that containsid_token
and would result in an ID token returned from the Authorization Endpoint. It's worked that way, per spec, since 2013. And it was OpenID Certified as a Hybrid OP (and other profiles) in in 2015.https://github.com/rohe/oidctest/issues/111#issuecomment-426450954 is indicative of an issue with the test suite. Which, if not fixed, puts us in the very bad position of having to introduce a breaking change to product in-order to re-certify.