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.

Sign up for GitHub

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

Jump to bottom

[dice] Refactor the dice library to support multiple implementations #24609

Merged
merged 2 commits into from
Oct 2, 2024
Merged

[dice] Refactor the dice library to support multiple implementations #24609

merged 2 commits into from
Oct 2, 2024

Conversation

tommychiu-github
Copy link
Contributor

@tommychiu-github tommychiu-github commented Sep 20, 2024 •
edited
Loading

To support mutliple DICE implementation (X.509 with ASN.1, and CWT with CBOR), it'll require to refactor current dice lib implementation.

  1. Add a dice_cwt lib (implementation details to be added later)
  2. Levaraging the third_party open_dice library to generate UDS_Pub structure

@tommychiu-github tommychiu-github requested a review from a team as a code owner September 20, 2024 06:18
@tommychiu-github tommychiu-github requested review from timothytrippel and removed request for a team September 20, 2024 06:18
@timothytrippel timothytrippel marked this pull request as draft September 20, 2024 18:45
timothytrippel
timothytrippel reviewed Sep 25, 2024
View reviewed changes
Copy link
Contributor

@timothytrippel timothytrippel left a comment

Choose a reason for hiding this comment

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

Couple of suggestions, otherwise this LGTM. Thanks @tommychiu-github !

sw/device/silicon_creator/lib/cert/BUILD Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/cert/dice_cwt.c
// UDS (Creator) attestation key diverisfier constants.
// Note: versions are always set to 0 so these keys are always valid from the
// perspective of the keymgr hardware.
const sc_keymgr_diversification_t kUdsKeymgrDiversifier = {
Copy link
Contributor

Choose a reason for hiding this comment

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

since these diversifiers are the same between both DICE libs (dice.c and dice_cwt.c), perhaps these should move to a separate lib that is included in both dice.c and dice_cwt.c so we don't repeat ourselves here. (OK to do in a follow up PR/commit).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Will push a commit for that later.

sw/device/silicon_creator/lib/cert/dice_cwt.c Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/cert/cbor_helper.h Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/cert/BUILD Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/cert/cbor_helper.h Outdated Show resolved Hide resolved
sw/device/silicon_creator/lib/cert/dice_cwt.c Show resolved Hide resolved
@timothytrippel timothytrippel marked this pull request as ready for review September 25, 2024 05:32
@timothytrippel timothytrippel changed the title Multiple ft_personalize_base refactor [dice] add dice_cwt lib for generating DICE CWT certs Sep 25, 2024
@timothytrippel
Copy link
Contributor

@tommychiu-github for future reference, please update the title and descriptions of your PRs to be more descriptive and formatted like commit message. Thanks!

@tommychiu-github
[manuf] declare multiple ft_personalize_base
906f50f 
We are having multiple DICE implementations for different SKUs. Declare
multiple ft_personalize_base* lib to link the related implementations.

Bug: 356532759
Bug: lowRISC/opentitan:lowRISC#24281
Test: ft_provision_dice_cwt_fpga_cw340_rom_with_fake_keys

Signed-off-by: Tommy Chiu <[email protected]>
@tommychiu-github tommychiu-github changed the title [dice] add dice_cwt lib for generating DICE CWT certs [dice] Refactor the dice library to support multiple implementations Sep 30, 2024
@tommychiu-github
Copy link
Contributor Author

@tommychiu-github for future reference, please update the title and descriptions of your PRs to be more descriptive and formatted like commit message. Thanks!

Updated.

@tommychiu-github
[dice] Add CWT COSE_Key implementation for Uds_pub
a471a83 
For dice_lib to support different formats including X509 and CWT(CBOR).

Bug: 356532759
Bug: lowRISC/opentitan:lowRISC#24281
Test: ft_provision_dice_cwt_fpga_cw340_rom_with_fake_keys

Signed-off-by: Tommy Chiu <[email protected]>
@timothytrippel timothytrippel self-requested a review October 2, 2024 05:33
@timothytrippel timothytrippel merged commit fbe88bb into lowRISC:master Oct 2, 2024
38 checks passed
@timothytrippel timothytrippel added the CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 label Oct 26, 2024
@github-actions GitHub Actions
Copy link

Successfully created backport PR for earlgrey_1.0.0:

@github-actions github-actions bot mentioned this pull request Oct 26, 2024
Closed
@timothytrippel timothytrippel added CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 and removed CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 labels Oct 27, 2024
@github-actions GitHub Actions
Copy link

Successfully created backport PR for earlgrey_1.0.0:

@github-actions github-actions bot mentioned this pull request Oct 27, 2024
Merged
@timothytrippel timothytrippel mentioned this pull request Oct 28, 2024
Closed
@timothytrippel timothytrippel added CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 and removed CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 labels Oct 28, 2024
@github-actions GitHub Actions
Copy link

Git push to origin failed for earlgrey_1.0.0 with exitcode 1

@github-actions github-actions bot added the Manually CherryPick This PR should be manually cherry picked. label Oct 28, 2024
@timothytrippel timothytrippel removed Manually CherryPick This PR should be manually cherry picked. CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 labels Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timothytrippel timothytrippel timothytrippel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tommychiu-github @timothytrippel