[MLDSA-Support] Add MLDSA support to provisioning services (2/3)#293
[MLDSA-Support] Add MLDSA support to provisioning services (2/3)#293willyzha merged 4 commits intolowRISC:mainfrom
Conversation
willyzha
force-pushed
the
partial-mldsa-support-2
branch
7 times, most recently
from
26c6820 to
24ca8bd
Compare
|
@timothytrippel - I added definitions the increased TLV header, should we bring those same definitions over to the OpenTitan repo? |
| crthNameSizeFieldShift = crthSizeFieldWidth | ||
| crthNameSizeFieldWidth = 4 | ||
| crthNameSizeFieldMask = (1 << crthNameSizeFieldWidth) - 1 | ||
| sizeOfObjectHeaderV0 = 2 |
There was a problem hiding this comment.
I think we should leave the comment saying where these originate from, and add a comment saying we need to update opentitan repo with new format (and file an issue in the opentitan repo to track this).
| template="${CERTGEN_TEMPLATES[$i]}" | ||
| key="${CERTGEN_KEYS[$i]}" | ||
| endorsing_key="${CERTGEN_ENDORSING_KEYS[$i]}" | ||
| key_type="${CERTGEN_KEY_TYPES[$i]}" |
There was a problem hiding this comment.
should we consider it an error for the key type not to match the endorsing key type? is there a scenario we would ever want them not to match?
There was a problem hiding this comment.
I think it might make testing easier right now since OT firmware doesn't support producing MLDSA DeviceID public keys. For testing we can have the HSM certify the exiting ECDSA public keys with an MLDSA key from the HSM.
In the long term if we're ok with being prescriptive on exactly what algorithms we support we can probably enforce that the key_type == endorsing_key_type.
There was a problem hiding this comment.
OK lets file an issue to track updating this to trigger an error in the future then so we dont lose track
| openssl x509 -req -engine "${ENGINE}" -keyform engine \ | ||
| -in "${CSR_FILE}" \ | ||
| -out "${CERT_FILE}" \ | ||
| -days 7300 \ |
There was a problem hiding this comment.
this reminds me: can you file an issue to parameterize this in a follow-up PR? (so we don't forget)
There was a problem hiding this comment.
specifically parametrize the expiry?
Yes; let's file an issue over there to track and bring them over |
Signed-off-by: Willy Zhang <willyzhang@google.com>
Signed-off-by: Willy Zhang <willyzhang@google.com>
…sizing Signed-off-by: Willy Zhang <willyzhang@google.com>
willyzha
force-pushed
the
partial-mldsa-support-2
branch
from
24ca8bd to
307a341
Compare
* Define perso blob version enum * Remove isLegacyV0 helpers and use version helpers Signed-off-by: Willy Zhang <willyzhang@google.com>
willyzha
force-pushed
the
partial-mldsa-support-2
branch
from
307a341 to
b6a59b8
Compare
Filed lowRISC/opentitan#29432 |
| // - V1 Cert Header (32-bit) | ||
| size_t expected_size = | ||
| sizeof(perso_tlv_object_header_t) + sizeof(perso_tlv_cert_header_t) + | ||
| sizeof(perso_tlv_object_header_v0_t) + sizeof(uint16_t) + |
There was a problem hiding this comment.
Why is both the size of a v0 and v1 object added here?
2 participants
Increase perso blob size limit to support larger MLDSA certificates.
The existing TLV structure allocated 12bits for data, which limits data to 4k. This is far too small for larger PQ algorithms.
This PR introduces a magic version component that if present as the first TLV in the blob will specify the version of the remainder of the blob. If the version TLV is not found it the perso blob will be assumed to use the legacy 16bit header for backwards compatibility.
This PR is chained on top of PR #292