Subject: [Rats] Benjamin Kaduk's Discuss on draft-ietf-rats-yang-tpm-charra-16: (with DISCUSS and COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 08 Mar 2022 21:57:51 -0800

Benjamin Kaduk has entered the following ballot position for
draft-ietf-rats-yang-tpm-charra-16: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-rats-yang-tpm-charra/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

(1) Section 2 references the "ietf-basic-remote-attestation YANG module",
but that appears to be an older name for what got renamed to the
"ietf-tpm-remote-attestation" module at time of WG adoption.

(2) There are some issues with references, either pointing to the wrong
document or to a draft version of a specification when there is a final
version available.

(2.1) The YANG reference stanza for "feature ima" (in the
ietf-tpm-remote-attestation module) still refers to a draft version of the
Canonical Event Log specification.

(2.2) The YANG reference stanza for "grouping ima-event" (in the same
module) does the same.

(2.3) The PDF link in the description of "feature tpm20" (in the
ietf-tcg-algs YANG module) appears to point to a draft version of the
specification.  Since the link is to a 2011 version, I trust that the
final version is already available, and the fix is just to update the
link.

(2.4) The PDF link in the descriptions of all three "enum" values of
"leaf type" in "container certificates" in the ietf-tpm-remote-attestation
module points to a draft version of the TCG specification.

(2.5) The PDF link in the description of "feature tpm20" in the
ietf-tcg-algs YANG module points to a draft version of the TCG
specification.

(2.6) The description for "identity TPM_ALG_HMAC" in the ietf-tcg-algs
YANG module points to RFC 2014 as one reference, but HMAC is RFC 2104.

(2.7) The reference for [TPM2.0-Arch] links to a draft version of the
TCG specification.

(2.8) The reference for [TPM2.0-Key] links to a draft version of the TCG
specification.

