tls: Set session only once before Client Hello

[cspiel1]

If the server rejects the session reusage then
the client should not re-set the session multiple
times in function tls_connect() => During the
TLS handshaking / After the new session ticket

Otherwise the first connection attempt in the
rejection-case will fail. Only the second attempt
succeeds.

[cspiel1]

@sreimers Where are the workflow jobs now?

I still try to find the reason for the segfault in retest on MaxOS with openssl 1.1.1s.

[sreimers]

@sreimers Where are the workflow jobs now?

Unsure, maybe a github action bug. Can you try a rebase with git push? or a dummy commit.

[cspiel1]

Great, it works and we don't know why. :-)

[cspiel1]

These warnings seem to be related to the SEGV. At least they are related to the commit.

tcp: conn peer get: getpeername(): Socket is not connected [57]
tls: tls_reuse_session: tcp_conn_peer_get failed: (Socket is not connected [57]).
tcp: conn peer get: getpeername(): Socket is not connected [57]
tls: tls_reuse_session: tcp_conn_peer_get failed: (Socket is not connected [57]).

tlstest: TEST_EQUALS: /Users/runner/work/re/retest/src/tls.c:283: test_tls_base(): expected=1(0x1), actual=0(0x0)
/Users/runner/work/_temp/35eddbf0-d2a1-4513-a4f4-a1c8d3381d4f.sh: line 4:  3483 Segmentation fault: 11  ./retest -r -v

Enabled retest verbosity.

This warnings are not printed on:

• Ubuntu 22.04 (OpenSSL 3.0.2)
• Ubuntu 22.04 with OpenSSL_1_1_1s

[cspiel1]

When looking in the code of tls_tcp.c and the tls test: It looks like on Ubuntu the TCP connection is established sychronously before tls_start_tcp() is called. Anyway, the warning makes sense and the solution for the unique TLS session should be done differently. But this is not an explanation for the SEGV.

@sreimers Should we fix the SEGV first? Do you have a Mac installation with openssl 1.1.1s?

I expect the SEGV in retest/src/tls.c somewhere in this mem_deref part:

 out:
	/* NOTE: close context first */
	mem_deref(tt.tls);

	mem_deref(tt.sc_cli);
	mem_deref(tt.sc_srv);
	mem_deref(tt.tc_cli);
	mem_deref(tt.tc_srv);
	mem_deref(tt.ts);

[sreimers]

This is the backtrace:

Better one, with re debug info:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
  * frame #0: 0x000000010038993f libre.11.dylib`hash_apply(h=0xb5b5b5b5b5b5b5b5, ah=(libre.11.dylib`remove_handler at tls.c:1619), arg=0x00000001006047f0) at hash.c:135:14
    frame #1: 0x00000001003fa9e1 libre.11.dylib`session_remove_cb(ctx=0x0000000102808200, sess=0x00000001006047f0) at tls.c:1642:9
    frame #2: 0x000000010047ccc4 libssl.3.dylib`SSL_CTX_flush_sessions + 153
    frame #3: 0x0000000100472b65 libssl.3.dylib`SSL_CTX_free + 149
    frame #4: 0x000000010047183b libssl.3.dylib`SSL_free + 770
    frame #5: 0x00000001003f66ac libre.11.dylib`destructor(arg=0x000060000391f7d0) at tls_tcp.c:50:3
    frame #6: 0x000000010039f046 libre.11.dylib`mem_deref(data=0x000060000391f7d0) at mem.c:376:3
    frame #7: 0x0000000100058cd0 retest`test_tls_base(keytype=TLS_KEYTYPE_EC, add_ca=false, exp_verr=80, test_sess_reuse=true, forced_version=771) at tls.c:299:2
    frame #8: 0x000000010005835d retest`test_tls_session_reuse_tls_v12 at tls.c:310:9
    frame #9: 0x00000001000469f5 retest`test_unit(name=0x0000000000000000, verbose=false) at test.c:506:10
    frame #10: 0x0000000100046831 retest`test_reg(name=0x0000000000000000, verbose=false) at test.c:724:8
    frame #11: 0x000000010002256a retest`main(argc=0, argv=0x00007ff7bfeffa78) at main.c:233:9
    frame #12: 0x00000001000a152e dyld`start + 462

[cspiel1]

Ah, sorry. Didn't update the view and missed your comment.
The last commit solves the original problem.

Will remove it again, in order we can solve the SEGV. It is when the SSL is freed.

[cspiel1]

Maybe I am able to debug this tomorrow. The question is what is wrong with the session_remove_cb when the test fails on MacOS?

[alfredh]

Maybe I am able to debug this tomorrow. The question is what is wrong with the session_remove_cb when the test fails on MacOS?

The object is destroyed, since the pattern is 0xb5b5b5b5

Set the callback handler to NULL in the destructor, or check the deref order.

[cspiel1]

@fAuernigg is on vacation and I am not sure. Isn't it a normal situation that the session_remove_cb() is called if an SSL is destroyed, thus in the tls_conn destructor?

I think there is some cleanup missing after first "round" in the test_tls_base:

for (i = 0; i < rounds; i++) {

[cspiel1]

Alfred, your right! The tls object is destroyed before the tls_conn. When SSL is freed, the callback is called and the tls pointer is already invalid. The last commit fixes the SEGV.

Edit: The test fails now "normally" without SEGV.

[cspiel1]

Now I split the PR.

[cspiel1]

SEGV fix: #611

This PR has to be re-based later.

[cspiel1]

This is ready for merge now.

[sreimers]

Thanks!