Re: [Ace] AD review of draft-ietf-ace-aif-04

[Carsten Bormann <[e-mail]>]

Hi Ben,

thank you for this in-depth review.

I have prepared a pull request with resulting changes at 

https://github.com/cabo/ace-aif/pull/1

Depending on further feedback from you and from the WG, I plan to submit the resulting updated I-D on Monday.

> On 2022-02-11, at 05:20, Benjamin Kaduk <[e-mail]> wrote:
> 
> Hi all,
> 
> There's enough that will be changing yet that I'll mark this as "Revised
> I-D needed" in the datatracker rather than starting an IETF Last Call
> directly.
> 
> We'll also need to change the "Intended RFC Status" field in the
> datatracker to match the Proposed Standard target.

Chairs: please do make this change.

> Without further ado...
> 
> Abstract, Introduction
> 
> Seeing ~identical abstract and introduction always makes me wonder if
> there's a way to pare down the abstract and/or flesh out the introduction
> further.  (I didn't get very far in my wonderment today, to be clear.)

I don’t see immediate opportunities for such changes, so I haven’t tried.

> Also, in the first sentence we say "Constrained Devices [...] need
> security."  I see that we go on to focus on the authorization aspect of such
> security functionality, but I think it would be good to have some more
> adjectives qualifying "security", which in isolation can mean very different
> things to different readers.

Hmm, the whole point of the rest of the first paragraph is to specify what kind of security is addressed here.

>   need to ascertain
>   that the authorization to request the operation does apply to the
>   actual requester, [...]
> 
> nit: maybe "actual authenticated requester"?

Good idea; I made it “actual requester as authenticated”.

> 
>                     and need to ascertain that other devices they place
>   requests on are the ones they intended.
> 
> nit: maybe s/place requests on/make requests of/?

“make requests of” is not a phrase I would have come up with, but I’ll defer to the native speaker here.

> 
>                      This document provides a suggestion for such a
>   format, the Authorization Information Format (AIF).  [...]
> 
> If we're going for Proposed Standard, we should say something stronger than
> just "a suggestion for" such a format.

Right.  s/provides a suggestion for/defines/

(It is too easy to forget making such changes as a draft progresses from straw man to solid specification…)

>                                                        AIF is defined
>   both as a general structure that can be used for many different
>   applications and as a specific refinement that describes REST
>   resources (potentially dynamically created) and the permissions on
>   them.
> 
> (editorial) I might consider a framing more like "defined both as [...] and
> as a specific instantiation tailored to REST resources and the permissions
> on them, including some provision for dynamically created resources."

Nice!  Adopted.

> Section 1.1
> 
>   The shape of data is specified in CDDL [RFC8610].  Terminology for
>   Constrained Devices is defined in [RFC7228].
> 
> I expect that some readers will find "the shape of data" to be too informal
> for their liking.  I'm willing to let it go through to IETF LC, myself, but
> am not going to push hard for it to remain if there is pushback.

“Data shape” is a technical term that we actually use a lot to describe the qualities that a structural (as opposed to semantic) data definition describes. 
Googling, I get about 257.000 results, e.g., https://www.w3.org/2014/data-shapes/wiki/Main_Page

>   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>   "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
>   "OPTIONAL" in this document are to be interpreted as described in
>   BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all
>   capitals, as shown here.  These words may also appear in this
>   document in lower case as plain English words, absent their normative
>   meanings.
> 
> I expect multiple future reviewers to ask you to use the BCP 14 boilerplate
> exactly as it appears in RFC 8174.

Indeed, with Standards Track, this can now be normalized.

>   (Note that this document is itself informational, but it is
>   discussing normative statements that MUST be put into concrete terms
>   in each specification that makes use of this document.)
> 
> This is stale now that we're not going for Informational anymore.

Ditto.

> Section 2
> 
>   We do not concern the AIF format with the subject for which the AIF
>   data item is issued, so we are focusing the AIF data item on a single
>   row in the access matrix (such a row traditionally is also called a
>   capability list).  [...]
> 
> (editorial?) we haven't exactly said *why* we don't concern ourselves with
> the subject yet -- that's only in the following sentence, which makes the
> deductive link via "so we are" rather weak.  One approach would be to flip
> around the order of the sentence, as in "in this document we focus on [a
> single row], without concern to the subject for which the data item is
> accessed", that looks like it would flow naturally into the existing "as a
> consequence, AIF MUST be used in a way that [the subject is identified]".

Makes sense.
(S/accessed/issued/)

