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.

Sign up for GitHub

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

Jump to bottom

SSL_new_session_ticket Support for QUIC #25

Closed
nibanks opened this issue Mar 26, 2021 · 14 comments
Closed

SSL_new_session_ticket Support for QUIC #25

nibanks opened this issue Mar 26, 2021 · 14 comments
Labels
1.1.1 Applies to 1.1.1 enhancement New feature or request

Comments

@nibanks
Copy link
Member

nibanks commented Mar 26, 2021

It seems SSL_new_session_ticket needs some updated to make it work with QUIC. It should allow QUIC servers to manually trigger a new session ticket to be sent.
@nibanks nibanks mentioned this issue Mar 26, 2021
Closed
@nibanks
Copy link
Member Author

nibanks commented Apr 2, 2021

@kaduk any idea how complicated this change would be? Is it something I (with extremely limited knowledge of OpenSSL) would be able to figure out? How big of a change would it be? I think this is the last thing that we'd need for complete 0-RTT support in MsQuic.

@nibanks nibanks added the enhancement New feature or request label Apr 2, 2021
@kaduk
Copy link
Member

kaduk commented Apr 2, 2021

Hey @nibanks thanks for checking back -- it's been a crazy week here!
I expect that the core of the change is just going to be kaduk@a2c9d10 but I want to come up with some way of testing that it works as intended before putting it in a PR. That said, if MSQuic finds it usable, that's a pretty good indication...

@nibanks
Copy link
Member Author

nibanks commented Apr 2, 2021

Thanks @kaduk. That's a pretty simple change. :) So, how exactly is this expected to work. Do I call SSL_new_session_ticket and that should directly trigger my add_handshake_data handler? Or do I need to call SSL_process_quic_post_handshake afterwards?

@kaduk
Copy link
Member

kaduk commented Apr 2, 2021

My expectation (again, untested), is that you call SSL_new_session_ticket(.); SSL_do_handshake(.) and that will provide the encoded ticket to the server's add_handshake_data() routine and kick off whatever bits are needed to send it to the client in a CRYPTO frame. The client would then process the contents of the CRYPTO frame with its add_handshake_data() function before calling SSL_process_quic_post_handshake() to do the TLS processing of the ticket.

@nibanks
Copy link
Member Author

nibanks commented Apr 3, 2021

@kaduk I just started looking at this, and it seem this is 3.0 specific code? Is there something I can CP to 1.1.1 to get the prereq first?

@kaduk
Copy link
Member

kaduk commented Apr 3, 2021

@nibanks Yes, this is only in 3.0 at the moment.
The three commits in openssl#11416 should be the only ones that are relevant. The first two look to cherry-pick cleanly (did not build-test) and the last one is just make update, which can be re-run in the 1.1.1 tree to get the needed artifacts.

@nibanks
Copy link
Member Author

nibanks commented Apr 4, 2021

After I manually integrates those changes into 1.1.1 and wired up MsQuic to call SSL_new_session_ticket it seems to be working. I just need to get the receive side wired up now. Thanks @kaduk. Any chance of that PR getting cherry-picked to 1.1.1 quictls?

@kaduk
Copy link
Member

kaduk commented Apr 4, 2021

I expect that we can put the whole thing in 1.1.1 quictls once this last bit lands on 3.0.0 quictls, yes.
My default plan would be to try to get this two-line change into stock openssl before putting it in quictls, but I'm not actually tied to that if it's helpful to get it in quictls faster.

@nibanks
Copy link
Member Author

nibanks commented Apr 4, 2021

I have everything working except getting the callback on the server side when a ticket is received. I call SSL_CTX_set_session_ticket_cb with just a dec_cb (gen_cb is NULL), but it's never getting called. I'm getting my set_encryption_secrets callback triggered for ssl_encryption_early_data secrets, so it must be using the ticket, right? Any suggestions on next steps?

@nibanks
Copy link
Member Author

nibanks commented Apr 4, 2021

Oh, wait. I was calling SSL_CTX_set_session_ticket_cb in the wrong place I think. Let me try something else.

@nibanks
Copy link
Member Author

nibanks commented Apr 4, 2021