(3) Some of the references don't seem to match up with their slugs --
e.g., [ima-log] seems to point to the TCB "canonical event log format" and
[netequip-boot-log] (and the YANG reference stanza for "feature
netequip_boot") points to what looks like a Linux IMA document.  While the
description in "grouping network-equipment-boot-event-log" does give some
indication that [netequip-boot-log] is intended to be very similar to the
IMA format, I don't think we can reasonably just directly reference the
IMA spec and call it [netequip-boot-log] without some additional
clarification either in the reference entry or where it is used.

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Section 2.1.1.1

   *  'TPMs': Indicates that multiple TPMs on the device can support
      remote attestation.  This feature is applicable in cases where
      multiple line cards are present, each with its own TPM.

It sounds like this is intending to emphasize "TPMs" as opposed to "TPM"
singular; making the plural 's' have semantic value seems to risk
confusion, and perhaps a "multi-tpm" feature name would be less
risk-prone.

Section 2.1.1.5

   Container 'rats-support-structures':  This houses the set of
      information relating to a device's TPM(s).

   Container 'tpms':  Provides configuration and operational details for
      each supported TPM, including the tpm-firmware-version, PCRs which

I believe that in the current YANG module, "tpms" is a child of
"rats-support-structures" (along with "compute-nodes" and
"attester-supported-algos"), which does not really seem very commensurate
with the prose summary here.

   +--rw tpms
      +--rw tpm* [name]
      [...]
         +--rw firmware-version    identityref
         +--rw tpm12-hash-algo?    identityref
         +--rw tpm12-pcrs*         pcr
         +--rw tpm20-pcr-bank* [tpm20-hash-algo]

It's fairly surprising that the firmware-version is writable, and I'm not
really sure what it means to have the set of pcrs be writable, either.

Section 2.1.1.6

     typedef pcr {
       type uint8 {
         range "0..31";
       }
       description
         "Valid index number for a PCR.  At this point 0-31 is viable.";

That's a fairly surprising description to see in a Proposed Standard.
If we are not sure that the range restriction will always be valid, should
we be enforcing it as part of the module?

     typedef compute-node-ref {
       type leafref {
         path "/tpm:rats-support-structures/tpm:compute-nodes"
            + "/tpm:compute-node/tpm:node-name";
       }
       description
         "This type is used to reference a hardware node.  It is quite
          possible this leafref will eventually point to another YANG
          module's node.";

Similarly here, I might expect the statement to be a caution to the reader
that they need to handle the case where it points to another YANG module's
node, rather than "it is quite possible" that such an eventuality might
occur.

     grouping tpm20-hash-algo {
     [...]
         default "taa:TPM_ALG_SHA256";
         description
           "The hash scheme that is used to hash a TPM1.2 PCR. This
            must be one of those supported by a platform.";

Is this description with "TPM1.2" really correct in the "tpm20-hash-algo"
grouping?

         default "taa:TPM_ALG_SHA1";
         description
           "The hash scheme that is used to hash a TPM1.2 PCR. This
            MUST be one of those supported by a platform.  This assumes
            that an algorithm other than SHA1 can be supported on some
            TPM1.2 cryptoprocessor variant.";

What exactly assumes that?  Use of a non-default value, perhaps?

     grouping nonce {
       description
         "A random number intended to be used once to show freshness
          and to allow the detection of replay attacks.";

"Show freshness" makes sense and seems intuitive.  Detecting replay
attacks seems like it would require some additional infrastructure (a
"replay cache", perhaps, ideally with some time-based aging out), about
which we say nothing.  Is this replay protection expected to be part of
the TPM chip itself, or on the composite device hosting the YANG server,
or somewhere else entirely?  It seems a bit disingenuous to suggest that
such functionality is possible while providing no pointers at all for how
to achieve it.

     grouping tpm12-pcr-selection {
       description
         "A Verifier can request one or more PCR values using its
          individually created Attestation Key Certificate (AC).
          The corresponding selection filter is represented in this
          grouping.
          Requesting a PCR value that is not in scope of the AC used,
          detailed exposure via error msg should be avoided.";

This last sentence seems to be missing a few words that are vital to the
meaning.

     grouping tpm12-pcr-selection {
       [...]
       leaf-list pcr-index {
         [...]
         description
           "The numbers/indexes of the PCRs. At the moment this is limited
            to 32.   In addition, any selection of PCRs MUST verify that

(A similar comment as above regarding "at the moment" and future-proofing
what goes into a Proposed Standard.)

     grouping tpm20-pcr-selection {
       [...]
       list tpm20-pcr-selection {
         unique "tpm20-hash-algo";

I confess to being only a YANG amateur, but am not really sure why the
"unique" qualifier is preferable to the "key" one in this case (and
several others later on, but I'll just mention this one).

     grouping tpm-name {
       description
         "A unique TPM on a device.";
       leaf name {
         type string;
         description
           "Unique system generated name for a TPM on a device.";

If it's system-generated would that imply config false?

       uses node-uptime;
       list unsigned-pcr-values {
         description
           [...]
            significant processing.  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.";

I'm not sure I understand this part -- is there a missing negation in the
first sentence?

     grouping boot-event-log {
       description
         "Defines a specific instance of an event log entry
          and corresponding to the information used to
          extend the PCR";
       leaf event-number {
         type uint32;
         description
           "Unique event number of this event";

Should we mention what scope the uniqueness is within?  (At only 32 bits
it cannot be globally unique.)

       leaf pcr-index {
         type pcr;
         description
           "Defines the PCR index that this event extended";
       }

Why is this not mandatory?

       leaf-list event-data {
         type uint8;
         description
           "The event data size determined by event-size";

Is there some missing punctuation or something here?  Otherwise the focus
on size seems strange, given that there is a separate "leaf event-size".
(Also, why is a leaf-list of uint8 better than "type binary"?)

     grouping ima-event {
       description
         "Defines an hash log extend event for IMA measurements";
       reference
         "ima-log:
          https://www.trustedcomputinggroup.org/wp-content/uploads/
          TCG_IWG_CEL_v1_r0p30_13feb2021.pdf  Section 4.3";

Is section 4.3 the best section to reference?  I only see specifics about,
e.g., the hash algorithm being encoded as a string later on, circa §5.1.6.

       leaf filename-hint {
         type string;
         description
           "File that was measured";

Is this just the file name, a full path, either, ...?

       leaf signature {
         type binary;
         description
           "The file signature";

Both this description and the reference for ima-event seem pretty sparse
on what the signature actually covers.

     rpc tpm20-challenge-response-attestation {
       if-feature "taa:tpm20";
       description
         "This RPC accepts the input for TSS TPM 2.0 commands of the
          managed device. ComponentIndex from the hardware manager YANG
          module to refer to dedicated TPM in composite devices,
          e.g. smart NICs, is still a TODO.";

I would prefer to not see "is still a TODO" in a Proposed Standard.
"Out of the scope of this specification" is seen much more often (though
in some sense it conveys much the same meaning...)

       container compute-nodes {
         [...]
         list compute-node {
           key "node-id";

If we're going to use "leaf node-name" for leafrefs, does that require a
uniqueness constraint on "node-name" here?

       container tpms {
         [...]
           leaf path {
             type string;
             config false;
             description
               "Path to a unique TPM on a device.  This can change across
                reboots.";

Is this a ... device-tree path?  Surely it is not a filesystem path,
right?

           leaf firmware-version {
             type identityref {
               base taa:cryptoprocessor;
             }
             mandatory true;
             description
               "Identifies the cryptoprocessor API set supported.  This
                is automatically configured by the device and should not
                be changed.";

(Per my previous comment on the tree diagram, "should not be changed"
strongly implies that it should be "config false" to me.  When might that
not be the case?)

       container attester-supported-algos {
         description
           "Identifies which TPM algorithms are available for use on an
            attesting platform.";
         [...]

Just to confirm I'm reading this properly: the lists herein are not scoped
to specific TPM instances, but are rather global to the entire composite
device; the "when" constraints just say that each leaf-list is only
present if there are any TPMs of the corresponding TPM version present but
do not otherwise limit the scope of applicability for the algorithms in
question?  That seems rather surprising, especially if one considers a
scenario where there are multiple line cards on a given device and one
could be hot-replaced to a newer model with a TPM that supports more
algorithms.  But perhaps I'm missing something.

Section 2.1.2.2

   3.  Specific algorithm types: Each algorithm type defines what
       cryptographic functions may be supported, and on which type of
       API specification.  It is not required that an implementation of
       a specific TPM will support all algorithm types.  The contents of
       each specific algorithm mirrors what is in Table 3 of
       [TCG-Algos].

This phrasing implies a strong sense of consistency between the two
documents.  I did not attempt to perform a rigorous verification of each
element between the two documents, but I hope someone has done so.

Section 2.1.2.3

     contact
       "WG Web:   <https://datatracker.ietf.org/wg/rats/>
        WG List:  <mailto:[email-masked]>
        Author:   Eric Voit <mailto:[email-masked]>";

Just to confirm: it's correct for Eric to be the only listed author for
this module (vs ietf-tpm-remote-attestation, that includes what appears to
be the whole author list of the I-D)?

     identity signing {
       description
         "A TCG recognized signing algorithm";
       reference
         "TCG-Algos:TCG Algorithm Registry Rev1.32  Table 2";

I understand a desire to remain consistent with what the TCG says, but
would we consider mentioning that this also includes symmetric crypto
"signing" operations?

     identity method {
       description
         "A TCG recognized method such as a mask generation function.";

Perhaps we want "cryptographic method" here, in light of the way the TCG
spec frames things.

     identity TPM_ALG_XOR {
       if-feature "tpm12 or tpm20";
       base tpm12;
       base tpm20;
       base hash;
       base symmetric;

I didn't see in the TCG Algorithm Registry Rev1.32 doc that indicated that
the "XOR encryption algorithm" derived from a hash.  Please confirm that
"base hash" is correct.

     identity TPM_ALG_SYMCIPHER {
       if-feature "tpm20";
       base tpm20;
       description
         "Object type for a symmetric block cipher";
       reference
         "TCG-Algos:TCG Algorithm Registry Rev1.32  Table 3 and
          TCG TPM 2.0 library specification. ALG_ID: 0x0025";

Is the lack of "base symmetric" correct?  (The algorithm registry doc
seems to list is as an "object" as well as symmetric.)
Also, I guess this is a tolerable place to ask why the "AES" alg is
specific to TPM 1.2.  I found mention of (e.g.) "AES128" in at least one
TPM 2.0 spec, so presumably I'm just missing how those algorithms are
represented in the TPM 2.0 model (and why symmetric ciphers are treated
differently than the different output lengths of the SHA2 and SHA3 family
of hash functions, which do get individual algorithm identities in this
YAND module).  Perhaps they use this TPM_ALG_SYMCIPHER!

Section 4

The YANG security considerations template has a section about readable
data nodes that are considered sensitive.  Does that apply to any of the
nodes in the modules defined by this document?

   Container '/rats-support-structures/attester-supported-algos':  'tpm1
      2-asymmetric-signing', 'tpm12-hash', 'tpm20-asymmetric-signing',
      and 'tpm20-hash'.  All could be populated with algorithms that are
      not supported by the underlying physical TPM installed by the
      equipment vendor.

We should say what the impact of that configuration would be at runtime.

   Container: '/rats-support-structures/tpms':  'name': Although shown
      as 'rw', it is system generated.  Therefore it should not be

Why is it shown as 'rw', then?

Also, separately, if we're going to keep 'firmware-version' as writable,
there seems to be some consideration there, as telling a TPM 2.0-only chip
to use TPM 1.2 is going to be a DoS.

      'certificates': It is possible to provision a certificate which
      does not correspond to an Attestation Identity Key (AIK) within
      the TPM 1.2, or an Attestation Key (AK) within the TPM 2.0
      respectively.

Could we have a sentence about the runtime impact of such a
misconfiguration?

   RPC 'tpm12-challenge-response-attestation':  It must be verified that
   [...]
   RPC 'tpm20-challenge-response-attestation':  It must be verified that

Must be verified by whom?  In order to achieve what property?

Section 6.1

   [ISO-IEC-9797-2]
              "Message Authentication Codes (MACs) - ISO/IEC
              9797-2:2011", n.d.,
              <https://www.iso.org/standard/51618.html>.

Following the link indicates that there's a 2021 revision available; do we
need the older version specifically?

NITS

Section 2.1.1.1

   *  'TPMs': Indicates that multiple TPMs on the device can support
      remote attestation.  This feature is applicable in cases where
      multiple line cards are present, each with its own TPM.

An "e.g." or "for example" seems appropriate here.

Section 2.1.1.6

         description
           "Name of one or more unique TPMs on a device.  If this object
            exists, a selection should pull only the objects related to
            these TPM(s).  If it does not exist, all qualifying TPMs that
            are 'hardware-based' equals true on the device are selected.";

I think s/are/have/ would make sense.

     grouping tpm20-attestation {
       description
         "Contains an instance of TPM2 style signed cryptoprocessor
          measurements.  It is supplemented by unsigned Attester
          information.";
       leaf TPMS_QUOTE_INFO {
         type binary;
         mandatory true;
         description
           "A hash of the latest PCR values (and the hash algorithm used)
            which have been returned from a Verifier for the selected PCRs
            and Hash Algorithms.";
         reference
           "TPM2.0-Structures:
            https://www.trustedcomputinggroup.org/wp-content/uploads/
            TPM-Rev-2.0-Part-2-Structures-01.38.pdf  Section 10.12.1";

The reference appeears to include two sections marked 10.12.1(!), though
clearly we do not want the one entitled "introduction".

       leaf filedata-hash {
         type binary;
         description
           "Hash of filedata";

It is perhaps banal, but mentioning that the interpretation is controlled
by the filedata-hash-algorithm might be useful.

     rpc tpm12-challenge-response-attestation {
       [...]
       input {
         container tpm12-attestation-challenge {
           [...]
           leaf-list certificate-name {
             if-feature "tpm:tpms";
             type certificate-name-ref;
             must "/tpm:rats-support-structures/tpm:tpms"
                + "/tpm:tpm[tpm:firmware-version='taa:tpm12']"
                + "/tpm:certificates/"
                + "/tpm:certificate[name=current()]" {
               error-message "Not an available TPM1.2 AIK certificate.";

This is the first instance of "AIK" in the document (it does not appear in
the prose until the security considerations).  Perhaps it should get
mentioned in the Introduction as another term inported from TPM2.0-key?

     rpc tpm20-challenge-response-attestation {
       if-feature "taa:tpm20";
       description
         "This RPC accepts the input for TSS TPM 2.0 commands of the
          managed device. ComponentIndex from the hardware manager YANG
          module to refer to dedicated TPM in composite devices,
          e.g. smart NICs, is still a TODO.";

The second sentence seems to be missing a verb.

       output {
         list tpm20-attestation-response {
           unique "certificate-name";
           description
             "The binary output of TPM2b_Quote in one TPM chip of the
              node which identified by node-id. [...]

Does "chip" imply hardware chip, which is perhaps more specific than the
introductory materials indicated to be the scope of applicability?

     container rats-support-structures {
       container compute-nodes {
         if-feature "tpm:tpms";
         description
           "Holds the set device subsystems/components in this composite
            device that support TPM operations.";

"set of"

           leaf status {
             type enumeration {
               enum operational {
                 value 0;
                 description
                   "The TPM currently is currently running normally and
                    is ready to accept and process TPM quotes.";
                 reference
                   "TPM2.0-Arch:
                    TPM-Rev-2.0-Part-1-Architecture-01.07-2014-03-13.pdf
                    Section 12";

Huh, why is this not an https URI?

           container certificates {
             description
               "The TPM's certificates, including EK certificates
                and AK certificates.";

While we're expanded LAK and IAK previously, we haven't expanded just "AK"
itself yet.

Section 2.1.2.3

     organization
       "IETF RATS Working Group";

Do we want to use the same spelling as in the ietf-tpm-remote-attestation
module?

     identity tpm12 {
       if-feature "tpm12";
       base cryptoprocessor;
       description
         "Supportable by a TPM1.2.";
       reference
         "TPM1.2-Structures:
          https://trustedcomputinggroup.org/wp-content/uploads/
          TPM-Main-Part-2-TPM-Structures_v1.2_rev116_01032011.pdf
          TPM_ALGORITHM_ID values, page 18";

We referenced it as section 4.8 (vs page 18) for the YANG "feature"
stanza.

     identity tpm20 {
       if-feature "tpm20";
       base cryptoprocessor;
       description
         "Supportable by a TPM2.";
       reference
         "TPM2.0-Structures:
          https://trustedcomputinggroup.org/wp-content/uploads/
          TPM-Rev-2.0-Part-2-Structures-01.38.pdf
          The TCG Algorithm Registry. Table 9";

Is table 9 the right reference?

     identity TPM_ALG_SM4 {
       if-feature "tpm20";
       base tpm20;
       base symmetric;
       description
         "SM4 symmetric block cipher";
       reference
         "TCG-Algos:TCG Algorithm Registry Rev1.32  Table 3.
         ALG_ID: 0x0013";

Isn't SM4 an ISO standard now as well?

     identity TPM_ALG_RSASSA {
       if-feature "tpm20";
       base tpm20;
       base asymmetric;
       base signing;
       description
         "Signature algorithm defined in section 8.2 (RSASSAPKCS1-v1_5)";
       reference
         "TCG-Algos:TCG Algorithm Registry Rev1.32  Table 3 and
         RFC 8017.  ALG_ID: 0x0014";
     }

     identity TPM_ALG_RSAES {
       if-feature "tpm20";
       base tpm20;
       base asymmetric;
       base encryption_mode;
       description
         "Signature algorithm defined in section 7.2 (RSAES-PKCS1-v1_5)";

I think you need to say what document the section numbers are from, since
two references are listed in the reference stanza.

     identity TPM_ALG_ECDH {
       if-feature "tpm20";
       base tpm20;
       base asymmetric;
       base method;
       description
         "Secret sharing using ECC";
       reference
         "TCG-Algos:TCG Algorithm Registry Rev1.32  Table 3 and
          NIST SP800-56A and RFC 7748. ALG_ID: 0x0019";

RFC 7748 is specific to the so-called "CFRG curves"; is it really the
right reference for generic ECDH?

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Wed, 9 Mar 2022 20:02:06 +0000

Hi Benjamin,

Thanks for the comments.  Tweaks in line.  Everything was covered excepting one ***.     Each fixed/updated area in the document corresponds to a pull request in:

https://github.com/ietf-rats-wg/basic-yang-module

It is not possible to post a new draft yet since we are close to the IETF, so hopefully this will be sufficient for now.

> From: Benjamin Kaduk, March 9, 2022 12:58 AM

> When responding, please keep the subject line intact and reply to all email

> addresses included in the To and CC lines. (Feel free to cut this introductory

> paragraph, however.)

> Please refer to  <https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/> https://www.ietf.org/about/groups/iesg/statements/handling-

 <https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/> > ballot-positions/

>  <https://datatracker.ietf.org/doc/draft-ietf-rats-yang-tpm-charra/> https://datatracker.ietf.org/doc/draft-ietf-rats-yang-tpm-charra/

> "ietf-tpm-remote-attestation" module at time of WG adoption.

Good catch.  Fixed.

> Canonical Event Log specification.

Fixed.  Updated to the just approved version.

> module) does the same.

Fixed.  Updated to the just approved version.

> link.

Fixed the link to the latest 2019 approved version.  Also there was a new version update everywhere the TPM2.0 Architecture was referenced.  They were updated as well.

> module points to a draft version of the TCG specification.

Fixed.  All now point to the official Oct 2021 version:

https://trustedcomputinggroup.org/wp-content/uploads/TPM-2p0-Keys-for-Device-Identity-and-Attestation_v1_r12_pub10082021.pdf

> specification.

Duplicate of comment (2.3), fixed.

> YANG module points to RFC 2014 as one reference, but HMAC is RFC 2104.

Fixed.   Eight years ago my fingers were programmed that way I guess.

> TCG specification.

Fixed, per above.

> specification.

Fixed, per above

> clarification either in the reference entry or where it is used.

*** Defer to Henk who has a few questions here.

> risk-prone.

The longer feature name would make almost all of trees contained in the figures line wrap.  Currently none of the lines word wrap.   While the change could be made, it doesn't seem unreasonable to instead optimize for easier readability in the figures.

> with the prose summary here.

Changed the description of Container 'rats-support-structures' to:

This houses the set of information relating to remote attestation for a device.  This includes specific device TPM(s), the compute nodes (such as line cards) on which the TPM(s) reside, and the algorithms supported across the platform.

> really sure what it means to have the set of pcrs be writable, either.

This is an artifact of YANG.  If you make the information read only, then these nodes are not available to be used as constraints within the configuration datastore.  There were several discussions on this last year including:

https://mailarchive.ietf.org/arch/msg/rats/JgNtYx0LPWlL8E0yEXkK9ZW_0CU/

And during the YANG Doctor review:

https://mailarchive.ietf.org/arch/msg/rats/xzcjDbMIObdyCbUyWqsWCODJ34c/

The alternative is to simply not do the  consistency check, which would drive errors.

> we be enforcing it as part of the module?

Custom TPM implementations can have additional PCRs.  We did not want to have the model restrict such flexibility.

> occur.

Changed the text to:

      "This type is used to reference a hardware node.  Note that an

       implementer might include an alternative leafref pointing to a

       different YANG module node specifying hardware structures.";

> grouping?

Fixed.  (It looks like a previous fix here got dropped somehow.)

> What exactly assumes that?  Use of a non-default value, perhaps?

SHA1 is the hash default for TPM1.2.  But at a global level, SHA1 is deprecated for signatures. 

https://csrc.nist.gov/News/2006/NIST-Comments-on-Cryptanalytic-Attacks-on-SHA-1 

It seems wise to require alternative algorithms so that users do not get confused.

> to achieve it.

There have been a number of discussions on this

https://mailarchive.ietf.org/arch/msg/rats/KvUkqzBuisozqKfKoAzbRtbvV_k/

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

> meaning.

Deleted the last sentence.  It was trying (and failing) to describe a fairly obvious error case which is not core the grouping definition.

> what goes into a Proposed Standard.)

Removed "at the moment this is limited to 32", as this not necessary to point out within this specific definition.  It is better to just leave up to the definition of the PCR range.   

> several others later on, but I'll just mention this one).

https://datatracker.ietf.org/doc/html/rfc7950#section-7.8.3

> If it's system-generated would that imply config false?

Same issue as above.  Making it config=false means it cannot be used for constraining configuration data.  Many discussions on this are in the archives.  More reading is in RFC8342, Network Management Datastore Architecture (NMDA).

> first sentence?

Tweaked the definition to:

         significant processing.  Note that there should never be a 

         result where an unsigned PCR value differs from what may be 

         reconstructed from the within the PCR quote and the event logs.

> it cannot be globally unique.)

Text refined to:

        "Identifies a unique event which extended a PCR.

        Uniqueness is since last boot.";

With 32 bits, there are billions of events recordable.  Tracking more than this since boot is unlikely.

> Why is this not mandatory?

Making it mandatory would restrict informational events which could also be placed into the log. Eliminating this possibility could be unnecessarily restrictive as PCR reconstruction would only occur on entries with a pcr-index.

> (Also, why is a leaf-list of uint8 better than "type binary"?)

The object is driven by the TCG:

https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf 

Section 5.1 provides the object definitions.   In multiple notes later in section 5, this particular object has the note:

"this is a memory copy of EventSize bytes".  All the examples I see fill this with "0x00, 0x00, 0x00, 0x00"

So mostly this object data will be ignored as it will not be used for PCR validation.  But as it arrive as part of the TCG EFI log format, we include it.

>            <https://www.trustedcomputinggroup.org/wp-content/uploads/> https://www.trustedcomputinggroup.org/wp-content/uploads/

> e.g., the hash algorithm being encoded as a string later on, circa §5.1.6.

5.1.6 also works.  Changed to that.

> Is this just the file name, a full path, either, ...?

As each vendor might do this their own way, we didn't want to specify how.

> on what the signature actually covers.

Now says "Digital file signature which provides a fingerprint for the file being measured."

> in some sense it conveys much the same meaning...)

Made "is not is covered".

> uniqueness constraint on "node-name" here?

Yes.  Good catch.   Added unique "node-name";

> right?

Yes a device tree path.  Made it "Device path".

> not be the case?)

See earlier discussions.

> algorithms.  But perhaps I'm missing something.

This interpretation is correct.  In surveying the vendors of the devices, nobody wanted to mix and match algorithms for a platform.  Only when the full platform supported it, would they be advertised.  Doing so simplifies the management interfaces.

> element between the two documents, but I hope someone has done so.

Henk did a scrub to make sure I copied everything verbatim.

>        "WG Web:   < <https://datatracker.ietf.org/wg/rats/> https://datatracker.ietf.org/wg/rats/>

>         WG List:  < <mailto:[email-masked]> mailto:[email-masked]>

>         Author:   Eric Voit < <mailto:[email-masked]> mailto:[email-masked]>";

> be the whole author list of the I-D)?

I don't have a problem with it ;-).   Note: while *lots* of people contributed initial objects and structures for their companies and did reviews, I did the vast majority of the synthesis.  This included a couple full rewrites after earlier authors departed.

That said, I really don’t really care if other authors are listed.  Anyone who wants just needs to speak up and say so.  Or add their name to a GitHub pull before it is too late.

> "signing" operations?

To minimize any chance of perceived divergence, prefer to say exactly what TCG says in the referenced document.

> spec frames things.

By using their text, reduces the chance for perceived divergence.

> "base hash" is correct.

H in the TCG table means "hash algorithm that compresses input data to a digest value or indicates a method that uses a hash".  And "H" appears in the Table next to TPM_ALG_XOR.

> seems to list is as an "object" as well as symmetric.)

Looks like this got dropped somewhere.  Thanks for the catch!

> YAND module).  Perhaps they use this TPM_ALG_SYMCIPHER!

AES is supported as a symmetric algorithm on TPM2.   But symmetric keys are not use with specific PCR signatures or hashing extensions driven from outside a router.  So putting a "Base" statement would make the algorithm appear available when in fact it is not used in this context.

> nodes in the modules defined by this document?

The colon after the following sentence is supposed to indicate these specific vulnerabilities.

> We should say what the impact of that configuration would be at runtime.

Added: "A vendor should restrict the ability to configure unsupported algorithms."  In this case there shouldn't be the ability to configure these.

> Why is it shown as 'rw', then?

See note above.  YANG isn't always pretty.

> to use TPM 1.2 is going to be a DoS.

It should not be possible, this is a YANG artifact.

> misconfiguration?

Added: "In such a case, calls to an RPC requesting this specific certificate could result in either no response or a response for an unexpected TPM."

> Must be verified by whom?  In order to achieve what property?

Now says:

RPC 'tpm12-challenge-response-attestation'

The receiver of the RPC response must verify that the certificate is for an active AIK, i.e., the certificate has been confirmed by a third party as being able to support Attestation on the targeted TPM

RPC 'tpm20-challenge-response-attestation'

The receiver of the RPC response must verify that the certificate is for an active AK, i.e., the private key confirmation of the quote signature within the RPC response has been confirmed by a third party to belong to an entity legitimately able to perform Attestation on the targeted TPM 2.0.

>               < <https://www.iso.org/standard/51618.html> https://www.iso.org/standard/51618.html>.

> need the older version specifically?

As the TCG table is from before that standard was released, it seems safer to leave the older reference (as getting/verifiying the new version would require buying it.)

> An "e.g." or "for example" seems appropriate here.

I don't know of another example, so the current text looks good.

> I think s/are/have/ would make sense.

Updated.

>              <https://www.trustedcomputinggroup.org/wp-content/uploads/> https://www.trustedcomputinggroup.org/wp-content/uploads/

> clearly we do not want the one entitled "introduction".

:-) Hopefully the reader can go down a couple lines. 

> by the filedata-hash-algorithm might be useful.

Now says:  "Hash of filedata as updated based upon the filedata-hash-algorithm."

> mentioned in the Introduction as another term inported from TPM2.0-key?

Good idea.  Added.

> The second sentence seems to be missing a verb.

Now says: "is used to refer"

> introductory materials indicated to be the scope of applicability?

Removed chip.  Now says "from one TPM of the"

> "set of"

Updated

> Huh, why is this not an https URI?

Updated

> itself yet.

Expanded to Attestation Key.

> module?

Made:

"IETF RATS (Remote ATtestation procedureS) Working Group";

>            <https://trustedcomputinggroup.org/wp-content/uploads/> https://trustedcomputinggroup.org/wp-content/uploads/

> stanza.

Updated

>            <https://trustedcomputinggroup.org/wp-content/uploads/> https://trustedcomputinggroup.org/wp-content/uploads/

> Is table 9 the right reference?

Removed "The TCG Algorithm Registry. Table 9"

> Isn't SM4 an ISO standard now as well?

Wiki says no, 

https://en.wikipedia.org/wiki/SM4_(cipher)

but a web search says it happened in the middle of last year.

https://www.iso.org/standard/81564.html

Leaving current text as is, since I don't want to buy the document.

> two references are listed in the reference stanza.

Done. Put RFC 8017 at the front of the description

>           NIST SP800-56A. ALG_ID: 0x0019";

> right reference for generic ECDH?

This is what the TCG table says.  Nevertheless removed 7748.   

Thanks again,

Eric

---

From: Henk Birkholz <[email-masked]>
Date: Wed, 9 Mar 2022 21:41:21 +0100

Thanks Eric,
hi Ben!

there is no perfect choice here.

I hear your point that an "Linux IMA document" is not normative. Yes.

While that is technically correct, corresponding normative references 
are actually not better. We could inject a normative reference, such as 
the documents found at:

> https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p05p_r14_pub.pdf

or

> https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf

Would it be possible to retain IMA kernel reference as a informative 
reference in this case? Effectively, I'd find that more useful to 
implementers. Or maybe both? Maybe you could provide us with some 
guidance here.

Viele Grüße,

Henk

On 09.03.22 21:02, Eric Voit (evoit) wrote:
> Hi Benjamin,
> 
> Thanks for the comments.  Tweaks in line.  Everything was covered 
> excepting one ***.     Each fixed/updated area in the document 
> corresponds to a pull request in:
> 
> <https://github.com/ietf-rats-wg/basic-yang-module>
> 
> It is not possible to post a new draft yet since we are close to the 
> IETF, so hopefully this will be sufficient for now.
> 
> email
>  > Please refer to 
> https://www.ietf.org/about/groups/iesg/statements/handling- 
> <https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/>
> 
>> ballot-positions/ 
> <https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/>
> 
> <https://datatracker.ietf.org/doc/draft-ietf-rats-yang-tpm-charra/>
> 
>  >
> 
>  >
> 
>  >
> 
> Fixed the link to the latest 2019 approved version.  Also there was a 
> new version update everywhere the TPM2.0 Architecture was referenced.  
> They were updated as well.
> 
> <https://trustedcomputinggroup.org/wp-content/uploads/TPM-2p0-Keys-for-Device-Identity-and-Attestation_v1_r12_pub10082021.pdf>
> 
> The longer feature name would make almost all of trees contained in the 
> figures line wrap.  Currently none of the lines word wrap.   While the 
> change could be made, it doesn't seem unreasonable to instead optimize 
> for easier readability in the figures.
> 
>  > Section 2.1.1.5
> 
>  >
> 
> This houses the set of information relating to remote attestation for a 
> device.  This includes specific device TPM(s), the compute nodes (such 
> as line cards) on which the TPM(s) reside, and the algorithms supported 
> across the platform.
> 
>  >    +--rw tpms
> 
>  >       +--rw tpm* [name]
> 
>  >       [...]
> 
> This is an artifact of YANG.  If you make the information read only, 
> then these nodes are not available to be used as constraints within the 
> configuration datastore.  There were several discussions on this last 
> year including:
> 
> <https://mailarchive.ietf.org/arch/msg/rats/JgNtYx0LPWlL8E0yEXkK9ZW_0CU/>
> 
> <https://mailarchive.ietf.org/arch/msg/rats/xzcjDbMIObdyCbUyWqsWCODJ34c/>
> 
> Custom TPM implementations can have additional PCRs.  We did not want to 
> have the model restrict such flexibility.
> 
> <https://csrc.nist.gov/News/2006/NIST-Comments-on-Cryptanalytic-Attacks-on-SHA-1> 
> 
> 
> <https://mailarchive.ietf.org/arch/msg/rats/KvUkqzBuisozqKfKoAzbRtbvV_k/>
> 
> Deleted the last sentence.  It was trying (and failing) to describe a 
> fairly obvious error case which is not core the grouping definition.
> 
> Removed "at the moment this is limited to 32", as this not necessary to 
> point out within this specific definition.  It is better to just leave 
> up to the definition of the PCR range.
> 
> <https://datatracker.ietf.org/doc/html/rfc7950#section-7.8.3>
> 
>  >      grouping tpm-name {
> 
>  >        description
> 
> Same issue as above.  Making it config=false means it cannot be used for 
> constraining configuration data.  Many discussions on this are in the 
> archives.  More reading is in RFC8342, Network Management Datastore 
> Architecture (NMDA).
> 
>  >        uses node-uptime;
> 
> Making it mandatory would restrict informational events which could also 
> be placed into the log. Eliminating this possibility could be 
> unnecessarily restrictive as PCR reconstruction would only occur on 
> entries with a pcr-index.
> 
>  >        leaf-list event-data {
> 
>  >          type uint8;
> 
>  >          description
> 
> <https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf> 
> 
> 
> Section 5.1 provides the object definitions.   In multiple notes later 
> in section 5, this particular object has the note:
> 
> So mostly this object data will be ignored as it will not be used for 
> PCR validation.  But as it arrive as part of the TCG EFI log format, we 
> include it.
> 
>  >      grouping ima-event {
> 
>  >        description
> 
> <https://www.trustedcomputinggroup.org/wp-content/uploads/>
> 
> This interpretation is correct.  In surveying the vendors of the 
> devices, nobody wanted to mix and match algorithms for a platform.  Only 
> when the full platform supported it, would they be advertised.  Doing so 
> simplifies the management interfaces.
> 
>  > Section 2.1.2.2
> 
>  >
> 
>  >        "WG Web:   <https://datatracker.ietf.org/wg/rats/ 
> <https://datatracker.ietf.org/wg/rats/>>
> 
>  >         WG List:  <mailto:[email-masked] <mailto:[email-masked]>>
> 
>  >         Author:   Eric Voit <mailto:[email-masked] 
> <mailto:[email-masked]>>";
> 
>  >
> 
> I don't have a problem with it ;-).   Note: while *lots* of people 
> contributed initial objects and structures for their companies and did 
> reviews, I did the vast majority of the synthesis.  This included a 
> couple full rewrites after earlier authors departed.
> 
> That said, I really don’t really care if other authors are listed.  
> Anyone who wants just needs to speak up and say so.  Or add their name 
> to a GitHub pull before it is too late.
> 
>  >      identity signing {
> 
>  >        description
> 
> To minimize any chance of perceived divergence, prefer to say exactly 
> what TCG says in the referenced document.
> 
>  >      identity method {
> 
>  >        description
> 
> H in the TCG table means "hash algorithm that compresses input data to a 
> digest value or indicates a method that uses a hash".  And "H" appears 
> in the Table next to TPM_ALG_XOR.
> 
> AES is supported as a symmetric algorithm on TPM2.   But symmetric keys 
> are not use with specific PCR signatures or hashing extensions driven 
> from outside a router.  So putting a "Base" statement would make the 
> algorithm appear available when in fact it is not used in this context.
> 
>  > Section 4
> 
>  >
> 
> Added: "A vendor should restrict the ability to configure unsupported 
> algorithms."  In this case there shouldn't be the ability to configure 
> these.
> 
> Added: "In such a case, calls to an RPC requesting this specific 
> certificate could result in either no response or a response for an 
> unexpected TPM."
> 
> The receiver of the RPC response must verify that the certificate is for 
> an active AIK, i.e., the certificate has been confirmed by a third party 
> as being able to support Attestation on the targeted TPM
> 
> The receiver of the RPC response must verify that the certificate is for 
> an active AK, i.e., the private key confirmation of the quote signature 
> within the RPC response has been confirmed by a third party to belong to 
> an entity legitimately able to perform Attestation on the targeted TPM 2.0.
> 
>  > Section 6.1
> 
>  >
> 
>  >    [ISO-IEC-9797-2]
> 
>  >               <https://www.iso.org/standard/51618.html 
> <https://www.iso.org/standard/51618.html>>.
> 
>  >
> 
> As the TCG table is from before that standard was released, it seems 
> safer to leave the older reference (as getting/verifiying the new 
> version would require buying it.)
> 
>  > NITS
> 
>  >
> 
>  > Section 2.1.1.1
> 
>  >
> 
> <https://www.trustedcomputinggroup.org/wp-content/uploads/>
> 
> <https://trustedcomputinggroup.org/wp-content/uploads/>
> 
> <https://trustedcomputinggroup.org/wp-content/uploads/>
> 
> <https://en.wikipedia.org/wiki/SM4_(cipher)>
> 
> <https://www.iso.org/standard/81564.html>
>

---

From: Benjamin Kaduk <[email-masked]>
Date: Sun, 13 Mar 2022 12:44:43 -0700

Hi Henk,

> implementers. Or maybe both? Maybe you could provide us with some 

I think this question is just about the [netequip-boot-long] topic (correct
me if I'm wrong).

My core point here is that if we're going to label the reference as
"netequip-boot-long" (network equipment boot long), it had better point to
something that talks about a boot log for network equipment.  Having
thought about this a bit more, prompted by some other discussion, I'm
currently thinking that the most appealing option is to make a short
appendix that discusses the topic of a network equipment boot log, and
refer to that where we currently need [netequip-boot-log].  The YANG
description for grouping network-equipment-boot-event-log already has some
of the content for such an appendix, which could then go on to reference
the Linux IMA document in the context of an explanation that the IMA format
is reused in an expanded scope as part of the network equipment boot log
functionality.

It's not really clear to me that the TCG docs you link above would be very
helpful for this purpose, as they don't really talk about "network
equipment" specifically.

In closing, as Lars notes, a reference to just "the current Linux
documentation" is not a particularly good stable reference, so we'd
probably want to pull a specific git hash from the source tree or use an
archive.org snapshot or similar in order to get it as a stable reference.

-Ben

---

From: Benjamin Kaduk <[email-masked]>
Date: Sun, 13 Mar 2022 18:57:54 -0700

Hi Eric,

> It is not possible to post a new draft yet since we are close to the IETF, so hopefully this will be sufficient for now.

If you have a file you'd want to submit, feel free to send me (or Roman)
the XML and we can approve a manual posting during the submissions blackout.
That would make for an easier workflow for me at least, though maybe it is
harder for you.

> Duplicate of comment (2.3), fixed.

Oops, I guess my pre-balloting review missed the duplicate.  Sorry about
that.

> The longer feature name would make almost all of trees contained in the figures line wrap.  Currently none of the lines word wrap.   While the change could be made, it doesn't seem unreasonable to…

Ah, I hadn't realized there was an additional constraint in place.
That said, I count just four affected instances, which is not huge.

Still, even using "mtpm" or "m-tpm" (if that fits) would seem better than
"tpms", since "tpm" and "tpms" are very close linguistically but these
other options are not.

> This houses the set of information relating to remote attestation for a device.  This includes specific device TPM(s), the compute nodes (such as line cards) on which the TPM(s) reside, and the alg…

My point was more that we had a listing here of the two containers as
siblings in the list, but they are in a parent/child relationship in the
YANG module.  It's a little jarring (at least to me) to have them in
different relationships in the two places without having the skew
acknowledged.

> The alternative is to simply not do the  consistency check, which would drive errors.

Okay.  Thanks for the explanation.

> Custom TPM implementations can have additional PCRs.  We did not want to have the model restrict such flexibility.

That would seem to suggest not encoding the range limit into the YANG, or
am I misunderstanding something?

> It seems wise to require alternative algorithms so that users do not get confused.

Yes, it does seem wise to use hashes other than SHA1 for signatures.

But my question is about the sentence that starts "This assumes that..." --
what is the noun that the pronoun "this" is standing in for?  I don't think
it's "the hash scheme that is used to hash a TPM 1.2 PCR", but there aren't
many other choices floating around to pick from.

> sent into a specific TPM type:

I agree with what you say here, but it doesn't seem responsive to my
question.  That is, using a nonce for freshness is quite intuitive -- you
pick a random value, include it in a query, and by including that value
(which has only existed since it was created) in the response it proves
that the response was generated in response to the specific query.  This is
pretty standard stuff, and I don't expect this document to need to go into
detail about it.  Replay detection, however, is not so intuitive: there are
even some "intuitive" ways to go about it that fail to provide the needed
properties.  Building a reliable replay detection system using nonces
requires more complicated machinery than we allude to here.  I think that
if we are going to say anything about replay detection in the context of
the nonce grouping, we need to allude to the complications of replay
detection in practice, even if that's just by saying "for use as part of a
replay-detection mechanism".  The current phrasing seems to imply that the
nonce will magically allow us to detect replays, which isn't really true.

> https://datatracker.ietf.org/doc/html/rfc7950#section-7.8.3

I read the RFC for what it means, yes.  Why are those semantics what we
need here rather than the "key" semantics?

>          verified from retrieved logs is considered definitive.";

Thanks.  (nit: s/from the within/from within/)

> So mostly this object data will be ignored as it will not be used for PCR validation.  But as it arrive as part of the TCG EFI log format, we include it.

That just looks like "a binary string of length EventSize" to me; there are
no special semantics to being a list of individual octets.

I think you are well within your rights to encode it as the YANG "binary"
type instead of trying to index each octet individually.

> Now says "Digital file signature which provides a fingerprint for the file being measured."

So it's just a signature over the file, with no padding or headers applied?
That's slightly surprising, but it is what it is...

> Henk did a scrub to make sure I copied everything verbatim.

Thanks, Henk!

> H in the TCG table means "hash algorithm that compresses input data to a digest value or indicates a method that uses a hash".  And "H" appears in the Table next to TPM_ALG_XOR.

Ah, okay.  I guess I just looked at the prose and not the actual table
contents...

> The colon after the following sentence is supposed to indicate these specific vulnerabilities.

I don't understand this response.  To reiterate my comment, there is a
paragraph in the YANG security considerations template that is not present
in this document.  That paragraph concerns sensitive readable nodes.  Why
is that paragraph not present in this document?  (To be clear: "there are no
sensitive readable nodes that were not already listed in the section of
writeable nodes" is a perfectly fine answer, and there are probably other
fine answers that do not imply a need to add such a paragraph to the
document.)

> I don't know of another example, so the current text looks good.

The current text implies that the only possible use case for multi-TPM
operation is the case of multiple line cards.  I find that highly unlikely
to actually be true across all potential future developments, so I proposed
"applicable in (e.g.) cases where ..."

> Leaving current text as is, since I don't want to buy the document.

Okay.  (I didn't want to buy it, either :)

> This is what the TCG table says.  Nevertheless removed 7748.   

Thanks for all the updates and responses!

-Ben

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Mon, 14 Mar 2022 14:58:21 +0000

Hi Ben,

> From: Benjamin Kaduk, March 13, 2022 9:58 PM
> 
> Hi Eric,
> 
> > Thanks for the comments.  Tweaks in line.  Everything was covered excepting
> one ***.     Each fixed/updated area in the document corresponds to a pull
> request in:
> >
> > It is not possible to post a new draft yet since we are close to the IETF, so
> hopefully this will be sufficient for now.
> 
> If you have a file you'd want to submit, feel free to send me (or Roman) the XML
> and we can approve a manual posting during the submissions blackout.

Will unicast to both you and Roman as soon as the Pull Requests are approved.  Hopefully in the next day or so.

> > Fixed the link to the latest 2019 approved version.  Also there was a new
> version update everywhere the TPM2.0 Architecture was referenced.  They were
> updated as well.
> >
> > https://trustedcomputinggroup.org/wp-content/uploads/TPM-2p0-Keys-for-
> > Device-Identity-and-Attestation_v1_r12_pub10082021.pdf
> >
> >
> >
> Oops, I guess my pre-balloting review missed the duplicate.  Sorry about that.
> 
> >
> >
> figures line wrap.  Currently none of the lines word wrap.   While the change
> could be made, it doesn't seem unreasonable to instead optimize for easier
> readability in the figures.
> 
> Still, even using "mtpm" or "m-tpm" (if that fits) would seem better than "tpms",
> since "tpm" and "tpms" are very close linguistically but these other options are
> not.

Changed to 'mtpm' as it solves both problems.

> > This houses the set of information relating to remote attestation for a device.
> This includes specific device TPM(s), the compute nodes (such as line cards) on
> which the TPM(s) reside, and the algorithms supported across the platform.
> 
> My point was more that we had a listing here of the two containers as siblings in
> the list, but they are in a parent/child relationship in the YANG module.  It's a
> little jarring (at least to me) to have them in different relationships in the two
> places without having the skew acknowledged.

This was a design decision which solves a digestibility problem.  The majority of devices only have one TPM.  Enforcing a parent/child hierarchy (and its keying complexity) for everyone based on the minority of platforms would be a barrier to use.  This structure means that devices with one TPM never have to ask the question "why am I being asked to addressed against a hierarchy".

While it is possible to add prose describing YANG design decisions, I believe it just adds unnecessary reading complexity to implementers who won't have a reason to care.
 
> > This is an artifact of YANG.  If you make the information read only, then these
> nodes are not available to be used as constraints within the configuration
> datastore.  There were several discussions on this last year including:
> >
> That would seem to suggest not encoding the range limit into the YANG, or am I
> misunderstanding something?

We could have chosen not to encode the range.  But this means the possibility of errors for the majority of implementations.  

For those devices which do choose to use more than 0-31, there is the option of deviating away from the constraint.  See RFC-7950 section 7.20.3 for more on the "deviation" statement.  Deviations are very common in real world-deployments.

> But my question is about the sentence that starts "This assumes that..." -- what
> is the noun that the pronoun "this" is standing in for?  I don't think it's "the hash
> scheme that is used to hash a TPM 1.2 PCR", but there aren't many other choices
> floating around to pick from.

I will simply delete the sentence with "This assumes...".  It is unnecessary and doesn't add anything to the conversation.
 
> I agree with what you say here, but it doesn't seem responsive to my question.
> That is, using a nonce for freshness is quite intuitive -- you pick a random value,
> include it in a query, and by including that value (which has only existed since it
> was created) in the response it proves that the response was generated in
> response to the specific query.  This is pretty standard stuff, and I don't expect
> this document to need to go into detail about it.  Replay detection, however, is
> not so intuitive: there are even some "intuitive" ways to go about it that fail to
> provide the needed properties.  Building a reliable replay detection system using
> nonces requires more complicated machinery than we allude to here.  I think
> that if we are going to say anything about replay detection in the context of the
> nonce grouping, we need to allude to the complications of replay detection in
> practice, even if that's just by saying "for use as part of a replay-detection
> mechanism".  The current phrasing seems to imply that the nonce will magically
> allow us to detect replays, which isn't really true.

Change the text to:

"A random number intended for use as part of a replay-detection mechanism.";

> > Deleted the last sentence.  It was trying (and failing) to describe a fairly
> obvious error case which is not core the grouping definition.
> >
> > Removed "at the moment this is limited to 32", as this not necessary to point
> out within this specific definition.  It is better to just leave up to the definition of
> the PCR range.
> >
> >
> >
> I read the RFC for what it means, yes.  Why are those semantics what we need
> here rather than the "key" semantics?

You must include a "key" in a query.  "Unique" allows it the key to be omitted from the RPC, and the default will be used instead.  I.e., it makes things far easier for the user in that they can make the request on the RPC without even knowing what algorithms are supported on a platform.
 
> constraining configuration data.  Many discussions on this are in the archives.
> More reading is in RFC8342, Network Management Datastore Architecture
> (NMDA).
> >
> > >        uses node-uptime;
> >

Fixed based on a previous reviewer catch.
 
> > Making it mandatory would restrict informational events which could also be
> placed into the log. Eliminating this possibility could be unnecessarily restrictive
> as PCR reconstruction would only occur on entries with a pcr-index.
> >
> >
> >
> > >        leaf-list event-data {
> >
> > >          type uint8;
> >
> > >          description
> >
> > So mostly this object data will be ignored as it will not be used for PCR
> validation.  But as it arrive as part of the TCG EFI log format, we include it.
> 
> That just looks like "a binary string of length EventSize" to me; there are no
> special semantics to being a list of individual octets.
> 

uint8 is the datatype from the source specification.  It seemed best to leave things as the source defined.

> > This interpretation is correct.  In surveying the vendors of the devices, nobody
> wanted to mix and match algorithms for a platform.  Only when the full platform
> supported it, would they be advertised.  Doing so simplifies the management
> interfaces.
> >
> >
> >
> > > Section 2.1.2.2
> >
> > >
> >
> > I don't have a problem with it ;-).   Note: while *lots* of people contributed
> initial objects and structures for their companies and did reviews, I did the vast
> majority of the synthesis.  This included a couple full rewrites after earlier
> authors departed.
> >
> > That said, I really don’t really care if other authors are listed.  Anyone who
> wants just needs to speak up and say so.  Or add their name to a GitHub pull
> before it is too late.
> >
> > >      identity signing {
> >
> > >        description
> >
> digest value or indicates a method that uses a hash".  And "H" appears in the
> Table next to TPM_ALG_XOR.
> 
> Ah, okay.  I guess I just looked at the prose and not the actual table contents...
> 
> >
> >
> >
> >
> > AES is supported as a symmetric algorithm on TPM2.   But symmetric keys are
> not use with specific PCR signatures or hashing extensions driven from outside a
> router.  So putting a "Base" statement would make the algorithm appear
> available when in fact it is not used in this context.
> >
> > > Section 4
> >
> > >
> >
> I don't understand this response.  To reiterate my comment, there is a paragraph
> in the YANG security considerations template that is not present in this
> document.  That paragraph concerns sensitive readable nodes.  Why is that
> paragraph not present in this document?  (To be clear: "there are no sensitive
> readable nodes that were not already listed in the section of writeable nodes" is
> a perfectly fine answer, and there are probably other fine answers that do not
> imply a need to add such a paragraph to the
> document.)

Ok, I get the question now.   And there are no sensitive nodes which did not previously appear in the previous writeable section.
 
> algorithms."  In this case there shouldn't be the ability to configure these.
> >
> > Added: "In such a case, calls to an RPC requesting this specific certificate could
> result in either no response or a response for an unexpected TPM."
> >
> >
> >
> > The receiver of the RPC response must verify that the certificate is
> > for an active AIK, i.e., the certificate has been confirmed by a third
> > party as being able to support Attestation on the targeted TPM
> >
> >
> >
> > The receiver of the RPC response must verify that the certificate is for an
> active AK, i.e., the private key confirmation of the quote signature within the
> RPC response has been confirmed by a third party to belong to an entity
> legitimately able to perform Attestation on the targeted TPM 2.0.
> >
> > > Section 6.1
> >
> > >
> >
> > >    [ISO-IEC-9797-2]
> >
> > >               < <https://www.iso.org/standard/51618.html>
> https://www.iso.org/standard/51618.html>.
> >
> > >
> >
> The current text implies that the only possible use case for multi-TPM operation
> is the case of multiple line cards.  I find that highly unlikely to actually be true
> across all potential future developments, so I proposed "applicable in (e.g.)
> cases where ..."

Changed to:

"Indicates that multiple TPMs on the device can support remote attestation. For example, this feature could be used in cases where multiple line cards are present, each with its own TPM."
 
> Thanks for all the updates and responses!

And thanks for your comments as well.

Eric

---

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 14 Mar 2022 09:53:59 -0700

Hi Eric,

Things are getting sparse in here, but leaving the full context in place.

> Will unicast to both you and Roman as soon as the Pull Requests are approved.  Hopefully in the next day or so.

Ok, I will keep an eye out.

> While it is possible to add prose describing YANG design decisions, I believe it just adds unnecessary reading complexity to implementers who won't have a reason to care.

Ok.

> For those devices which do choose to use more than 0-31, there is the option of deviating away from the constraint.  See RFC-7950 section 7.20.3 for more on the "deviation" statement.  Deviations a…

Perhaps we could say something more like "at the time of this writing,
typical TPMs have at most 32 PCRs", then?

> I will simply delete the sentence with "This assumes...".  It is unnecessary and doesn't add anything to the conversation.

Okay, that works.

> "A random number intended for use as part of a replay-detection mechanism.";

Please go with "A random number intended to guarantee freshness and for use
as part of a replay-detection mechanism."; I'd hate to lose the mention of
freshness entirely.

> You must include a "key" in a query.  "Unique" allows it the key to be omitted from the RPC, and the default will be used instead.  I.e., it makes things far easier for the user in that they can ma…

Thank you!

> uint8 is the datatype from the source specification.  It seemed best to leave things as the source defined.

In this case, I respectfully disagree.  The encoding and processing
overhead is quite significant and the intent seems to clearly be "binary
string", which more naturally maps to YANG "binary".  In my reading the
source material is just using the native (C?) spelling for an octet string,
which happens to involve the type 'uint8' but that's just cosmetic and the
actual semantics are "octet/byte string".

> Ok, I get the question now.   And there are no sensitive nodes which did not previously appear in the previous writeable section.

Ok, thanks for confirming!

> "Indicates that multiple TPMs on the device can support remote attestation. For example, this feature could be used in cases where multiple line cards are present, each with its own TPM."

Thank you!

> And thanks for your comments as well.

Likewise.  I think we're looking to be in good shape, now.

-Ben

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Wed, 16 Mar 2022 00:47:44 +0000

Hi Benjamin,

Snipped away the resolved stuff.   A new version will be sent to you and Roman 
for posting shortly.

> > > > > --------------------------------------------------------------------

Being worked by Henk in a parallel thread.

> > > > > --------------------------------------------------------------------
> > > > > Section 2.1.1.6
> > > >
> > > > >
> > > >
> > > > >      typedef pcr {
> > > >
> > > > >        type uint8 {
> > > >
> > > > >          range "0..31";
> > > >
> > > > >        }
> > > >
> > > > >        description
> > > >
> > We could have chosen not to encode the range.  But this means the
> > possibility
> of errors for the majority of implementations.
> >
> > For those devices which do choose to use more than 0-31, there is the
> > option
> of deviating away from the constraint.  See RFC-7950 section 7.20.3 for more
> on
> the "deviation" statement.  Deviations are very common in real world-
> deployments.
>

Changed definition per below:

  typedef pcr {
    type uint8 {
      range "0..31";
    }
    description
      "Valid index number for a PCR.  A {{TPM2.0}} compliant PCR index
      extends from 0-31.  At this time a typical TPM would have no
      more than 32 PCRS.";
  }

> question.
> > > That is, using a nonce for freshness is quite intuitive -- you pick a
> > > random
> value,
> > > include it in a query, and by including that value (which has only
> > > existed since
> it
> > > response to the specific query.  This is pretty standard stuff, and I
> > > don't
> expect
> > > this document to need to go into detail about it.  Replay detection,
> > > however,
> is
> > > not so intuitive: there are even some "intuitive" ways to go about it
> > > that fail
> to
> > > provide the needed properties.  Building a reliable replay detection
> > > system
> using
> > > that if we are going to say anything about replay detection in the
> > > context of
> the
> > > nonce grouping, we need to allude to the complications of replay
> > > detection
> in

Done.

> > > > https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-
> Spec
> > > > ification-rev13-160330final.pdf
> > > >
> > > > in
> section 5,
> > > this particular object has the note:
> > > >
> > > > "this is a memory copy of EventSize bytes".  All the examples I see
> > > > fill this
> with
> > > "0x00, 0x00, 0x00, 0x00"
> > > >
> > > >
> > > >
> > > > PCR
> > uint8 is the datatype from the source specification.  It seemed best to
> > leave
> things as the source defined.
>

Made it binary.

Eric

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 15 Mar 2022 20:59:19 -0700

Hi Eric,

> Snipped away the resolved stuff.   A new version will be sent to you and Roman 

Likewise.  (That is, no further comments.)
I'll keep an eye out for the new version.

Many thanks,

Ben

---

From: Henk Birkholz <[email-masked]>
Date: Wed, 16 Mar 2022 22:07:51 +0100

Hi Ben,

we tried to accommodate your suggestion as quickly as possible.

There is now a PR that should address your comments:

> https://github.com/ietf-rats-wg/basic-yang-module/pull/110

If you'd assess that this proposed change addresses your comments, we'd 
include it in the next I-D submit. I sincerely hope that we managed to 
improve on:

1.) "a reference to just "the current Linux
 > documentation" is not a particularly good stable reference" and
2.) "the most appealing option is to make a short appendix that 
discusses the topic of a network equipment boot log".

The appendix is not "super" short, but it is now comprehensive, we think.

Please let us know you thoughts.

Viele Grüße,

Henk

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 16 Mar 2022 14:53:17 -0700

Hi Henk,

On a very quick skim, this looks like the sort of thing I was asking for,
thank you.  Unfortunately I'm being kept occupied by a dayjob emergency at
the moment and am not sure how quickly I can do a proper review, but it
seems preferable to include this change in the next I-D rather than omit it
entirely.

-Ben

---

From: Henk Birkholz <[email-masked]>
Date: Wed, 16 Mar 2022 23:17:25 +0100

Hi Ben,

I pushed a corresponding -17 to you and Roman. With Roman taking on the 
task of submitting that push during the moratorium, I hope that this 
helps to off-load some of the burden while your take care of little 
fires everywhere :-) I sincerely relate, which is why I am moving this 
action forward in my TZ right now.

Thanks a ton for your encouraging input. We'll stay tuned!

Viele Grüße,

Henk