Last Call Review of draft-ietf-netconf-ssh-client-server-36
review-ietf-netconf-ssh-client-server-36-opsdir-lc-wu-2024-02-05-00
Request | Review of | draft-ietf-netconf-ssh-client-server |
---|---|---|
Requested revision | No specific revision (document currently at 40) | |
Type | Last Call Review | |
Team | Ops Directorate (opsdir) | |
Deadline | 2024-02-12 | |
Requested | 2024-01-29 | |
Authors | Kent Watsen | |
I-D last updated | 2024-02-05 | |
Completed reviews |
Genart Last Call review of -37
by Elwyn B. Davies
(diff)
Intdir Telechat review of -38 by Sheng Jiang (diff) Opsdir Last Call review of -36 by Qin Wu (diff) Yangdoctors Last Call review of -03 by Andy Bierman (diff) Yangdoctors Last Call review of -24 by Andy Bierman (diff) Secdir Last Call review of -24 by Barry Leiba (diff) |
|
Assignment | Reviewer | Qin Wu |
State | Completed | |
Request | Last Call review on draft-ietf-netconf-ssh-client-server by Ops Directorate Assigned | |
Posted at | https://mailarchive.ietf.org/arch/msg/ops-dir/Kkc5fL-VNa_q0lUq2YJ-aGZHQlI | |
Reviewed revision | 36 (document currently at 40) | |
Result | Ready | |
Completed | 2024-02-05 |
review-ietf-netconf-ssh-client-server-36-opsdir-lc-wu-2024-02-05-00
I have reviewed this document as part of the Operational directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written with the intent of improving the operational aspects of the IETF drafts. Comments that are not addressed in last call may be included in AD reviews during the IESG review. Document editors and WG chairs should treat these comments just like any other last call comments. This document defines 7 YANG modules for Communication between SSH Client and SSH Server. In these 7 YANG modules, there are three "IETF" modules and four "IANA" modules. This document is well written and I believe it is ready for publication. However I have a few comments on the latest version v-36: Major issues: None Minor issues 1. The abstract is not consistent with Introduction section, in the abstract, only 3 YANG modules are described but in section 1 or introduction section, 7 YANG modules are explored. Suggest to change abstract to align with section 1. 2. The title of each subsection of section 5.1 doesn’t reflect what is documented within it, I think each subsection focuses on security consideration, suggest the following change to each subsection of section 5.1 s/ Template for the ……/ Consideration for the …… 3. Appendix A.1 said: “ This section provides an overview of the "iana-ssh-encryption-algs" module in terms of its identities and protocol-accessible nodes. ” It is clear that iana-ssh-encryption-algs" module doesn’t define Any protocol-accessible nodes such as container node, list node, leaf node, etc. It only defines identities and typedefs.Suggest the following change as follows: NEW TEXT: “This section provides an overview of the "iana-ssh-encryption-algs" module in terms of its identities and typedefs. ” 4. Appendix A.2 said: “ This section provides an overview of the "iana-ssh-mac-algs" module in terms of its identities and protocol-accessible nodes. “ It is clear that "iana-ssh-mac-algs " module doesn’t define Any protocol-accessible nodes such as container node, list node, leaf node, etc. It only defines identities and typedefs. Suggest the following change as follows: NEW TEXT: “This section provides an overview of the " iana-ssh-mac-algs" module in terms of its identities and typedefs. ” 5. Appendix A.3 said: “ This section provides an overview of the "iana-ssh-public-key-algs" module in terms of its identities and protocol-accessible nodes. “ It is clear that " iana-ssh-public-key-algs " module doesn’t define Any protocol-accessible nodes such as container node, list node, leaf node, etc. It only defines identities and typedefs. Suggest the following change as follows: NEW TEXT: “This section provides an overview of the " iana-ssh-public-key-algs " module in terms of its identities and typedefs. 6. Appendix A.4 said: “ This section provides an overview of the "iana-ssh-key-exchange-algs" module in terms of its identities and protocol-accessible nodes. ” It is clear that " iana-ssh-key-exchange-algs " module doesn’t define Any protocol-accessible nodes such as container node, list node, leaf node, etc. It only defines identities and typedefs. Suggest the following change as follows: NEW TEXT: “This section provides an overview of the " iana-ssh-key-exchange-algs " module in terms of its identities and typedefs. 7. Section 6 IANA Section Section 6.3,Section 6.4, Section 6.5, Section 6.6 describe IANA maintained module, but doesn’t follows Guidance for Writing the IANA Considerations for RFCs Defining IANA-Maintained Modules defined in RFC8047bis. So the question is for IANA maintained Module, if such module shadows algorithm name sub-registry of SSH protocol parameters registry, should IANA section follow guidance for Defining IANA Maintained Modules defined in RFC8047bis, My understanding is yes. Similar example can be found in the section 3 of RFC7224. 8. Appendix A It looks the get identity python code is specific to SSH related encryption algorithm, I am wondering whether there is more generic code for get identity or get enum to automatically help IANA generate IANA maintained modules every time the new value is added? If the answer is, I think such code can be integrated into RFC8407bis as well. 9. There are 31 errors spot by the current YANG validation tool. It looks line wrapping per RFC 8792 is root cause for such errors. I am wondering whether YANG validation tool in the datatracker should be upgraded to ignore line folding per RFC8792 or provide YANG validation check after removing line wrapping from YANG module code.