Dear colleagues
Unfortunately, we lost the Shepherd for rfc8366bis to retirement. And all the
usual suspects, especially those with yang expertise turned me down because
of too much conflicting work (WG chair, nomcom chair, AD... *sigh*), or they
are co-authors.
So i wanted to reach out to the WG to see if anyone of you would be interested
to take over this job - before passing it on to the default usual suspect,
Sheng (as a co-chair).
This should not really be a lot of work left. The current Shepherd did provide
a thorough review in 2023 which was the basis of the mayor changes we did since
then, including a lot of pain on Michael Richardsons side to investigate
if/which
of the notational improvements would work in a backward compatible fashion,
and be supported as much as possible by existing YANG tools. All that feedback
was disucssed during 2024 IETFs and i think we exploited the best YANG options
feasible.
So, IMHO what's left is mostly procedural and should be easy to do for anyone
knowing rfc8366. So, here is your opportunity to try to be a shepherd ;-))
Pls. let me know!
Oh, and i just see that the original review wasn't Cc'ed to the list, so
i am appending it here. And thanks again, Alex!
Cheers
Toerless
>From a...@clemm.org Tue Aug 22 03:13:56 2023
To: draft-ietf-anima-rfc8366bis.auth...@ietf.org, anima-cha...@ietf.org
Cc: "a...@futurewei.com" <a...@futurewei.com>
From: Alexander Clemm <a...@clemm.org>
Subject: Shepherd review of draft-ietf-anima-rfc8366bis-09
Hi,
I have been assigned as document shepherd for
draft-ietf-anima-rfc8366bis-09. Toerless recently pinged me, indicating
that with rev -09 the document document has now progressed far enough
for me to start getting involved. Accordingly, I have reviewed the
document and have a number of comments and suggestions. Some of these
do concern design decisions and are not merely editorial suggestions; if
you would like to discuss these in a meeting just let me know.
I am sending my comments just to the authors and chairs, not the entire
WG (to reduce what many might consider spam); that being said, it is
possible that some of the issues raised below may result in broader
discussion in which we can always move their discussion to the list.
I am not sure if there is any formal procedure to follow at this point
as a shepherd. I would like to ask the ANIMA chairs for guidance if
need to submit this anywhere as part of the shepherd "collateral" - it
is too early for shepherd writeup and I assume having email discussion
at this point is just fine.
To make it easier to reference to the text, I downloaded the text
version of the document into an editor and will in the following refer
to the corresponding line numbers. For your convenience, I am attaching
it here as well.
With this, here goes:
SECTIONS 1 - 5
1) Header, Line 11: Toerless' affiliation should probably be Futurewei,
not Huawei.
2) Abstract, Lines 34-36:
" This document updates RFC8366, merging a number of extensions into
the YANG. The RFC8995 voucher request is also merged into this
document."
2a) It is not sufficiently clear what you mean with "merging" here.
Likewise "into the YANG" is a bit too colloquial here. Presumably you
want to say that you are updating a YANG data model that was previously
defined, adding a number of extensions to it. Also, are you revising the
data model (and expect previous versions of it to remain in use), or are
you really intending to replace the old with the new?
2b) Likewise, you may want to clarify the relationship of this document
with RFC 8366 and RFC 8995. Presumably this document is intended to
update and replace RFC 8366; if that is the case, please state so. And
what about RFC 8995? If you are redefining what was already defined
there, do you need to update RFC 8995 as well (to remove it from
there)? Why replicate what is already part of another RFC (even if in
hindsight this might have been structure differently) instead of
referencing it?
2c) Line 22: Will it be clear to the reader that "pledge" refers to a
device? (The term is later introduced, of course, but might be expanded
here to provide context for the uninitiated.)
3) Introduction, around Line 153: The text here assumes familiarity with
the overall process and is a bit dense. A brief explanation may be
helpful, e.g. expanding a bit on what is meant with "assigning
ownership" here: presumably, associating a pledge with an operator's
network and allowing the operator to take control.
4) Terminology, around Line 233: do you need to introduce
"pinned-domain-cert" certificate as a term? Somewhere (perhaps earlier
in the introduction) it might be useful to briefly introduce the
different actors and how they relate: i.e. MASA used by the manufacturer
of the device to sign a voucher for a pledge, which may indicate the
owner of the device (?), the Registrar, acting for the network domain
and deciding who may or may not join a given network, the Owner or
Network Operator who wants to securely deploy the plege, etc
5) TOFU, Line 242: Is this a special case to what is defined here, or is
it different? The relevance here is not quite clear and it is not
really used in the remainder of the text. The way it is explained is
not clear to me (probably due to my ignorance); at the same time if it
not really relevant for the remainder of the draft is this even needed?
6) e.g. lines 236, 242, 261, 272: The terms "pledge", "pledge device",
"pledge endpoint", "prospective device" are all used - if they all mean
the same, better to use a single term consistently.
7) Line 273: "The join registrar might use this information". Or it
might not, I guess. This seems very unspecific. Is this really needed;
how would you verify this?
8) Line 287: What does "pin" mean here? Is pinning the same as
authentication? Perhaps this is a term that should be introduced/added
to acronyms, and/or it might be useful to summarize the process.
9) Lines 303, 306: Presumably lines 304 and 305 are part of the header;
should the "===" delineation be moved from 303 to 306? Also 301 / 302:
Maybe delineate the columns between assertion, registrar ID, and
validity more clearly so it is clear which column belongs to which.
10) Lines 316, 317, 359: Is "Bearer out-of-scope [voucher]" the same as
"Bearer Voucher"? Presumably you mean that Bearer Vouchers are out of
scope. Why not simply omit them from the table then and state that
separately - this would be clearer. As a side note, this is the only
place where the TOFU term is mentioned; since it is scope is it really
needed (see also comment 5)?
11) Line 331: For clarity and better context, it might be useful to
spell out what "ownership knowledge" refers to hear. Ownership of what
by who - presumably ownership of the pledge by a network operator?
12) Line 341: Pardon my ignorance here, but could you elaborate on the
relationship between nonce and validity period? Since the term is about
"nonceless audit voucher", it may be better to state that because there
is no validity period associated with the voucher, a nonce is not
require. It would also be good to spell out what "without a validity
period" means: does it mean the audit voucher is presumed perpetually
valid?
13) Section 5: I am wondering if it would make sense to divide Section 5
into 2 subsections: One providing background (circa lines 370 to at
least 403, or possibly as far as 427 or so), another summarizing the
actual updates that were made? This would seem both clearer and cleaner
than what it is now.
14) Line 384: Do you have a reference for the consortia?
15) Line 427ff: As per comment #2, please state more clearly what you
mean with "merge", as well as what happens to the other documents. Are
you expecting that what you "merge" here will "replace" stuff elsewhere
- will [BRSKI] be updated to remove the voucher-request here; hat are
the extensions from [cBRSKI], [CLOUD] and [PRM] and will those be
updated; if the other documents are not updated, how are they expected
to coexist?
16) "There are some difficulties with this approach" - what do mean
here; if there are difficulties, why are you taking it; are there other
more feasible alternatives?
17) Line 455: s/definies/defines/
18) Line 459ff: It is not clear what you mean with uses in Netconf. So
far, the impression the document has given is that the YANG data model
that you are defining is used to describe the structure that you are
planning to encode as part of the voucher. Presumably you are using YANG
to have the option of using different encodings, rather than committing
to one particular encoding and defining the semantics of the fields
there. However, the assumption is that you are not using this as actual
management / configuration information to be used with management
protocols such as Netconf (or RESTCONF); for example, the fields are not
up for modification through edit-config operations etc. With this
context, I find the reference to Netconf here confusing. Presumably, if
you were to retrieve / edit / move around a voucher, you would do it as
a "blob" (with the blob including data fields encoded as per encoding
rules applied to YANG), but the components of the voucher would not
represent data nodes themselves. Is this correct? If so, this may be a
good place to state so explicitly, to avoid misunderstandings. If not,
please state this as well. Eitherway, the statement "uses [what uses?]
(such as in NETCONF)" needs to be explained (or perhaps be taken out?).
19) Jumping backwards - section 2: Please consider adding acronyms such
as CMS, CBOR, CMS. Just expand the abbreviations. (Cryptographic
Message Syntax is probably not a household word)
20) Line 465 "Which type of voucher": Does the MIME Content-Type
indicate a voucher, or what specific type of voucher? (Please rephrase
as this is currently ambgiuous.) If it is the former (and per section
10.3 I think it is), why (and not the latter)? What is a voucher type;
where are the different voucher types defined in this document; how
would new voucher types be introduced?
YANG MODEL - VOUCHER ARTIFACT
21) Preamble: Section 6.1: To confirm, this tree seems to suggest that a
voucher consists of a mandatory serial number and a bunch of optional
parameters. All of them are optional and they can occur in any
combination.
22) Actually, I don't think this is quite correct, as there are some
interdependencies. Specifically, you indicate later that you cannot
have both a nonce and an expires-on. Why did not choose to represent
this via an optional choice? (It would be preferrable to indicate it
that way - clearer structure, less context dependencies, easier edit
configurations (should that ever be necessary), etc.)
To do so, you would need to do something like:
choice validity {
case perpetual: {leaf nonce {....}}
case limited: {leaf exprires-on {....}}
}
Now, the question if the choice itself would be optional or mandatory.
If it can be one or the other, or none (but not both), what is put here
is what you want. If you want it to be an either-or, you need to make
the choice mandatory.
23) From your descriptions of the data nodes, it seems that there are
other interdependencies. For example, what is the relationship between
"pinned-domain-cert", "pinned-domain-pubk",
"pinned-domain-pubk-sha256"? If they are alternatives, again, I would
put them in a choice. If there are some other data nodes that should be
present with one of the alternatives, but not others (e.g.
"comain-cert-revocation-checks"?), those should be included as part of
the choice. Are there any other such interdependencies?
24) Section 6.2 examples (Lines 496ff): Both examples are JSON.
Presamably, JSON is only one option to encode a voucher, correct? Would
it make sense to put another example in XML and/or in CBOR?
25) "last-renewal-date": Some questions regarding the underlying data
model:
25a) would it make sense to put the last-renewal-date next to the
created-on?
25b) It seems that "last-renewal-date" can be present only if there is
also an "expires-on". Again, you could simply move this into a
corresponding choice, e.g. something like:
choice validity {
case perpetual: {leaf nonce {....}}
case limited: {leaf expires on {...}
{leaf last-renewal-date{...}}
(and, if within the choice any node must be present, indicate by making
the corresponding leaf mandatory within the choice, not just the choice
itself - not sure if this applies here)
26) Lines 578ff: Is there a deeper meaning to the order of authors?
This is different from the order of authors on the document itself.
27) Line 612: Revision date should match document date.
28) Lines 624 and 609: Probably both should say "RFC ZZZ", and then put
a note to the RFC Editor to replace RFC ZZZ with the RFC number of this
document when it is published.
29) Line 627: I would probably remove this comment as it does not
really add anything.
30) Line 629: Reusable groupings, like type definitions etc, should be
defined before they are being used. This makes the module definition
cleaner and more easy to read. Please move the definition of
voucher-artifact-grouping before your top level statement.
31)
31a) Line 632: I don't understand this statement. What do you mean
with "grouping defined for future augmentations"?
31b) If you want to allow for future extensions, it would be good to
indicate what you have in mind, specifically. Provide an example (not
inside the data model definition, but in a subsequent section).
31c) Lines 634ff: Why are you choosing to have the voucher container
within the grouping? Would it make more sense to me is to have a design
pattern as follows: Define a container, then define a separate grouping
for each type of voucher.
Groupings define groups of nodes that can be reused in various parts of
your data model. In general, you would augment data nodes, not
groupings. I would hesitate to augment groupings and I am not sure if
it is even legal, really you should apply an augmentation where the
grouping is used.
For example, if you have a new type of voucher for which new / different
data nodes apply, here is your boilerplate for how you would do it. It
would be useful if you could explain what scenarios for future
augmentation you have in mind. If you wanted to add groups of
additional parameters later, a preferred way of doing this would be to
define a new grouping, then augment the data nodes where you want to
apply whatever is defined in the new grouping with a uses statement.
Likewise, if you wanted to add a new choice for a new type of voucher,
you would define the grouping for the new voucher, then augment the
choice with a new case that uses the grouping. (This scenario, by the
way, is another reason to make use of the "choice" construct, instead of
trying to model interdependencies via when/must statements. You can add
a new case to the choice without having to update any of the existing
definitions - e.g. when adding a new node "foo", you don't need to
update the old definitions of "bar" which may have some condition that
bar can only be when you don't have foo, etc.)
For example:
voucher
+-- General Voucher Attributes (serial-number, created-on, assertion,
type(?))
+-- (Voucher-type)
+--:(Type-A) uses type-a-grouping;
+--:(Type-B) uses type-b-grouping;
If later you define a Type-C:
augment /voucher/Voucher-type {
case Type-C: uses type-c-grouping
}
32) Line 665 - leaf assertion misses a description statement
33) Line 667: type enumeration - and all the enums in the lines that
follow: Why are you choosing an enumeration here?
33a) IMHO it is preferable to define a type for the enumeration, then
make the assertion of that type. This will allow you to reuse the same
data type somewhere else (e.g. in notifications, or wherever it may come
up).
33b) Did you consider using identityref instead? This may be more
appropriate here, specifically if you are looking to introduce new
options over time. What you would do is the following:
- Define an identity "assertion-type" for your assertion types
- Define a separate identity for each of your enum options, with
assertion-type as base identity
- Then, define your assertion leaf to be an identityref to
assertion-type (you could define a separate type for the identityref
itself)
Now, whenever you want to add a new option, you simply define a new
identity (still with assertion-type as the base type). This will be
automatically picked up without needing to revise other modules. I
believe this is a more elegant and more easily maintainable design
versus what you have now.
34) Line 717: Is it possible that you would like to restrict the length
of the string, or that there will be syntax restrictions for the serial
number that is being applied? If that is the case, it may be preferable
to define a custom type. (This one depends a bit on how you are
planning to use it: if you are never going to validate anything, what
you have now may be sufficient.)
35) Line 752:
35a) Also here the question if you would like to introduce a type
definition for an X.509 v3 certificate? This will allow you to reuse
this & make verification easier to implement. (If you expect to never
verify anything and just need a place where to put it, what you have
will be sufficient.)
35b) Is there a chance that you might ever have to support different
types of certificate here? For example, what if an X.509 v4 came out,
would you want to have the option of supporting this as part of
pinned-domain-cert as well? If no, maybe consider reflecting X509v3 in
the name (i.e., use a specific name instead of the current fairly
generic one). If yes, be aware that the way it is currently defined
does not allow for that.
36) This leaf is optional. What if it is absent - what is the normal
PKIX behavior that applies?
37) Lines 798 and 853: as per the earlier comments, by using the choice
construct you do not need these constraints.
38) Lines 815 ff - leaf pinned-domain-pubk:
38a) The description is a bit confusing. What happens if both
algorithms are supported? Why are the two algorithms combined into one
leaf, another one in a separate leaf (pinned-domain-pubk-sha256)?
38b) The description contains information that probably does not belong
here. This should define the semantics of the data model and what the
data nodes represent, not which algorithms an implementation should or
must support. Please rephrase accordingly (indicating what the leaf may
or may not contain).
38c) Analogous comment to earlier - have you considered defining a
separate type for this (since you are very specific about the
constraints that the contents of this must adhere to - phrasing this as
part of a separate typedef may make the implementatin of certain aspects
such as validation easier).
39) Line 865 // from BRSKI-CLOUD: What does this mean? What does it do
to BRSKI-CLOUD if it is defined here as well - is it removed there, will
there be redundant information? If this is merely intended as mental
note to the authors, this should probably be removed.
40)Line 871: Cloud Registrar is a new term that has not been
introduced. Is this the same as the Join Registrar?
41) Line 877: Leaf should probably renamed to reflect that is not a
configuration, but a location where the configuration is found.
42) Line 881: s/to which/from which/
43) Lines 903ff: I am not sure if I understand this.
43a) What if tooling becomes mature? Would this then no longer
considered normative? What happens if something changes? This section
needs to be rephrased to be clear on what is being standardized and what
not. Probably it should also simply state how to extend this or modify
this should it become required in the future, and the circumstances
under which this would occur.
43b) As it sounds that this may be a growing catalogue, really it seems
more appropriate to define a registry and have that registry be
maintained by IANA. In that case, you probably need to add a section
10.4 for this.
44) Lines 928ff: With assertion type is subject to extension, I strongly
recommend using identity and identityref instead of enum (as outlined
earlier).
45) Lines 980 and 988: Please be consistent - CMS SignedData structure
vs CMS structure
46) Line 988ff: Is this background, or does this specify the artifact?
Might be good to tighten up the language a little bit.
47) Section 6.5: It would be good to show an example.
SECTION 7 FF
48) Lines 1030 ff: Analogous comments to earlier to be clear on what it
means that this definition has been moved here. If this is simply a
historical explanation, this can probably be removed.
49) YANG module for the IETF-Voucher-Request: Several analogous
comments from section 6 apply here as well, e.g.
49a) Line 1194: Define groupings before they are used
49b) e.g. Lines 1238, 1262, 1278, 1302, 1320, 1329, 1362: Consider type
definitions for binary leaf types
49c: Lines 1169 and 1184: Add RFC author instruction (to replace RFC
XXXX with proper RFC number)
49d) Line 1172: Align revision date with draft.
50) Line 1139: Michael Behringer is author of the YANG module but not
author on the draft? (Not sure about the IETF policy here; it just
strikes me as odd.)
51) Line 1199: A refinement to make mandatory false, when it is not
mandatory by default, is not needed.
52) Lines 1202, 1221, and from their description also 1208, 1214, etc:
Note that "mandatory false" means "optional". It does not mean "must
not be included. I am not sure if this is what you mean. Putting into
the description that if you have this, it is really malformed, makes
even less sense. If you want certain fields to be part of a voucher but
not of a voucher request, a better approach may be to use groupings:
One with the data nodes that are only part of voucher-request (but not
the voucher), one which are only part of the voucher (but not the
voucher request), and one that is common. Then say the voucher "uses"
generally-applicable grouping and voucher-only-grouping, voucher request
uses generally-applicable grouping and roucher-request-only-grouping.
Done.
53) Lines 1262 ff (the various certificates): analagous to the earlier
comments, consider using of choice (and augmenting to add cases) to
cover more cases.
54) Line 1672: Add email address? (perhaps of ANIMA WG chairs or ops
directorate?)
55) Line 1938: Having Toerless acknowledged when he joined the authors
sounds a bit odd; his name should probably be removed here. At the same
time, per comment 50, Michael Behringer probably should be acknowledged,
possibly even as a contributor (since coauthor of a module that makes up
a substantial part of the document).
56) Please note that the YANG module validation contains errors and
warnings. (I would not worry about this at this point as a number of
revisions will likely have to be made anyhow.)
Cheers
--- Alex
_______________________________________________
Anima mailing list -- anima@ietf.org
To unsubscribe send an email to anima-le...@ietf.org