Sorry for the false alarm. Everything is working now! As I mentioned before, I manually integrated the changes from openssl#11416 and kaduk/openssl@a2c9d10 locally. Then I wired up everything in MsQuic as necessary. All client and server functionality seems to be working as expected.

It'd be great if we can get these officially integrated into quictls 1.1.1 as soon as possible.

@nibanks nibanks added the 1.1.1 Applies to 1.1.1 label Apr 4, 2021
@kaduk
Copy link
Member

kaduk commented Apr 7, 2021

as soon as possible

To set some expectations, I unepxectedly had to take Monday off and am playing catch-up yesterday/today. I expect to have useful progress by the end of the week.

@nibanks
Copy link
Member Author

nibanks commented Apr 7, 2021

as soon as possible

To set some expectations, I unepxectedly had to take Monday off and am playing catch-up yesterday/today. I expect to have useful progress by the end of the week.

That sounds great! Really appreciate all the support you continue to provide! Please let me know when you have something and I can help send it through our pipeline to validate.

@kaduk
Copy link
Member

kaduk commented Jun 29, 2021

I believe that once #26 was cherry-picked to 1.1.1k+quic as a6e9d76, this issue is resolved for 1.1.1 as well, so closing.

@kaduk kaduk closed this as completed Jun 29, 2021
tmshort pushed a commit that referenced this issue Jun 21, 2022
@bernd-edlinger
Fix a memory leak in crl_set_issuers
046b0d7 
This can be reproduced with my error injection patch.

The test vector has been validated on the 1.1.1 branch
but the issue is of course identical in all branches.

$ ERROR_INJECT=1653520461 ../util/shlib_wrap.sh ./cms-test ./corpora/cms/3eff1d2f1232bd66d5635db2c3f9e7f23830dfd1
log file: cms-3eff1d2f1232bd66d5635db2c3f9e7f23830dfd1-32454-test.out
ERROR_INJECT=1653520461
    #0 0x7fd5d8b8eeba in __sanitizer_print_stack_trace ../../../../gcc-trunk/libsanitizer/asan/asan_stack.cpp:87
    #1 0x402fc4 in my_realloc fuzz/test-corpus.c:129
    #2 0x7fd5d8893c49 in sk_reserve crypto/stack/stack.c:198
    #3 0x7fd5d8893c49 in OPENSSL_sk_insert crypto/stack/stack.c:242
    #4 0x7fd5d88d6d7f in sk_GENERAL_NAMES_push include/openssl/x509v3.h:168
    #5 0x7fd5d88d6d7f in crl_set_issuers crypto/x509/x_crl.c:111
    #6 0x7fd5d88d6d7f in crl_cb crypto/x509/x_crl.c:246
    #7 0x7fd5d85dc032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #8 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #9 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #10 0x7fd5d85db2b5 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:259
    #11 0x7fd5d85dc813 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:611
    #12 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #13 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #14 0x7fd5d85dca28 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:633
    #15 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #16 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #17 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #18 0x7fd5d85dd7d3 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:494
    #19 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #20 0x7fd5d85ddd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #21 0x7fd5d85dde35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #22 0x7fd5d85a77e0 in ASN1_item_d2i_bio crypto/asn1/a_d2i_fp.c:69
    openssl#23 0x402845 in FuzzerTestOneInput fuzz/cms.c:43
    #24 0x402bbb in testfile fuzz/test-corpus.c:182
    #25 0x402626 in main fuzz/test-corpus.c:226
    #26 0x7fd5d7c81f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
    #27 0x402706  (/home/ed/OPC/openssl/fuzz/cms-test+0x402706)

