Skip to content

Comments

remove static cache objects#2

Merged
baentsch merged 2 commits intobsslmlkem768from
mb-evptest
Nov 5, 2024
Merged

remove static cache objects#2
baentsch merged 2 commits intobsslmlkem768from
mb-evptest

Conversation

@baentsch
Copy link
Owner

@baentsch baentsch commented Nov 4, 2024

This PR removes the static and incorrectly initialized (no libctx usage) performance-enhancing EVP_MD cache structures in favour of a new mlkem_ctx structure correctly initialized using libctx.

Also added (and now passing) is the EVP-level test originally contributed in openssl#25403.

API change impact is significant, performance impact not insignificant (mostly has to do with correct placement of ctx object in provider context): In general, during review please check/provide feedback on "TODO" comments.

@baentsch
Copy link
Owner Author

baentsch commented Nov 4, 2024

@t8m @mattcaswell @andrewkdinh as discussed today, I'll merge this to "bsslmlkem768" branch (and thereby to openssl#25848) tomorrow unless hearing howls of protest.

@baentsch baentsch merged commit ead4c37 into bsslmlkem768 Nov 5, 2024
baentsch pushed a commit that referenced this pull request Jan 8, 2025
Here the undefined value "npa" passed to a function
WPACKET_sub_memcpy_u16(pkt, npa, npalen).
However the value is not really used, because "npalen" is zero,
but the call statememt itself is considered an invalid operation
by the new sanitizer.

The original sanitizer error report was:

==49175==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55a276b29d6f in tls_construct_stoc_next_proto_neg /home/runner/work/openssl/openssl/ssl/statem/extensions_srvr.c:1518:21
    #1 0x55a276b15d7d in tls_construct_extensions /home/runner/work/openssl/openssl/ssl/statem/extensions.c:909:15
    #2 0x55a276b513dc in tls_construct_server_hello /home/runner/work/openssl/openssl/ssl/statem/statem_srvr.c:2471:10
    #3 0x55a276b2e160 in write_state_machine /home/runner/work/openssl/openssl/ssl/statem/statem.c:896:26
    #4 0x55a276b2e160 in state_machine /home/runner/work/openssl/openssl/ssl/statem/statem.c:490:21
    open-quantum-safe#5 0x55a276b2f562 in ossl_statem_accept /home/runner/work/openssl/openssl/ssl/statem/statem.c:309:12
    open-quantum-safe#6 0x55a276a9f867 in SSL_do_handshake /home/runner/work/openssl/openssl/ssl/ssl_lib.c:4890:19
    open-quantum-safe#7 0x55a276a9f605 in SSL_accept /home/runner/work/openssl/openssl/ssl/ssl_lib.c:2169:12
    open-quantum-safe#8 0x55a276a3d4db in create_bare_ssl_connection /home/runner/work/openssl/openssl/test/helpers/ssltestlib.c:1281:24
    open-quantum-safe#9 0x55a276a3d7cb in create_ssl_connection /home/runner/work/openssl/openssl/test/helpers/ssltestlib.c:1350:10
    open-quantum-safe#10 0x55a276a64c0b in test_npn /home/runner/work/openssl/openssl/test/sslapitest.c:12266:14
    open-quantum-safe#11 0x55a276b9fc20 in run_tests /home/runner/work/openssl/openssl/test/testutil/driver.c:377:21
    open-quantum-safe#12 0x55a276ba0b10 in main /home/runner/work/openssl/openssl/test/testutil/main.c:31:15

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26269)
baentsch pushed a commit that referenced this pull request Aug 27, 2025
The new malloc failure test caught an asan error in this code:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
2025-08-07T03:22:20.3655117Z     #0 0x7fb88d8fd9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
2025-08-07T03:22:20.3655796Z     #1 0x5584f0e4670a in CRYPTO_malloc crypto/mem.c:211
2025-08-07T03:22:20.3656291Z     #2 0x5584f0e4679d in CRYPTO_zalloc crypto/mem.c:231
2025-08-07T03:22:20.3657040Z     #3 0x5584f11c4c10 in EVP_RAND_CTX_new crypto/evp/evp_rand.c:353
2025-08-07T03:22:20.3657656Z     #4 0x5584f0e93b27 in rand_new_drbg crypto/rand/rand_lib.c:666
2025-08-07T03:22:20.3658289Z     open-quantum-safe#5 0x5584f0e949d0 in rand_get0_public crypto/rand/rand_lib.c:843
2025-08-07T03:22:20.3658914Z     open-quantum-safe#6 0x5584f0e9305b in RAND_bytes_ex crypto/rand/rand_lib.c:490
2025-08-07T03:22:20.3659486Z     open-quantum-safe#7 0x5584f0b2405f in SSL_CTX_new_ex ssl/ssl_lib.c:4191
2025-08-07T03:22:20.3660183Z     open-quantum-safe#8 0x5584f0ae313c in create_ssl_ctx_pair test/helpers/ssltestlib.c:958
2025-08-07T03:22:20.3660871Z     open-quantum-safe#9 0x5584f0adeaf6 in do_handshake test/handshake-memfail.c:56
2025-08-07T03:22:20.3661539Z     open-quantum-safe#10 0x5584f0adee50 in test_alloc_failures test/handshake-memfail.c:125
2025-08-07T03:22:20.3662161Z     open-quantum-safe#11 0x5584f0cd9da8 in run_tests test/testutil/driver.c:342
2025-08-07T03:22:20.3662664Z     open-quantum-safe#12 0x5584f0cda9e5 in main test/testutil/main.c:31
2025-08-07T03:22:20.3663450Z     open-quantum-safe#13 0x7fb88d42a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 282c2c16e7b6600b0b22ea0c99010d2795752b5f)
2025-08-07T03:22:20.3664630Z     open-quantum-safe#14 0x7fb88d42a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 282c2c16e7b6600b0b22ea0c99010d2795752b5f)
2025-08-07T03:22:20.3666608Z     open-quantum-safe#15 0x5584f0ade864 in _start (/home/runner/work/openssl/openssl/test/handshake-memfail+0x22a864) (BuildId: 19659a44d8bed2c082918d25425f77e3a98df534)

It occurs because when rand_get0_public/rand_get0_private sets an
EVP_RAND_CTX object in its thread local storage, it neglects to check
the return code of the operation, which may fail when the associated
sparse array is expanded.

fix it by checking the return code and failing the get0_[public|private]
operation so the failure is graceful.

Fixes openssl/project#1315

Reviewed-by: Paul Yang <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from openssl#28195)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants