[Rats] AD review of draft-ietf-rats-yang-tpm-charra-11

[Roman Danyliw <[e-mail]>]

Hi!

I performed an AD review on draft-ietf-rats-yang-tpm-charra-11.  Thanks for this document.  My comments are below.

Thanks for being making specific section reference to the various TCG specs in the YANG module.  These exact citations were very helpful.

In a few comments below, I may be too closely associating this document with draft-ietf-rats-tpm-based-network-device-attest so please let me know when this is the case.

** There are eight authors.  Customarily (per RFC7322), the total number of authors/editors on the first page is limited to five.  Could I have a write-up/confirmation on why this current arrangement makes sense.

** Section 2.  
One or more TPMs MUST be embedded in a Composite Device that provides
   attestation evidence via the YANG module defined in this document.

Should this text be left this flexible, or should it explicitly reference [TPM1.2] and [TPM2.0]?

** Section 2.  Per "A fresh nonce with an appropriate amount of entropy MUST be supplied ...", where can the guidance that characterizes "an appropriate amount of entropy" be found?

** Section 2.  
The functions of this YANG module are restricted to 0-1 TPMs per
   hardware component.

Can the phrase "... are restricted to 0-1 TPMs per hardware components" be stated differently.  I'm don't follow the intent.

** Section 2.1.1.1.  Could a forward reference (just as is used in the YANG module) be provided for each of these features to improve readability.  

** Section 2.1.1.1 and [e-mail] module.

As listed in Section 2.1.1.1 and later in [e-mail] module, it appears that this document supports the following log types:

(1) feature bios ==> "https://trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v51.pdf,           Section 9.4.5.2";

(2) feature ima ==> "https://www.trustedcomputinggroup.org/wp-content/uploads/TCG_IWG_CEL_v1_r0p30_13feb2021.pdf  Section 4.3";

(3) feature netequip_boot ==> no reference, "The device supports the netequip_boot logs."

-- per bios, this document and draft-ietf-rats-tpm-based-network-device-attest use different references for BIOS logs.

Here there is:
https://trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v51.pdf,  Section 9.4.5.2

draft-ietf-rats-tpm-based-network-device-attest uses:
   [EFI-TPM]  Trusted Computing Group, "TCG EFI Platform Specification
              for TPM Family 1.1 or 1.2, Specification Version 1.22,
              Revision 15", January 2014,
              <https://trustedcomputinggroup.org/resource/tcg-efi-
              platform-specification/>.

Are these complimentary?  Should they be the same?