=================================================================
==29625==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x7fd5d8b8309f in __interceptor_malloc ../../../../gcc-trunk/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fd5d87c2430 in CRYPTO_zalloc crypto/mem.c:230
    #2 0x7fd5d889501f in OPENSSL_sk_new_reserve crypto/stack/stack.c:209
    #3 0x7fd5d85dcbc3 in sk_ASN1_VALUE_new_null include/openssl/asn1t.h:928
    #4 0x7fd5d85dcbc3 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:577
    #5 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #6 0x7fd5d85db104 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:178
    #7 0x7fd5d85ddd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #8 0x7fd5d85dde35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #9 0x7fd5d88f86d9 in X509V3_EXT_d2i crypto/x509v3/v3_lib.c:142
    #10 0x7fd5d88d6d3c in crl_set_issuers crypto/x509/x_crl.c:97
    #11 0x7fd5d88d6d3c in crl_cb crypto/x509/x_crl.c:246
    #12 0x7fd5d85dc032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #13 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #14 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #15 0x7fd5d85db2b5 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:259
    #16 0x7fd5d85dc813 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:611
    #17 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #18 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #19 0x7fd5d85dca28 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:633
    #20 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #21 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #22 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#23 0x7fd5d85dd7d3 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:494
    #24 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #25 0x7fd5d85ddd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #26 0x7fd5d85dde35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #27 0x7fd5d85a77e0 in ASN1_item_d2i_bio crypto/asn1/a_d2i_fp.c:69
    #28 0x402845 in FuzzerTestOneInput fuzz/cms.c:43
    #29 0x402bbb in testfile fuzz/test-corpus.c:182
    #30 0x402626 in main fuzz/test-corpus.c:226
    openssl#31 0x7fd5d7c81f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

SUMMARY: AddressSanitizer: 32 byte(s) leaked in 1 allocation(s).

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#18391)

(cherry picked from commit e9007e0)
tmshort pushed a commit that referenced this issue Jun 21, 2022
@bernd-edlinger
Fix a memory leak in crl_set_issuers
8754fa5 
This can be reproduced with my error injection patch.

The test vector has been validated on the 1.1.1 branch
but the issue is of course identical in all branches.

$ ERROR_INJECT=1653520461 ../util/shlib_wrap.sh ./cms-test ./corpora/cms/3eff1d2f1232bd66d5635db2c3f9e7f23830dfd1
log file: cms-3eff1d2f1232bd66d5635db2c3f9e7f23830dfd1-32454-test.out
ERROR_INJECT=1653520461
    #0 0x7fd5d8b8eeba in __sanitizer_print_stack_trace ../../../../gcc-trunk/libsanitizer/asan/asan_stack.cpp:87
    #1 0x402fc4 in my_realloc fuzz/test-corpus.c:129
    #2 0x7fd5d8893c49 in sk_reserve crypto/stack/stack.c:198
    #3 0x7fd5d8893c49 in OPENSSL_sk_insert crypto/stack/stack.c:242
    #4 0x7fd5d88d6d7f in sk_GENERAL_NAMES_push include/openssl/x509v3.h:168
    #5 0x7fd5d88d6d7f in crl_set_issuers crypto/x509/x_crl.c:111
    #6 0x7fd5d88d6d7f in crl_cb crypto/x509/x_crl.c:246
    #7 0x7fd5d85dc032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #8 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #9 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #10 0x7fd5d85db2b5 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:259
    #11 0x7fd5d85dc813 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:611
    #12 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #13 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #14 0x7fd5d85dca28 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:633
    #15 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #16 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #17 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #18 0x7fd5d85dd7d3 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:494
    #19 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #20 0x7fd5d85ddd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #21 0x7fd5d85dde35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #22 0x7fd5d85a77e0 in ASN1_item_d2i_bio crypto/asn1/a_d2i_fp.c:69
    openssl#23 0x402845 in FuzzerTestOneInput fuzz/cms.c:43
    #24 0x402bbb in testfile fuzz/test-corpus.c:182
    #25 0x402626 in main fuzz/test-corpus.c:226
    #26 0x7fd5d7c81f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
    #27 0x402706  (/home/ed/OPC/openssl/fuzz/cms-test+0x402706)

