refactor(core): move session keys to platform secure storage#67
refactor(core): move session keys to platform secure storage#67Grolleau-Benjamin wants to merge 2 commits intoSTMicroelectronics:mainfrom
Conversation
TofMassilia13320
force-pushed
the
main
branch
from
c452b45 to
2f6a547
Compare
|
Branch is out-of-date with the base branch. |
Grolleau-Benjamin
force-pushed
the
refactor-key-management
branch
from
5125dbe to
942bfc6
Compare
|
@TofMassilia13320 Maybe we should add a Discussions section to this repository, or create a Discord server to discuss this kind of change. It could also be useful to maintain a changelog. Your last change introduced a regression that could have been flagged through a proper changelog process (and I also introduced a huge regression in this PR). |
Signed-off-by: Benjamin Grolleau <benjamin.grolleau@outlook.com>
Signed-off-by: Benjamin Grolleau <benjamin.grolleau@outlook.com>
Grolleau-Benjamin
force-pushed
the
refactor-key-management
branch
from
4de1a70 to
99f3889
Compare
|
I've just rebased my branch on I also discovered the release notes ( Additionally, it might make more sense to maintain a classic Let me know what your thoughts are on this! |
|
Looking at the commits, I also believe that @Grom- would be interested in this discussion. |
|
Hello @Grolleau-Benjamin , Indeed , I am interested to discuss/review with you the different needs/requirements highlighted in this PR . I will contact you via mail in the coming days. |
Grom-
left a comment
Grom-
left a comment
There was a problem hiding this comment.
Thanks @Grolleau-Benjamin , I have reviewed your PR . Please do not hesitate to contact me if you want to discuss my comments .
In addition to the in-line code review . the PR commit messages are not in line with our contribution guideline . But I think being able to identify bug-fix for feature and refactor is an important point for change traceability and for release note content clarification . Please do not change them now , I will discuss this with the team and come back to you .
| * \param[in] key_length Length of the keys in bytes | ||
| * \return \ref STSE_OK on success; \ref stse_ReturnCode_t error code otherwise | ||
| */ | ||
| stse_ReturnCode_t stse_platform_store_session_key(PLAT_UI8 *pCypherKey, PLAT_UI32 *pCypherKeyIdx, |
There was a problem hiding this comment.
I would suggest to be more generic here . Something like store AES key . This would help to decorrelate this from STSAFE-A sessions context
| * \param[in] MACKeyIdx Index of the MAC key to delete | ||
| * \return \ref STSE_OK on success; \ref stse_ReturnCode_t error code otherwise | ||
| */ | ||
| stse_ReturnCode_t stse_platform_delete_key(PLAT_UI32 CypherKeyIdx, PLAT_UI32 MACKeyIdx); |
There was a problem hiding this comment.
Same here, I suggest to manage only one AES key (single index) . APIs will call the functions multiple times if needed
| pSession->context.host.MAC_counter = ARRAY_3B_SWAP_TO_UI32(host_key_slot.cmac_sequence_counter); | ||
| } | ||
|
|
||
| PLAT_UI32 HostMacKeyIdx; |
There was a problem hiding this comment.
I am not sure this would fit with the STSAFE-A host key approach.
When the host key is provisioned in the STSAFE-A, it is immutable (except in a specific A120 evaluation configuration, which is not applicable to the full STSAFE-A portfolio and is not recommended for production).
Therefore, I would recommend replacing the current stsafea_open_host_session with your stsafea_open_host_session_from_idx approach.
The new stse_platform_store_session_key, which allows the user to set the key, can optionally be used prior to the stsafea_open_host_session call (depending on the host key storage option at the host level).
| } | ||
|
|
||
| if (pSession->context.host.Host_MAC_key_idx && pSession->context.host.Host_cypher_key_idx) { | ||
| stse_platform_delete_key(pSession->context.host.Host_cypher_key_idx, pSession->context.host.Host_MAC_key_idx); |
There was a problem hiding this comment.
I would not delete the key here . Deleting the index is enough. The Host platform usually keep the Host key in its secure storage or in static memory (this key immutable in STSAFE-A) . Only the ID and counters could change from session to session as it can target a different STSAFE
|
Hi @Grom- , @TofMassilia13320, I think that I should move those changes to dev/v1.2 and introduce a changelog as we discussed with @Grom- in this PR? |
|
Yes exactly |
4 participants
Summary
Session keys are no longer kept in RAM. It is now up to the user to decide where they are stored, referencing them via a
uint32_tkey index.This improves security, enables backend-agnostic key management, and allows integration with PSA and TrustZone Secure Storage.
Changes
stse_platform_store_session_keyHost_MAC_key_idxandHost_cipher_key_idxSecurity
Note
STSELib remains backend-agnostic (no PSA assumption). The platform decides how keys are stored and used.