Skip to content

Conversation

fdamato
Copy link
Collaborator

@fdamato fdamato commented Aug 23, 2025

No description provided.

Signed-off-by: Fabrizio Damato <[email protected]>
- Updated Contributors list

Signed-off-by: Fabrizio Damato <[email protected]>
+=====================+=====================+=====================+==============================================+
| 0 | CommandCode | 1 | Shall be 02h to indicate GET_EAT. |
+---------------------+---------------------+---------------------+----------------------------------------------+
| 1 | CommandVersion | 1 | The version of this request structure. |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally better to put the CommandVersion first in case the command is parsed and we need to key off-of that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont disagree w/ the comment in general, but I dont have strong opinion as well for this specific case. Followed same convention of GET_ENVELOPE_SIGNED_CSR (https://github.com/opencomputeproject/Security/blob/main/specifications/device-identity-provisioning/spec.ocp).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be worth changing there too. Generally version and size being the first two fields of the structure helps with maximum versioning compatiblity, and easier to write code for.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, @bluegate010 any thoughts on this ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shuffling thusly sounds good to me.

+---------------------+---------------------+---------------------+----------------------------------------------+
| 8 | EATToken | EATLength | Shall be the Entity Attestation Token |
| | | | conforming to this OCP Profile. This |
| | | | field shall be CBOR-encoded |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: is the encoding necessary to be specified as a shall? can be a stream of bytes i nthe response and specify elsewhere what the format of the EATToken is, which we already specify ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not being prescriptive ? I have no issue to remove SHALLL if you feel strong about it

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no strong opinion either way. being prescriptive in a single place generally means you only need to change one place. i think CBOR encoding is implied for EAT token in other parts of the spec, so didnt think it was needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. I can remove

- Certificate chain validation **MUST** be performed according to the requester's trust policy
- The x5-chain in the unprotected header **MUST** be validated before trusting the EAT contents

### Replay Protection

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm not sure i follow this section. i realize all of this is should, but i'm not sure this is necessary since "requesters" may implement something unnecessary. If the nonce requirements are met, this shouldnt be required at all right? this feels like it leads to temptation to do something non standard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree requester is misleading, would it replacing to "verifier" sound more reasonable ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was questioning the need for this section at all? If a verifier follows the nonce requirements, not sure what maintaining the cache would help with, and the SHOULD NOT makes it sound like it is okay to reuse a nonce within some time window (which should never be the case?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it is redundant...will strip out


## Overview

The GET_EAT command enables requesters to obtain attestation evidence from a device in the form of an Entity Attestation Token (EAT) that conforms to this OCP Profile. This command is designed to be transport-agnostic while providing a standardized interface for attestation requests.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requester is confusing, specially in the context of SPDM. SPDM has a very specific meaning for requester, here you really mean verifier?
might be good to delineate SPDM requester from this specs requester.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I will replace with verifier


2. **Requesters** using GET_EAT:
- **SHOULD** implement appropriate timeout handling
- **MUST** be prepared to handle ERROR responses

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SPDM ERROR responses?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SPDM is not the only supported transport layer.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack, might be good to clarify "error responses from the trasport". the capitalized ERROR sounds like it is referring to something specific.

  - Replace Requester with Verifier
  - Swap Command Version and Command Code
  - Strip unnecessary description of Security Considerations
  - Add Clarification for large buffer handling

Signed-off-by: Fabrizio Damato <[email protected]>
@fdamato fdamato requested a review from raghuncstate August 26, 2025 14:36
@@ -552,6 +553,76 @@ algorithms:

See <https://github.com/opencomputeproject/Security/tree/main/specifications/ietf-eat-profile>.

## OCP Command Registry {#sec:ocp-command-registry}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this registry is at the level of OCP, not OCP Security, then it should go in (something like) https://github.com/opencomputeproject/ocp-oid-registry. Maybe @bluegate010 can rename that repository to ocp-registry and not make it specific to OIDs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, on reflection I think it at least needs to be in a separate document, and it's possible that other projects will want to define their own. Will ask OCP to change the registry repo name.

Copy link
Collaborator Author

@fdamato fdamato Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds good... I`ll move to https://github.com/opencomputeproject/ocp-registry. Is a markdown document the right place to include the OCP Command Registry, or are you expecting a different format ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Markdown is fine, yeah.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will create a separate commit, as it is a separate github repo

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's drop the changes to this spec from the PR?

@steven-bellock steven-bellock added the EAT OCP Entity Attestation Token spec label Aug 26, 2025
@fdamato fdamato force-pushed the fadamato/get_eat_support branch 2 times, most recently from 2d07c0d to 6101b25 Compare August 27, 2025 18:07
… from Attestation Main Spec

Signed-off-by: Fabrizio Damato <[email protected]>
@fdamato fdamato force-pushed the fadamato/get_eat_support branch from 6101b25 to 65a9c1a Compare August 27, 2025 18:08
@@ -552,6 +553,76 @@ algorithms:

See <https://github.com/opencomputeproject/Security/tree/main/specifications/ietf-eat-profile>.

## OCP Command Registry {#sec:ocp-command-registry}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's drop the changes to this spec from the PR?

Comment on lines +328 to +330
| 2 | Status | 1 | Response status. |
| | | | 0x00 = SUCCESS |
| | | | 0x01 = ERROR |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transport protocol should have an error reporting mechanism that we can use?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I`m not really convinced on this one... because we would lose control on ERROR type, if we want to expand more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to use SPDM's ExtendedErrorData field for vendor-defined errors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this is a heavy requirement to bring for NON SPDM adopters... don`t you agree?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, we can define in the transport binding how error codes should be returned. For SPDM we can avail ourselves of their existing machinery, and for other transports we can do something else.

2. The EAT Profile claim (265) **MUST** be present and contain the OCP Profile OID
3. The Nonce claim (10) **MUST** be present and contain the nonce value from the request
4. The Measurements claim (273) **MUST** be present and contain concise evidence
5. The issuer claim (1) **SHALL** be present if binding to a certificate chain
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this line uses SHALL instead of MUST?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer consistency and move everything to MUST

- The Responder **MUST** use the SPDM VENDOR_DEFINED_RESPONSE format
- The StandardID field **MUST** contain 4 (IANA)
- The VendorID field **MUST** contain 42623 (OCP)
- The first byte of the VendorDefinedReqPayload/VendorDefinedRespPayload is the Command Code, and must contain the value 02h to indicate GET_EAT
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- The first byte of the VendorDefinedReqPayload/VendorDefinedRespPayload is the Command Code, and must contain the value 02h to indicate GET_EAT
- The first byte of the VendorDefinedReqPayload/VendorDefinedRespPayload is the Command Code, and **MUST** contain the value 02h to indicate GET_EAT

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack

Comment on lines +374 to +375
TSM engines and other transport mechanisms **MAY** define their own bindings for the GET_EAT command, provided they:
- Maintain semantic equivalence of request and response structures
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will need a blank line between these two in order for the bullet points to render properly. Here and above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET_EAT should go in (something like) OCP Attestation. How the EAT is transported should decoupled from the EAT itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no problem with that, but we initially discussed to keep it together with the profile. @bluegate010 you ok to move to OCP Attestation 1.1 spec ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EAT OCP Entity Attestation Token spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants