Re: [OAUTH-WG] Review of draft-ietf-oauth-dyn-reg-16

[Hannes Tschofenig <[e-mail]>]

Hi Justin,

thanks for the quick feedback.

A few remarks below.

>> - Protocol Flow
>>
>> In Figure 1 you show the client and the developer in the same box. The
>> protocol defined in the specification clearly runs between a client and
>> client registration endpoint at an authorization server. So, I would
>> suggest to put the developer (which is a human) outside the box and to
>> draw another box around the client registration endpoint to indicate
>> that this is part of the authorization server.
> 
> There are two known modes of deployment for this protocol: either the
> client calls the registration endpoint directly and gets its own
> client_id and client_secret, or the developer uses some tool (part of
> their build process, a software publication process, a self-service
> portal) to register the client. While the first use case is the original
> driver, several people wanted to make sure that this other use case
> wasn't inadvertently written out.

Makes sense to me. Maybe you just want to provide that piece of
background to the figure.

> 
> 
>> - Section 2
>>
>> What exactly does this sentence mean?
>>
>> "
>>    Authorization servers MUST accept all fields in this list.
>> "
>>
>> I believe I cannot mean that the authorization server supports all
>> mechanisms.
> 
> All I was trying to say was that the AS isn't allowed to crash if it
> sees a field in this list as part of the registration, to give us some
> kind of interoperability baseline. I am welcome to re-wording
> suggestions. The AS is free to ignore any field that it doesn't like, or
> reject a registration for a value that it doesn't like, or replace a
> value that it doesn't like with something that it does like and return
> that. We enumerate all of those cases separately, so perhaps this
> sentence isn't necessary any more.

Understood.

To clarify the context one could write the following.

"
Future extensions will extend the list of grant_types, and
response_types with new mechanisms. To avoid interoperability problems
authorization server MUST ignore values they do not understand.

For instance, the [OAuth.Registration.Metadata] specification defines
additional client metadata values. A properly implemented authorization
server that does not understand any of the metadata values defined in
[OAuth.Registration.Metadata] sent by the client would ignore those.
"

> 
>> You write:
>>
>> "
>>    Client metadata values can either be communicated directly in the
>>    body of a registration request, as described in Section 4.1, or
>>    included as claims in a software statement, as described in
>>    Section 3.  If the same client metadata name is present in both
>>    locations, the value in the software statement SHOULD take
>>    precedence.
>> "
>>
>> It might be worthwhile to note that the two options exist to allow (a)
>> the client to suggest values and (b) to have the organizing issuing the
>> software assertion to provide further values.
> 
> It's actually the other way around -- the assertion if present defines a
> set of core values for a class of clients and the client suggests values
> on its own in the plain JSON. The vast majority of registrations will
> use only the JSON object, in my estimation.
> 
>> Regarding the SHOULD in the last sentence I guess it might make more
>> sense to just say that it is deployment dependent what policy is used.
> 
> The idea here is that the software statement, if present and trusted,
> should really take precedence since it's cryptographically protected and
> the plain JSON isn't.

Reasonable thought. Maybe turn the SHOULD into a MUST?
The challenge with the SHOULD is always to explain when it shouldn't be
done.


> 
>> - Section 2.1
>>
>> You write:
>>
>> "
>> As such, a server supporting these fields
>>    SHOULD take steps to ensure that a client cannot register itself into
>>    an inconsistent state.
>> "
>>
>> Any guidance on how the authorization server would do that?
> 
> Either return an error ("invalid_client_metadata") or replace the values
> with sane defaults. Probably the former for most servers. Should we
> state this?

Might be a useful addition for someone implementing the spec to know
what to do.

I didn't know what to do when reading that sentence.

> 
>> - Section 3
>>
>> I don't understand this sentence:
>>
>> "
>>   In some cases, authorization servers MAY choose to accept a software
>>    statement value directly as a Client ID in an authorization request,
>>    without a prior dynamic client registration having been performed.
>> "
>>
>> Does this mean that the client id is the software statement or that the
>> software statement is embedded in the client id or something else?
> 
> The idea here is that the software statement itself would be the
> client_id, but the details of such usage are outside the scope of
> dynamic registration (since it's not really registration at that point,
> but a stateless client identifier).

If the software statement is the client_id then would you only send the
software statement but no client_id in the message?


