Re: [OPSAWG] Yangdoctors last call review of draft-ietf-opsawg-mud-tls-10

[tirumal reddy <[e-mail]>]

Thanks Xufeng for the detailed review. Please see inline

On Sat, 10 Dec 2022 at 05:56, Xufeng Liu via Datatracker <[e-mail]>
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
>
>
>
>