tests/sys/psa_crytpo_se: disable test on esp32-wroom-32 #20150

maribu · 2023-12-05T14:10:16Z

Contribution description

As the title says

Testing procedure

This PR should pass the CI, unlike most CI runs recently.

Issues/PRs references

None

Teufelchen1 · 2023-12-05T14:12:25Z

Why? To little memory?

riot-ci · 2023-12-05T14:20:10Z

Murdock results

✔️ PASSED

167a70c tests/sys/psa_crytpo_se: disable test on esp32-wroom-32

Success	Failures	Total	Runtime
49	0	49	02m:54s

Artifacts

Documentation preview

maribu · 2023-12-05T14:21:00Z

So that we can get PRs merged again ;-)

This test fails in master most of the time with something like this:

-- running on worker pi-c06df873 thread 1, build number 16109.
-- executing tests for tests/sys/psa_crypto_se on esp32-wroom-32 (compiled with gnu toolchain):
- executing hook run_test_pre
- resetting USB power...
- waiting for tty to re-attach...
- hook run_test_pre finished
make: Entering directory '/tmp/dwq.0.9066599401575657/83fe40e8580de8df04f2496930fb2db8/tests/sys/psa_crypto_se'
 You are going to use the PSA Crypto module, which is only partly implemented and not yet thouroughly tested.\n Please do not use this module in production, as it may introduce security issues!
esptool.py --chip esp32 --port /dev/ttyUSB0 --baud 460800 --before default_reset write_flash -z --flash_mode dout --flash_freq 40m --flash_size detect 0x1000 /tmp/dwq.0.9066599401575657/83fe40e8580de8df04f2496930fb2db8/tests/sys/psa_crypto_se/bin/esp32-wroom-32/esp_bootloader/bootloader.bin 0x8000 /tmp/dwq.0.9066599401575657/83fe40e8580de8df04f2496930fb2db8/tests/sys/psa_crypto_se/bin/esp32-wroom-32/partitions.bin 0x10000 /tmp/dwq.0.9066599401575657/83fe40e8580de8df04f2496930fb2db8/tests/sys/psa_crypto_se/bin/esp32-wroom-32/tests_psa_crypto_se.elf.bin
esptool.py v4.4
Serial port /dev/ttyUSB0
Connecting....
Chip is ESP32-D0WDQ6 (revision v1.0)
Features: WiFi, BT, Dual Core, 240MHz, VRef calibration in efuse, Coding Scheme None
Crystal is 40MHz
MAC: 30:ae:a4:f4:7a:e0
Uploading stub...
Running stub...
Stub running...
Changing baud rate to 460800
Changed.
Configuring flash size...
Auto-detected Flash size: 4MB
Flash will be erased from 0x00001000 to 0x00005fff...
Flash will be erased from 0x00008000 to 0x00008fff...
Flash will be erased from 0x00010000 to 0x0002cfff...
Warning: Image file at 0x1000 is protected with a hash checksum, so not changing the flash size setting. Use the --flash_size=keep option instead of --flash_size=4MB in order to remove this warning, or use the --dont-append-digest option for the elf2image command in order to generate an image file without a hash checksum
Compressed 17440 bytes to 11674...
Writing at 0x00001000... (100 %)
Wrote 17440 bytes (11674 compressed) at 0x00001000 in 0.7 seconds (effective 213.7 kbit/s)...
Hash of data verified.
Compressed 3072 bytes to 85...
Writing at 0x00008000... (100 %)
Wrote 3072 bytes (85 compressed) at 0x00008000 in 0.1 seconds (effective 318.8 kbit/s)...
Hash of data verified.
Compressed 116912 bytes to 57617...
Writing at 0x00010000... (25 %)
Writing at 0x000171b4... (50 %)
Writing at 0x000248b0... (75 %)
Writing at 0x000299bc... (100 %)
Wrote 116912 bytes (57617 compressed) at 0x00010000 in 1.8 seconds (effective 512.1 kbit/s)...
Hash of data verified.

Leaving...
Hard resetting via RTS pin...
make: Nothing to be done for 'termdeps'.
make: Leaving directory '/tmp/dwq.0.9066599401575657/83fe40e8580de8df04f2496930fb2db8/tests/sys/psa_crypto_se'
make: Entering directory '/tmp/dwq.0.9066599401575657/83fe40e8580de8df04f2496930fb2db8/tests/sys/psa_crypto_se'
 You are going to use the PSA Crypto module, which is only partly implemented and not yet thouroughly tested.\n Please do not use this module in production, as it may introduce security issues!
