Re: [Ace] WGLC for draft-ietf-ace-aif

[Daniel Migault <[e-mail]>]

Thanks for the review Olaf.

Yours,
Daniel

On Fri, Feb 12, 2021 at 4:05 AM Olaf Bergmann <[e-mail]> wrote:

> Hi all, hi Carsten,
>
> here is my review for draft-ietf-ace-aif-01.
>
> The document is well-written and straight forward. I have only a few
> minor comments (see below).
>
> Grüße
> Olaf
>
> # Abstract/1. Introduction
>
>   "To transfer detailed authorization information from an
>   authorization manager (such as an ACE-OAuth Authorization Server) to
>   a device, a representation format is needed."
>
> Maybe add "compact" before "representation format is needed" to
> indicate that suitability for constrained devices is an important
> design goal here.
>
> ## 1.1. Terminology
>
> Mention that CDDL is used as a formal syntax?
>
> # 2. Information Model
>
>   "For the purposes of this specification, the underlying access
>   control model will be that of an access matrix, which gives a set of
>   permissions for each possible combination of a subject and an
>   object. We do not concern the AIF format with the subject for which
>   the AIF object is issued, focusing the AIF object [...]"
>
> Here, the different use of "object" might be confusing (first as one
> dimension of the access matrix, then as "AIF object", i.e., the
> serialization of permission+object; in the next paragraph it is again
> the first meaning of object). Maybe this can be solved by
> unfolding the abbreviation:
>
>  "[...] We do not concern the AIF format with the subject for which
>   the Authorization Information is issued, focusing the Authorization
>   Information [...]"
>
> Alternatively, these terms could be explained in the terminology
> section.
>
> Also, "AIF format" would double as "Authorization Information Format
> format". I would not bother, though.
>
> In the same sentence:
>
>   "We do not concern the AIF format with the subject for which the AIF
>   object is issued, focusing the AIF object on a single row [...]"
>
> I am not sure if "[...] focusing the AIF object [...]" is really meant
> here. Another, also valid statement would be "[...] focusing on the AIF
> object [...]"?
>
> ## 2.1 REST-specific model
>
> Heading: s/model/Model/  (also for 2.3 Extended REST-specific Model)
>
> The first paragraph refers to CoAP which has not been introduced (and
> motivated). It might be good to note that CoAP is used as an
> explanatory example here which is motivated by the target devices'
> constraints.
>
> Table 1: To readers who are not familiar with the notion of the "/s"
> and "/a" prefixes, it might be surprising that the light (sensor)
> should allow GET only. Maybe "/s/temp" would be more intuitive?
> (also in Figure 3 and Figure 5)
>
> ## 2.2. Limitations
>
> In the first sentence: s/e.g. URIs/e.g., URIs/
>
> Second paragraph: s/doesn't/does not/
>
> Last paragraph: s/specific to a subject, e.g. that/specific to a subject,
> e.g., that/
>
> ## 2.3 Extended REST-specific model
>
> First paragraph: s/in a CoAP result/in a CoAP response/
>
> s/rfc5661/RFC5661/
>
> Table 2 is missing a reference in the text.
>
> The question that may arise here is how a receiver of an AIF object is
> supposed to react when it does not support the extended model? Is it
> safe to simply ignore the Dynamic-* part? (cf. Security Considerations
> below).
>
> # 3. Data Model
>
> OLD:
>
> "In CDDL [...] a specification of the data model [...] is:"
>
> NEW:
>
> "In CDDL [...] a specification of the data model [...] is shown in Figure
> 4."
>
> ## 5.2. Registries
>
> Table 4 names the reference to the AIF RFC "[RFCthis]" whereas it is
> "RFC XXXX" in the other subsections of Section 5 (including the note
> to the RFC Editor).
>
> # 6. Security Considerations
>
> I think that the question how to deal with AIF that is not understood
> warrants a short discussion here, e.g., motivating the usual
> "everything is denied until it is explicitly allowed" should hold here
> as well.
>
> _______________________________________________
> Ace mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ace
>


-- 
Daniel Migault
Ericsson