Thanks Xufeng for the detailed review. Please see inline
On Sat, 10 Dec 2022 at 05:56, Xufeng Liu via Datatracker <[email protected]>
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
>
>
>
>
_______________________________________________
OPSAWG mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/opsawg