r
 You are going to use the PSA Crypto module, which is only partly implemented and not yet thouroughly tested.\n Please do not use this module in production, as it may introduce security issues!
socat - open:/dev/ttyUSB0,b115200,echo=0,raw,cs8,parenb=0,cstopb=0 
READY
s
START
main(): This is RIOT! (Version: buildtest)
HMAC SHA256 took 2808 us
HMAC SHA256 failed: PSA_ERROR_COMMUNICATION_FAILURE
Cipher AES 128 took 2240 us
Cipher AES 128 failed: PSA_ERROR_COMMUNICATION_FAILURE
Tests failed...
{ "threads": [{ "name": "idle", "stack_size": 2048, "stack_used": 2048 }]}
Timeout in expect script at "child.expect_exact('All Done')" (tests/sys/psa_crypto_se/tests/01-run.py:8)

make: *** [/tmp/dwq.0.9066599401575657/83fe40e8580de8df04f2496930fb2db8/makefiles/tests/tests.inc.mk:26: test] Error 1
make: Leaving directory '/tmp/dwq.0.9066599401575657/83fe40e8580de8df04f2496930fb2db8/tests/sys/psa_crypto_se'

This effectively prevents merging any PR.

maribu · 2023-12-05T15:51:31Z

Thx :-)

gschorcht · 2023-12-06T09:15:24Z

Do we have any idea why all the PSA tests fail on ESP32x SoCs?

maribu · 2023-12-06T10:08:53Z

@Einhornhool guessed it could be a stack overflow, but didn't have the toolchain/hardware at hand to reproduce.

I was able to reproduce with my ESP32 Ethernet Kit, but when stepping through the code with GDB I got

Info : [esp32.cpu0] Core was reset.
Info : [esp32.cpu1] Debug controller was reset.
Info : [esp32.cpu1] Core was reset.
Info : Set GDB target to 'esp32.cpu0'
Error: gdb requested a non-existing register (reg_num=220)
Info : dropped 'gdb' connection

I updated the Espressif fork of OpenOCD afterwards, but still the exact same issue. I could help to debug, but I really want to get GDB working again to be more productive at debugging.

gschorcht · 2023-12-06T11:37:50Z

@Einhornhool guessed it could be a stack overflow, but didn't have the toolchain/hardware at hand to reproduce.

🤔 But wouldn't a stack overflow cause a crash? According to the ps output it doesn't seem to be a stack problem.

START
main(): This is RIOT! (Version: 2024.01-devel-306-g8f01d)
HMAC SHA256 took 2841 us
HMAC SHA256 failed: PSA_ERROR_COMMUNICATION_FAILURE
	pid | name                 | state    Q | pri | stack  ( used) ( free) | base addr  | current     
	  1 | esp_timer            | sleeping _ |   2 |   3636 (  452) ( 3184) | 0x3ffb2ec0 | 0x3ffb3b30 
	  2 | idle                 | pending  Q |  15 |   2048 (  440) ( 1608) | 0x3ffb0f28 | 0x3ffb1570 
	  3 | main                 | running  Q |   7 |   3584 ( 1496) ( 2088) | 0x3ffb1728 | 0x3ffb1f50 
	    | SUM                  |            |     |   9268 ( 2388) ( 6880)
Cipher AES 128 took 2209 us
Cipher AES 128 failed: PSA_ERROR_COMMUNICATION_FAILURE
	pid | name                 | state    Q | pri | stack  ( used) ( free) | base addr  | current     
	  1 | esp_timer            | sleeping _ |   2 |   3636 (  452) ( 3184) | 0x3ffb2ec0 | 0x3ffb3b30 
	  2 | idle                 | pending  Q |  15 |   2048 (  440) ( 1608) | 0x3ffb0f28 | 0x3ffb1570 
	  3 | main                 | running  Q |   7 |   3584 ( 1544) ( 2040) | 0x3ffb1728 | 0x3ffb1f20 
	    | SUM                  |            |     |   9268 ( 2436) ( 6832)
Tests failed...
{ "threads": [{ "name": "idle", "stack_size": 2048, "stack_used": 440 }]}

Einhornhool · 2023-12-06T15:00:46Z

Actually, these tests need a secure element to perform those operations, so they should actually fail on all boards.
These test should only test the build configurations and make sure all the modules are declared correctly.
Is there a way to only test the build but not run them?

maribu · 2023-12-06T15:28:56Z

Is there a way to only test the build but not run them?

Yes, the can be disabled on a per-board basis as done in this PR.

Alternativly, the python test script could be placed in a test-with-config folder. This will only run the tests with make test-with-config and not with make test, effectively disabling them for all boards in the CI.

