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

[Benjamin Kaduk <[e-mail]>]

On Fri, Feb 11, 2022 at 06:48:21PM +0100, Carsten Bormann wrote:
> 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.

That sounds good; I should be able to request that the Secretariat start
the IETF LC shortly thereafter.

Comments are pretty sparsely intertwined; most of your responses look good
and unctonrtoversial.

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

I think it was done already before you sent this :)

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

Looking at it again, there may be opportunity to pare down the Abstract.
E.g.,

Providing securely protected information about which entities are
authorized to perform what operations on which other entities is a crucial
component of producing an overall system that is secure.  This exchange of
authorization information is especially critical in highly automated
systems with large number of entities, such as the "Internet of Things".

This document provides a generic information model and format for
representing such authorization information, as well as a specific
instantiation of that format for use with REST resources identified by URI
path.

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

I was thinking to add a few more words, like "need security protections in
order to operate correctly and prevent misuse to attack other systems".

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

Me, too.  It's in the RFC Editor queue, but that didn't stop DTLS 1.3 from
holding up some other documents while the WG fixed a late-breaking issue
:-/

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

Sounds good.

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

Ok.  (Giving the Dynamic-Offset status as a named variable makes it sound
like it's intended to be a customizable parameter.  Which, maybe it is,
in the abstract model, and it's just the specific CoAP/RESTful case where
we have a natural and obvious value for it.)  But we can leave this be.

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

Agreed, no further impact here that I can see.  (IIRC, the MQTT profile
will base64 encode.)

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

Ah, right, only the default parameters make it REST-specific; good catch.

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

I am not so sure.

To me, the core question is whether these registries are used for anything
other than media-type parameters.  What such other usage do you envision?

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

Ok.

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

It's only February! ;)

Thanks,

Ben