Skip to content

mbedTLS ITS is not thread safe #59362

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.

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

Closed
alxelax opened this issue Jun 19, 2023 · 6 comments
Closed

mbedTLS ITS is not thread safe #59362

alxelax opened this issue Jun 19, 2023 · 6 comments
Assignees
Labels
area: Crypto / RNG bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug Stale

Comments

@alxelax
Copy link
Collaborator

alxelax commented Jun 19, 2023

Describe the bug
mbedTLS has PSA ITS (Internal Trusted Storage) implementation that is based on posix file API. It doesn't consider ability to run multiple processes\treads with mbedTLS instances. As a consequence, all instances compete for the single file that reproduces ITS functionality.
It impacts on BLE mesh tests behavior. All mesh bsim tests are run in parallel environment. Tests those check mesh functionality related to storing\restoring\usage keys with PSA_KEY_LIFETIME_PERSISTENT fail.

Tests pass successfully if disable parallel environment.
Finally, it causes the situation when it is not possible to run bsim tests for mesh with PSA in Zephyr CI.

  • Platform: nrf52_bsim (this is posix based platform)
  • Potential workaround: disable parallel environment for bsim tests (this will increase time of BabbleSim step in CI dramatically)

To Reproduce
Steps to reproduce the behavior:

  1. config build ble mesh with mbedTLS PSA.
diff --git a/subsys/bluetooth/mesh/Kconfig b/subsys/bluetooth/mesh/Kconfig
index 4f2438b9bb..7813311f05 100644
--- a/subsys/bluetooth/mesh/Kconfig
+++ b/subsys/bluetooth/mesh/Kconfig
@@ -15,7 +15,7 @@ if BT_MESH
 
 choice BT_MESH_CRYPTO_LIB
        prompt "Crypto library selection for mesh security"
-       default BT_MESH_USES_TINYCRYPT
+       default BT_MESH_USES_MBEDTLS_PSA
 
 config BT_MESH_USES_TINYCRYPT
        bool "Use TinyCrypt"
  1. build mesh bsim tests
    All steps and links how to deploy bsim environment is possible to find in Zephyr documentation
/zephyr/tests/bsim/bluetooth/mesh$ bash compile.sh
  1. run bsim tests
/zephyr/tests/bsim$ bash run_parallel.sh
  1. See error

If remove parallel environment, all tests pass

diff --git a/tests/bsim/run_parallel.sh b/tests/bsim/run_parallel.sh
index 395dc7f608..d8bebe3dbd 100755
--- a/tests/bsim/run_parallel.sh
+++ b/tests/bsim/run_parallel.sh
@@ -70,26 +70,26 @@ export CLEAN_XML="sed -E -e 's/&/\&amp;/g' -e 's/</\&lt;/g' -e 's/>/\&gt;/g' \
 
 echo -n "" > $tmp_res_file
 
-if [ `command -v parallel` ]; then
-  parallel '
-  echo "<testcase name=\"{}\" time=\"0\">"
-  {} $@ &> {#}.log
-  if [ $? -ne 0 ]; then
-    (>&2 echo -e "\e[91m{} FAILED\e[39m")
-    (>&2 cat {#}.log)
-    echo "<failure message=\"failed\" type=\"failure\">"
-    cat {#}.log | eval $CLEAN_XML
-    echo "</failure>"
-    rm {#}.log
-    echo "</testcase>"
-    exit 1
-  else
-    (>&2 echo -e "{} PASSED")
-    rm {#}.log
-    echo "</testcase>"
-  fi
-  ' ::: $all_cases >> $tmp_res_file ; err=$?
-else #fallback in case parallel is not installed
+#if [ `command -v parallel` ]; then
+#  parallel '
+#  echo "<testcase name=\"{}\" time=\"0\">"
+#  {} $@ &> {#}.log
+#  if [ $? -ne 0 ]; then
+#    (>&2 echo -e "\e[91m{} FAILED\e[39m")
+#    (>&2 cat {#}.log)
+#    echo "<failure message=\"failed\" type=\"failure\">"
+#    cat {#}.log | eval $CLEAN_XML
+#    echo "</failure>"
+#    rm {#}.log
+#    echo "</testcase>"
+#    exit 1
+#  else
+#    (>&2 echo -e "{} PASSED")
+#    rm {#}.log
+#    echo "</testcase>"
+#  fi
+#  ' ::: $all_cases >> $tmp_res_file ; err=$?
+#else #fallback in case parallel is not installed
   for case in $all_cases; do
     echo "<testcase name=\"$case\" time=\"0\">" >> $tmp_res_file
     $case $@ &> $i.log
@@ -107,7 +107,7 @@ else #fallback in case parallel is not installed
     rm $i.log
     let i=i+1
   done
-fi
+#fi
 echo -e "</testsuite>\n</testsuites>\n" >> $tmp_res_file
 dur=$(($SECONDS - $start))
 echo -e "<testsuites>\n<testsuite errors=\"0\" failures=\"$err\"\

Expected behavior
All tests should pass without errors in parallel environment

Impact
Impossible to add ble mesh bsim tests with mbedTLS PSA crypto in Zephyr's CI

Environment (please complete the following information):

  • OS: Linux
  • Toolchain: zephyr sdk 0.16.1
  • Commit SHA: 68589ca

Additional context
Alternative solution to fix the issue is implementation PSA ITS based on Zephyr's settings subsystem. If mbedTLS supports ITS based on settings, then each simulated device will include itself its own flash area. There is no competition for the common source.

@alxelax alxelax added the bug The issue is a bug, or the PR is fixing a bug label Jun 19, 2023
@jgl-meta
Copy link
Collaborator

@PavelVPV have you seen this?

@jgl-meta jgl-meta added the priority: medium Medium impact/importance bug label Jun 20, 2023
@PavelVPV
Copy link
Collaborator

@jgl-meta this is I guess issue with mbedTLS, not with Bluetooth mesh. As an mbedTLS user I can't use different filenames for ITS in mbedTLS.

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Aug 21, 2023
@alxelax alxelax removed the Stale label Aug 21, 2023
@alxelax
Copy link
Collaborator Author

alxelax commented Aug 21, 2023

I removed Stale label since the issue is actual. mbedtls with built-in ITS is not possible to use in BabbleSim tests.
The temporary workaround based on --allow-multiple-definition linker option and ITS emulator was implemented: https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/bsim/bluetooth/mesh/CMakeLists.txt#L87-L97

If mbedtls PSA is going to be the main security library in Zephyr then it should be compatible with Zephyr's code base including tests approaches.

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Oct 21, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 4, 2023
@alxelax alxelax reopened this Feb 17, 2025
@github-actions github-actions bot removed the Stale label Feb 18, 2025
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Apr 19, 2025
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Crypto / RNG bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug Stale
Projects
None yet
Development

No branches or pull requests

5 participants