Re: [Rats] Review of draft-ietf-rats-yang-tpm-charra-21

[Henk Birkholz <[e-mail]>]

As soon as Kent & Rob unblock C455, we can talk to RFCed and check what 
they think of the changes. We are past all content changes, but I agree 
that these changes look editorial. We'll have to wait for our missref.

On 29.08.23 03:24, Panwei (William) wrote:
> 
> I've reviewed the changes at https://github.com/ietf-rats-wg/basic-yang-module/pull/132, they look fine to me.
> The current changes are really minor and I think we don't need to reset the document in the IETF review cycle.
> By the way, I think the change of the reference of RATS architecture from I-D to RFC 9334 will also be made by the RFC editor when publishing it.
> 
> --snip--
>> The reason for the first part of the additional text is that there are router/switch
>> vendors (including Cisco) provide cryptoprocessors with interfaces compliant
>> to the TPM1.2 and TPM2.0 interface specification.
>> However these cryptoprocessors have not gone through the TCG's compliance
>> verification process.  So a decision was made not to claim compliance.
>>
>> As for the statement ", included in the device components of the composite
>> device the YANG server is running on."   Likely we can delete this as the
>> information is covered outside the Abstract.   Do you have a strong
>> preference
>> for deletion?
> 
> I suggest leaving the text as it is.
> --snip--
> 
> Regards & Thanks!
> Wei Pan (潘伟)
> 
>> -----Original Message-----
>> From: RATS <[e-mail]> On Behalf Of Eric Voit (evoit)
>> Sent: Tuesday, August 29, 2023 1:41 AM
>> To: Hannes Tschofenig <[e-mail]>; Henk Birkholz
>> <[e-mail]>
>> Cc: [e-mail]
>> Subject: Re: [Rats] Review of draft-ietf-rats-yang-tpm-charra-21
>>
>> Hi Hannes,
>>
>> Changes below are proposed into:
>> https://github.com/ietf-rats-wg/basic-yang-module/blob/master/draft-ietf-ra
>> ts-yang-tpm-charra.md
>>
>> Once we are all aligned, we can do a single push to update the draft version.
>> Before doing that, we will need to make sure that this doesn't reset the
>> document in the IETF review cycle first as already cleared the IESG.
>>
>>> From: RATS <[e-mail]> On Behalf Of Hannes Tschofenig
>>> Sent: Monday, July 3, 2023 4:22 AM
>>> To: [e-mail]
>>> Subject: [Rats] Review of draft-ietf-rats-yang-tpm-charra-21
>>>
>>> Hi all,
>>>
>>> I have read draft-ietf-rats-yang-tpm-charra-21. The document reads
>>> well. I just have a few small remarks.
>>>
>>> # Abstract
>>>
>>> Expand the acronym: RPCs - Remote Procedure Calls
>>
>> Done
>>
>>> s/roots of trust for measurement/root of trust for measurements
>>
>> Google has:
>> - 3850 results for "roots of trust for measurements" and
>> - 3 results for "roots of trust for measurements"
>>
>> Are you able to describe some advantages for this change?
>>
>>> This sentence is very complex: What are you trying to say? The
>>> document is really about TPMs.
>>>
>>> I suggest to shorten it
>>>
>>> FROM:
>>>
>>> The module defined requires at least one TPM 1.2 or TPM 2.0 as well as
>>> a corresponding TPM Software Stack (TSS), or equivalent hardware
>>> implementations that include the protected capabilities as provided by
>>> TPMs as well as a corresponding software stack, included in the device
>>> components of the composite device the YANG server is running on.
>>>
>>> TO:
>>>
>>> The module defined requires at least one TPM 1.2 or TPM 2.0 as well as
>>> a corresponding TPM Software Stack (TSS).
>>
>> The reason for the first part of the additional text is that there are router/switch
>> vendors (including Cisco) provide cryptoprocessors with interfaces compliant
>> to the TPM1.2 and TPM2.0 interface specification.
>> However these cryptoprocessors have not gone through the TCG's compliance
>> verification process.  So a decision was made not to claim compliance.
>>
>> As for the statement ", included in the device components of the composite
>> device the YANG server is running on."   Likely we can delete this as the
>> information is covered outside the Abstract.   Do you have a strong
>> preference
>> for deletion?
>>
>>> # Section 1
>>>
>>> rolling hash is the incorrect term for how PCR values are generated.
>>> See https://en.wikipedia.org/wiki/Rolling_hash for a definition of how
>>> a rolling hash works. You would use a rolling hash in creating a
>>> transcript hash for TLS when you do not want to maintain a copy of the
>>> entire message exchange transcript.
>>
>> Both are needed.  The sequential series of messages is recorded in the logs.
>> And the PCR (maintained in protected hardware) is the hash this sequential
>> series of entries.   It is this hash which allow you to verify the
>> completeness/accuracy of the log entries.
>>
>>> [I-D.ietf-rats-architecture] became an RFC. Update the reference
>>> throughout the document
>>
>> Done.
>>
>>> # Section 2 The YANG Module for Basic Remote Attestation Procedures
>>>
>>> What does "Basic" refer to? Is there a "sophisticated" or "advanced"
>>> version somewhere else? If not, omit the term "basic".
>>
>> There are advanced versions.  For an example, see:
>> draft-voit-rats-trustworthy-path-routing
>> which is an instance of nested attestations as described in draft-ietf-rats-ar4si
>> Section 3.2
>>
>>> You write:
>>> "
>>> The method for communicating the relationship of each individual TPM
>>> to specific measured component within the Composite Device is out of
>>> the scope of this document.
>>> "
>>>
>>> Is this functionality described in some other document? If so, where
>>> is it described? If not, what is the implication: what am I unable to do?
>>
>> You need an out-of-band provisioning mechanism which lets you know the
>> type of cryptoprocessor, the equipment within which it sits, and its public key.
>> This document does not describe this because these elements of business
>> context are inherited from the companion document:
>> draft-ietf-rats-tpm-based-network-device-attest
>>
>>> 2.1.1.3.2. 'tpm20-challenge-response-attestation'
>>>
>>> Example is not really an example. Here is a snippet:
>>>
>>>      <certificate-name
>>>          xmlns="urn:ietf:params:xml:ns:yang:ietf-keystore">
>>>          (instance of Certificate name in the Keystore)
>>>      </certificate-name>
>>>
>>> Could you include an example with real values? Should be easy for you
>>> to copy-and-paste an example from one of your implementations. It
>>> would also be good to have examples for the other functionality as
>>> well. Readers, like me, like examples. In this specific case I am
>>> interested to see how the certificate identification looks like.
>>
>> If you look at the authoritative YANG draft "draft-ietf-netconf-keystore"
>> instance data in Section 3 it would be "Manufacturer-Generated IDevID Cert"
>> which is less descriptive than what we have now.   This is because the
>> instance data is just a reference to the full set of key info described throughout
>> "draft-ietf-netconf-keystore".  Trying to keep the documents as independent
>> is traditional with yang models so that we don't have to worry about keeping
>> instance data in synch across a large number of drafts.
>>
>>> A small note on the example: Shouldn't the nonce element be called
>>> nonce-value? Here is the text from the example:
>>>
>>>      <nonce>
>>>
>>> 0xe041307208d9f78f5b1bbecd19e2d152ad49de2fc5a7d8dbf769f6b8ffdeab9
>>>      </nonce>
>>
>> Yes, Good catch.  Fixed.
>>
>>> 2.1.1.5. Data Nodes
>>>
>>> You use the term "compute node" several times throughout the document
>>> but it is not defined. Is there a definition you can reference?
>>
>> The compute node is defined in the YANG model, which itself references
>> RFC6933
>>
>>        list compute-node {
>>          key "node-id";
>>          unique "node-name";
>>          config false;
>>          min-elements 2;
>>          description
>>            "A component within this composite device which
>>             supports TPM operations.";
>>          leaf node-id {
>>            type string;
>>            description
>>              "ID of the compute node, such as Board Serial Number.";
>>          }
>>          leaf node-physical-index {
>>            if-feature "hw:entity-mib";
>>            type int32 {
>>              range "1..2147483647";
>>            }
>>            config false;
>>            description
>>              "The entPhysicalIndex for the compute node.";
>>            reference
>>              "RFC 6933: Entity MIB (Version 4) - entPhysicalIndex";
>>          }
>>          leaf node-name {
>>            type string;
>>            description
>>              "Name of the compute node.";
>>          }
>>          leaf node-location {
>>            type string;
>>            description
>>              "Location of the compute node, such as slot number.";
>>          }
>>        }
>>
>>> Figure 1 has no title
>>
>> As the top of the figure shows <CODE BEGINS> file
>> "[e-mail]" it appeared redundant.
>>
>> Thanks again,
>> Eric
>>
>>
>>>
>>> Ciao
>>> Hannes
>