Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

attest : remove "storage" and "volume" from the field names #76

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shjala
Copy link
Member

@shjala shjala commented Nov 27, 2024

Remove "storage" and "volume" from the field names, this is to make the field names more generic and not tied to storage or volume. This will make adding new keys to the attestation process easier. There has been no change in the wire format.

@shjala shjala requested a review from eriknordmark as a code owner November 27, 2024 18:38
ZAttestResponseCode response = 1; // Result of quote validation
bytes integrity_token = 2; // Valid if response is ATTEST_RESPONSE_SUCCESS, contains integrity token for device to
// Cache and present in the next config request
repeated AttestKey storage_keys = 3; // Valid if response is ATTEST_RESPONSE_SUCCESS, contain keys (recived encrypted
Copy link
Contributor

Choose a reason for hiding this comment

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

note typo "recived"

Copy link
Member Author

Choose a reason for hiding this comment

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

corrected.

ZAttestReqType reqType = 1; // type of the request
ZAttestQuote quote = 2; // attestation quote msg
repeated org.lfedge.eve.certs.ZCert certs = 3; // X509 certs in .PEM format, signed by device certificate
AttestStorageKeys storage_keys = 4; // encrypted secrets to be saved by the Controller, like encrypted keys for the volume storage vaults
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this field also have "Storage" removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

ATTEST_REQ_CERT = 1; // EVE X.509 certificates
ATTEST_REQ_NONCE = 2; // nonce request to Controller
ATTEST_REQ_QUOTE = 3; // quote msg from device
Z_ATTEST_REQ_TYPE_KEYS = 4; // to store device keys in Controller
Copy link
Contributor

Choose a reason for hiding this comment

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

Here STORE is a verb having nothing to do with storage, thus I think you should keep it.

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

ATTEST_RESP_CERT = 1; // response to cert msg
ATTEST_RESP_NONCE = 2; // response to quote request
ATTEST_RESP_QUOTE_RESP = 3; // response to quote msg
Z_ATTEST_RESP_TYPE_KEYS = 4; //response to Z_ATTEST_REQ_TYPE_KEYS
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@shjala shjala force-pushed the generic_attest branch 3 times, most recently from 93859e5 to 10acaa1 Compare November 28, 2024 10:17
Remove "storage" and "volume" from the field names, this is to make the
field names more generic and not tied to storage or volume. This will
make adding new keys to the attestation process easier.

There has been no change in the wire format.

Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
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.

2 participants