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

Justin Richer <jricher@mitre.org> Thu, 24 April 2014 13:46 UTC

Return-Path: <jricher@mitre.org>
X-Original-To: oauth@ietfa.amsl.com
Delivered-To: oauth@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 9753C1A01DC for <oauth@ietfa.amsl.com>; Thu, 24 Apr 2014 06:46:19 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.472
X-Spam-Level:
X-Spam-Status: No, score=-4.472 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RP_MATCHES_RCVD=-0.272] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id VfTasuIR_dGq for <oauth@ietfa.amsl.com>; Thu, 24 Apr 2014 06:46:15 -0700 (PDT)
Received: from smtpksrv1.mitre.org (smtpksrv1.mitre.org [198.49.146.77]) by ietfa.amsl.com (Postfix) with ESMTP id 3966A1A019B for <oauth@ietf.org>; Thu, 24 Apr 2014 06:46:15 -0700 (PDT)
Received: from smtpksrv1.mitre.org (localhost.localdomain [127.0.0.1]) by localhost (Postfix) with SMTP id 1685A1F0894; Thu, 24 Apr 2014 09:46:09 -0400 (EDT)
Received: from IMCCAS01.MITRE.ORG (imccas01.mitre.org [129.83.29.78]) by smtpksrv1.mitre.org (Postfix) with ESMTP id 0AE9C1F0886; Thu, 24 Apr 2014 09:46:09 -0400 (EDT)
Received: from [10.146.15.6] (129.83.31.51) by IMCCAS01.MITRE.ORG (129.83.29.78) with Microsoft SMTP Server (TLS) id 14.3.174.1; Thu, 24 Apr 2014 09:46:08 -0400
Message-ID: <53591579.9090103@mitre.org>
Date: Thu, 24 Apr 2014 09:45:29 -0400
From: Justin Richer <jricher@mitre.org>
User-Agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.4.0
MIME-Version: 1.0
To: Hannes Tschofenig <hannes.tschofenig@gmx.net>, "oauth@ietf.org" <oauth@ietf.org>
References: <5357BB37.1080803@gmx.net> <5357D4FF.5040800@mitre.org> <5358D170.1060303@gmx.net>
In-Reply-To: <5358D170.1060303@gmx.net>
Content-Type: text/plain; charset="ISO-8859-1"; format=flowed
Content-Transfer-Encoding: 7bit
X-Originating-IP: [129.83.31.51]
Archived-At: http://mailarchive.ietf.org/arch/msg/oauth/03iOAebYpjVh2wIrGaFo4A6byOE
Subject: Re: [OAUTH-WG] Review of draft-ietf-oauth-dyn-reg-16
X-BeenThere: oauth@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: OAUTH WG <oauth.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/oauth>, <mailto:oauth-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/oauth/>
List-Post: <mailto:oauth@ietf.org>
List-Help: <mailto:oauth-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/oauth>, <mailto:oauth-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 24 Apr 2014 13:46:19 -0000

On 04/24/2014 04:55 AM, Hannes Tschofenig wrote:
> 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.

Makes sense -- it's in the introductory text but it might not have been 
propagated through.

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

OK, we can work on that text.

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

I'll let the proponents of including the software statement in the core 
chime in on that one.

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

I think we can do that with a small example, something like:

    As such, a server supporting these fields
    SHOULD take steps to ensure that a client cannot register itself into
    an inconsistent state. For instance, returning an "invalid_client_metadata" error
    if a client tries to register for the "code" response type and the "implicit" grant
    type.



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

You'd send the whole software statement as the client_id, in the 
client_id field. There would need to be another draft to describe how to 
do that interoperably, and I think John was looking to put that out 
sometime. This statement as I understand it is more of a pointing toward 
another use case that's outside the scope of dynamic registration (and 
in fact can be an alternative to dynamic registration for some use cases).

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

Maybe we should instead prohibit what we really *don't* want instead of 
trying to say what we do, since the former is more narrowly defined?

>
>>> - 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
>>> OAuth@ietf.org
>>> https://www.ietf.org/mailman/listinfo/oauth