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

[rom_ext_e2e] Add an ownership transfer test #24419

Merged
merged 4 commits into from
Sep 3, 2024
Merged

[rom_ext_e2e] Add an ownership transfer test #24419

merged 4 commits into from
Sep 3, 2024

Conversation

cfrantz
Copy link
Contributor

@cfrantz cfrantz commented Aug 27, 2024

  1. Create a library of helper functions for facilitating ownership
    transfer tests.
  2. Create a basic ownership transfer test that transfers chip ownership
    from the default fake test owner to a dummy owner.

@cfrantz cfrantz requested a review from a team as a code owner August 27, 2024 22:47
@cfrantz cfrantz removed the request for review from a team August 27, 2024 22:47
moidx
moidx approved these changes Aug 28, 2024
View reviewed changes
sw/host/tests/ownership/transfer_lib.rs Outdated Show resolved Hide resolved
sw/host/tests/ownership/transfer_lib.rs Show resolved Hide resolved
sw/device/silicon_creator/rom_ext/e2e/ownership/BUILD
package(default_visibility = ["//visibility:public"])

opentitan_test(
name = "ownership_transfer_test",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we start maintaining a test plan for ownership transfer?

Similar to sw/device/silicon_creator/rom/data/rom_e2e_testplan.hjson. I think it is ok if we add it as a separate test plan.

It will be useful certification later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the current ownership transfer test plan:
https://github.com/lowRISC/opentitan/blob/earlgrey_es_sival/sw/device/silicon_creator/rom_ext/data/rom_ext_e2e_testplan.hjson#L221-L488

This covers basic functionality; I think more test cases are needed however.

sw/host/tests/ownership/transfer_test.rs
transport.reset_target(Duration::from_millis(50), /*clear_uart=*/ true)?;
let capture = UartConsole::wait_for(
&*uart,
r"(?msR)ownership_state = UANY$.*ownership_transfers = (\d+)$.*PASS!$",
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it may be useful for supply chain to be able to log a hash or some other identifier for the previous owner. This is something we can track as an open.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #24429

@moidx
Copy link
Contributor

moidx commented Aug 29, 2024

LGTM

@cfrantz cfrantz force-pushed the owner-init-tests branch 2 times, most recently from 6eaa95f to 0e1f2f3 Compare August 30, 2024 16:22
@cfrantz
Copy link
Contributor Author

cfrantz commented Aug 30, 2024

Fixes: #24465

cfrantz added 4 commits August 30, 2024 13:40
@cfrantz
[opentitantool] Improve rescue CLI
4c593f5 
1. Enhance the boot-svc commands to retrieve and display the response.
2. Emit any boot-svc response `status` as hex.

Signed-off-by: Chris Frantz <cfrantz@google.com>
@cfrantz
[ownership] Add a dummy owner
e5c2899 
The `dummy` owner is a secondary owner used for testing ownership
transfer flows.

Signed-off-by: Chris Frantz <cfrantz@google.com>
@cfrantz
[opentitanlib] Bindgen rom_error_t into Rust
5ecb3fb 
Signed-off-by: Chris Frantz <cfrantz@google.com>
@cfrantz
[rom_ext_e2e] Add an ownership transfer test
0ad99c9 
1. Create a library of helper functions for facilitating ownership
   transfer tests.
2. Create a basic ownership transfer test that transfers chip ownership
   from the default `fake` test owner to a `dummy` owner.

Signed-off-by: Chris Frantz <cfrantz@google.com>
cfrantz added a commit to cfrantz/opentitan that referenced this pull request Aug 30, 2024
@cfrantz
[rom_ext_e2e] More ownership transfer tests
1f21fd3 
The tests added by this change are all simple variations on
the ownership transfer test added in lowRISC#24419.  They involve verifying the
different modes and error conditions (e.g. using the wrong key).

Adds the following tests:
- `bad_unlock_test`; Fixes lowRISC#24466
- `bad_activate_test`; Fixes lowRISC#24467
- `bad_owner_block_test`; Fixes lowRISC#24468
- `bad_app_key_test`; Fixes lowRISC#24469
- `transfer_endorsed_test`; Fixes lowRISC#24470
- `bad_endorsee_test`; Fixes lowRISC#24471
- `locked_update_test`; Fixes lowRISC#24472
- `bad_locked_update_test` & `bad_locked_update_no_exec_test`; Fixes lowRISC#24473

Signed-off-by: Chris Frantz <cfrantz@google.com>
@cfrantz cfrantz mentioned this pull request Aug 30, 2024
Merged
@cfrantz cfrantz merged commit 0412396 into lowRISC:earlgrey_es_sival Sep 3, 2024
25 checks passed
cfrantz added a commit to cfrantz/opentitan that referenced this pull request Sep 3, 2024
@cfrantz
[rom_ext_e2e] More ownership transfer tests
4d520bd 
The tests added by this change are all simple variations on
the ownership transfer test added in lowRISC#24419.  They involve verifying the
different modes and error conditions (e.g. using the wrong key).

Adds the following tests:
- `bad_unlock_test`; Fixes lowRISC#24466
- `bad_activate_test`; Fixes lowRISC#24467
- `bad_owner_block_test`; Fixes lowRISC#24468
- `bad_app_key_test`; Fixes lowRISC#24469
- `transfer_endorsed_test`; Fixes lowRISC#24470
- `bad_endorsee_test`; Fixes lowRISC#24471
- `locked_update_test`; Fixes lowRISC#24472
- `bad_locked_update_test` & `bad_locked_update_no_exec_test`; Fixes lowRISC#24473

Signed-off-by: Chris Frantz <cfrantz@google.com>
cfrantz added a commit that referenced this pull request Sep 4, 2024
@cfrantz
[rom_ext_e2e] More ownership transfer tests
137c3e9 
The tests added by this change are all simple variations on
the ownership transfer test added in #24419.  They involve verifying the
different modes and error conditions (e.g. using the wrong key).

Adds the following tests:
- `bad_unlock_test`; Fixes #24466
- `bad_activate_test`; Fixes #24467
- `bad_owner_block_test`; Fixes #24468
- `bad_app_key_test`; Fixes #24469
- `transfer_endorsed_test`; Fixes #24470
- `bad_endorsee_test`; Fixes #24471
- `locked_update_test`; Fixes #24472
- `bad_locked_update_test` & `bad_locked_update_no_exec_test`; Fixes #24473

Signed-off-by: Chris Frantz <cfrantz@google.com>
@cfrantz cfrantz mentioned this pull request Oct 28, 2024
Merged
github-actions bot pushed a commit that referenced this pull request Oct 29, 2024
@cfrantz @github-actions
[rom_ext_e2e] More ownership transfer tests
fd1898f 
The tests added by this change are all simple variations on
the ownership transfer test added in #24419.  They involve verifying the
different modes and error conditions (e.g. using the wrong key).

Adds the following tests:
- `bad_unlock_test`; Fixes #24466
- `bad_activate_test`; Fixes #24467
- `bad_owner_block_test`; Fixes #24468
- `bad_app_key_test`; Fixes #24469
- `transfer_endorsed_test`; Fixes #24470
- `bad_endorsee_test`; Fixes #24471
- `locked_update_test`; Fixes #24472
- `bad_locked_update_test` & `bad_locked_update_no_exec_test`; Fixes #24473

Signed-off-by: Chris Frantz <cfrantz@google.com>
(cherry picked from commit 4d520bd)
moidx pushed a commit that referenced this pull request Oct 30, 2024
@cfrantz @moidx
[rom_ext_e2e] More ownership transfer tests
f626f9f 
The tests added by this change are all simple variations on
the ownership transfer test added in #24419.  They involve verifying the
different modes and error conditions (e.g. using the wrong key).

Adds the following tests:
- `bad_unlock_test`; Fixes #24466
- `bad_activate_test`; Fixes #24467
- `bad_owner_block_test`; Fixes #24468
- `bad_app_key_test`; Fixes #24469
- `transfer_endorsed_test`; Fixes #24470
- `bad_endorsee_test`; Fixes #24471
- `locked_update_test`; Fixes #24472
- `bad_locked_update_test` & `bad_locked_update_no_exec_test`; Fixes #24473

Signed-off-by: Chris Frantz <cfrantz@google.com>
(cherry picked from commit 4d520bd)
@cfrantz cfrantz mentioned this pull request Dec 16, 2024
Merged
cfrantz added a commit to cfrantz/opentitan that referenced this pull request Dec 19, 2024
@cfrantz
[rom_ext_e2e] More ownership transfer tests
64f8a0a 
The tests added by this change are all simple variations on
the ownership transfer test added in lowRISC#24419.  They involve verifying the
different modes and error conditions (e.g. using the wrong key).

Adds the following tests:
- `bad_unlock_test`; Fixes lowRISC#24466
- `bad_activate_test`; Fixes lowRISC#24467
- `bad_owner_block_test`; Fixes lowRISC#24468
- `bad_app_key_test`; Fixes lowRISC#24469
- `transfer_endorsed_test`; Fixes lowRISC#24470
- `bad_endorsee_test`; Fixes lowRISC#24471
- `locked_update_test`; Fixes lowRISC#24472
- `bad_locked_update_test` & `bad_locked_update_no_exec_test`; Fixes lowRISC#24473

Signed-off-by: Chris Frantz <cfrantz@google.com>
(cherry picked from commit 4d520bd)
@cfrantz cfrantz mentioned this pull request Dec 19, 2024
Open
cfrantz added a commit to cfrantz/opentitan that referenced this pull request Dec 20, 2024
@cfrantz
[rom_ext_e2e] More ownership transfer tests
05808cb 
The tests added by this change are all simple variations on
the ownership transfer test added in lowRISC#24419.  They involve verifying the
different modes and error conditions (e.g. using the wrong key).

Adds the following tests:
- `bad_unlock_test`; Fixes lowRISC#24466
- `bad_activate_test`; Fixes lowRISC#24467
- `bad_owner_block_test`; Fixes lowRISC#24468
- `bad_app_key_test`; Fixes lowRISC#24469
- `transfer_endorsed_test`; Fixes lowRISC#24470
- `bad_endorsee_test`; Fixes lowRISC#24471
- `locked_update_test`; Fixes lowRISC#24472
- `bad_locked_update_test` & `bad_locked_update_no_exec_test`; Fixes lowRISC#24473

Signed-off-by: Chris Frantz <cfrantz@google.com>
(cherry picked from commit 4d520bd)
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

@moidx moidx moidx 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.
3 participants
@cfrantz @moidx @timothytrippel