-- per ima, does this document only support IMA in the Canonical Event Log Format or all of the Canonical Event Log Format (asking because draft-ietf-rats-tpm-based-network-device-attest isn't that specific)? Citing Section 4.3 is helpful but doe not resolve the ambiguity as the CONTENT_TYPE TLV=7, 8 is IMA but 4 or 5 would not be.

If it only support IMA, please be clear down to the TLV reference.  Additionally, there is a conversation to have about draft-ietf-rats-tpm-based-network-device-attest which in Section 2.3 says that "Attestation logs SHOULD be formatted according to the Canonical Event Log format [Canonical-Event-Log]" - that is, it doesn't seem to restrict down to IMA.

If it supports more than IMA, than this feature name needs a more expansive name.

Also, this draft cites v1, Revision 0.30, Dec 11, 2020 version of Canonical Event log.  draft-ietf-rats-tpm-based-network-device-attest cites v1, Revision .12, Oct 2018. Should they be the same?

-- Per netequip_boot, is there a reference which explains what this is?

Also, since this document is supposed to use "the operational context defined in [I-D.ietf-rats-tpm-based-network-device-attest]", how does netequip boot fit into that document?  I can't link that log type to the existing text.

** Section 4, Section 2.1.1.3.1 and 2.1.1.3.2.  There is an implicit verification workflow/protocol built into using these challenge-response RPCs.  A few clarifying questions:

-- Where can one find the guidance on generating the nonce.  For example, for TPM1.2, Section 16.4 of https://trustedcomputinggroup.org/wp-content/uploads/mainP3Commandsrev103.pdf says it should 160-bits.  Does that apply here?  What about for TPM2?
-- Where is the guidance on how to verify the signature on TPM_QUOTE2 and TPM_QUOTE_INFO?

-- Where is the guidance on how the certificate-names-ref might be set?

** 2.1.1.3.2.  Per the RPC challenge example:

       <tpm20-hash-algo
           xmlns:taa="urn:ietf:params:xml:ns:yang:ietf-tcg-algs">
         taa:TPM_ALG_SHA256
       </tpm20-hash-algo>

I'm missing something.  Why is there an XML namespace in the element body? (i.e., why the "taa:" for "TPM_ALG_SHA256")?  The response also has a "ks:something".

** [e-mail] .  grouping tpm12-attestation. Can a URL be provided for the reference?  The current text says to use rev116.  I found the following link to revision 103.
https://trustedcomputinggroup.org/wp-content/uploads/mainP3Commandsrev103.pdf

** [e-mail] .  list unsigned-pcr-values. What should be the behavior if an unsigned pcr value is found to deviate from the signed one?

** Section 2.1.2.  I'm not sure the history is needed here and it won't age well in an archival series  This section could just start with "This document as encoded the TCG Algorithm ...

** [e-mail]. Typo. s/collasping/collapsing/

** [e-mail]. Identity asymmetric.  The URL to the document containing Table 2 returns a 404.  I believe this is the correct link:
https://trustedcomputinggroup.org/wp-content/uploads/TCG-_Algorithm_Registry_r1p32_pub.pdf
For all of the identities that use "TCG_Algorithm_Registry_r1p32_pub" as a reference, please provide the full URL or the formal name of the registry.  The partial file name does not help.

** [e-mail]. identity tpm12 and tpm20.  Thanks for include the PDF name.  Can you please add the full URL.

** [e-mail]. For all of the identities that use "TCG_Algorithm_Registry_r1p32_pub Table 3" as a reference.  Please either use the formal name of the TCG reference or provide a URL.

** Identity TPM_ALB_MGF1.  Typo. s/IEEE Std 1363a -2004/IEEE Std 1363a-2004/

** Identity TPM_ALG_KEYEDHASH.  Typo. s/specification. ./specification./

** Identity TPM_ALG_SM3_256.  The reference provided here does not match the TCG registry.  TGC says it should be "ISO/IEC 10118-3:2018".

** Identity TPM-ALG_SM2.  The references doesn't match the TCG registry which says "GB/T 32918.1-2016, GB/T 32918.2-2016, GB/T 32918.3-2016, GB/T 32918.4-2016, GB/T 32918.5-2017"

** Section 4.  draft-ietf-rats-tpm-based-network-device-attest said that "RIV implementations SHOULD use TPM2.0".  Is that a recommendation here?

** Section 4.  

Is there a reason that this document doesn't provide any additional caution or normative recommendations against using certain algorithms in the YANG module - TPM_ALG_TDES, TPM_ALG_SHA, TPM_ALG_SHA1, TPM_ALG_XOR, TPM_ALG_NULL?  If not, at least something weaker to remind implementers to carefully select which algorithms, and recognize that sub-optimal choices may need to be made given the constraints of the fielded TPMs in the attesters.

** Section 4.  Thank you for using part of the YANG  security considerations template from https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines.  A few additional considerations:

-- [RFC 8446], [RFC6241], [RFC6242], [RFC8341], and [RFC8040] must all be "normative references"

-- What measures should be put in place to ensure that only authorized verifiers are permitted to engage the attesters?  For example, will NACM be used?

-- Per sensitivity of read, could the information collected through the RPCs reveal that specific versions of software and configurations of endpoints which could identify vulnerabilities on those systems?  

-- Per the existing text in about verifying the appropriateness of the AK.  Is there a reference for that?

** Section 4.  Editorial. s/ i. e./i.e.,/

** Section 4

   RPC 'log-retrieval':  Pulling lots of logs can chew up system
      resources.

Consider using a less colloquial description, perhaps "requesting a large volume of logs from the attester could require significant system resources and create a denial of service".

Regards,
Roman