-
Notifications
You must be signed in to change notification settings - Fork 36
dpe: Resolve feedback items from #66 #88
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
base: main
Are you sure you want to change the base?
Conversation
fc6c886 to
0913e48
Compare
| | 0x08 | `U32` | 31:0 | `PROFILE` | One of `DPE_PROFILE_*`. | ||
| | 0x0C | `U32` | 31:0 | `CERTIFICATE_SIZE` | Number of bytes used in `CERTIFICATE_CHAIN`. Can be smaller than requested if no bytes are left to read. | ||
| | 0x10 | `BYTES` | 16383:0 | `CERTIFICATE_CHAIN` | Returned certificate chain. This may be a partial certificate chain. | ||
| | 0x0C | `U32` | 31:0 | `REMAINING` | Number of bytes remaining after this portion of the certificate chain. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jhand2 Instead of changing this ABI, what if we added a return status that was "There is still more data"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, ya I think that's how it works today: https://github.com/chipsalliance/caliptra-dpe/blob/main/verification/client/abi.go#L548-L551
Although in retrospect, InvalidArgument was probably a bad return code to choose :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I aligned the functionality more with how the upstream spec handles multi-part operations. This allows us to simplify some aspects along with being more compliant. Let me know what you think.
Fixes opencomputeproject#86 * Adds maximum certificate size for ML-DSA * Adds a remaining field to GetCertificateChain to fix the corner case where GetCertificateChainResponse.size == the size of the certificate * Adds detail about ML-DSA private key derivation Signed-off-by: Zach Halvorsen <zhalvorsen@google.com>
0913e48 to
cc25817
Compare
Fixes #86
GetCertificateChainResponse.size == the size of the certificate