Re: [yang-doctors] Yangdoctors early review of draft-ietf-rats-network-device-subscription-02

["Eric Voit (evoit)" <[e-mail]>]

Hi Jürgen,

Thanks for the comments.  They have been adopted as described in-line, and posted as pull-requests on our GitHub site.

> From: Jürgen Schönwälder, October 27, 2022 11:30 AM
> 
> Reviewer: Jürgen Schönwälder
> Review result: On the Right Track
> 
> * Abstract
> 
>   s/other Conceptual Message/other Conceptual Messages/

Fixed

> * Introduction
> 
>   s/there are can be additional/there can be additional/

Fix already applied on GitHub.

>   For my taste, this is a somewhat difficult to understand
>   introduction, as the text immediately gets lost in details.

This document is an extension of draft-ietf-rats-yang-tpm-charra.   The context needs to be understood to parse this document.

> * Sequence Diagram
> 
>   What does "Figure 1 below is a sequence diagram which updates Figure
>   5" mean? Is this a formal update of the figure? Also "This sequence
>   diagram replaces the [..]" makes me wonder. Perhaps you mean extend
>   or adapt?

Text now says...

Figure 1 below is a sequence diagram which updates Figure 5 of [I-D.ietf-rats-tpm-based-network-device-attest]. This sequence diagram adapts [I-D.ietf-rats-tpm-based-network-device-attest] by replacing the TPM-specific challenge-response interaction model with a {{RFC8639}} Dynamic Subscription to an <attestation> Event Stream
 
>   I have not tried to understand all the RATS details.
> 
> * Configuring the <attestation> Event Stream
> 
>   s/current configuration of stream/current configuration of a stream/

Fix already applied on GitHub.

> * YANG Module
> 
>   Did the module not change since 2021-05-11? Otherwise, update the
>   timestamp.
> 
>   Update the copyright year to 2022.
> 
>   There is no reference for ietf-tcg-algs, did not know where to find
>   it. I figured out by searching that it is also in
>   draft-ietf-rats-yang-tpm-charra.
> 
>   What are "conceptual YANG specifications"??

Removed 'conceptual'.
 
>      identity pcr-unsubscribable {
>        base sn:establish-subscription-error;
>        description
>          "Requested PCR is subscribable by the Attester.";
>      }
> 
>   So is it subscribable or unsubscribable?

Fixed.   (By adding 'un'.)

>   Add a units statement to tpm20-subscription-heartbeat.

Added 
            units seconds;
 
>   I did not spot any YANG specific issues, whether the definitions
>   make sense from the RATS perspective, I can't judge. This requires
>   reviews by people who understand RATS technology.
> 
>   Security considerations are missing.

Needs to be written

>   IANA considerations are missing.

Needs to be written.
 
>   I did not compile the YANG module since getting all YANG modules it
>   depends on and their dependents into place is, well, non-trivial, in
>   particular with intermitted Internet access.

Just compiled ok at https://www.yangcatalog.org/yangvalidator: with:
confd: confd-7.8    pyang: 2.5.3    xym: 0.6.1    yangdump: 20.10-9    yanglint: 2.0.231    


Thanks,
Eric