[Anima] Iotdir early review of draft-ietf-anima-constrained-voucher-21

[Henk Birkholz via Datatracker <[e-mail]>]

Reviewer: Henk Birkholz
Review result: Ready with Nits

I am the assigned IoT Directorate reviewer for
I-D.ietf-anima-constrained-voucher-21 as part of the IoT Directorate's
effort to provide early feedback to IoT-related IETF documents before
being processed by the IESG. Document authors, document editors, WG
chairs and Area Directors should treat these comments just like any other
WG comments. This early review focuses on the body of the memo and does
not extend to the examples in the appendices.

Summary: Between my review of -12 and this review of -21, the document
scope has not changed. The document proposes a solution that introduces
CBOR-based voucher and EST-CoAPS based enrollment as a more concise
variant of JSON-based vouchers and EST over HTTPS based enrollment as
defined by RFC8995 and RFC8366. The general concept makes a lot of
sense as CBOR's (and in consequence COSE's) footprint on message size,
processing code size, and computation resources combined is
significantly lower in comparison with JSON (or CMS), ultimately
enabling future interoperability with a host of constrained node types.

# Comparison to Early Review of draft-ietf-anima-constrained-voucher-12

The document improved significantly in content, structure, and style
since -12 and is now Ready with Nits. A detailed comparison follows.

(0) Title

Title, abstract and introduction look pretty much in sync now. Thanks
for addressing that issue.

(1) Abstract

Abstract reads conclusive by itself now. Thanks for addressing that
issue.

Small non-blocking nit in abstract:

OLD:
"Constrained BRSKI is a variant of the BRSKI protocol, which uses an
artifact signed by the device manufacturer called the "voucher" which
enables a new device and the owner's network to mutually authenticate."

NEW:
"Constrained BRSKI is a variant of the BRSKI protocol, which uses an
artifact signed by the device manufacturer called the "voucher" that
enables a new device and the owner's network to mutually authenticate."

(2) Introduction

Domain context and terms used are mostly well introduced now. Thanks
for addressing that issue.

Small non-blocking nit in introduction:
Not sure if Registrar is a term used colloquial enough to be applied
without reference to the duties of that role.

(3) Overview of Protocol

Terms used here, such as 'proximity' and also introduced earlier and
now applied here, such as 'Registrar', are well-introduced in the
Terminology Section. Thanks for addressing that issue.

(4) Resource Discovery, URIs and Content Formats

The examples and expositional text provided are integrated organically
and quite comprehensive. Thanks for addressing that issue.

Small non-blocking nit in this Section:

Table 1 does not include the term 'short name' typically used in the
current document in its content our its label. Slight disconnect with
the text.

(5) Extensions to BRSKI

Avoiding 'CoRE Link Format parsing' found a better place in 6.4.1. and
is now a logical conclusion. The text is still silent about the why
avoidance is useful (same as in current 6.4.1 and 6.5.1), but that's
a nit. Thanks for addressing that issue.

(6) Extensions to EST-coaps

Extensions are introduced quite well now. See next item.

(7) Pledge Extensions

The Table makes creates an excellent overview to help the reader
navigate the context. Thanks for addressing that issue.

(8) Registrar Extensions

Security Considerations content is now aggregate well in this version.

(9) BRSKI-MASA Protocol

Security Considerations content is now aggregate well in this version.

(10) Registrar Identity Selection and Encoding

Nits fixed and current text has significantly improved.

(11) MASA Pinning Policy

Nonce and Nonceless scenarios are illustrated well and the case-by-case
description in the last paragraph is really useful to the reader.
Thanks for addressing that issue.

(12) Pinning of Raw Public Keys

Thanks for adapting normative language suggestions.

# Review of draft-ietf-anima-constrained-voucher-21

All feedback below is composed of non-blocking nits and editorial
suggestions (mostly phrasing, missing commas, etc., that I found along
the way). Four high-level comments:

0. Quite a lot of sentences start with "Note"/"Note that", which I
   think can easily be omitted - especially, if there is normative
   language in that sentence.

1. This document heavily relies on certification paths. Certification
   paths are referred to as chains in the document. While it is not the
   primary purpose of this document to educate the reader on
   certification paths and trust anchors, I think the document might
   still benefit from a quick recap on  "what are the two ends of a
   certification path", "most specific certificate", and "certification
   paths are called chains in this document" in on place.

2. Why are fours dots ("....") used as a readable abbreviation of bytes
   in a CBOR diagnostic instead of three dots ("...")?

3. Some DT references are outdated.

Some editorial nits (certainly not all):

(0) Abstract

OLD
"Constrained BRSKI is a variant of the BRSKI protocol, which uses"

NEW
"Constrained BRSKI is a variant of the BRSKI protocol that uses"

(1) Introduction

OLD
"Secure enrollment of new nodes into constrained networks with
constrained nodes"

NEW
"Secure enrollment of new nodes into constrained node networks"

(2) Updates to RFC8366 and RFC8995

If you span a sentence through an itemized list, you need an 'and'.

OLD
"clarifies when new trust anchors should be retrieved (Section 6.5.1),"

NEW
"clarifies when new trust anchors should be retrieved (Section 6.5.1),
and"

OLD
"provides the option to return trust anchors in a simpler format
(Section 6.5.3),"

NEW
"provides the option to return trust anchors in a simpler format
(Section 6.5.3), and"

(3) DTLS Version

As only two protocol versions are allowed, this sentence can be simplified.

