[cryptolib] Move security_config_check() to public API

[nasahlpa]

security_config_check() checks, whether certain countermeasure are enabled to ensure a secure execution of the cryptolib. Until now, this check was conducted within the cryptolib itself.

However, as this function accesses the cpuctrlsts register, which is only accessible in the M privilege mode, this restricted CL to only be executable in M mode.

To increase the usage of the CL, this PR moves the security_config_check() function to the public CL API. Hence, now it is the responsibility of the caller of the CL to ensure that this check is first conducted.

[siemen11]

Thank you Pascal! Good catch to move this to the user space, and thank you for implementing the new check in the pentest interface as well!

One question, would it be enough that the user calls this only once at the initialization of the library or is it needed for each crypto call?

[nasahlpa]

Thank you Pascal! Good catch to move this to the user space, and thank you for implementing the new check in the pentest interface as well!

One question, would it be enough that the user calls this only once at the initialization of the library or is it needed for each crypto call?

Thanks for the review & suggestion.

Currently I've placed the check into each pentest subfunction that uses the CL. However, if a pentest subfunction has multiple calls into the CL, the check is only placed at the very beginning. E.g., in the cryptolib_fi_rsa_sign_impl function.

Would it make sense to move the check into the init function, e.g. into here?

opentitan/sw/device/tests/penetrationtests/firmware/fi/cryptolib_fi_asym.c

Line 457 in da96c41

status_t handle_cryptolib_fi_asym_init(ujson_t *uj) {

[siemen11]

Thank you Pascal! Good catch to move this to the user space, and thank you for implementing the new check in the pentest interface as well!
One question, would it be enough that the user calls this only once at the initialization of the library or is it needed for each crypto call?

Thanks for the review & suggestion.

Currently I've placed the check into each pentest subfunction that uses the CL. However, if a pentest subfunction has multiple calls into the CL, the check is only placed at the very beginning. E.g., in the cryptolib_fi_rsa_sign_impl function.

Would it make sense to move the check into the init function, e.g. into here?

opentitan/sw/device/tests/penetrationtests/firmware/fi/cryptolib_fi_asym.c

Line 457 in da96c41

status_t handle_cryptolib_fi_asym_init(ujson_t *uj) {

Yeah that would be great! We have space for it in

opentitan/sw/device/tests/penetrationtests/firmware/fi/cryptolib_fi_asym.c

Line 514 in da96c41

// Add things like versioning.

[nasahlpa]

Thank you Pascal! Good catch to move this to the user space, and thank you for implementing the new check in the pentest interface as well!
One question, would it be enough that the user calls this only once at the initialization of the library or is it needed for each crypto call?

Thanks for the review & suggestion.
Currently I've placed the check into each pentest subfunction that uses the CL. However, if a pentest subfunction has multiple calls into the CL, the check is only placed at the very beginning. E.g., in the cryptolib_fi_rsa_sign_impl function.
Would it make sense to move the check into the init function, e.g. into here?

opentitan/sw/device/tests/penetrationtests/firmware/fi/cryptolib_fi_asym.c

Line 457 in da96c41

status_t handle_cryptolib_fi_asym_init(ujson_t *uj) {

Yeah that would be great! We have space for it in

opentitan/sw/device/tests/penetrationtests/firmware/fi/cryptolib_fi_asym.c

Line 514 in da96c41

// Add things like versioning.

Makes sense - I've moved it to the init function that we always call before a pentest run.

[johannheyszl]

Thanks @nasahlpa

[h-filali]

Thanks @nasahlpa for adding this.
I checked the following:

• The first commit only moves the check ✔
• The second commit only removes the function calls inside CL ✔
• The third commit only switches the security level ✔
• The last commit adds calls to the relevant locations ✔

For the last commit I have a small addendum. For cryptolib users it might not be clear that they have to call the security config check. A user will probably base their code off of what they can find in the cryptotests or in the pentests library. This makes incorrect use in this specific case pretty likely. I will start a document that captures any such cases from now on.

[lowrisc-ci]

Backport failed for earlgrey_1.0.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin earlgrey_1.0.0
git worktree add -d .worktree/backport-28882-to-earlgrey_1.0.0 origin/earlgrey_1.0.0
cd .worktree/backport-28882-to-earlgrey_1.0.0
git switch --create backport-28882-to-earlgrey_1.0.0
git cherry-pick -x 86bbee37acc5f3989edf65aa995e9678af695b84 94e3de705d69ce5d9b6250421ac12a687240d4ab 24bc1b2cb33bb5d9d45d8acf0bb82e91b95e9534 a9afc85d6c36c2ab521c65ffcadc965dbd6ced12

[lowrisc-ci]

Backport failed for earlgrey_1.0.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin earlgrey_1.0.0
git worktree add -d .worktree/backport-28882-to-earlgrey_1.0.0 origin/earlgrey_1.0.0
cd .worktree/backport-28882-to-earlgrey_1.0.0
git switch --create backport-28882-to-earlgrey_1.0.0
git cherry-pick -x 86bbee37acc5f3989edf65aa995e9678af695b84 94e3de705d69ce5d9b6250421ac12a687240d4ab 24bc1b2cb33bb5d9d45d8acf0bb82e91b95e9534 a9afc85d6c36c2ab521c65ffcadc965dbd6ced12