session BUGFIX do not close SSH socket twice

Currently, SSH socket is closed twice when created by libssh, causing
undefined behavior in multithreaded programs (e.g. netopeer2-server).

ssh_disconnect() used to close the SSH socket. However, this behavior
changed with libssh 0.10: the socket is no longer closed when passed
through options.

Only close SSH socket when created by us after disconnecting from
the session (if libssh >= 0.10).

Fixes #450
Fixes: 70e9062 ("session BUGFIX close SSH socket")
Signed-off-by: Ilyes Ben Hamouda <[email protected]>

Author: Ilyes Ben Hamouda

src/session.c

@@ -816,12 +816,18 @@ nc_session_free_transport(struct nc_session *session, int *multisession)
                     free(siter);
                 } while (session->ti.libssh.next != session);
             }
-            /* remember sock so we can close it */
-            sock = ssh_get_fd(session->ti.libssh.session);
             if (connected) {
-                /* does not close sock */
+                /* resets ssh sock but only closes it if not passed through
+                 * options since libssh 0.10
+                 */
                 ssh_disconnect(session->ti.libssh.session);
+#if (LIBSSH_VERSION_MAJOR == 0 && LIBSSH_VERSION_MINOR >= 10) || LIBSSH_VERSION_MAJOR > 0
+                if (session->ti.libssh.opt_sock != NC_SSH_INVALID_SOCKET) {
+                    close(session->ti.libssh.opt_sock);
+                }
+#endif
             }
+            /* closes ssh sock if it is still open */
             ssh_free(session->ti.libssh.session);
         } else {
             /* remove the session from the list */

src/session_client_ssh.c

@@ -1655,6 +1655,7 @@ _nc_connect_libssh(ssh_session ssh_session, struct ly_ctx *ctx, struct nc_keepal
     NC_CHECK_ERRMEM_RET(!session, NULL);
     session->status = NC_STATUS_STARTING;
     session->ti_type = NC_TI_SSH;
+    session->ti.libssh.opt_sock = NC_SSH_INVALID_SOCKET;
     session->ti.libssh.session = ssh_session;
 
     /* was port set? */
@@ -1683,6 +1684,7 @@ _nc_connect_libssh(ssh_session ssh_session, struct ly_ctx *ctx, struct nc_keepal
             free(host);
             goto fail;
         }
+        session->ti.libssh.opt_sock = sock;
         ssh_options_set(session->ti.libssh.session, SSH_OPTIONS_FD, &sock);
         ssh_set_blocking(session->ti.libssh.session, 0);
 
@@ -1809,6 +1811,7 @@ nc_connect_ssh(const char *host, uint16_t port, struct ly_ctx *ctx)
 
     /* transport-specific data */
     session->ti_type = NC_TI_SSH;
+    session->ti.libssh.opt_sock = NC_SSH_INVALID_SOCKET;
     session->ti.libssh.session = ssh_new();
     if (!session->ti.libssh.session) {
         ERR(session, "Unable to initialize SSH session.");
@@ -1833,6 +1836,7 @@ nc_connect_ssh(const char *host, uint16_t port, struct ly_ctx *ctx)
         ERR(session, "Unable to connect to %s:%u (%s).", host, port, strerror(errno));
         goto fail;
     }
+    session->ti.libssh.opt_sock = sock;
     ssh_options_set(session->ti.libssh.session, SSH_OPTIONS_FD, &sock);
     ssh_set_blocking(session->ti.libssh.session, 0);
 
@@ -1903,6 +1907,7 @@ nc_connect_ssh_channel(struct nc_session *session, struct ly_ctx *ctx)
 
     /* share some parameters including the IO lock (we are using one socket for both sessions) */
     new_session->ti_type = NC_TI_SSH;
+    new_session->ti.libssh.opt_sock = session->ti.libssh.opt_sock;
     new_session->ti.libssh.session = session->ti.libssh.session;
     new_session->io_lock = session->io_lock;
 
@@ -2016,6 +2021,9 @@ nc_accept_callhome_ssh_sock(int sock, const char *host, uint16_t port, struct ly
         return NULL;
     }
 
+    if (session->ti.libssh.opt_sock == NC_SSH_INVALID_SOCKET) {
+        session->ti.libssh.opt_sock = sock;
+    }
     session->flags |= NC_SESSION_CALLHOME;
     return session;
 }

src/session_p.h

@@ -142,6 +142,11 @@ enum nc_store_type {
 #include <curl/curl.h>
 #include <libssh/libssh.h>
 
+/**
+ * Invalid SSH socket ID.
+ */
+#define NC_SSH_INVALID_SOCKET -1
+
 /**
  * Timeout set for libssh (s).
  */
@@ -786,6 +791,7 @@ struct nc_session {
         } unixsock;              /**< NC_TI_UNIX transport implementation structure */
 #ifdef NC_ENABLED_SSH_TLS
         struct {
+            int opt_sock;                /**< socket file descriptor passed through options */
             ssh_channel channel;
             ssh_session session;
             struct nc_session *next; /**< pointer to the next NETCONF session on the same

src/session_server_ssh.c

@@ -1449,6 +1449,7 @@ nc_server_ssh_channel_subsystem(struct nc_session *session, ssh_channel channel,
 
         new_session->status = NC_STATUS_STARTING;
         new_session->ti_type = NC_TI_SSH;
+        new_session->ti.libssh.opt_sock = session->ti.libssh.opt_sock;
         new_session->io_lock = session->io_lock;
         new_session->ti.libssh.channel = channel;
         new_session->ti.libssh.session = session->ti.libssh.session;
@@ -1955,6 +1956,7 @@ nc_accept_ssh_session(struct nc_session *session, struct nc_server_ssh_opts *opt
 
     /* other transport-specific data */
     session->ti_type = NC_TI_SSH;
+    session->ti.libssh.opt_sock = NC_SSH_INVALID_SOCKET;
     session->ti.libssh.session = ssh_new();
     if (!session->ti.libssh.session) {
         ERR(NULL, "Failed to initialize a new SSH session.");