> 
>> - Section 4
>>
>> The story around the initial access token is a bit strange. Here is the
>> text:
>>
>>    The client registration endpoint MAY be an OAuth 2.0 protected
>>    resource and accept an initial access token in the form of an OAuth
>>    2.0 [RFC6749] access token to limit registration to only previously
>>    authorized parties.  The method by which the initial access token is
>>    obtained by the registrant is generally out-of-band and is out of
>>    scope for this specification.  The method by which the initial access
>>    token is verified and validated by the client registration endpoint
>>    is out of scope for this specification.
>>
>>
>> First, the term 'registrant' is used here for the first time. Then, it
>> is outside the scope of how the client got this initial access token.
>> Normally for access tokens the client does not have to care about the
>> content and does not verify anything but here the last sentence hints to
>> the verification (although it is outside the scope of how it is used).
> 
> The client doesn't verify the token, the client registration endpoint
> verifies it. It's a vanilla OAuth token.

Read that incorrectly. Thanks for the clarification!

> 
>> I am curious whether the software assertion could actually be re-use
>> here in case the unauthorized use of the registration by clients is a
>> concern!?
> 
> This is exactly what BlueButton+ does: the software statement equivalent
> is presented as the initial access token. I personally think that this
> makes a lot more sense, but some folks wanted to be able to separate
> these two things, so that the authority to register with a server is
> differentiable from the fixed registration parameters. Also, you don't
> want to define the initial access token to *always* be a software
> statement, since it could just represent authorization to call the
> registration endpoint with no other strings attached.

I would like to see some text (from some of the proponents of this
two-token idea) where it would make sense to have these two tokens.
Sounds quite complex to me, particularly when there are a lot of
"out-of-scope" statements.

We don't want to get to a model where the client does not have the
client_id configured and then needs to use the dynamic client
registration protocol to subsequently require even more information to
get registered....

> 
>> - Section 4.2
>>
>> You write:
>>
>> "
>>  This client identifier MUST be
>>    unique at the server and MUST NOT be in use by any other client.
>> "
>>
>> This is a bit unclear given the text you provide in the subsequent
>> section, Section 5.1.
>> You write:
>>
>> "
>>   client_id  REQUIRED.  Unique client identifier.  It MUST NOT be
>>       currently valid for any other distinct registered client.  It MAY
>>       be the same as the Client ID value used by other instances of this
>>       client, provided that the Redirection URI values and potentially
>>       other values dictated by authorization server policy are the same
>>       for all instances.
>> "
> 
> Those are still inconsistent and I'm not positive where we came down on
> that issue -- I personally don't like the idea of re-using client_ids
> for different instances of the client (I see it causing nothing but
> trouble down the line), but there was a push to relax that. Can someone
> who wanted this comment on its utility?
> 
>> You write:
>>
>> "
>> If a software statement was used as part of the registration, its
>>    value SHOULD be returned in the response and its value MUST be
>>    returned if the authorization server supports registration management
>>    operations [OAuth.Registration.Management] that would require its
>>    presence in subsequent operations.
>> "
>>
>> I am not sure I understand that. Are you saying that the software
>> assertion is returned in the response from the authorization server to
>> the client? Why is that?
> 
> It is effectively part of the client's registration metadata and should
> be returned as-is. If the client is going to be doing management, it
> needs to be able to post that back to the server again as part of its
> update and the server needs to be able to check against it. What we
> really don't want is the registration endpoint to generate a new
> software statement as part of the registration response.

But the client had that data in the first place and so it is a waste of
bandwidth to return the same data back to him again (maybe even twice --
once in the software assertion and then separately as meta-data).


> 
>> - References
>>
>> I believe you should delete the dependency on the registration
>> management specification.
> 
> This makes sense if the operations can be completely insular and we
> don't need any forward references as to why to do certain things. It
> would be easy to build a registration endpoint that precludes any kind
> of management/lifecycle API, and we want to avoid that in the design of
> the core protocol. I think we've successfully done that, but if it makes
> it cleaner to remove the references and just state how to do things, I
> don't see why not.
> 

I am just saying it because of the way how the discussions at the last
IETF meeting went and creating these dependencies could lead to a
problem with progressing this spec.

> Thanks for the thorough review.
No problem - I am here to help.


Ciao
Hannes

>  -- Justin
> 
>>
>> Ciao
>> Hannes
>>
>>
>>
>> _______________________________________________
>> OAuth mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/oauth
>