=================================================================
==29625==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x7fd5d8b8309f in __interceptor_malloc ../../../../gcc-trunk/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fd5d87c2430 in CRYPTO_zalloc crypto/mem.c:230
    #2 0x7fd5d889501f in OPENSSL_sk_new_reserve crypto/stack/stack.c:209
    #3 0x7fd5d85dcbc3 in sk_ASN1_VALUE_new_null include/openssl/asn1t.h:928
    #4 0x7fd5d85dcbc3 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:577
    #5 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #6 0x7fd5d85db104 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:178
    #7 0x7fd5d85ddd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #8 0x7fd5d85dde35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #9 0x7fd5d88f86d9 in X509V3_EXT_d2i crypto/x509v3/v3_lib.c:142
    #10 0x7fd5d88d6d3c in crl_set_issuers crypto/x509/x_crl.c:97
    #11 0x7fd5d88d6d3c in crl_cb crypto/x509/x_crl.c:246
    #12 0x7fd5d85dc032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #13 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #14 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #15 0x7fd5d85db2b5 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:259
    #16 0x7fd5d85dc813 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:611
    #17 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #18 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #19 0x7fd5d85dca28 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:633
    #20 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #21 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #22 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#23 0x7fd5d85dd7d3 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:494
    #24 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #25 0x7fd5d85ddd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #26 0x7fd5d85dde35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #27 0x7fd5d85a77e0 in ASN1_item_d2i_bio crypto/asn1/a_d2i_fp.c:69
    #28 0x402845 in FuzzerTestOneInput fuzz/cms.c:43
    #29 0x402bbb in testfile fuzz/test-corpus.c:182
    #30 0x402626 in main fuzz/test-corpus.c:226
    openssl#31 0x7fd5d7c81f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

SUMMARY: AddressSanitizer: 32 byte(s) leaked in 1 allocation(s).

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#18391)

(cherry picked from commit e9007e0)
tmshort pushed a commit that referenced this issue Jul 5, 2022
@bernd-edlinger
Fix a memory leak in EC_GROUP_new_from_ecparameters
cc7c127 
This can be reproduced with my error injection patch.

The test vector has been validated on the 1.1.1 branch
but the issue is of course identical in all branches.

$ ERROR_INJECT=1656112173 ../util/shlib_wrap.sh ./x509-test ./corpora/x509/fe543a8d7e09109a9a08114323eefec802ad79e2
    #0 0x7fb61945eeba in __sanitizer_print_stack_trace ../../../../gcc-trunk/libsanitizer/asan/asan_stack.cpp:87
    #1 0x402f84 in my_malloc fuzz/test-corpus.c:114
    #2 0x7fb619092430 in CRYPTO_zalloc crypto/mem.c:230
    #3 0x7fb618ef7561 in bn_expand_internal crypto/bn/bn_lib.c:280
    #4 0x7fb618ef7561 in bn_expand2 crypto/bn/bn_lib.c:304
    #5 0x7fb618ef819d in BN_bin2bn crypto/bn/bn_lib.c:454
    #6 0x7fb618e7aa13 in asn1_string_to_bn crypto/asn1/a_int.c:503
    #7 0x7fb618e7aa13 in ASN1_INTEGER_to_BN crypto/asn1/a_int.c:559
    #8 0x7fb618fd8e79 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:814
    #9 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    #10 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    #11 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    #12 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    #13 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    #14 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    #15 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    #16 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #17 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #18 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #19 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #20 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #21 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #22 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#23 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #24 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #25 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    #26 0x402afb in testfile fuzz/test-corpus.c:182
    #27 0x402656 in main fuzz/test-corpus.c:226
    #28 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
    #29 0x402756  (/home/ed/OPC/openssl/fuzz/x509-test+0x402756)

=================================================================
==12221==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7fb61945309f in __interceptor_malloc ../../../../gcc-trunk/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fb619092430 in CRYPTO_zalloc crypto/mem.c:230
    #2 0x7fb618ef5f11 in BN_new crypto/bn/bn_lib.c:246
    #3 0x7fb618ef82f4 in BN_bin2bn crypto/bn/bn_lib.c:440
    #4 0x7fb618fd8933 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:618
    #5 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    #6 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    #7 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    #8 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    #9 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    #10 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    #11 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    #12 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #13 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #14 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #15 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #16 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #17 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #18 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #19 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #20 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #21 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    #22 0x402afb in testfile fuzz/test-corpus.c:182
    openssl#23 0x402656 in main fuzz/test-corpus.c:226
    #24 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

Indirect leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x7fb61945309f in __interceptor_malloc ../../../../gcc-trunk/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fb619092430 in CRYPTO_zalloc crypto/mem.c:230
    #2 0x7fb618ef7561 in bn_expand_internal crypto/bn/bn_lib.c:280
    #3 0x7fb618ef7561 in bn_expand2 crypto/bn/bn_lib.c:304
    #4 0x7fb618ef819d in BN_bin2bn crypto/bn/bn_lib.c:454
    #5 0x7fb618fd8933 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:618
    #6 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    #7 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    #8 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    #9 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    #10 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    #11 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    #12 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    #13 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #14 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #15 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #16 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #17 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #18 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #19 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #20 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #21 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #22 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    openssl#23 0x402afb in testfile fuzz/test-corpus.c:182
    #24 0x402656 in main fuzz/test-corpus.c:226
    #25 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

