Thanks Xufeng for the detailed review. Please see inline
On Sat, 10 Dec 2022 at 05:56, Xufeng Liu via Datatracker <noreply@xxxxxxxx> wrote:
Reviewer: Xufeng Liu
Review result: Ready with Issues
This is a review of the YANG module in draft-ietf-opsawg-mud-tls-10.
Sec 5.1. and 5.2
1) The container “client-profile” is duplicated twice. Is there any reason for
the duplication?
It was added by mistake, I will remove the duplication.
2) As a convention, in IETF YANG modules, the node name of a list is in the
singular form. Above the list there can be a container with a name in the
plural form. For example, in RFC8519, there are a container “acls” and a list
“acl”. Using such a convention, the container “client-profile” would be better
named as “client-profiles”, and the list “tls-dtls-profiles” would be better
named as “tls-dtls-profile”. The same is applicable to other list names, such
as “tls-dtls-profiles”, “cipher-suites”, “extension-types”, and
“signature-algorithms-cert”.
Thanks, I wil fix the above names accordingly.
3) The leaf-list “acceptlist-ta-certs” can be better named as
“accept-list-ta-certs” per RFC8407.
Sure.
4) Default values should be specified or explained for optional leaves like
“spki-hash-algorithm”.
Okay.
5) The leaf “profile-name” is suggested to be renamed to “name”. As described
in Sec 4.3.1. of RFC8407, child nodes within a container or list SHOULD NOT
replicate the parent identifier.
Done.
Sec. 5.3. IANA (D)TLS profile YANG Module
1) This section indicates that there are no IANA-maintained values for
spki-pin-set, and certificate-authority parameters. If so, what are the reasons
to include these two types in this IANA YANG module? What do we expect IANA to
maintain and update?
I see your point, I will remove these types from the IANA YANG module.
Sec. 5.4. MUD (D)TLS Profile Extension
1) The file name of the YANG module is wrong. It seems to be a typo.
Yes, fixed.
2) The statement “import ietf-mud” needs to have a “reference” sub-statement.
Added.
3) The leaf “is-tls-dtls-profile-supported” should have a default value or a
description of the default behavior.
Okay.
Sec 7.
1) In the example, the leaf “profile-name” is needed as it is the key of the
list.
Yes.
Sec 10.1.
1) For iana module, iana-tls-profile, instead of “Registrant Contact: IESG”, we
should have Registrant Contact: IANA
Fixed.
[Nit]:
1) The following code contains a line longer than 69 characters:
leaf hash {
type ianatp:hash-algorithm;
description
"Hash algorithm used with HKDF as defined in RFC5869.";
}
Done.
Cheers,
-Tiru
Thanks,
- Xufeng
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
