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

From: Roman Danyliw <[email-masked]>
Date: Thu, 4 Nov 2021 20:47:48 +0000

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 [email-masked] module.

As listed in Section 2.1.1.1 and later in [email-masked] 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".

** [email-masked] .  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

** [email-masked] .  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 ...

** [email-masked]. Typo. s/collasping/collapsing/

** [email-masked]. 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.

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

** [email-masked]. 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

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Tue, 16 Nov 2021 14:48:20 +0000

Hi Roman,

Thanks for the detailed questions and recommendations.  Several of us
authors have gotten together on a joint response with comments coordinated
across this document, and the RIV document.  Details below....

> 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.

The initial YANG model was pieced together from the submissions of many
vendor YANG models.  Reflecting the broad contributions of many in the
industry seemed important as YANG model leadership far too often driven by
too small a group.  And we wanted the Engineering organizations of lots of
vendors to contribute parts.  And we wanted the security organizations to
also contribute on the operations of the TPM.
 
> Should this text be left this flexible, or should it explicitly reference
[TPM1.2] and
> [TPM2.0]?

This should be left flexible.  For example, Cisco uses its own home-grown
cryptographic hardware chips which are not exactly TPM1.2 or TPM2.0, but
which can nevertheless meet the API requirements expected from this
specification.  We didn't want to disqualify such hardware.

> ** 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?

I have included a new reference to the NIST publication "True Randomness
Can't be Left to Chance: Why entropy is important for
information security"
https://tsapps.nist.gov/publication/get_pdf.cfm?pub_id=915121

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

Changed:
The functions of this YANG module are restricted to 0-1 TPMs per hardware
component.

To:
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.

> ** 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.

Done for each reference.   Other changes requested with RIV are also
included.  See below for many specifics.

> As listed in Section 2.1.1.1 and later in
ietf-tpm-remote-attestation@2021-05-
> 11.yang 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";
> 
> -- 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 (RIV) talks about the
overall mechanisms and references for BIOS logs.  This link eventually above
resolves to: 
https://trustedcomputinggroup.org/wp-content/uploads/TCG_EFI_Platform_1_22_F
inal_-v15.pdf

The Charra model points to the specific object definitions in the log within
the PC platform profile containing the measurement object which goes into
the PCR.  This profile is 
https://trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platf
orm_Profile_for_TPM_2p0_Systems_v51.pdf,  Section 9.4.5.2

> -- 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.

There are lots of IMA event log types which will be used.  This is the
reason that
The Canonical Event Log Format was created.  But this doesn't mean it will
Displace what is already out there.  In the end, the delivered objects
should be the 
same for measurement mapping.   It is really a internal Verifier issue
of how the delivered log measurement object is extracted from the referenced
log and used in PCR hash verification.   This has been more accurately
documented in the 
YANG model object definition for the log:

  feature ima {
    description
      "The device supports Integrity Measurement Architecture logs.
      Many variants of IMA logs exist in the deployment.  Each encodes 
      the log entry contents as the specific measurements which get 
      hashed into a PCRs as Evidence.  See the reference below for 
      one example of such an encoding.";
    reference
      "https://www.trustedcomputinggroup.org/wp-content/uploads/
       TCG_IWG_CEL_v1_r0p30_13feb2021.pdf  Section 4.3";
  }

> 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.

Correct.

> 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?
> 

Added:

> 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.

Henk and Guy have made the changes to that document.  You should see them
shortly.

> ** 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?

We didn't want to tie the nonce down to a specific size, as the proper size
is a function of how many quotes per second can be generated from a physical
TPM1.2, TPM2, or perhaps more speedy custom hardware that support the TPM
API.  To help clarify, the description of the nonce in the YANG model now
defines specific predictable behavior for padding/trimming as the nonce is
sent into a specific TPM type:

    leaf nonce-value {
      type binary;
      mandatory true;
      description
        "A cryptographically generated random number which should
         not be predictable prior to its issuance from a random
         number generation function. The random number MUST be
         derived from an entropy source external to the Attester.
         
         Note that a nonce sent into a TPM will typically be 160 or 256
         binary digits long.  (This is 20 or 32 bytes.) So if fewer
         binary are sent, this nonce object will be padded 
         with leading zeros any in Quotes returned from the TPM.  
         Additionally if more bytes are sent, the nonce will be trimmed
         to the most significant binary digits.";
    }

> -- Where is the guidance on how to verify the signature on TPM_QUOTE2 and

These should be well documented in the referenced TCG documents.  