There was a way to tell to selectively enable the test-with-config for certain app-board tuples somehow. But I don't recall and the person implemented it is now working for a new employer and no longer available to the community.

benpicco · 2023-12-06T15:40:35Z

If the test needs a secure element, a whitelist might be a better approach than a blacklist as only few boards come with one (I can only think of same54-xpro now).

But those affect the entire build when we indeed just want to whitelist the automatic test.

miri64 · 2023-12-06T15:53:23Z

But those affect the entire build when we indeed just want to whitelist the automatic test.

Can't this be expressed as a feature? Then we could just set FEATURE_REQUIRED for the test.

miri64 · 2023-12-06T15:54:37Z

Otherwise, there is always TEST_ON_CI_WHITELIST += <board>.

maribu · 2023-12-06T16:17:34Z

Can't this be expressed as a feature?

If I understood @Einhornhool correctly, it was a deliberate design choice to allow compile-testing it for all platforms regardless of whether a secure element is present or not.

But personally, I like it if users get an error message at build time for configurations that are known to not work, rather than letting them debug runtime errors. So strong +1 for a feature.

Einhornhool · 2023-12-06T16:31:08Z

PSA Crypto only supports one type of secure element so far and it needs a specific configuration for this test. So it can even fail on boards with a builtin SE. This means it's probably best to not run these at all.

It is not really necessary to compile-test for all boards, one should be enough.
We're probably going to restructure PSA Crypto internally and these tests are supposed to make sure we don't accidentally include unused structures, uninitialized variables, etc. in the build.
So a feature is totally fine.

tests/sys/psa_crytpo_se: disable test on esp32-wroom-32

167a70c

maribu requested review from smlng, leandrolanzieri, aabadie, MichelRottleuthner and fjmolinas as code owners December 5, 2023 14:10

maribu requested review from Einhornhool and benpicco and removed request for aabadie, leandrolanzieri, smlng, MichelRottleuthner and fjmolinas December 5, 2023 14:10

github-actions bot added the Area: tests Area: tests and testing framework label Dec 5, 2023

maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 5, 2023

Teufelchen1 approved these changes Dec 5, 2023

View reviewed changes

maribu added this pull request to the merge queue Dec 5, 2023

Merged via the queue into RIOT-OS:master with commit 9ed2da1 Dec 5, 2023
27 checks passed

maribu mentioned this pull request Dec 5, 2023

tests/sys/psa_crypto_se_cipher: disable test on esp32-wroom-32 #20152

Merged

maribu deleted the tests/sys/psa_crypto_se branch December 5, 2023 15:51

Einhornhool mentioned this pull request Dec 13, 2023

tests/sys/psa_crypto*: Fix failing tests #20178

Merged

MrKevinWeiss added this to the Release 2024.01 milestone Feb 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests/sys/psa_crytpo_se: disable test on esp32-wroom-32 #20150

tests/sys/psa_crytpo_se: disable test on esp32-wroom-32 #20150

maribu commented Dec 5, 2023

Teufelchen1 commented Dec 5, 2023

riot-ci commented Dec 5, 2023

maribu commented Dec 5, 2023

maribu commented Dec 5, 2023

gschorcht commented Dec 6, 2023

maribu commented Dec 6, 2023

gschorcht commented Dec 6, 2023 •

edited

Loading

Einhornhool commented Dec 6, 2023

maribu commented Dec 6, 2023

benpicco commented Dec 6, 2023

miri64 commented Dec 6, 2023

miri64 commented Dec 6, 2023

maribu commented Dec 6, 2023

Einhornhool commented Dec 6, 2023

tests/sys/psa_crytpo_se: disable test on esp32-wroom-32 #20150

tests/sys/psa_crytpo_se: disable test on esp32-wroom-32 #20150

Conversation

maribu commented Dec 5, 2023

Contribution description

Testing procedure

Issues/PRs references

Teufelchen1 commented Dec 5, 2023

riot-ci commented Dec 5, 2023

Murdock results

Artifacts

maribu commented Dec 5, 2023

maribu commented Dec 5, 2023

gschorcht commented Dec 6, 2023

maribu commented Dec 6, 2023

gschorcht commented Dec 6, 2023 • edited Loading

Einhornhool commented Dec 6, 2023

maribu commented Dec 6, 2023

benpicco commented Dec 6, 2023

miri64 commented Dec 6, 2023

miri64 commented Dec 6, 2023

maribu commented Dec 6, 2023

Einhornhool commented Dec 6, 2023

gschorcht commented Dec 6, 2023 •

edited

Loading