Skip to content

Conversation

eldios
Copy link

@eldios eldios commented Jan 22, 2025

Add custom feature for AMD SNP to get a derived-key via a dedicated REST API endpoint.

@eldios eldios requested a review from a team as a code owner January 22, 2025 04:15
@Xynnn007
Copy link
Member

Xynnn007 commented Feb 6, 2025

Update: SGX has similar API.

Copy link
Member

@Xynnn007 Xynnn007 left a comment

Choose a reason for hiding this comment

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

Hi @eldios , could you help to squash the commits with some commit messages? It would help review

}

service AttestationAgentService {
rpc GetDerivedKey(GetDerivedKeyRequest) returns (GetDerivedKeyResponse) {};
Copy link
Member

Choose a reason for hiding this comment

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

Could we add some documents or notes about the RPC, like
what is it for?
what does it require from the underlying TEE?
what attributes of the key would/would not have?

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems rather intrusive change (given the API impact) to enable an SNP specific feature. Might be easier to just make /dev/sev_guest available to the workload and have the ioctl logic implemented there.

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure whether the interface that Lele wants to add is to use the TEE hardware features as a trusted cryptographic seed source to derive keys for other cryptographic operations. If so, SGX/TPM have similar interfaces to derive key, and I think it can be further designed as a high-level interface, using different hardware features at the bottom layer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, it would be good to understand (CoCo) use-case first.

Copy link
Member

Choose a reason for hiding this comment

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

one twist is that I'm not sure configfs supports this stuff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants