TLS: do not ignore accept callback result

Before this change the TLS code would ignore the accept callback result,
and would not try to gracefully close the connection. This had not been
noticed, as it is not really required for DoH. Now the code tries to
shut down the TLS connection gracefully when accepting it is not
successful.

(cherry picked from commit ffcb54211e148ecc7e145f6ea79a0ab2a58a2287)

diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c
index 2f3314d4f70f0763736d24d235760a9aacc0fa6f..a7cebd4669a08a837fccf5d565b4b74c50fa8ec0 100644 (file)
--- a/lib/isc/netmgr/tlsstream.c
+++ b/lib/isc/netmgr/tlsstream.c
@@ -324,7 +324,7 @@ tls_process_outgoing(isc_nmsocket_t *sock, bool finish,
 }
 
 static int
-tls_try_handshake(isc_nmsocket_t *sock) {
+tls_try_handshake(isc_nmsocket_t *sock, isc_result_t *presult) {
        int rv = 0;
        isc_nmhandle_t *tlshandle = NULL;
 
@@ -342,13 +342,18 @@ tls_try_handshake(isc_nmsocket_t *sock) {
                isc__nmsocket_log_tls_session_reuse(sock, sock->tlsstream.tls);
                tlshandle = isc__nmhandle_get(sock, &sock->peer, &sock->iface);
                if (sock->tlsstream.server) {
-                       sock->listener->accept_cb(tlshandle, result,
-                                                 sock->listener->accept_cbarg);
+                       result = sock->listener->accept_cb(
+                               tlshandle, result,
+                               sock->listener->accept_cbarg);
                } else {
                        tls_call_connect_cb(sock, tlshandle, result);
                }
                isc_nmhandle_detach(&tlshandle);
                sock->tlsstream.state = TLS_IO;
+
+               if (presult != NULL) {
+                       *presult = result;
+               }
        }
 
        return (rv);
@@ -397,7 +402,7 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
                        SSL_set_connect_state(sock->tlsstream.tls);
                }
                sock->tlsstream.state = TLS_HANDSHAKE;
-               rv = tls_try_handshake(sock);
+               rv = tls_try_handshake(sock, NULL);
                INSIST(SSL_is_init_finished(sock->tlsstream.tls) == 0);
        } else if (sock->tlsstream.state == TLS_CLOSED) {
                return;
@@ -420,7 +425,21 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
                         * handshake is done.
                         */
                        if (sock->tlsstream.state == TLS_HANDSHAKE) {
-                               rv = tls_try_handshake(sock);
+                               isc_result_t hs_result = ISC_R_UNSET;
+                               rv = tls_try_handshake(sock, &hs_result);
+                               if (sock->tlsstream.state == TLS_IO &&
+                                   hs_result != ISC_R_SUCCESS) {
+                                       /*
+                                        * The accept callback has been called
+                                        * unsuccessfully. Let's try to shut
+                                        * down the TLS connection gracefully.
+                                        */
+                                       INSIST(SSL_is_init_finished(
+                                                      sock->tlsstream.tls) ==
+                                              1);
+                                       INSIST(!atomic_load(&sock->client));
+                                       finish = true;
+                               }
                        }
                } else if (send_data != NULL) {
                        INSIST(received_data == NULL);
@@ -448,7 +467,7 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
                /* Decrypt and pass data from network to client */
                if (sock->tlsstream.state >= TLS_IO && sock->recv_cb != NULL &&
                    !atomic_load(&sock->readpaused) &&
-                   sock->statichandle != NULL)
+                   sock->statichandle != NULL && !finish)
                {
                        uint8_t recv_buf[TLS_BUF_SIZE];
                        INSIST(sock->tlsstream.state > TLS_HANDSHAKE);
@@ -467,9 +486,9 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
                                 * nullified (it happens in netmgr.c). If it is
                                 * the case, then it means that we are not
                                 * interested in keeping the connection alive
-                                * anymore. Let's shutdown the SSL session, send
-                                * what we have in the SSL buffers, and close
-                                * the connection.
+                                * anymore. Let's shut down the SSL session,
+                                * send what we have in the SSL buffers,
+                                * and close the connection.
                                 */
                                if (sock->statichandle == NULL) {
                                        finish = true;
@@ -527,8 +546,13 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
                        return;
                }
 
+               if (sock->outerhandle == NULL) {
+                       return;
+               }
+
+               INSIST(VALID_NMHANDLE(sock->outerhandle));
+
                if (sock->tlsstream.reading) {
-                       INSIST(VALID_NMHANDLE(sock->outerhandle));
                        isc_nm_resumeread(sock->outerhandle);
                } else if (sock->tlsstream.state == TLS_HANDSHAKE) {
                        sock->tlsstream.reading = true;
@@ -1086,8 +1110,8 @@ isc__nm_tls_cleanup_data(isc_nmsocket_t *sock) {
        } else if (sock->type == isc_nm_tlssocket) {
                if (sock->tlsstream.tls != NULL) {
                        /*
-                        * Let's shutdown the TLS session properly so that the
-                        * session will remain resumable, if required.
+                        * Let's shut down the TLS session properly so that
+                        * the session will remain resumable, if required.
                         */
                        tls_try_shutdown(sock->tlsstream.tls, true);
                        tls_keep_client_tls_session(sock);