>   In a specific data model, the object identifier (Toid) will often be
>   a text string, and the set of permissions (Tperm) will be represented
>   by a bitset in turn represented as a number (see Section 3).
> 
> (editorial) maybe "a specific data model, including the one specified in
> this document"?  Hmm, though then the placement of "often" is awkward...

I went for s/including/such as/, and then I think the statement stands.

>   AIF-Specific = AIF-Generic<tstr, uint>
> 
>                  Figure 2: Likely shape of a specific AIF
> 
> Do we want to keep "likely" as we go for proposed standard?

It is still likely, so likely that we actually define it here…
S/likely/commonly used/

> Section 2.1
> 
>   In the specific instantiation of the REST resources and the
>   permissions on them, for the object identifiers (Toid), we use the
>   URI of a resource on a CoAP server.  More specifically, the parts of
>   the URI that identify the server ("authority" in [RFC3986]) are
>   considered the realm of the authentication mechanism (which are
>   handled in the cryptographic armor); [...]
> 
> The current "are considered the realm of" phrasing makes it sound like we're
> stating an intrinsict fact or universally held belief, whereas it's really
> just a choice that we make for convenience (not least because the URI
> authority component is already authenticated by HTTPS and CoAPS).  

I certainly meant “are considered” as a design decision, not a belief.

> So I'd
> recommend a phrasing more like "Since the parts of the URI that identify the
> server (...) are what are authenticated during REST resource access (Section
> 4.2.2 of draft-ietf-httpbis-semantics and Section 6.2 of [RFC7252]), they
> naturally fall into the realm handled by the cryptographic armor; [...]".

Adopted.  Thanks!

> 
>                                        we therefore focus on the "path-
>   absolute" and "query" parts of the URI (URI "local-part" in this
>   specification, as expressed by the Uri-Path and Uri-Query options in
>   CoAP).  [...]
> 
> To follow RFC 3986 we should talk about the generic "path" component and
> explain why it is okay to specialize to just "path-absolute" for
> authorization purposes.  

Looking at 3986 again, we should actually focus on path-abempty.
Fixed.

>   For the permissions (Tperm), we simplify the model permissions to
>   giving the subset of the CoAP methods permitted.  This model is
>   summarized in Table 1.
> 
> Do we want to mention HTTP in addition to CoAP, since this is claiming to be
> a generic REST-ful model?

Probably.  Added.  This is in terms of CoAP method names/numbers (FETCH is called QUERY this week in HTTP [draft-ietf-httpbis-safe-method-w-body], and I don’t think we want to wait for this as a normative reference).

>   For the permissions (Tperm), we simplify the model permissions to
>   giving the subset of the CoAP methods permitted.  [...]
> 
> (editorial) it feels a bit odd to say that we "simplify" something without a
> clear picture of what the ("complicated") baseline is.  Maybe we could just
> say that we use a simple permissions model that lists the subset of CoAP (or
> HTTP) methods permitted?
> Also, is "giving" the right word here?  Maybe "listing" or "conveying"?

Lists.  Text appropriately adapted.


>   In this example, a device offers a temperature sensor /s/temp for
>   read-only access and a LED actuator /a/led for read/write.
> 
> Table 1 also lists a /dtls endpoint, shouldn't we mention it in the prose?

Yes, added.

> Section 2.3
> 
> It feels a little strong to call this model just the "extended"
> REST-specific model, since the extension is limited to resources created by
> the subject of the autorization -- the limitations mentioned in §2.2 of
> conditionalizing based on state other than identification of objects, and of
> access to resources created by others for the subject, still apply.  Perhaps
> we should just call it a REST-specific model with dynamic resource creation?

Nice!  Adopted.

>   itself, specifically, a resource that is made known to the subject by
>   providing Location-* options in a CoAP response or using the Location
>   header field in HTTP [RFC7231] (the Location-indicating mechanisms).
> 
> (draft-ietf-httpbis-semantics is a better reference than 7231, in my
> opinion.)

Fixed.  (I hope this will not slow us down in a monster cluster.)

>   which the subject had access.)  In other words, it addresses the
>   third limitation mentioned in Section 2.2.
> 
> It *partially* addresses that limitation (which as written covers both
> self-created and externally-created objects that are specifically for the
> subject of the authorization).

To me, addressing implies partial addressing…
Added “an important subset”.

