search

ietf-rats / draft-birkholz-rats-basic-yang-module

This repository is abandoned. The adopted I-D can be found at:
https://github.com/ietf-rats-wg/basic-yang-module
Other
0 stars 3 forks source link

To address comments from @Xialiang #4

Closed shwethab closed 5 years ago

shwethab commented 5 years ago

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?

[SB]Good idea. Done The certs are TPM specific makes sense to group them under tpm

    +--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~~

[SB] Right. Addressed.

|  |  |  +---w pcr-list* []    //this is an array of array of PCRs, why do you design in this way?

[SB] how else? We need multiple PCRs to be selected for attestation. | | | | +---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? [SB] No, it is binary the wire encapsulation of the model will have a way to frame it and indicate the length. IT doesnt have to be explicit.

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

[SB] How will it be different for TPM1.2? This RPC fetches the AIK certificate that can be validated and then used for verifying attested data.

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

[SB] rats-support-structures provides all the unique certs for the TPM. Should provide both EK and AK.

|  +--ro output
|     +--ro attestation-certificates* [tpm_name]    // challenge message has only one tpm_name, why are there multiple tpm’s response here?

[SB] updated the input. We did have the input selection based on compute node name earlier. But i think it didnt make it to the draft model.

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

[SB] updated the description in rats-support-structure

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

[SB] This can be considered.

|           +--:(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?

[SB] Can you please propose an alternate that you have in mind? It is a list so we can represent multiple PCR in the selection.

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

[SB] You are right, The logs only which PCR index was extended. So it makes sense to organize the logs with node-name,tpm_name as the key. If a node has multiple TPMs it would extend the PCR and maintain logs separately per TPM from my experience. Bill, Eric, Henk is this right?

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

[SB] No it is pcr-index. The logs tell which pcr is extended. the digest will indicate the hash that was extended into this pcr.
| +--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? [SB] No it is pcr-index. The logs tell which pcr is extended. the digest will indicate the hash that was extended into this pcr. 

                           +--ro signature?
henkbirkholz commented 5 years ago

This pr has been vetted by the authors. There might be more merges or pushes before cut-off.