Jens Geyer created THRIFT-6214:
----------------------------------
Summary: C++: TSSLSocketMatchNameTest is killed by SIGPIPE in
about a third of runs
Key: THRIFT-6214
URL: https://issues.apache.org/jira/browse/THRIFT-6214
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
h2. What happens
{{TSSLSocketMatchNameTest}} is killed by {{SIGPIPE}} on some runs:
{noformat}
47/58 Test #47: TSSLSocketMatchNameTest ...............SIGPIPE***Exception:
0.17 sec
{noformat}
Run directly, the binary exits with status 141 and prints nothing, because the
signal ends it before Boost.Test gets to report. Built from master it failed 3
of 10 consecutive runs; the others pass.
h2. Where it comes from
Under gdb the signal arrives in the server thread of the negative handshake
case (backtrace abridged):
{noformat}
Entering test case "common_name_is_not_consulted_when_a_dns_san_is_present"
Thread 3 "TSSLSocketMatch" received signal SIGPIPE, Broken pipe.
#5 BIO_write () from /lib/x86_64-linux-gnu/libcrypto.so.3
#7 BIO_ctrl () from /lib/x86_64-linux-gnu/libcrypto.so.3
#8 ?? () from /lib/x86_64-linux-gnu/libssl.so.3
#10 apache::thrift::transport::TSSLSocket::initializeHandshake() at
lib/cpp/src/thrift/transport/TSSLSocket.cpp:650
#11 apache::thrift::transport::TSSLSocket::write(buf="OK", len=2) at
lib/cpp/src/thrift/transport/TSSLSocket.cpp:509
#13 TSSLSocketMatchNameTest::(anonymous namespace)::OneShotServer::run() at
lib/cpp/test/TSSLSocketMatchNameTest.cpp:195
{noformat}
That case exists to have the client refuse the server's certificate, and it
does: the client gives up and closes the connection while
{{OneShotServer::run()}} is still inside {{SSL_accept()}}, which its
{{write()}} started. {{TSSLSocket}} hands the descriptor to OpenSSL
({{SSL_set_fd}}), and OpenSSL's socket BIO writes without {{MSG_NOSIGNAL}}.
When the server's next handshake write reaches a connection the client has
already torn down, it raises {{SIGPIPE}}, and since nothing in this test module
ignores the signal, the process dies. When the write gets out first, the
handshake fails on the following read instead, and {{run()}} catches that as
intended -- hence the flakiness.
Every other test that drives TLS connections ignores the signal for exactly
this reason -- {{SecurityTest}}, {{SecurityFromBufferTest}},
{{TNonblockingSSLServerTest}} and {{TSSLSocketInterruptTest}} in
{{lib/cpp/test}}, and {{test/cpp/src/TestServer.cpp}}:
{code:cpp}
// OpenSSL calls send() without MSG_NOSIGPIPE so writing to a socket that has
// disconnected can cause a SIGPIPE signal...
signal(SIGPIPE, SIG_IGN);
{code}
{{TSSLSocketMatchNameTest}} does not. It only called
{{DefaultClientAccessManager::verify()}} until cd199715b added the handshake
cases, and the signal handling did not come along with them.
h2. Suggested fix
Ignore {{SIGPIPE}} in {{TSSLSocketMatchNameTest}} the way {{SecurityTest.cpp}}
does, with a {{BOOST_GLOBAL_FIXTURE}}. The server's write then fails with
{{EPIPE}}, {{SSL_accept()}} returns an error, and {{OneShotServer::run()}}
swallows the resulting exception, which it already expects in this case.
h2. Seen with
Current master -- built from 2f75006ca; {{lib/cpp}} is unchanged up to
e1176aff9 -- in the {{thrift:jammy}} build image: Ubuntu 22.04, OpenSSL 3.0.2,
GCC 11.4.0, Boost 1.74, CMake Debug build.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