> 
>   For a method X, the presence of a Dynamic-X permission means that the
>   subject holds permission to exercise the method X on resources that
>   have been returned by a Location-indicating mechanism to a request
>   that the subject made to the resource listed (/a/make-coffee in the
>   example shown in Table 2, which might return the location of a
>   resource that allows GET to find out about the status and DELETE to
>   cancel the coffee-making operation).
> 
> I think we may need to be more precise than just "resources that have been
> returned ... to a request that the subject made".  As discussed in
> https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-semantics#section-10.2.2
> the semantics of Location: differ on the response code, and in my reading we
> probably only want the semantics for 201 (Created) responses.  It seems
> potentially risky for us to grant access to the resources referred to by 3xx
> (Redirection) responses without limitation.  For example, in the
> "make-coffee" case, if we get redirected from /a/make-coffee to
> /a/better-coffee, we probably should only be able to POST to
> /a/better-coffee (if anything), but not DELETE it.

Added "in a 2.01 (201) response".
(For many reasons, including the ones alluded to here, there is no redirection in CoAP.)

> There is also potentially some question about how both the authorization
> subject and the RESTful server independently track the dynamically granted
> authorizations.  It is *probably* going to just be an internal matter in
> each case, but might be worth mentioning as something that needs to be
> implemented (carefully).

Good point.
Added…

An implementation of the REST-specific Model With Dynamic Resource
Creation needs to carefully keep track of the dynamically created
objects and the subjects to which the Dynamic-X permissions apply —
both on the server side to enforce the permissions and on the client
side to know which permissions are available.

…to the security considerations.

> Section 3
> 
>   In this section, we will give the data model for basic REST
>   authorization as per Section 2.1 and Section 2.3.  As discussed, in
> 
> We say "basic" here, but (per my previous comment) currently say "extended"
> about §2.3.

S/basic/simple/

>   *  The (non-dynamic) methods in the permission sets are converted
>      into their CoAP method numbers, minus 1.
> 
> This seems to make it quite challenging to use AIF with full-featured HTTP,
> seeing as there are thirty-odd HTTP methods that do not have CoAP analogue.
> Do we perhaps want to tone down the potential application to HTTP (which I
> guess mostly takes the form of claiming to be compatible with REST in the
> abstract rather than specific HTTP references) in the earlier mentions in
> the document?

I’m not sure who really cares that much about the non-REST HTTP methods.
We could be more clear about that limitation.
(I’m not sure we can mention QUERY without damage, but of course the number 4 is available for QUERY.)

I added:

>> As will be seen in the data model ({{data-model}}), the representations
>> of REST methods provided are limited to those that have a CoAP method
>> number assigned; an extension to the model may be necessary to represent
>> permissions for exotic HTTP methods.

