Kent: Thank for addressing all my comments, thank for clarification for the python code. I just feel your python code in the appendix make IANA manager's life easier,:-) -Qin -----邮件原件----- 发件人: Kent Watsen [mailto:kent+ietf@xxxxxxxxxx] 发送时间: 2024年2月6日 7:15 收件人: Qin Wu <bill.wu@xxxxxxxxxx> 抄送: ops-dir@xxxxxxxx; draft-ietf-netconf-ssh-client-server.all@xxxxxxxx; last-call@xxxxxxxx; netconf@xxxxxxxx 主题: Re: [netconf] Opsdir last call review of draft-ietf-netconf-ssh-client-server-36 Hi Qin, Thank you for the OpsDir review. Please find responses below. FWIW, many of your comments (and my responses) apply to the tls-client-server draft as well. Kent > On Feb 5, 2024, at 7:51 AM, Qin Wu via Datatracker <noreply@xxxxxxxx> wrote: > > Reviewer: Qin Wu > Review result: Ready > > 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. Quite right. Below is the updated abstract. I made a similar update to the TLS draft. This document defines seven YANG 1.1 modules. Three "IETF" modules, and four supporting "IANA" modules. The three IETF modules are: ietf-ssh-common, ietf-ssh-client, and ietf-ssh-server. The "ietf-ssh-client" and "ietf-ssh-server" modules are the primary productions of this work, enabling the configuration and monitoring of SSH clients and servers. The four IANA modules are: iana-ssh-encryption-algs, iana-ssh-key- exchange-algs, iana-ssh-mac-algs, and iana-ssh-public-key-algs. These modules each define YANG identities for an IANA-maintained algorithm registry. > 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 …… I was just thinking exactly the same. The “Template for” prefix was suggested in one of the IESG-reviews but, in context, “Considerations for" makes more sense. Fixed in all nine drafts. > 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. ” Fixed in both the SSH and TLS drafts. > 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. ” Fixed (per above comment) > 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. Fixed (per above comment) > 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. Fixed (per above comment) > 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. Yes, they should follow rfc8047bis template. I just updated the four instances in this document and the one instance in the TLS document. > 8. Appendix A > It looks the gen-identities 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. The problem with that is that each IANA registry is different: - column names - column ordering - how deprecate/obsolete are expressed, if at all - how reserved/unassigned entries are expressed - etc. The `gen-identities.py` script included in the SSH and TLS drafts are 85% overlap, but vary in important ways. > 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. Fixed. It was easy once I found the “textwrap” Python library. PS: I could publish these update now, but I think that I’ll wait until getting (hopefully) closure on the first three drafts going thru IESG review... Kent -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