Additionally there are open source libraries which do this, so extra text
here is unlikely to bring deployment clarity.  
But it would require much text to discuss how recreate the expected PCR
values so that the signatures align with the transported quotes.
 
> -- Where is the guidance on how the certificate-names-ref might be set?

The node structures point to a primary key reference which can be associated
to 
                 type leafref {
                   path "/ks:keystore/ks:asymmetric-keys/ks:asymmetric-key"
                      + "/ks:certificates/ks:certificate/ks:name";

Which imports
     import ietf-keystore {
       prefix ks;
     }

That is defined in Normative reference #1:
    I-D.ietf-netconf-keystore

So that is the IETF draft which defines the reference to a keystore.

That said, while normative references have been met, I don't know of
implementations of ietf-keystore.  So what is going to happen in practice is
that vendor implementations will augment the yang model with a new leaf
pointing to their own certificate-names-ref equivalent.  Such things like
this are common in yang models.

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

I don't believe the syntax incorrect.  But it could be redundant/confusing.
So I simplified to:

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

The response also has a
> "ks:something".

Also made the corresponding simplification.

> ** [email-masked] .  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.

I embedded the link to:
https://trustedcomputinggroup.org/wp-content/uploads/TPM-Main-Part-3-Command
s_v1.2_rev116_01032011.pdf

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

It shouldn't happen.  If it does, always go with the signed values.  I have
updated the text as such to:

         There should never be a result where
         an unsigned PCR value is actually that that within a quote.
         If there is a difference, a signed result which has been 
         verified from retrieved logs is considered definitive. 
 
> ** 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 ...

Text removed as the intended action has been completed.

> ** [email-masked]. Identity asymmetric.  The URL to the
> document containing Table 2 returns a 404.  I believe this is the correct
> _Algorithm_Registry_r1p32_pub.pdf

Fixed

> 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.

Made them all: TCG Algorithm Registry Revision 01.32
 
> ** [email-masked]. identity tpm12 and tpm20.  Thanks for
> include the PDF name.  Can you please add the full URL.

Done.  Now says...

  feature tpm12 {
    description
      "This feature indicates algorithm support for the TPM 1.2 API
       as per Section 4.8 of TPM Main Part 2 TPM Structures
       https://trustedcomputinggroup.org/wp-content/uploads/
       TPM-main-1.2-Rev94-part-2.pdf";
  }

  feature tpm20 {
    description
      "This feature indicates algorithm support for the TPM 2.0 API 
       as per Section 11.4 of Trusted Platform Module Library
       Part 1: Architecture
       https://trustedcomputinggroup.org/wp-content/uploads/
       TPM-Rev-2.0-Part-1-Architecture-01.38.pdf";
  }
 
> ** [email-masked]. 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.

Done with formal name, like above.

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

Fixed

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

Fixed

> ** 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".

Updated
 
> ** 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"

Updated

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

When this document is used, you are going to an instance of hardware which
has been deployed.  So by then the hardware exists, and as such the SHOULD
has already been determined.  Maybe there are some really small corner cases
where hardware or firmware could support both, but it doesn't seem
meaningful to point out.
 
> ** 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.

Added the following text...

"For the YANG module ietf-tcg-algs.yang, please use care when selecting
specific algorithms.  The introductory section of [TCG-Algos] highlights
that some algorithms should be considered legacy, and recommends
implementers and adopters diligently evaluate available information such as
governmental, industrial, and academic research before selecting an
algorithm for use."

This seemed a good way to handle it as the recommendations of algorithms is
made by [TCG-Algos]  within the Algorithm registry is TCG stewarded.

> ** 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:
> 

Moved from informative to normative.

> -- 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?

Extracting information related to Attestation doesn't seem to need deeper
protections than would be needed for any other YANG RPC.  Still saying this
is a good idea, so the following has been added:

"Information collected through the RPCs above could reveal that specific
versions of software and configurations of endpoints that could identify
vulnerabilities on those systems.  Therefore RPCs should be protected by
NACM [RFC8341] to limit the extraction of attestation data by only
authorized Verifiers."

> -- 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 change above.

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

Could you provide more context, we don't understand this question.
 
> ** Section 4.  Editorial. s/ i. e./i.e.,/

Fixed

> 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".

Change made.

Thanks again for the comments.   And you can see many of the changes above
in our Git Repository:
https://github.com/ietf-rats-wg/basic-yang-module

Eric on behalf of the authors.

> Regards,
> Roman
> 
> _______________________________________________
> RATS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rats