>   *  Dynamic-X permissions are converted into what the number would
>      have been for X, plus a Dynamic-Offset chosen as 32 (e.g., 35 for
>      Dynamic-DELETE).
> 
> Would a note about how this puts the start of the dynamic range immediately
> after the end of the non-dynamic range (due to the CoAP field size) be
> useful?  That is, why the value 32 was chosen.  (I do see the note at the
> end of the section, but don't think that precludes saying something here.)

I was assuming that the rationale for this was obvious.
There is also some text at the end of Section 3, including the limitation for I-JSON to dynamic forms for the first 21 CoAP methods.

>   This data model could be interchanged in the JSON [RFC8259]
>   representation given in Figure 3.
> 
> I expect we want to say something about Figure 3 being a representation of
> the permissions shown in Table 1, to tie the examples together.
> 
>   [["/s/temp", 1], ["/a/led", 5], ["/dtls", 2]]
> 
>       Figure 3: An authorization instance encoded in JSON (46 bytes)
> 
> I count 45 bytes (not 46),

You didn’t count the newline at the end…
I deleted all blank space and stopped counting the newline, so the caption now says 40 bytes.

> and it would be even fewer if the internal spaces
> were omitted.  (The topic of internal spaces is on my mind due to the MQTT
> profile putting AIF content in the "scope" field of a JWT, which gives
> semantic meaning to internal spaces.)

Oh.  Of course, a URI could have blank space in itself, so I’m not sure shoving JSON into spaces that aren’t really prepared for that is such a great idea.
But I don’t think this has any further impact on the current draft.

>   methods = &(
> 
> I guess new CoAP methods are rare enough that we don't need to use a socket
> here?

We are not providing extensibility (except by registering a new Tperm), so there is no socket in the CDDL either.  We could change this, but then also would want to provide a registry for the individual method values for the two REST-specific models.

> 
>   A representation of this information in CBOR [RFC8949] is given in
>   Figure 5; again, several optimizations/improvements are possible.
> 
> We had a bunch of stuff between Figure 3 and here, so "this information"
> feels like a stale reference.  Let's name Figure 3 or Table 1 specifically
> instead.

I put in both.

> Section 5.1
> 
> (I will comment only on the aif+cbor one, but aif+json should get the
> corresponding treatment.)
> 
>   Required parameters:
>      *  Toid: the identifier for the object for which permissions are
>         supplied.  A value from the subregistry for Toid.  Default
>         value: "local-uri" (RFC XXXX).
> 
>      *  Tperm: the data type of a permission set for the object
>         identified via a Toid.  A value from the subregistry for Tperm.
>         Default value: "REST-method-set" (RFC XXXX).
> 
> I am apparently failing to find the actual right reference to be looking at,
> but RFC 6838 has some discussion that seems to imply that it's incompatible
> to have a default value for a required parameter.  Are these actually
> optional parameters with default value?

In the data model, yes :-)
Fixed.

>   Applications that use this media type:  No known applications
>      currently use this media type.
> 
> I feel like the registrations I see go by usually make a generic statment
> about "applications that need to convey structured authorization data for
> resources accessible via REST mechanisms", even if the set of such
> applications is the empty set at the time of the registration.

Good point.  This is not limited to REST, though.

>> Applications that use this media type:
>> : Applications that need to convey structured authorization data for
>>   identified resources, conveying sets of permissions.


> 
> Section 5.2
> 
>   IANA is requested to create a registry for AIF with two sub-
>   registries for Toid and Tperm, populated with:
> 
> Should the sub-registries for Toid and Tperm be on the Media Type
> Sub-Parameter Registries page
> (https://www.iana.org/assignments/media-type-sub-parameters/media-type-sub-parameters.xhtml)
> rather than on a dedicated AIF page?

I think we are better off not burying the information there.

> Section 6
> 
>   *  ensure that the cryptographic armor employed around this format
>      fulfills the security objectives, and that the armor or some
> 
> I think we should clearly enumerate these security objectives in a paragraph
> just prior to this list.

We don’t know these objectives.
They come from the application.

Changed to:

>> * ensure that the cryptographic armor employed around this format
>>   fulfills the referencing specification's security objectives, and that the armor or some


>      additional information included in it with the AIF information
>      unambiguously identifies the subject to which the authorizations
>      shall apply, and
> 
> Not just the subject, but also (for the specific RESTful case) the [scheme
> and?] authority component of the URI of the object to which authorization is
> being granted.

So this is now:

>> * ensure that the cryptographic armor employed around this format
>>   fulfills the referencing specification's security objectives, and that the armor or some
>>   additional information included in it with the AIF information
>>   (1) unambiguously identifies the subject to which the authorizations
>>   shall apply and provides (2) any context information needed to derive the
>>   identity of the object to which authorization is being granted
>>   from the object identifiers (such as, for
>>   the data models defined in the present specification, the scheme and
>>   authority information that is used to construct the full URI), and



> 
>   *  ensure that the types used for Toid and Tperm provide the
>      appropriate granularity so that application requirements on the
> 
> Maybe say "precision" as also being provided?
> (FWIW, "granularity" is sometimes a problematic word in that it can be used
> to indicate both a coarse-grained nature or a fine-grained nature.)

Fortunately, there is no problem in technical documents with using the same word twice with only five other words interspersed :-)

>   A generic implementation of AIF might implement just the basic REST
>   model as per Section 2.1.  If it receives authorizations that include
>   permissions that use the Section 2.3, it needs to either reject the
> 
> nit: missing word; "the Section 2.3 [syntax/behavior/extension/...]"
> Also, "generic" is maybe not the right word here, since we introduce AIF as
> both a generic framework and the specific instantiation of that framework
> for REST.

generic ➔ plain

>   AIF data item entirely or act only on the permissions that it does
>   understand.  In other words, the usual principle "everything is
>   denied until it is explicitly allowed" needs to hold here as well.
> 
> I'd consider moving "everything not specifically allowed is denied" into a
> dedicated paragraph.  That is, AIF conveys only specific precisely
> identified bits of positive authorization information, and everything else
> is forbidden unless separately authorized.  That's true regardless of
> whether it's a generic implementation or not.

Added

>> The semantics of the authorization information defined in this
>> documents are that of an *allow-list*:
>> everything is denied until it is explicitly allowed.

And then changed to:

>> In other words, the semantics underlying an allow-list as discussed
>> above need to hold here as well.
> 
> Section 7.2
> 
> I think our discussions of "local-part" and "path-absolute"+"query"
> components probably promote RFC 3986 to a normative reference.

Done.

> 
> RFC 7231 (or rather, draft-ietf-httpbis-semancis) seems like it needs to be
> normative as well, since it is the source of the Location header that is
> defined to be a way that a related resource is made known to the subject.

There goes any hope of getting the document published this year :-)

Grüße, Carsten