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

Responses inline.

On 04/23/2014 09:08 AM, Hannes Tschofenig wrote:
> Hi all,
>
> here are a few comments regarding the draft-ietf-oauth-dyn-reg-16 spec.
> The first two are editorial and a matter of taste.
>
> - Abstract
>
> The abstract is awfully short. Could you at least add a few more lines
> to tell the reader what to expect in the document?

Makes sense -- there was a lot of tumult about what would actually be in 
the document, but now that I think we've got general agreement on the 
proper split (i.e., merging "core" and "metadata", keeping "management" 
separate) this can happen.

> - Introduction
>
> I believe the problem statement in the introduction section could be a
> bit clearer. I would phrase the story as follows: today client software
> developers need to manually interact with a deployment organization to
> obtain relevant parameters for their client software and/or to provide
> meta-data about it. This can be time-consuming. This document provides a
> mechanism for dynamically provisioning this information.

Makes sense, we can clarify. Most notably, we need to be clear that this 
is an automated API to do client registration, and that you can still do 
manual registration or some other nonstandard process if you want to.

> - Terminology
>
> Please avoid RFC 2119 language in the terminology section. I also
> believe it is unnecessary, for example in the client instance definition.

Noted.

> There is a bit of inconsistency in the terminology when I look at the
> client software - client instance and the software api publisher -
> software api deployment relationship. I would have expected to see
> software api instance - software api publisher.
>
> It might help readers to provide a few examples to show typical
> relationships.
>
> For example, a social network site (=deployment organization) develops
> their own social network software and makes it available to application
> developers. They are also a software API publisher and use OAuth to
> protect access to the data. A software developer writes a client
> software for use with the social network site and distributes different
> client instances to smart phones.

Many of these terms and relationship were pulled in wholesale from 
Phil's draft, so they probably need to be massaged a bit to fit with the 
rest.

> I wonder whether it would make sense to use a different name for the
> 'initial access token', such as registration token to make it easier to
> differentiate it from a regular access token. (Later in my review I will
> argue that this access token is a bit strange...)
>
> In the text you use the term 'client' but the terminology section only
> defines 'client instance' and 'client software'. Does the term 'client'
> refer to 'client software'?

The draft inherits the term "client" from RFC6749. We *do* need to be 
careful to use the most exact term throughout the text.

> - 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.

> - 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.

> 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.

> - 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?

> - 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).

> - 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.

> 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.

> - 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.

> - 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.

Thanks for the thorough review.

  -- Justin

>
> Ciao
> Hannes
>
>
>
> _______________________________________________
> OAuth mailing list
> OAuth@ietf.org
> https://www.ietf.org/mailman/listinfo/oauth