SUMMARY: AddressSanitizer: 80 byte(s) leaked in 2 allocation(s).

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from openssl#18633)

(cherry picked from commit be50862)
tmshort pushed a commit that referenced this issue Jul 5, 2022
@bernd-edlinger
Fix a memory leak in EC_GROUP_new_from_ecparameters
6c8879c 
This can be reproduced with my error injection patch.

The test vector has been validated on the 1.1.1 branch
but the issue is of course identical in all branches.

$ ERROR_INJECT=1656112173 ../util/shlib_wrap.sh ./x509-test ./corpora/x509/fe543a8d7e09109a9a08114323eefec802ad79e2
    #0 0x7fb61945eeba in __sanitizer_print_stack_trace ../../../../gcc-trunk/libsanitizer/asan/asan_stack.cpp:87
    #1 0x402f84 in my_malloc fuzz/test-corpus.c:114
    #2 0x7fb619092430 in CRYPTO_zalloc crypto/mem.c:230
    #3 0x7fb618ef7561 in bn_expand_internal crypto/bn/bn_lib.c:280
    #4 0x7fb618ef7561 in bn_expand2 crypto/bn/bn_lib.c:304
    #5 0x7fb618ef819d in BN_bin2bn crypto/bn/bn_lib.c:454
    #6 0x7fb618e7aa13 in asn1_string_to_bn crypto/asn1/a_int.c:503
    #7 0x7fb618e7aa13 in ASN1_INTEGER_to_BN crypto/asn1/a_int.c:559
    #8 0x7fb618fd8e79 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:814
    #9 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    #10 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    #11 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    #12 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    #13 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    #14 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    #15 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    #16 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #17 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #18 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #19 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #20 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #21 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #22 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#23 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #24 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #25 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    #26 0x402afb in testfile fuzz/test-corpus.c:182
    #27 0x402656 in main fuzz/test-corpus.c:226
    #28 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
    #29 0x402756  (/home/ed/OPC/openssl/fuzz/x509-test+0x402756)

=================================================================
==12221==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7fb61945309f in __interceptor_malloc ../../../../gcc-trunk/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fb619092430 in CRYPTO_zalloc crypto/mem.c:230
    #2 0x7fb618ef5f11 in BN_new crypto/bn/bn_lib.c:246
    #3 0x7fb618ef82f4 in BN_bin2bn crypto/bn/bn_lib.c:440
    #4 0x7fb618fd8933 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:618
    #5 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    #6 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    #7 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    #8 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    #9 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    #10 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    #11 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    #12 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #13 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #14 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #15 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #16 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #17 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #18 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #19 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #20 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #21 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    #22 0x402afb in testfile fuzz/test-corpus.c:182
    openssl#23 0x402656 in main fuzz/test-corpus.c:226
    #24 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

Indirect leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x7fb61945309f in __interceptor_malloc ../../../../gcc-trunk/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fb619092430 in CRYPTO_zalloc crypto/mem.c:230
    #2 0x7fb618ef7561 in bn_expand_internal crypto/bn/bn_lib.c:280
    #3 0x7fb618ef7561 in bn_expand2 crypto/bn/bn_lib.c:304
    #4 0x7fb618ef819d in BN_bin2bn crypto/bn/bn_lib.c:454
    #5 0x7fb618fd8933 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:618
    #6 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    #7 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    #8 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    #9 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    #10 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    #11 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    #12 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    #13 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #14 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #15 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #16 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #17 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #18 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #19 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #20 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #21 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #22 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    openssl#23 0x402afb in testfile fuzz/test-corpus.c:182
    #24 0x402656 in main fuzz/test-corpus.c:226
    #25 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

SUMMARY: AddressSanitizer: 80 byte(s) leaked in 2 allocation(s).

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from openssl#18632)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
1.1.1 Applies to 1.1.1 enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kaduk @nibanks