Skip to content

Commit 2006eca

Browse files
bbockelmmatyasselmeci
authored andcommitted
[#36] Do a clean TLS shutdown for HTTPS
Unlike XrdTlsSocket, the HTTP protocol did a one-sided TLS shutdown. This saved network a round-trip but at the cost of correctness: if the server shut down the connection after its response while the client was still sending data then the client may recieve a TCP reset prior to reading out the response. This exact behavior was observed in unit tests and the correct approach is outlined in latest HTTP 1.1 RFC: https://datatracker.ietf.org/doc/html/rfc9112#name-tls-connection-closure Basically, we now do the same as `XrdTlsSocket` and perform a bidirectional TLS shutdown.
1 parent 02d6a53 commit 2006eca

File tree

1 file changed

+15
-12
lines changed

1 file changed

+15
-12
lines changed

src/XrdHttp/XrdHttpProtocol.cc

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1938,23 +1938,26 @@ void XrdHttpProtocol::Cleanup() {
19381938

19391939
if (ssl) {
19401940
// Shutdown the SSL/TLS connection
1941-
// https://www.openssl.org/docs/man1.0.2/man3/SSL_shutdown.html
1942-
// We don't need a bidirectional shutdown as
1943-
// when we are here, the connection will not be re-used.
1944-
// In the case SSL_shutdown returns 0,
1945-
// "the output of SSL_get_error(3) may be misleading, as an erroneous SSL_ERROR_SYSCALL may be flagged even though no error occurred."
1946-
// we will then just flush the thread's queue.
1947-
// In the case an error really happened, we print the error that happened
1941+
// This triggers a bidirectional shutdown of the connection; the bidirectional
1942+
// shutdown is useful to ensure that the client receives the server response;
1943+
// a one-sided shutdown can result in the server sending a TCP reset packet, zapping
1944+
// the contents of the TCP socket buffer on the client side. The HTTP 1.1 RFC has a
1945+
// description of why this is important:
1946+
// https://datatracker.ietf.org/doc/html/rfc9112#name-tls-connection-closure
1947+
// Once we get the clean SSL shutdown message back from the client, we know that
1948+
// the client has received the response and we can safely close the connection.
19481949
int ret = SSL_shutdown(ssl);
19491950
if (ret != 1) {
19501951
if(ret == 0) {
1951-
// Clean this thread's error queue for the old openssl versions
1952-
#if OPENSSL_VERSION_NUMBER < 0x10100000L
1953-
ERR_remove_thread_state(nullptr);
1954-
#endif
1952+
// ret == 0, the unidirectional shutdown was successful; wait for the acknowledgement.
1953+
ret = SSL_shutdown(ssl);
1954+
if (ret != 1) {
1955+
TRACE(ALL, "SSL server failed to receive the SSL shutdown message from the client");
1956+
ERR_print_errors(sslbio_err);
1957+
}
19551958
} else {
19561959
//ret < 0, an error really happened.
1957-
TRACE(ALL, " SSL_shutdown failed");
1960+
TRACE(ALL, "SSL server failed to send the shutdown message to the client");
19581961
ERR_print_errors(sslbio_err);
19591962
}
19601963
}

0 commit comments

Comments
 (0)