Correct recent additions to attest.proto #133
Conversation
shjala
force-pushed
the
version.key.encryption.correction
branch
from
7c58298 to
b6d1c1d
Compare
|
@eriknordmark The yetus complaints are expected, appreciate if you double check my understanding of the |
eriknordmark
left a comment
eriknordmark
left a comment
There was a problem hiding this comment.
OK, we can ignore the bufcompat warnings since this API hasn't be used yet.
It would be good to eye-ball the diffs against the older version which is in use.
| // which PCRs to use, for example excluding PCRs that are volatile in nature | ||
| // like PCR 1 (Host Platform Configuration). If not present, default PCRs | ||
| // as per EVE design will be used. | ||
| bool has_policy_pcr_list = 3; //whether policy_pcr_list is present |
There was a problem hiding this comment.
Here a reserved = 3;
would also make sense.
There was a problem hiding this comment.
This struct is stored as binary in cloud db, we are free to make changes.
There was a problem hiding this comment.
I'd still prefer the hygene to keep the tags reserved in cases like this,
eriknordmark
left a comment
eriknordmark
left a comment
There was a problem hiding this comment.
Please mark the removed tags using reserved = X
shjala
force-pushed
the
version.key.encryption.correction
branch
from
b6d1c1d to
3f49a17
Compare
shjala
force-pushed
the
version.key.encryption.correction
branch
2 times, most recently
from
d97d1e9 to
3890539
Compare
eriknordmark
left a comment
eriknordmark
left a comment
There was a problem hiding this comment.
Looks good but it would make sense to squash this into two commits (one for the proto files and one for the derived files)
- Partially revert "attest: add versioning support to AttestVolumeKey", commit 8fe5236. - Partially revert "proto: Add PCR policy list to AttestVolumeKeyData", commit 5ac5235. - Add AttestPolicyPcrList message to specify which PCR indices are included in a TPM policy, along with a policy ID for versioning. - Extend AttestVolumeKey with policy_pcr_list and has_policy_pcr_list field to associate encrypted volume keys with their corresponding PCR-based policies. This enables EVE to use dynamic PCR policies for volume key management. - Add AttestVolumeKeyVersion enum to support multiple encryption formats for volume storage keys. The version field in AttestVolumeKeyData message provides backwards compatibility with legacy format (version 0) and enables new formats (V1 uses AES-GCM). - Correct field numbering in AttestVolumeKey and AttestVolumeKeyData for backward compatibility. Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
Add generate golang and python code for the changes. Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
shjala
force-pushed
the
version.key.encryption.correction
branch
from
3890539 to
d294a68
Compare
4 participants
This PR corrects the recent additions to the attest.proto, specifically :
AttestVolumeKeyData, notAttestVolumeKey, commercial controller implementation is expected to to store marshalledAttestVolumeKeyDataas binary, this allows us to change the volume key format without any change in the controller implementation.AttestVolumeKey, notAttestVolumeKeyData, as it is expected that EVE receives this information from controller.The logic for neither of these changes were part of any official EVE or commercial controller release, so the change is expected to be harmless 🤞🏼.