[Openid-specs-ab] First Release Candidates for final OpenID Connect specifications

Torsten Lodderstedt torsten at lodderstedt.net
Wed Oct 23 19:17:40 UTC 2013


Hi all,

see replies inline.

regards,
Torsten.


Am 21.10.2013 18:23, schrieb George Fletcher:
> Comments in line...
>
> On 10/21/13 12:28 AM, Mike Jones wrote:
>>
>> Replies are inline.  Some of these are questions to the working 
>> group, so everyone, please read them.
>>
>> -- Mike
>>
>> *From:*Torsten Lodderstedt [mailto:torsten at lodderstedt.net]
>> *Sent:* Friday, October 18, 2013 1:16 PM
>> *To:* Mike Jones
>> *Cc:* openid-specs-ab at lists.openid.net; board at openid.net
>> *Subject:* Re: [Openid-specs-ab] First Release Candidates for final 
>> OpenID Connect specifications
>>
>> Hi all,
>>
>> @Mike: first of all let me thank you for taking the burden and rework 
>> the whole document structure. I think the new structure is a major 
>> leap forward for OpenID Connect.
>>
>> I focused my review on the new core specification. Please find my 
>> comments below.
>>
>> I won't attend IIW but will attend IETF-88. So, if needed, we can 
>> talk through my comments there.
>>
>> Best regards,
>> Torsten.
>>
>> 1.  Introduction
>>
>> I would suggest to a reference to RFC 6749 in the first sentence. It 
>> probably also makes sense to explicitly point out that the reader is 
>> expected to be familiar with RFC 6749 and RFC 6750 as well as other 
>> IETF I-Ds (notably JOSE,  JWT and JWT Assertion Profile).
>>
>> 1.3.  Overview
>> The flow description is a good starting point for readers. I would 
>> suggest to add the following information in this section:
>>
>> - OpenID Connect authentication is basically an extension to the 
>> standard OAuth authorization process. This extension is defined for 
>> most OAuth grant types.
>> - Clients wishing to acquire identity information indicate this by 
>> sending the scope value "openid" as part of the authorization request 
>> parameters. (There are much more parameters used to control the 
>> process but this is the "main switch".)
>> - Such a client is also called relying party (RP). An authorization 
>> server also supporting OpenID Connect is called OpenID Provider (OP).
>>
>> Adding this information will help the reader to understand the way 
>> connect utilizes/integrates into OAuth.
>>
>> I would also suggest to move the definition (syntax and contents) of 
>> the ID Token here and make it section 1.4 because this is THE core 
>> concepts used throughout the specification. It's introduction in 
>> section 2.1.3.6 is to late (in my opinion) because it is cited 
>> roughly 20 times in previous sections.
>>
>> What do people think of defining the ID Token earlier, and in a 
>> higher-level section?  I think Torsten is probably right about this.
>>
> I had similar thoughts (I'm only through section 2.3 so far) as 
> concepts like 'audience' are mentioned in text before the ID Token is 
> defined. This would be confusing for someone who isn't familiar with 
> the whole overall spec.
>>
>>
>> 2.  Authentication
>> "The Authorization Code Flow is suitable for Clients that can 
>> securely maintain a Client Secret between themselves and the 
>> Authorization Server ..." - this is confusing since public clients 
>> can use the code as well. The key benefits of this grant type in my 
>> opinion are:
>> - AS _can_ authenticate clients
>> - AS _can_ return refresh tokens
>> - simplest way for web application backends to acquire tokens
>> That's why is best suited for web applications and native apps.
>>
>> Proposal:
>> "The Authorization Code Flow is appropriate for web applications and 
>> native apps as it allows to authenticate clients and obtain refresh 
>> tokens whereas the implicit flow does not support these features."
>> Or just remove the assessment of OAuth grant types and leave it to 
>> the implenentors to carry out their assessment.
>>
>> Torsten, I'm curious what your objection to the statement "The 
>> Authorization Code Flow is suitable for Clients that can securely 
>> maintain a Client Secret between themselves and the Authorization 
>> Server ..." is.  I say that, because unless the client can keep the 
>> client_secret a secret, client impersonation is possible.
>>
> I know that we are using the code flow for native clients (which can't 
> really protect the secret) specifically because of the refresh_token 
> capability. In the mobile environment we don't want users to have to 
> continually authorize the app and I don't want to enable long lived 
> access_tokens. Is client impersonation possible in this case? 
> Absolutely, so we manage access to scopes accordingly. This native app 
> case is also one of the drivers of the 'client instance' registration 
> flow so that impersonation is on a per client basis and not a client 
> class.
>
> If we identify the client impersonation case in the security 
> considerations, then maybe we could just point to it here. I do like 
> the inclusion of native apps in the set of clients where the code flow 
> makes sense.

Do you see security considerations beyound RFC 6749 and RFC 6819? I 
would like to omit further duplication of existing text.

>>
>> 2.1.  Authentication using the Authorization Code Flow
>>
>> OLD: "This provides the benefit of not exposing the Access Token to 
>> the Resource Owner ..."
>>
>> The same indeed holds for the ID Token, which is more important from 
>> a security perspective.
>>
>> NEW: "This provides the benefit of not exposing the Access Token and 
>> the ID Token to the Resource Owner ..."
>> NEW (alternative): "This provides the benefit of not exposing any 
>> Token to the Resource Owner ..."
>>
>> 2.1.1.  Authorization Code Flow Steps
>>
>> OLD: "8. Client validates the tokens and retrieves the End-User's 
>> subject identifier."
>>
>> I assume the client is supposed to validate the ID token, only?
>>
>> NEW: "8. Client validates the ID token and retrieves the End-User's 
>> subject identifier."
>>
>> 2.1.2.1.  Authorization Request
>>
>> "When the Client wishes to access a Protected Resource and the 
>> End-User Authorization has not yet been obtained, the Client prepares 
>> an Authorization Request to the Authorization Endpoint" - Why is this 
>> relevent in this context? I suggest to remove this sentence.
>>
>> "An Authorization Request is a message sent from an RP to the OP's 
>> Authorization Endpoint. It is an extended OAuth 2.0 [RFC6749] 
>> Authorization Request. Section 4.1.1 and 4.2.1 of OAuth 2.0 [RFC6749] 
>> define the OAuth 2.0 Authorization Request parameters." - Why 
>> Authorization Request? Shouldn't this be called an "Authentication 
>> Request"?
>>
>> "Communication with the Authorization Endpoint MUST utilize TLS. See 
>> Section 15.17 for more information on using TLS.
>> Authorization Servers MUST support the use of the HTTP GET and POST 
>> methods defined in RFC 2616 [RFC2616] at the Authorization 
>> Endpoint.Clients MAY use the HTTP GET or POST methods to send the 
>> Authorization Request to the Authorization Server. If using the HTTP 
>> GET method, the request parameters are serialized using URI Query 
>> String Serialization, perSection 12.1. If using the HTTP POST method, 
>> the request parameters are serialized using Form Serialization, per 
>> Section 12.2."
>>
>> Seems to be standard OAuth stuff, I suggest to remove it.
>>
>> Yes, some of the spec repeats standard OAuth stuff.  Particularly for 
>> security related information, such as the requirement to use TLS, 
>> etc., it was thought to be better to repeat it. Repeating the HTTP 
>> POST stuff, etc. is just a convenience to the reader.  Obviously if 
>> we say something that conflicts with OAuth, that's a problem...
>>
> I'm ok with leaving the text in the spec. Having to constantly switch 
> specs to try and understand something is problematic for developers. 
> The normative text is still OAuth2, but having the text in the doc is 
> helpful (at least for me).

just a general statement: redundencies may serve the reader but may lead 
to inconsistencies.

>> - redirect_uri Parameter
>>
>> "This URI MUST exactly match one of the redirect_uris registered for 
>> the Client" - Why is the redirect_uri validation more restrictive 
>> than http://tools.ietf.org/html/rfc6749#section-3.1.2.2?
>>
>> The redirect_uri validation is more restrictive than 
>> http://tools.ietf.org/html/rfc6749#section-3.1.2.2 to make it simpler 
>> and thereby increase the chance of it being done correctly.  (Doing 
>> this wrong has been the biggest source of OAuth security problems 
>> that I'm aware of.)
>>
> I understand the security implications of not doing an exact match... 
> my only real concern here is that our implementation will not be 
> compliant because I don't know that we will stop allowing some form of 
> regex matches in order to ease the development environments. When 
> things get to production we tend to move to exact matches, but not always.

I share George's concerns. Our OpenID implementation IS A OAuth AS 
complying to the requirements given in RFC 6749. I see two ways to 
comply to the more restrictive Connect requirements:
- restrict the URL matching for all clients (OIDC and pure OAuth)
- implement different behavior depending on the actual request 
parameters (scope value "openid")

None of these options look appealing to me.

BTW: Exact matching restricts the aiblity of the client to manage its 
state. Generally, I think URL matching is a security measure primarily 
relevant for public clients. For confidential client, client 
impersonation is prevented by authentication.

>>
>> "When using this flow, the redirection URI MAY use the http scheme, 
>> provided that the Client Type is confidential, as defined in Section 
>> 2.1 of OAuth 2.0; otherwise, it MUST use the https scheme" - This, 
>> surprisingly enough, is relaxed in comparison to 
>> http://tools.ietf.org/html/rfc6749#section-10.5.
>>
>> RFC 6749 states: "Authorization codes operate as plaintext bearer 
>> credentials, used to verify that the resource owner who granted 
>> authorization at the authorization server is the same resource owner 
>> returning to the client to complete the process.  Therefore, if the 
>> client relies on the authorization code for its own resource owner 
>> authentication, the client redirection endpoint MUST require the use 
>> of TLS."
>>
>> Why is Connect, in this particular case, less restrictive than OAuth?
>>
>> John, can you speak to why we're allowing http redirect_uri values 
>> when apparently OAuth doesn't?
>>
> I had some questions on this point as well. I believe the thinking is 
> that since the client is protecting the secret and the code is sent to 
> the /token endpoint with client authentication, even if someone was 
> able to hijack the code value they couldn't exchange it for the access 
> (and possibly refresh) tokens. If we are trying to make things 
> simpler, then just moving all of it to TLS makes sense. To me, the 
> only exception is localhost.

The attacker could inject the authorization code into the same 
application as used by the victim in order to impersonate her/him.

>>
>> - nonce Parameter
>>
>> "One method to achieve this is to store a random value as a signed 
>> session cookie, and pass the value in the nonce parameter. In that 
>> case, the nonce in the returned ID Token can be compared to the 
>> signed session cookie to detect ID Token replay by third parties." - 
>> I would recommend to move this text into an "implementation note" section
>>
>> id_token_hint Parameter - "Previously issued ID Token passed to the 
>> Authorization Server .." issued by the AS being requested? or any AS? 
>> I assume by the same AS
>> NEW: "ID Token previously issued by this Authorization server to the 
>> client ..."
>>
>> "... it SHOULD return a login_required error." - Does this mean the 
>> OP shall try to authenticate the user account identified by the ID 
>> token and refuses authentication otherwise? This sounds more like a 
>> requirement than a hint.
>>
>> "When possible, an id_token_hint SHOULD be present when prompt=none 
>> is used and an invalid_request error MAY be returned if it is not; 
>> however, the server SHOULD respond successfully when possible, even 
>> if it is not present."  - Why is the login hint recommended for this 
>> prompt value? checkid_immediate in OpenID 2.0 worked very well w/o a 
>> hint?
>>
>> Can someone please answer this one?
>>
>>
>> 2.1.2.2.  Authorization Request Validation
>>
>> "3. If the sub (subject) Claim is requested with a specific value for 
>> the ID Token ...." The meaning of the text is unclear to me. How is a 
>> specific sub value requested? by the login_hint or the id_token_hint?
>>
>> "As specified in OAuth 2.0 [RFC6749], Authorization Servers SHOULD 
>> ignore unrecognized request parameters.
>>
>> If the Authorization Server encounters any error, it MUST return an 
>> error response."
>>
>> Standard OAuth stuff, I recommend to remove it.
>>
>> 2.1.2.4.  Authorization Server Obtains End-User Consent/Authorization
>>
>> "When permitted by the request parameters used, this MAY be done 
>> through an interactive dialogue with the End-User ..." - What if the 
>> parameters do not allow for an interactive dialogue, e.g. 
>> prompt==none? I assume an error response with return code 
>> consent_required or interaction_required is appropriate. I would 
>> prefer interaction_required because to RP does not need to know more.
>>
>> 2.1.2.5.  Authorization Successful Response
>>
>> This is a vanilla OAuth 2.0 response, right? I would suggest to just 
>> say so.
>>
>> BTW: This piece of text is not applicable to the code grant type: 
>> "This specification only describes OAuth 2.0 Bearer Token Usage 
>> [RFC6750]. The OAuth 2.0 response parameter token_type MUST be set to 
>> Bearer unless another Token Type has been negotiated with the Client."
>>
>> I don't understand this comment, since a token_type value is returned 
>> from the Token Endpoint.
>>

That's correct. But 2.1.2.5 talks about the response from the 
authorization endpoint. As the example in the same section illustrates, 
it just returns the code along with a state parameter.

>>
>> 2.1.3.  Tokens Endpoint
>>
>> "Clients MUST use the HTTP POST method to make requests to the Token 
>> Endpoint. Request parameters are added using Form Serialization, per 
>> Section 12.2. The Token Endpoint MUST support the use of the HTTP 
>> POST method defined in RFC 2616 [RFC2616] at the Token Endpoint.
>>
>> Communication with the Token Endpoint MUST utilize TLS. See Section 
>> 15.17 for more information on using TLS.
>>
>> All Token Endpoint responses that contain tokens, secrets, or other 
>> sensitive information MUST include the following HTTP response header 
>> fields and values: ..."
>>
>> This seems to be standard OAuth stuff. I recommend to remove it.
>>
>>
>> 2.1.3.1.  Token Request
>>
>> "To obtain an ID Token, Access Token, or Refresh Token, the Client 
>> MUST authenticate to the Token Endpoint using the authentication 
>> method registered for its client_id, as described in Section 8 ..." - 
>> At this point the reader is not familiar with the different 
>> authentication methods supported by an OpenID OP. I therefore suggest 
>> to move the client authentication section before the authentication 
>> section (e.g. make it a section 1.5).
>>
>> What do people think of this suggestion?  I understand that it makes 
>> logical sense, but if you read Section 8, it introduces a bunch of 
>> stuff that would interrupt the flow of just understanding how the 
>> Code Flow works.
>>
> I agree. I don't think we should put all the different client 
> authentication options early in the spec. It might make sense to 
> introduce the concept so that readers at least understand what 'client 
> authentication' means. This might work for the ID Token concept as well.
>>
>>
>> 2.1.3.2.  Token Request Validation
>>
>> The whole sections seems to re-phrase standard OAuth stuff. I 
>> recommend to remove it.
>>
>> 2.1.3.3.  Token Successful Response
>>
>> "Servers SHOULD support OAuth 2.0 Bearer Token Usage [RFC6750] for 
>> interoperability" - I think this topic is better covered in the MTI 
>> section.
>>
>> "the following parameters MUST be included in the response if the 
>> grant_type value is authorization_code and the Authorization Request 
>> scope parameter contains openid:" - Seems to be redundant since this 
>> whole section is about exactly this use case. I recommend to remove 
>> this text. Same holds true for the following text
>>
>> "An id_token MUST be returned when the grant_type value is 
>> authorization_code and MAY be returned when other grant types are used."
>>
>> The sentence above is definitely not "standard OAuth stuff", as it 
>> adds the requirement to return the ID Token.
>>
> I do think it is a little confusing regarding when id tokens are 
> returned and when not. For example, a response_type of 'code token' 
> will still return an id token, but only when the code is exchanged for 
> the requested tokens. This is because an OpenID Connect authentication 
> request MUST contain 'openid' is the list of scopes requested. If you 
> don't remember this it can get confusing.
>
> I do agree that the scope of 'openid' and returned 'id_token' are NOT 
> standard OAuth and need to be called out.
>>
>>
>> 2.1.3.6.  ID Token
>>
>> That's the key elements of OpenID Connect! As already stated, I 
>> recommend to move the (full) description of its content and syntax to 
>> section 1 (section 1.4). I think this will facilitate readability. 
>> The validation rules should stay in the sections of the respective 
>> grant types.
>>
>> I was thinking of making it 2.1.2 -- putting it before the 
>> Authorization Endpoint stuff.  What do others think?  It's not really 
>> introduction or overview stuff -- it's part of the meat of the 
>> specification.
>>
> From an overview perspective, I do think that having something in 
> section 1 makes sense. It's the key feature of OpenID Connect. I don't 
> think it needs to have all the syntax, but the core concepts would be 
> helpful. Something to give context to the reader as they read the rest 
> of the spec.

2.1. is good

>>
>> 2.1.3.7.  ID Token Validation
>>
>> "1. If the Client has provided an id_token_encrypted_response_alg 
>> parameter during Registration, decrypt the ID Token using the key 
>> pair specified during Registration." - text depends on dynamic 
>> registration and should therefore be generalized.
>> NEW: "1. If the ID Token is encrypted, the Client first decrypts it 
>> using the key agreed upon with the authorization server."
>>
>> "5. If the id_token is received via direct communication between the 
>> Client and the Token Endpoint, the TLS server validation MAY be used 
>> to validate the issuer in place of checking the token signature. The 
>> Client MUST validate the signature of all other ID Tokens according 
>> to JWS [JWS] using the algorithm specified in the alg parameter of 
>> the JWT header." - text seems rather generic. As this is about the 
>> code flow, the ID token is received via direct communication, so the 
>> text can be simplified.
>> NEW: "5. Since the ID token is received via direct communication, the 
>> TLS server validation MUST be used to validate the issuer in place of 
>> checking the token signature."
>>
>> Steps 6-8 can be removed.
>>
>> 2.1.3.8.  Access Token Validation
>>
>> I would recommend to add the text from section 2.2.2.9. because this 
>> is the first point where the concept is used.
>>
>> a_hash validation is mentioned. What's about c_hash validation?
>>
>> As I understand it, the c_hash is only needed for hybrid flows, in 
>> which the ID Token is returned in a fragment.  That's why it's 
>> introduced there.  Do others agree with this decision?
>>
>
> If we separate out the description and syntax of the id_token, then 
> all capabilities should be defined. Otherwise, I'm ok with introducing 
> the required elements when they are needed. Now if most AS are going 
> to include c_hash even when it's not required (as in the code flow) 
> then it would make sense to add it as OPTIONAL in the code flow 
> description.

But doesn't the same hold true for the at_hash. I would assume it is 
only used in the fragment-based flows (e.g. token id_token). If thats 
true and there is no need to check the AT at all, I would therefore 
suggest to remove 2.1.3.8..


>>
>> 2.2.  Authentication using the Implicit Flow
>>
>> "...which may expose them to the Resource Owner and other 
>> applications that have access to the Resource Owner's User-Agent." - 
>> I recommend to add: "In contrast to the authorization code flow, this 
>> requires additional security mechanisms (described below) to detect 
>> falsified ID tokens."
>>
>> 2.2.2.1.  Authorization Request
>> see comments regarding redirect_uri and nonce for section 2.1.2.1
>>
>> 2.2.2.2 - 2.2.2.4 - I recommend to state in section 2.2.2 that those 
>> steps are performed in the same manner as for the code flow and to 
>> remove these section?
>>
>> These are here to keep the structure of the sections parallel with 2.1.
>>
>>
>> 2.2.2.5.  Authorization Successful Response
>>
>> access_token Parameter - "Access Token for the UserInfo Endpoint." - 
>> In contrast to section 2.1.3.3, this section also described standard 
>> OAuth response parameters. I don't think this is needed. Moreover, 
>> the term "User Info" is used before it is really introduced. In my 
>> opinion, authentication should not talk about user info. The access 
>> token returned as part of the authentication result might suited for 
>> interactions with any protected resource, including user info.
>>
>> 2.2.2.7.  Redirect URI Fragment Handling
>>
>> This section needs a bit more of context and description. While the 
>> introduction of 2.2 states: "The Implicit Flow is mainly used by 
>> Clients implemented in a browser using a scripting language", this 
>> section suddenly _requires_ the client to send data to a web server 
>> ("The Client MUST provide ...").
>>
>> First of all, I don't understand the MUST.
>>
>> This could become "The Client needs to implement...".
>>
> For native clients, I'm not sure the client needs to send data to a 
> web server at all. The connection with a web back is only true for 
> clients implemented in a browser (correct)? So we can restrict the 
> 'implicit flow' to browser based clients that do not implement any 
> browser plugins, or we should relax the MUST as with a native or 
> desktop client, you can capture the redirect before it's loaded into 
> the webkit view and process it locally.
>>
>> Second, a description is needed of the different patterns, scripting 
>> clients vs. scripted frontends of server-based web-application (here 
>> the implicit grant offers better performance and scalability at the 
>> cost of a more complex client implementation. Therefore, 
>> implementation advices are given.
>>
>> I also think this section should be part of an "implementation note" 
>> section.
>>
>> 2.2.2.11.  ID Token Validation
>>
>> Text around signature validation should be moved from 2.1.3.7 to this 
>> section as it is required for the implicit grant (in contrast to code).
>>
>> 2.3.  Authentication using the Hybrid Flow
>>
>> 2.3.2.2.-2.3.2.4. can be removed
>>
>> 2.3.2.5.  Authorization Successful Response
>>
>> Again, I would recommend to focus on additional response parameters, 
>> as in section 2.1.3.3
>>
>> 2.3.2.7.  Redirect URI Fragment Handling
>>
>> see comment at section 2.2.2.7
>>
>> I think 2.3.3.1.-2.3.3.9 can be removed - Instead it should be stated 
>> that for the hybrid flow client and AS must conform to ALL 
>> requirements for code and implicit.
>>
>> These are here to keep the structure of the sections parallel with 2.1.
>>
>> 3.  Initiating Login from a Third Party
>>
>> I assume this is mainly intended to support OP initiated logins? I 
>> don't think it deserves a top-level section. I would recommend to 
>> make it part of the Authentication section.
>>
>> "The Client MAY optionally register [OpenID.Registration] an 
>> initiate_login_uri that can be used by the Authorization Server or 
>> another party to initiate a login for an End-User at the Client." I 
>> assume this feature shall also be supported by OPs w/o dynamic 
>> registration? I therefore suggest to move this text to the dynamic 
>> registration spec. Instead one could state: "The approach utilized by 
>> the 3rd party or the OP to determine the client's respective URL is 
>> out-of-scope of this document."
>>
>> Generally, I don't think any meta data element registered via dynamic 
>> registration or discovered via OpenID Discovery should be 
>> specified/mentioned in the core spec. I think the core function must 
>> work w/o both functions in place.
>>
>> I agree that it shouldn't be mentioned in a way that implies or 
>> states that Discovery or Registration are required.  That being said, 
>> we've tried to point people to the relevant Discovery & Registration 
>> parameters where they're relevant to help implementers more easily 
>> grasp the bigger picture.
>>
> I like the pointers to the other specs. I agree that we should make it 
> clear that these specs are NOT required for the core spec to work.

+1

>
> I'll have to stop here as I haven't finished my detailed reading of 
> the spec.
>
> Thanks,
> George
>>
>>
>> 4.  Claims
>>
>> "This section specifies how the Client can obtain Claims about the 
>> End-User ..." claims about the authentication process are supported 
>> as well.
>> NEW: "This section specifies how the Client can obtain Claims about 
>> the End-User and the authentication process ...
>>
>> 4.1.  Requesting Claims using Scope Values
>>
>> This is an extension to the authentication part, it should be 
>> specified that way. For example, there is no need to specify the use 
>> of the scope value "openid" again. IMHO it suffices to say that 
>> clients may request access to user data by adding more scope values 
>> in conjunction with "openid".
>>
>> 4.2.  Standard Claims
>>
>> I think this section should be the first section as it describes 
>> standard claims on a conceptual level and which ways exist for a 
>> client to retrieve them.
>>
>> This seems reasonable.  Do others agree with this reordering?
>>
>>
>> 4.3.  UserInfo Endpoint
>>
>> I think this section should be merged with Section 4.1 as the scope 
>> values defined there control access to this resource, only.
>>
>> Disagree, since 4.5 (Requesting Claims using the "claims" Request 
>> Parameter) also controls the access to this resource.
>>

That's certainly true :-) I neverless feel the current location "in 
between" is not optimal. Would it make sense to start this chapter with 
a 4.2, followd by a description of the user info endpoint and cover 
scopes and claims afterwards?

>>
>> 4.4.  Requesting Claims Locales with the "claims_locales" Request 
>> Parameter
>>
>> I would suggest to move this either before 4.1. or after 4.5. as it 
>> seems to be orthogonal to the functions described there.
>>
>> If we move the Standard Claims to 4.1, I agree that this could 
>> logically become 4.2
>>
>>
>> 4.6.  Claim Types
>>
>> This seems to be out of order because after a description of how a 
>> client may allocate claims to ID token and user info, this section 
>> again exclusively talks about UserInfo. Is it really the intention to 
>> support aggregated and distributed claims at the User Info endpoint, 
>> only? If so I recommend to move this section before 4.4. Otherwise, 
>> please state that such claims can be provided in the ID Token as well.
>>
>> Fair point. While I understand the general argument that this might 
>> apply to the ID Token as well, from a practical matter, they're only 
>> likely to ever be used with UserInfo claims.
>>

The spec should clearly spell that out.

>>
>> How is a aggregated or distributed claim requested by a client?
>>
>> It's up to the OP when and whether to provide them -- not up to the 
>> RP to ask.  I'll try to make that clearer.
>>
>>
>> 5.  Passing Request Parameters as JWTs
>>
>> I would suggest to move this topic into another document. The 
>> features described here allow implementors to achieve higher security 
>> levels and may reduce the request size but I consider them beyond the 
>> scope of a core document.
>>
>> We considered moving lots of things out to separate documents, but in 
>> the end, decided that things would be easier for developers to 
>> understand if we avoided a proliferation of documents.
>>

Sorry, I cannot accept this statement. This is the first version of a 
consolidated document, which pretends to be THE core document of OpenID 
Connect. Who decided based on what data/information/stats that 
everything needs to be in a single spec? And why weren't registration 
and discovery also included?

What are developers supposed to better understand that way? As an 
implementor I still have a hard time to understand what is really 
relevant for me in order to implement a compliant OpenID Connect OP. And 
I don't think signed request objects belong into this category.

>>
>> 6.  Self-Issued OpenID Provider
>>
>> How mature is the concept? How many implementations exist? Is this 
>> really part of a core specification or rather an extension for 
>> mobile/personal devices? I would opt for making this section a 
>> separate document.
>>
>> Same response as for 5.  And yes, there are implementations.
>>

Same response as for 5. Additionally: You didn't not answer my question 
regarding maturity of this module. Given there are a couple of notes 
starting "The OpenID Foundation plans ..." it appears to me there is 
some way to go until it will be mature. Who did actually implement this 
feature?

>>
>> 6.2.  Self-Issued OpenID Provider Registration
>>
>> "When using a Self-Issued OP, the Client is deemed to have registered 
>> with the OP and obtained following Client Registration Response." - 
>> Does this mean dynamic registration is required for self-issued ID 
>> providing?
>>
>> 7.  Subject Identifier Types
>>
>> This section is completely about discovery and registration - it 
>> should go there, consequently.
>>
>> No, it's about core functionality -- the meaning of the subject claim 
>> in the ID Token.
>>

The section starts as follows: "The OpenID Provider's Discovery document 
SHOULD list its supported identifier types in the 
subject_types_supported element ..." - This is not about discovery?

Further: According to 2.1.3.6, the meaning of the sub claim is "Subject 
identifier. A locally unique and never reassigned identifier within the 
Issuer for the End-User, which is intended to be consumed by the Client, 
e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT 
exceed 255 ASCII characters in length. The sub value is a case sensitive 
string."

Seems the client does not need to know about "pairwise identifiers". 
Furthermore, I didn't find any other place in this spec, which refers to 
identifier types (e.g. request parameters). So why do you think this is 
core functionality?

I think this is an aspect of the OP's implementation. It may be relevant 
to actually setup a client_id at an OP, but that's Dynamic Registration.


>>
>> 8.  Client Authentication
>>
>> This section provides fundamental information about the client 
>> authentication methods supported/introduced by OpenID Connect. I 
>> would suggest to move it into section 1 (e.g. section 1.5).
>>
>> Section 1 is for introductory material -- not specifying 
>> requirements. We could move it earlier, if you feel that it's 
>> warranted.  Where would like you like it to go, other than Section 1?
>>

Section 2, before the description of the authentication flows.

>>
>> "During Client Registration, the RP (Client) MAY register an 
>> authentication method. " I assume all client authentication methods 
>> shall work even if the OP does not support dynamic registration. 
>> Consequently, this text must be replaced by a general description of 
>> what methods are supported and how the OP is supposed to determine 
>> the right method for a particular client.
>>
>> OLD: "During Client Registration, the RP (Client) MAY register an 
>> authentication method. If no method is registered, the default method 
>> of client_secret_basic MUST be used."
>> NEW: "OpenID connect supports the authentication methods listed 
>> below. The authorization server determines the authentication method 
>> to be used in a particular authorization transaction based on the 
>> client_id. The way client and authorization server negotiate the 
>> authentication method is out of scope of this specification."
>>
>> Actually, the Authorization Server advertises what methods it 
>> supports and the Client chooses the method it uses from among that set.
>>

If discovery and registration are supported by that particular OP. Since 
this is not a MTI requirement, the core should avoid to create 
contradicting impressions and focus on documenting the relevant concepts 
and assumptions.

>>
>>  9.  Signatures and Encryption
>>
>> Most of this section talks about discovery and dynamic registration - 
>> it should go there, consequently.
>>
>> Things like key rollover are Core functionality.  The references to 
>> the discovery and registration parameters are there to aid developers 
>> in understanding the bigger pictures.  The use of them is optional.  
>> (If you believe that the text isn't clear on this, please propose 
>> specific language changes to clarify this.)
>>

I would suggest to add the following sentence to the introduction of 
section 9: "This specification assumes that the Client and OpenID 
Provider negotiate algorithms to sign and encrypt data before actually 
starting an authentication flow. Normally Discovery and Dynamic 
Registration are utilized for that purpose, other mechanisms MAY be used 
as well."

I repeat my suggestion the rest of the text, as all values are specified 
in Discovery and Dynamic Registration and only relevant for OPs 
supporting the respective features.

>>
>> I think the core spec needs MTI algorithms and clear definition when 
>> signatures are required. In my opinion, there are two use cases:
>> - Login via implicit grant
>> - azp - Login at another AS via ID token
>>
>> In both cases, Signing ID Tokens with RSA SHA-256 could be good 
>> baseline. This could be documented in the ID Tokens section or the 
>> MTI section.
>>
>> 14.1 (Mandatory to Implement Features for All OpenID Providers) 
>> already does specify RS256 as MTI.  Is there something else you want 
>> us to say in this regard?
>>
>>
>> 9.3.1.  Rotation of Asymmetric Signing Keys
>>
>> Isn't this an implementation note?
>>
>> No, it's normative
>>

To me the introduction "Rotation of signing keys can be accomplished 
with the following approach." sounds like an implementation note.

>> 10.  Offline Access
>>
>> Given the description (" that grants access to the End-User's 
>> UserInfo Endpoint ..."), I would say this text can go to the User 
>> Info section.
>>
>> 11.  Using Refresh Tokens
>>
>> I think this should go to the authentication section (2.4?), as it 
>> describes usage of the refresh token grant type in the Connect context.
>>
>> It's optional functionality, whereas Authentication is not.  Connect 
>> implementations do not need to support refresh tokens.
>>

I don't get your argument. Our apps use the code grant type for initial 
login and refresh tokens to sub-sequently login and obtain fresh 
tokens/data. So from my perspective, esp. from the perspective of mobile 
apps, refresh tokens are in the same way relevant for authentication 
than all other grant types.

Furthermore: Refresh tokens are in the same optional as nearly all other 
grant types. For example, Connect implementations do not need to support 
the hybrid flows.

>>
>> 11.2.  Refresh Successful Response
>>
>> "If an ID Token is returned as a result of a token refresh request 
>> ..." - Can't we specify the conditions under which an ID token is 
>> returned? Otherwise, an interoperable client does not know what to 
>> expect or how to control the outcome of this request. For the 
>> standard authentication flow, presence of the scope value "openid" is 
>> the trigger. I would suggest to use the same logic for the refresh 
>> token grant type.
>>
>> The refresh functionality is outside the scope of the spec.  The 
>> current language is there to place constraints on what it needs to 
>> do, if supported.
>>

if it is out of scope then please remove it. The current text leaves 
large gaps, which will cause interop problems.

>>
>> 14.  Implementation Considerations
>>
>> I would suggest to move MTI requirements to another document as it 
>> refers to nearly every document of the OpenID Connect suite. For the 
>> core document, I would suggest to (at most) specify the MTI 
>> requirements for closed systems, only:
>> - code flow extension
>> - prompt
>> - display
>> - language
>>
>> The logic behind the current organization is that specifying both the 
>> closed and open MTI requirements in one place makes them easier for 
>> developers to understand.
>>

One place does not nessecarily mean the core document, right?

>>
>> 14.1.  Mandatory to Implement Features for All OpenID Providers
>>
>> "Signing ID Tokens with RSA SHA-256" - I think, in Berlin, we agreed 
>> to not require signatures for simple, static setups using authz code. 
>> We agreed to move this to the MTI section for dynamic OPs.
>>
>> Actually, what we agreed was to allow signing to be optional for the 
>> Code flow, provided the client registers saying that it wants 
>> "none".   This is now in the spec.
>>
>>
>> 14.5.  Compatibility Notes
>>
>> Isn't such information better covered on the working group page. Such 
>> information typically change more often than the specification itself.
>>
>> Developers are more likely to read the spec than other working group 
>> pages.  Again this is there for developer convenience, to help them 
>> understand the whole context as easily as possible.
>>

Which evidences is your statement based on?

>>
>> 14.6.  Related Specifications and Implementer's Guides
>>
>> same here
>>
>> Same answer.
>>
>>
>>
>> _______________________________________________
>> Openid-specs-ab mailing list
>> Openid-specs-ab at lists.openid.net
>> http://lists.openid.net/mailman/listinfo/openid-specs-ab
>
> -- 
> George Fletcher <http://connect.me/gffletch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openid.net/pipermail/openid-specs-ab/attachments/20131023/f0d9f134/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 80878 bytes
Desc: not available
URL: <http://lists.openid.net/pipermail/openid-specs-ab/attachments/20131023/f0d9f134/attachment-0001.png>


More information about the Openid-specs-ab mailing list