Thanks to the authors for working on this draft. I want to specially thank the authors for consolidating the YANG module ietf-voucher-request along with ietf-voucher and its changes into this draft. By combining support for both JSON/CMS and CBOR/COSE, the draft provides a unified "Voucher" module across BRSKI and cBRSKI.
I will note that this review covers the -26 version of the draft. The authors have posted the -27 version of the draft. If there is text in -27 that is different and I want to comment on, I will do that separately. I will admit I am not a security expert, and most of this review focuses on the YANG modules in the draft. I will probably have a security expert review this document. Onto the review. Obsoletes: 8366 (if approved) Updates: 8366, 8995 (if approved) The document says that it updates as well as obsoletes RFC 8366. What gives? "Abstract", paragraph 3 > This document updates RFC8366: it includes a number of desired > extensions into the YANG module. The Voucher Request YANG module > defined in RFC8995 is also updated and now included in this document, > as well as other YANG extensions needed for variants of BRSKI/ > RFC8995. Same question here. Does this document update or obsoletes RFC 8366? Section 2, paragraph 3 > Bootstrapping: The process where a Pledge obtains cryptographic key > material to identify and trust future interactions within a > specific Domain network. Based on imprinted key material provided > during the manufacturing process (see: Imprint). This term was > used in [RFC8366], but has been supplanted by the term Onboarding. The statement "Based on imprinted ..." sounds incomplete. Section 8.3, paragraph 2 > <CODE BEGINS> file "[email protected]" > =============== NOTE: '\' line wrapping per RFC 8792 ================ I note that the datatracker records 7 issues with the modules in question. Is this a false positive or an actual error? libyang err : Refine(s) target node "last-renewal-date" in grouping "voucher-artifact-grouping" was not found. (/ietf-voucher-request:{extension='sx:structure'}/voucher/{uses='voucher-request-grouping'}/{uses='vch:voucher-artifact-grouping'}) libyang err : Refine(s) target node "domain-cert-revocation-checks" in grouping "voucher-artifact-grouping" was not found. (/ietf-voucher-request:{extension='sx:structure'}/voucher/{uses='voucher-request-grouping'}/{uses='vch:voucher-artifact-grouping'}) libyang err : Refine(s) target node "assertion" in grouping "voucher-artifact-grouping" was not found. (/ietf-voucher-request:{extension='sx:structure'}/voucher/{uses='voucher-request-grouping'}/{uses='vch:voucher-artifact-grouping'}) libyang err : Refine(s) target node "idevid-issuer" in grouping "voucher-artifact-grouping" was not found. (/ietf-voucher-request:{extension='sx:structure'}/voucher/{uses='voucher-request-grouping'}/{uses='vch:voucher-artifact-grouping'}) YANGLINT[E]: Parsing schema module "[email protected]" failed. In addition, while xym is able to extract the module from the draft, pyang fails. % pyang [email protected] [email protected]:173: error: the escape sequence "\ " is illegal in double quoted strings Removing the two \ in the module fixes this issue. Section 8.3, paragraph 3 > import ietf-yang-types { > prefix yang; > reference > "RFC 6991: Common YANG Data Types"; > } > import ietf-inet-types { > prefix ietf; > reference > "RFC 6991: Common YANG Data Types"; > } Please update the reference to RFC 9911. Section 8.3, paragraph 3 > Author: Kent Watsen > <mailto:[email protected]> > Author: Michael Richardson > <mailto:[email protected]> > Author: Max Pritikin > <mailto:[email protected]> > Author: Toerless Eckert > <mailto:[email protected]> > Author: Qiufang Ma > <mailto:[email protected]> > Author: Esko Dijk > <mailto:[email protected]>"; The draft has 5 authors, but the YANG module has six? What gives? Section 8.3, paragraph 6 > grouping voucher-artifact-grouping { > description > "Grouping to allow reuse/extensions in future work."; No need to call a grouping a -grouping. If this is being used in this module only, then there is no backward compatibility issue in making the change. Section 9.2, paragraph 3 > import ietf-yang-structure-ext { > prefix sx; > } Please add a reference statement here. Section 9.2, paragraph 3 > Author: Kent Watsen > <mailto:[email protected]> > Author: Michael Richardson > <mailto:[email protected]> > Author: Max Pritikin > <mailto:[email protected]> > Author: Toerless Eckert > <mailto:[email protected]> > Author: Qiufang Ma > <mailto:[email protected]> > Author: Esko Dijk > <mailto:[email protected]>"; Same question on the number of authors. Section 9.2, paragraph 3 > Copyright (c) 2023 IETF Trust and the persons identified as > authors of the code. All rights reserved. Please update the Copyright tree from 2023 to 2023-2026. Section 9.2, paragraph 6 > grouping voucher-request-grouping { > description > "Grouping to allow reuse/extensions in future work."; No need to call a grouping a -grouping. If this is being used in this module only, then there is no backward compatability issue in making the change. No reference entries found for these items, which were mentioned in the text: [RFC8949], [RFC8995], [draft-ietf-tls-rfc4492bis-17], [ITU.X690.1994], and [RFC7250]. Possible DOWNREF from this Standards Track doc to [ITU-T.X690.2015]. If so, the IESG needs to approve it. Found terminology that should be reviewed for inclusivity; see https://www.rfc-editor.org/part2/#inclusive_language for background and more guidance: * Term "traditional"; alternatives might be "classic", "classical", "common", "conventional", "customary", "fixed", "habitual", "historic", "long-established", "popular", "prescribed", "regular", "rooted", "time-honored", "universal", "widely used", "widespread" ------------------------------------------------------------------------------- NIT ------------------------------------------------------------------------------- All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. Section 8.3, paragraph 2 > Copyright (c) 2023 IETF Trust and the persons identified as > authors of the code. All rights reserved. Please update the Copyright year from 2023 to 2023-2026. Section 8.3, paragraph 6 > reference > "RFC XXXX A Voucher Profile for Bootstrapping Protocols"; > } s/RFC XXXX/RFC XXXX:/ Reference [RFC6125] to RFC6125, which was obsoleted by RFC9525 (this may be on purpose). Found non-HTTP URLs in the document: * 4.2.1.1 * IoTconsultancy.nl * s://www.cl.cam.ac.uk/research/dtg/www/files/publications/public/files/tr.1999.2.pdf These URLs in the document did not return content: * IoTconsultancy.nl * https://en.wikipedia.org/w/index.php?title=Imprinting_(psychology * https://1.ieee802.org/security/802-1ar/ * s://www.cl.cam.ac.uk/research/dtg/www/files/publications/public/files/tr.1999.2.pdf These URLs in the document can probably be converted to HTTPS: * http://www.sandelman.ca/ Section 4, paragraph 15 > scribe the extended semantics. In addition [cBRSKI] uses the serialization me > ^^^^^^^^ A comma may be missing after the conjunctive/linking adverb "addition". Section 5.1, paragraph 5 > on between Pledge and Registrar. Therefore the Pledge's IDevID certificate c > ^^^^^^^^^ A comma may be missing after the conjunctive/linking adverb "Therefore”. Thanks Mahesh Jethanandani [email protected]
_______________________________________________ Anima mailing list -- [email protected] To unsubscribe send an email to [email protected]
