Implement a PRNG based on ascon_xof & crypto tests#2280
Implement a PRNG based on ascon_xof & crypto tests#2280nextgens wants to merge 7 commits intomeshcore-dev:devfrom
Conversation
jcjones
left a comment
There was a problem hiding this comment.
Very nice implementation! Should be plenty fast, and a clear improvement without compatibility risks.
|
Thank you for your review! I'll be away for a week or so but don't hesitate to let me know if there is something else. |
Fix a bunch of bias related bugs in the process
$ pio test -e heltec_v4_companion_radio_ble TEST_START ╔════════════════════════════════════════════════════╗ ║ MeshCore Crypto Test Suite (Arduino) ║ ╚════════════════════════════════════════════════════╝ test/test_crypto/main.cpp:54:test_ed25519_create_keypair_deterministic:PASS test/test_crypto/main.cpp:55:test_ed25519_create_keypair_different_seeds:PASS test/test_crypto/main.cpp:56:test_ed25519_key_exchange_commutative:PASS test/test_crypto/main.cpp:57:test_ed25519_key_exchange_different_peers:PASS test/test_crypto/main.cpp:58:test_ed25519_sign_verify_valid_signature:PASS test/test_crypto/main.cpp:59:test_ed25519_sign_verify_invalid_signature_wrong_message:PASS test/test_crypto/main.cpp:60:test_ed25519_sign_verify_invalid_signature_tampered:PASS test/test_crypto/main.cpp:61:test_ed25519_sign_verify_invalid_signature_wrong_key:PASS test/test_crypto/main.cpp:62:test_ed25519_sign_verify_empty_message:PASS test/test_crypto/main.cpp:63:test_ed25519_sign_verify_long_message:PASS test/test_crypto/main.cpp:64:test_ed25519_keypair_known_vector:PASS test/test_crypto/main.cpp:65:test_ed25519_signature_known_vector:PASS Ed25519 create_keypair: 503 ms for 100 iterations (5.03 ms avg) test/test_crypto/main.cpp:66:test_ed25519_benchmark_create_keypair:PASS Ed25519 key_exchange: 7114 ms for 500 iterations (14.23 ms avg) test/test_crypto/main.cpp:67:test_ed25519_benchmark_key_exchange:PASS Ed25519 sign: 1299 ms for 200 iterations (6.50 ms avg) test/test_crypto/main.cpp:68:test_ed25519_benchmark_sign:PASS Ed25519 verify: 3167 ms for 200 iterations (15.84 ms avg) test/test_crypto/main.cpp:69:test_ed25519_benchmark_verify:PASS test/test_crypto/main.cpp:72:test_encryptThenMAC_basic_small_payload:PASS test/test_crypto/main.cpp:73:test_encryptThenMAC_various_sizes:PASS test/test_crypto/main.cpp:74:test_MACThenDecrypt_valid_mac:PASS test/test_crypto/main.cpp:75:test_MACThenDecrypt_invalid_mac_tampered:PASS test/test_crypto/main.cpp:76:test_MACThenDecrypt_invalid_mac_ciphertext_tampered:PASS test/test_crypto/main.cpp:77:test_MACThenDecrypt_wrong_shared_secret:PASS test/test_crypto/main.cpp:78:test_MACThenDecrypt_max_payload:PASS test/test_crypto/main.cpp:79:test_encryptThenMAC_MACThenDecrypt_roundtrip_max_payload:PASS test/test_crypto/main.cpp:80:test_encryptThenMAC_MACThenDecrypt_empty_payload:PASS test/test_crypto/main.cpp:81:test_MACThenDecrypt_invalid_length_too_short:PASS test/test_crypto/main.cpp:82:test_encryptThenMAC_different_keys_different_output:PASS test/test_crypto/main.cpp:83:test_encryptThenMAC_deterministic:PASS encryptThenMAC produced 34 bytes of output test/test_crypto/main.cpp:84:test_encryptThenMAC_known_vector:PASS MACThenDecrypt produced 32 bytes of output test/test_crypto/main.cpp:85:test_MACThenDecrypt_known_vector:PASS encryptThenMAC (184 bytes): 56 ms for 100 iterations (0.56 ms avg) test/test_crypto/main.cpp:86:test_encryptThenMAC_benchmark:PASS MACThenDecrypt (184 bytes): 56 ms for 100 iterations (0.56 ms avg) test/test_crypto/main.cpp:87:test_MACThenDecrypt_benchmark:PASS encryptThenMAC + MACThenDecrypt roundtrip (184 bytes): 548 ms for 500 iterations (1.10 ms avg) test/test_crypto/main.cpp:88:test_encryptThenMAC_MACThenDecrypt_benchmark_roundtrip:PASS AsconRNG entropy benchmark: 8192 bytes hardware entropy: 45677 us (175.14 KB/s) PRNG feed : 29054 us (275.35 KB/s) PRNG speedup : 1.57x (higher is faster) test/test_crypto/main.cpp:89:test_AsconRNG_benchmark_entropy_vs_prng:PASS test/test_crypto/main.cpp:90:test_AsconRNG_deterministic_for_same_seed:PASS test/test_crypto/main.cpp:91:test_AsconRNG_stream_advances_between_calls:PASS test/test_crypto/main.cpp:92:test_AsconRNG_reseed_changes_stream:PASS test/test_crypto/main.cpp:93:test_AsconRNG_reseed_deterministic_for_equal_inputs:PASS ----------------------- 38 Tests 0 Failures 0 Ignored OK ═══════════════════════════════════════════════════════ TEST_DONE ═══════════════════════════════════════════════════════
I haven't tested it but it looks easy enough
- Ensure we add entropy regardless of whether we have restored the seed or not - Clear memory where appropriate - Improve the fallback on STM32 if there is no HWRNG
Go back to -O2 since that is what upstream uses
|
@446564 ready for review |
|
just a bit more to read over and test some variants work as expected still, but so far so awesome. this is how we should be doing tests, even native but with unity. |
446564
left a comment
There was a problem hiding this comment.
I think we should go with unity for all tests going forward, much cleaner experience.
Otherwise: tested on xiao nrf without issues, ran on xiao and firmware behaves as normal. Verified the ascon library is consistent with upstream. Read over the rest and full disclosure I am no crypto expert, but it looks good. Nothing jumped out, I saw that all previous feedback was adressed.
The current random number generation is slow and potentially biased (modulo arithmetic). It relies pretty much on "shake & pray".
This one will be faster, will make use of hardware RNGs if they are present (ESP32, nrf52, STM32 and RadioLib's !) and will persist entropy across reboots. The idea is to feed a single ascon_xof() instance with some entropy at bootup and just squeeze from it. There is no periodic reseeding (other than at bootup) or anything... that could be added later.
It also introduces on-device crypto tests (like #2171 but without the native parts).