Skip to content
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

Valgrind reports uninialized value in EVP_DecryptFinal_ex #1417

Closed
huitema opened this issue Dec 9, 2022 · 1 comment
Closed

Valgrind reports uninialized value in EVP_DecryptFinal_ex #1417

huitema opened this issue Dec 9, 2022 · 1 comment

Comments

@huitema
Copy link
Collaborator

huitema commented Dec 9, 2022

Running valgrind and using OpenSSL V3, we get reports such as:

==4410== HEAP SUMMARY:
==4410==     in use at exit: 0 bytes in 0 blocks
==4410==   total heap usage: 869,652 allocs, 869,652 frees, 55,528,993 bytes allocated
==4410== 
==4410== All heap blocks were freed -- no leaks are possible
==4410== 
==4410== ERROR SUMMARY: 50 errors from 1 contexts (suppressed: 0 from 0)
==4410== 
==4410== 50 errors in context 1 of 1:
==4410== Conditional jump or move depends on uninitialised value(s)
==4410==    at 0x4AF62A4: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==4410==    by 0x4AF6581: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==4410==    by 0x49F70F4: EVP_DecryptFinal_ex (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==4410==    by 0x1B53E4: aead_do_decrypt (openssl.c:962)
==4410==    by 0x1A7503: picoquic_server_encrypt_ticket_call_back (in /home/runner/work/picoquic/picoquic/build/picoquic_ct)
==4410==    by 0x1CA3E6: try_psk_handshake (picotls.c:3544)
==4410==    by 0x1CA3E6: server_handle_hello (picotls.c:3994)
==4410==    by 0x1BF6D5: handle_handshake_record.part.0 (picotls.c:4742)
==4410==    by 0x1CE0AF: handle_handshake_record (picotls.c:4717)
==4410==    by 0x1CE0AF: ptls_server_handle_message (picotls.c:5463)
==4410==    by 0x1ABFE3: picoquic_tls_stream_process (in /home/runner/work/picoquic/picoquic/build/picoquic_ct)
==4410==    by 0x187004: picoquic_incoming_client_initial (in /home/runner/work/picoquic/picoquic/build/picoquic_ct)
==4410==    by 0x1892C0: picoquic_incoming_segment (in /home/runner/work/picoquic/picoquic/build/picoquic_ct)
==4410==    by 0x1896B0: picoquic_incoming_packet_ex (in /home/runner/work/picoquic/picoquic/build/picoquic_ct)
==4410==  Uninitialised value was created by a stack allocation
==4410==    at 0x185090: picoquic_remove_header_protection (in /home/runner/work/picoquic/picoquic/build/picoquic_ct)
==4410== 
==4410== ERROR SUMMARY: 50 errors from 1 contexts (suppressed: 0 from 0)

This issue occurs when using the OpenSSL V3 implementation of AES GCM 128. The same test reported here uses the PTLS "fusion" implementation for the packet exchanges, and that one does not trigger any report. The reports here come from using the "openssl" implementation of encrypting and decrypting session tickets.

There is an OpenSSL bug for the same issue.

@huitema
Copy link
Collaborator Author

huitema commented Dec 9, 2022

The fix was to:

  1. Move the "valgrind" test from a "no-fusion" branch to the "ci-test" branch
  2. Force the ticket encryption to use "fusion", even if that consumes more mmory than using the OpenSSL variant of AES GCM.

Consider replacing that when the OpenSSL bug is fixed.

@huitema huitema closed this as completed Feb 4, 2023
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

No branches or pull requests

1 participant