Reviewer: Joe Clarke Review result: Ready with Nits I have been asked to perform a last-call review of this draft on behalf of YANG Doctors. This draft specifies a YANG module for conveying a CSR in a Secure Zero Touch Provisioning Bootstrapping Request. It augments the "get-bootstrapping-data" RPC defined in RFC 8572. Overall, I found the draft (and module) ready with the following small issues/nits. Editorial Note: You mention XXXX and AAAA, and you also use BBBB in the module ietf-yang-structure-ext. I know this section will be removed, but you may want to mention this to make sure that substitution is done. === Section 2.1 The YANG tree here deviates from the output produced by pyang --tree-print-structures --tree-line-length=69 --tree-module-name-prefix. In particular, the "flags" column is missing from each element under the augment branch (e.g., the 'w' that appears in pyang's output's flags field). === Section 2.2 You have this text: Some implementation may choose to convey it inside a script (e.g., SZTP's "pre-configuration-script") I'd like to see a reference to RFC 8572 here on where this is defined. I realize I'm being pedantic here, but there are a number of SZTP documents, and I think having a reference helps. === Section 2.2 s/so that the the device/so that the device/ === Module overall: I noticed that the module itself differs in various formatting from pyang -f yang. There are some line and spacing differences, and I don't think it would hurt to canonicalize the module in the draft. === Module's main description: You mention SZTP's 'onboarding-information' response, but this isn't a formal YANG node. That node is conveyed-information. Is it worth clarifying that here in the module description like you did in draft text in section 2.2? I have the same comment in the description of "container csr". The quoted and hyphenated "onboarding-information" might make one think that there is a node that contains this. === Under leaf cmc's description: s/is the TaggedCertificationRequest and it a bodyPartId/is the TaggedCertificationRequest and it consists of a bodyPartId/ There are two instances of this, and I wasn't sure exactly what you wanted to say here. This was my attempt to make it readable. === While you indicate that the algorithm-selected node is mandatory, should this not be the case for the selected-algorithm container that contains it since as you state in the description for cert-req-info, it is an error if the SubjectPublicKeyInfo's algorithm does not match the selected-algorithm. Could an implementer not return this container at all and still be compliant to the module while causing unexpected behavior for the client? In leaf cert-req-info's description: The word “another” (with respect to the 400 Server Error) doesn’t follow here unless one refers to the csr-support presence container above. I think dropping it might make this text a bit less confusing (or copying similar text from the csr-support node’s description. === Section 3.1.2: s/In cases where it it not possible/In cases where it is not possible/ s/key rather then generate/key rather than generate/ -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
