lowRISC / lowRISC/opentitan

[ownership] ownership_key_validate() does not enforce the requested key role

Open
#30,993 0 comments 0 reactions 2 assignees Claimed by @sasdf View on GitHub
SW:ROM_EXT
Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

### Description

The `ownership_key_validate` function takes an `ownership_key_t key` argument that specifies the key role for a given operation. The code checks if the key is permitted for the given operation, e.g.:
https://github.com/lowRISC/opentitan/blob/595a468d8c18f5bc10e460f8e8dc1b5f836104a5/sw/device/silicon_creator/lib/ownership/ownership_key.c#L94-L99
However, the owner key has no such check:
https://github.com/lowRISC/opentitan/blob/595a468d8c18f5bc10e460f8e8dc1b5f836104a5/sw/device/silicon_creator/lib/ownership/ownership_key.c#L107-L110
Hence, the owner key is accepted at each callsite.

This contradicts the documentation:
https://github.com/lowRISC/opentitan/blob/595a468d8c18f5bc10e460f8e8dc1b5f836104a5/sw/device/silicon_creator/rom_ext/doc/ownership.md?plain=1#L117-L128

The impact is not severe as owner key is private and not accessible to an attacker. Moreover, kNoOwnerRecoveryKey is a weak symbol that is currently not in use (it points to a test / fake key), i.e., we haven't defined a recovery flow yet.

We should fix this in the future.

Thanks to @flaviens for reporting this!

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.