Subject: [Rats] comments on draft-birkholz-rats-basic-yang-module-00:

From: "Xialiang (Frank, Network Standard & Patent Dept)" <[email-masked]>
Date: Wed, 19 Jun 2019 10:34:45 +0000

Hi authors,
please find below an proposed addition to the module from Henk and me, also a set of summarized comments for this draft to review:

module: ietf-basic-remote-attestation
  +--ro rats-support-structures
     +--ro supported-algos*            uint16
     +--ro tpms* [tpm_name]      //is version info needed here?
     |  +--ro tpm_name              string
     |  +--ro tpm-physical-index?   int32 {ietfhw:entity-mib}?
     +--ro compute-nodes* [node-name]
     |  +--ro node-name              string
     |  +--ro node-physical-index?   int32 {ietfhw:entity-mib}?
     +--ro endorsement-certificates       //Is this the TCG specified EK certificate, or AK used here for RA, or a unified one? Is it better to move this under the respective tpm_name in tpms above, and delete this structure?
        +--ro certificate* [tpm_name]
           +--ro tpm_name                   string
           +--ro tpm-physical-index?        int32 {ietfhw:entity-mib}?
           +--ro endorsement-certificate    binary

  rpcs:
    +---x tpm12-challenge-response-attestation
    |  +---w input
    |  |  +---w tpm1-attestation-challenge
    |  |     +---w pcr-indices*            uint8
    |  |     +---w nonce-value             binary
    |  |     +---w TPM_SIG_SCHEME-value    uint8
    |  |     +---w (key-identifier)?
    |  |     |  +--:(public-key)
    |  |     |  |  +---w pub-key-id?       binary
    |  |     |  +--:(TSS_UUID)
    |  |     |     +---w TSS_UUID-value
    |  |     |        +---w ulTimeLow?       uint32
    |  |     |        +---w usTimeMid?       uint16
    |  |     |        +---w usTimeHigh?      uint16
    |  |     |        +---w bClockSeqHigh?   uint8
    |  |     |        +---w bClockSeqLow?    uint8
    |  |     |        +---w rgbNode*         uint8
    |  |     +---w add-version?            boolean
    |  |     +---w tpm_name?               string
    |  |     +---w tpm-physical-index?     int32 {ietfhw:entity-mib}?
    |  +--ro output
    |     +--ro tpm12-attestation-response* [tpm_name]
    |        +--ro tpm_name                     string
    |        +--ro tpm-physical-index?          int32 {ietfhw:entity-mib}?
    |        +--ro up-time?                     uint32
    |        +--ro node-name?                   string
    |        +--ro node-physical-index?         int32 {ietfhw:entity-mib}?
    |        +--ro (tpm12-quote)
    |           +--:(tpm12-quote)
    |           |  +--ro version* []
    |           |  |  +--ro major?      uint8
    |           |  |  +--ro minor?      uint8
    |           |  |  +--ro revMajor?   uint8
    |           |  |  +--ro revMinor?   uint8
    |           |  +--ro fixed?                 binary
    |           |  +--ro digest-value?          binary
    |           |  +--ro external-data?         binary
    |           |  +--ro TPM_PCR_COMPOSITE* []
    |           |  |  +--ro pcr-indices*       uint8
    |           |  |  +--ro value-size?        uint32
    |           |  |  +--ro tpm12-pcr-value*   binary
    |           |  +--ro signature-size?        uint32
    |           |  +--ro signature?             binary
    |           +--:(tpm12-quote2)
    |              +--ro tag?                   uint8
    |              +--ro fixed?                 binary
    |              +--ro external-data?         binary
    |              +--ro pcr-indices*           uint8
    |              +--ro locality-at-release?   uint8
    |              +--ro digest-at-release?     binary
    |              +--ro signature-size?        uint32
    |              +--ro signature?             binary
    +---x tpm2-challenge-response-attestation
    |  +---w input
    |  |  +---w tpm2-attestation-challenge    //should we consider supporting multiple tpms in one challenge-response interaction? And the output below has already multiple tpms~~
    |  |  |  +---w pcr-list* []    //this is an array of array of PCRs, why do you design in this way?
    |  |  |  |  +---w pcr
    |  |  |  |     +---w pcr-indices*                  uint8
    |  |  |  |     +---w (algo-registry-type)
    |  |  |  |        +--:(tcg)
    |  |  |  |        |  +---w tcg-hash-algo-id?       uint16
    |  |  |  |        +--:(ietf)
    |  |  |  |           +---w ietf-ni-hash-algo-id?   uint8
    |  |  |  +---w nonce-value                   binary
    |  |  |  +---w (signature-identifier-type)
    |  |  |  |  +--:(TPM_ALG_ID)
    |  |  |  |  |  +---w TPM_ALG_ID-value?       uint16
    |  |  |  |  +--:(COSE_Algorithm)
    |  |  |  |     +---w COSE_Algorithm-value?   int32
    |  |  |  +---w (key-identifier)?
    |  |  |     +--:(public-key)
    |  |  |     |  +---w pub-key-id?             binary
    |  |  |     +--:(uuid)
    |  |  |        +---w uuid-value?             binary
    |  |  +---w tpm_name?                     string
    |  |  +---w tpm-physical-index?           int32 {ietfhw:entity-mib}?
    |  +--ro output
    |     +--ro tpm2-attestation-response* [tpm_name]
    |        +--ro tpm_name               string
    |        +--ro tpm-physical-index?    int32 {ietfhw:entity-mib}?
    |        +--ro up-time?               uint32
    |        +--ro node-name?             string
    |        +--ro node-physical-index?   int32 {ietfhw:entity-mib}?
    |        +--ro tpms-attest
    |        |  +--ro pcrdigest?                   binary
    |        |  +--ro tpms-attest-result?          binary
    |        |  +--ro tpms-attest-result-length?   uint32
    |        +--ro tpmt-signature?        binary    //is signature-length missed here?
    +---x basic-trust-establishment          //So, this rpc is used once before the attestation process for trust establishment and certificate & key-identifier exchange? If so, it is still tpm2.0 specific, I assume we need one for tpm1.2, too.
    |  +---w input
    |  |  +---w nonce-value                   binary
    |  |  +---w (signature-identifier-type)
    |  |  |  +--:(TPM_ALG_ID)
    |  |  |  |  +---w TPM_ALG_ID-value?       uint16
    |  |  |  +--:(COSE_Algorithm)
    |  |  |     +---w COSE_Algorithm-value?   int32
    |  |  +---w tpm_name?                     string
    |  |  +---w tpm-physical-index?           int32 {ietfhw:entity-mib}?
    |  |  +---w certificate-name?             string          //how does the verifier know the existing certificate names in advance? The "rats-support-structures" above only provides the EK certificate, not AK.
    |  +--ro output
    |     +--ro attestation-certificates* [tpm_name]    // challenge message has only one tpm_name, why are there multiple tpm's response here?
    |        +--ro tpm_name                   string
    |        +--ro tpm-physical-index?        int32 {ietfhw:entity-mib}?
    |        +--ro up-time?                   uint32
    |        +--ro node-name?                 string
    |        +--ro node-physical-index?       int32 {ietfhw:entity-mib}?
    |        +--ro certificate-name?          string
    |        +--ro attestation-certificate?   ietfct:end-entity-cert-cms     //Same problem, what and where is AK certificate?
    |        +--ro (key-identifier)?           //should here consider the TPM 1.2 TSS_UUID option? If we can use this RPC for TPM2.0 and TPM1.2 both, that should be added here. If not, it goes in an corresponding TPM1.2 RPC, analogously.
    |           +--:(public-key)
    |           |  +--ro pub-key-id?          binary
    |           +--:(uuid)
    |              +--ro uuid-value?          binary
    +---x log-retrieval
       +---w input
       |  +---w log-selector* [node-name]
       |  |  +---w node-name                 string
       |  |  +---w node-physical-index?      int32 {ietfhw:entity-mib}?
       |  |  +---w (index-type)?
       |  |     +--:(last-entry)
       |  |     |  +---w last-entry-value?   binary
       |  |     +--:(index)
       |  |     |  +---w index-number?       uint64
       |  |     +--:(timestamp)
       |  |        +---w timestamp?          yang:date-and-time
       |  +---w log-type              identityref
       |  +---w pcr-list* []      //this is an array of array of PCRs, why do you design in this way?
       |  |  +---w pcr
       |  |     +---w pcr-indices*                  uint8
       |  |     +---w (algo-registry-type)
       |  |        +--:(tcg)
       |  |        |  +---w tcg-hash-algo-id?       uint16
       |  |        +--:(ietf)
       |  |           +---w ietf-ni-hash-algo-id?   uint8
       |  +---w log-entry-quantity?   uint16
       +--ro output
          +--ro system-event-logs
             +--ro node-data* [node-name]
                +--ro node-name              string
                +--ro node-physical-index?   int32 {ietfhw:entity-mib}?
                +--ro up-time?               uint32
                +--ro tpm-updated* [tpm_name]         //so, does it mean that there are possibly multiple tpms in one node? If so, how to use the PCR values of them together to verify the log integrity?
                |  +--ro tpm_name              string
                |  +--ro tpm-physical-index?   int32 {ietfhw:entity-mib}?
                +--ro log-result
                   +--ro (log-type)
                      +--:(bios)
                      |  +--ro bios-event-logs
                      |     +--ro bios-event-entry* [event-number]
                      |        +--ro event-number    uint32
                      |        +--ro event-type?     uint32
                      |        +--ro pcr-index?      uint16         //should this be the pcr-value?
                      |        +--ro digest-list* []
                      |        |  +--ro (algo-registry-type)
                      |        |  |  +--:(tcg)
                      |        |  |  |  +--ro tcg-hash-algo-id?       uint16
                      |        |  |  +--:(ietf)
                      |        |  |     +--ro ietf-ni-hash-algo-id?   uint8
                      |        |  +--ro digest*                       binary
                      |        +--ro event-size?     uint32
                      |        +--ro event-data*     uint8
                      +--:(ima)
                         +--ro ima-event-logs
                            +--ro ima-event-entry* [event-number]
                               +--ro event-number               uint64
                               +--ro ima-template?              string
                               +--ro filename-hint?             string
                               +--ro filedata-hash?             binary
                               +--ro template-hash-algorithm?   string
                               +--ro template-hash?             binary
                               +--ro pcr-index?                 uint16    //should this be the pcr-value?
                               +--ro signature?                 binary

Thanks a lot!

B.R.
Frank