Shutdown TLS1.3 connection

[micheleselea]

I think we have to deal with SecureSocketImple::shutdown() and TLS1.3 (using for example OpenSSL 1.1.1c).
I found some strange behavior that broke connection to the end transfer if the client do not correctly close connection. Reading this I understand that we should do something more during shutdown or we can disable the session tickets sent after handshake like

#if OPENSSL_VERSION_NUMBER >= 0x1010100fL
        /* TLS 1.3 server sends session tickets after a handhake as part of
         * the SSL_accept(). If a client finishes all its job before server
         * sends the tickets, SSL_accept() fails with EPIPE errno. Since we
         * are not interested in a session resumption, we can not to send the
         * tickets. */
        /*if (1 != SSL_set_num_tickets(ssl, 0)) {
            fprintf(stderr, "SSL_set_num_tickets failed\n");
            exit(EXIT_FAILURE);
        }
        Or we can perform two-way shutdown. Client must call SSL_read() before
        the final SSL_shutdown(). */
#endif

I'm going to have some try, because I found some strange behavior for example in FTPS connection, after client sent all data and close connection the socket receive on server side throw exception.
If I find more I'll share with you

[micheleselea]

other reference
if I use the SSL_set_num_tickets(ssl, 0) in acceptSSL after SSL_new I think the problem is solved but we lost session resumption.
I'll try even with the client side walkaround of the shutdown, should be something like

 retval = SSL_shutdown(ssl);
    if (retval < 0) {
        int ssl_err = SSL_get_error(ssl, retval);
        fprintf(stderr, "Client SSL_shutdown failed: ssl_err=%d\n", ssl_err);
        kill(server_pid, SIGTERM);
        exit(EXIT_FAILURE);
    }
    printf("Client shut down TLS session.\n");

    if (retval != 1) {
        /* Consume all server's data to access the server's shutdown */
        while (1) {
            processed = SSL_read(ssl, buffer, sizeof(buffer));
            printf("Client SSL_read returned %d\n", processed);
            if (processed <= 0) {
                int ssl_error = SSL_get_error(ssl, processed);
                if (ssl_error == SSL_ERROR_ZERO_RETURN) {
                    printf("Client thinks a server finished sending data\n");
                    break;
                }
                if (ssl_error != SSL_ERROR_WANT_READ &&
                        ssl_error != SSL_ERROR_WANT_WRITE) {
                    fprintf(stderr, "Client read failed: ssl_error=%d errno=%s: ",
                            ssl_error, strerror(errno));
                    ERR_print_errors_fp(stderr);
                    fprintf(stderr, "\n");
                    exit(EXIT_FAILURE);
                }
            }
        };

        retval = SSL_shutdown(ssl);
        if (retval != 1) {
            int ssl_err = SSL_get_error(ssl, retval);
            fprintf(stderr,
                    "Waiting for server shutdown using SSL_shutdown failed: "
                    "ssl_err=%d\n", ssl_err);
            kill(server_pid, SIGTERM);
            exit(EXIT_FAILURE);
        }
    }
    printf("Client thinks a server shut down the TLS session.\n");

[micheleselea]

I found this in shutdown code

// A proper clean shutdown would require us to
			// retry the shutdown if we get a zero return
			// value, until SSL_shutdown() returns 1.
			// However, this will lead to problems with
			// most web browsers, so we just set the shutdown
			// flag by calling SSL_shutdown() once and be
			// done with it.

We already try to deal with shutdown in TLS1.2 too...probably we should fix once for all

[micheleselea]

this is the code we already tried in 2017

void SecureSocketImpl::shutdown()
{
	if (_pSSL)
	{        
        // Don't shut down the socket more than once.
        int shutdownState = SSL_get_shutdown(_pSSL);
        bool shutdownSent = (shutdownState & SSL_SENT_SHUTDOWN) == SSL_SENT_SHUTDOWN;
        if (!shutdownSent)
        {
			// A proper clean shutdown would require us to
			// retry the shutdown if we get a zero return
			// value, until SSL_shutdown() returns 1.
			// However, this will lead to problems with
			// most web browsers, so we just set the shutdown
			// flag by calling SSL_shutdown() once and be
			// done with it.
			int rc = SSL_shutdown(_pSSL);
			if (rc == 0 && _pSocket->getBlocking())
			{
				rc = SSL_shutdown(_pSSL);
			}
			if (rc < 0) handleError(rc);
			if (_pSocket->getBlocking())
			{
				_pSocket->shutdown();
			}
		}
	}
}

[micheleselea]

Did not solve with the client side shutdown....I'm able to fix it just setting SSL_set_num_tickets
no other way bring me to a correct connection

[micheleselea]

In new 1.10.0 poco versione there no this patch in acceptssl function, ai think we should add that code to make tls 1.3 shutdown work correctly. I don't know if you have other suggestions

[obiltschnig]

@micheleselea can you send a PR against poco-1.10.1 branch?

[micheleselea]

@obiltschnig ok I'll try

[github-actions]

This issue is stale because it has been open for 365 days with no activity.

[github-actions]

This issue was closed because it has been inactive for 60 days since being marked as stale.