OLD
"This may occur for example if a legacy device gets software-upgraded
to support Constrained BRSKI."

NEW
"This may occur, for example, if a legacy device gets software-upgraded
to support Constrained BRSKI."

OLD
"However, for security reasons the Registrar MAY be administratively
configured to support only a particular DTLS version or higher."

NEW
"However, for security reasons the Registrar MAY be administratively
configured to support only DTLS 1.3."

(4) TLS Client Certificates: IDevID authentication

OLD
"Subsequently the Pledge will send a Pledge Voucher Request (PVR)""

NEW
"Subsequently, the Pledge will send a Pledge Voucher Request (PVR)"

(5) DTLS Handshake Fragmentation Considerations

Maybe add a reference to RFC 7959 in the last paragraph.

(6) Registrar and the Server Name Indicator (SNI)

"Threfore" -> "Therefore"

(7) Resource Discovery, URIs and Content Formats

OLD
"To keep the protocol messages small the EST-coaps and Constrained
BRSKI URIs are shorter than the respective EST and BRSKI URIs."

NEW
"To keep the protocol messages small, the EST-coaps and Constrained
BRSKI URIs are shorter than the respective EST and BRSKI URIs."

Maybe put more context and less colloquial language into this note
and make it a sentence without the parentheses:
"(So, a query for rt=brski, without the wildcard character.)"

(8) CoAP EST Resource Discovery and BRSKI

OLD
"Once the Pledge discovers an IP address and port number that connects
to the Registrar (probably via a Join Proxy), and it establishes a DTLS
connection."

NEW
"Once the Pledge discovers an IP address and port number that connect
to the Registrar (probably via a Join Proxy), it establishes a DTLS
connection."

OLD
"No further discovery of hosts or port numbers is required, but a
pledge that can do more than one kind of enrollment (future work offers
protocols other than [RFC9148]), then a pledge may need to use CoAP
Discovery to determine what other protocols are available."

NEW
"No further discovery of hosts or port numbers is required, but if a
pledge can do more than one kind of enrollment (future work offers
protocols other than [RFC9148]), then a pledge may need to use CoAP
Discovery to determine what other protocols are available."

(9) Pledge Extensions

"1. if the voucher" -> "1. If the voucher"

OLD
"2. Using this CA certificate as trust anchor it proceeds with EST
simple enrollment (/sen) to obtain its provisionally trusted LDevID
certificate."

NEW
"3. Using this CA certificate as trust anchor, it proceeds with EST
simple enrollment (/sen) to obtain its provisionally trusted LDevID
certificate."

(10) Registrar Extensions

The fifth paragraph about "4.06 Not Acceptable" could use a normative
MUST, I think.

(11) Protocol and Formats

"oursourced" -> "outsourced"

(12) Registrar Certificate Requirement

OLD
"In summary for typical Registrar use, where a single Registrar
certificate is used, then the certificate MUST have EKU of:"

NEW
"In summary for typical Registrar use, where a single Registrar
certificate is used, the certificate MUST have EKU of:"

(13) Considerations for use of IDevID-Issuer

"but"?
"[RFC8366] and [RFC8995] define the idevid-issuer attribute for voucher
and voucher-request (respectively), but they summarily explain when to
use it."

OLD
"However, there situations where the one to one relationship may be
broken."

NEW
"However, there are situations where the one to one relationship may be
broken."

(14) Signing of Pledge Voucher Request (PVR)

OLD
"This can occur for example if a Pledge has multiple IDevID
identities."

NEW
"This can occur, for example, if a Pledge has multiple IDevID
identities."

(15) Signing of voucher by MASA

OLD
"This can occur for example if a Pledge has multiple IDevID
identities."

NEW
"This can occur, for example, if a Pledge has multiple IDevID
identities."

OLD
"CA identity in its Trust Store which is not the identity that signs
the voucher"

NEW
"CA identity in its Trust Store that is not the identity that signs
the voucher"

(16) GRASP discovery

Figure 7 (and later diagrams) use CDDL, which is not indicated, and is
breaking with the CBOR Diag example pattern before and after. The lables
should indicate what is seen. (Maybe add a cbor diag for the M_FLOOD
examples, too?)

(17) CoAP Discovery

The Figures in 10.1.2. would benefit from a label.

The acronym GUA is introduced but never used again.

(18) 6TSCH Deployments

Please rephrase the reference to I-D.ietf-lake-edhoc and I would also avoid
future work references here.

(19) Generic networks using GRASP & Generic networks using mDNS

Maybe:
"WIFI" -> "wireless networks"

(20) Thread networks using Mesh Link Establishment (MLE)

OLD
"Once a suitable router is selected the new device initiates a DTLS
transport-layer secured connection to the network's commissioning
application, over a link-local single radio hop to the selected Thread
router."

NEW
"Once a suitable router is selected, the new device initiates a DTLS
transport-layer secured connection to the network's commissioning
application, over a link-local single radio hop to the selected Thread
router."

(21) Design Considerations

The context change to YANG is pretty abrupt and it might not be clear
to the reader why that is coming up here.

(22) Duplicate serial-numbers

OLD
"For example, imagine the same manufacturer makes light bulbs"

NEW
"For example, the same manufacturer makes light bulbs"

(23) Acknowledgements

OLD
"Russ Housley , Daniel Franke and Henk Birkholtz"

NEW
"Russ Housley, Daniel Franke and Henk Birkholz"

(24) Library Support for BRSKI

The sub-Sections might benefit from a bit more expositional text.

Viele Grüße,

Henk