Page MenuHomePhabricator

0001-connector-Add-checks-if-file-descriptor-is-socket-or_v2.patch

0001-connector-Add-checks-if-file-descriptor-is-socket-or_v2.patch

From 5d217aee90088f9ca94fe60e8a4f17416133a429 Mon Sep 17 00:00:00 2001
From: David Wedderwille <davidwe@posteo.de>
Date: Tue, 25 Sep 2018 00:07:27 +0200
Subject: [PATCH] connector: Add checks if file descriptor is socket or a named
pipe on Windows.
Fixes T104
Signed-off-by: David Wedderwille <davidwe@posteo.de>
---
src/connector.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 78 insertions(+), 5 deletions(-)
diff --git a/src/connector.c b/src/connector.c
index 407aa522..dff7edec 100644
--- a/src/connector.c
+++ b/src/connector.c
@@ -26,6 +26,8 @@
#include "libssh/callbacks.h"
#include "libssh/session.h"
#include <stdlib.h>
+#include <errno.h>
+#include <stdbool.h>
#define CHUNKSIZE 4096
#ifdef _WIN32
@@ -50,6 +52,8 @@ struct ssh_connector_struct {
socket_t in_fd;
socket_t out_fd;
+
+ bool fd_is_socket;
ssh_poll_handle in_poll;
ssh_poll_handle out_poll;
@@ -76,6 +80,10 @@ static int ssh_connector_channel_write_wontblock_cb(ssh_session session,
ssh_channel channel,
size_t bytes,
void *userdata);
+static int ssh_connector_fd_read(ssh_connector connector, void *buffer, uint32_t len);
+static int ssh_connector_fd_write(ssh_connector connector, const void *buffer,
+ uint32_t len);
+static bool ssh_connector_fd_is_socket(socket_t socket);
ssh_connector ssh_connector_new(ssh_session session)
{
@@ -90,6 +98,8 @@ ssh_connector ssh_connector_new(ssh_session session)
connector->session = session;
connector->in_fd = SSH_INVALID_SOCKET;
connector->out_fd = SSH_INVALID_SOCKET;
+
+ connector->fd_is_socket = false;
ssh_callbacks_init(&connector->in_channel_cb);
ssh_callbacks_init(&connector->out_channel_cb);
@@ -167,12 +177,14 @@ int ssh_connector_set_out_channel(ssh_connector connector,
void ssh_connector_set_in_fd(ssh_connector connector, socket_t fd)
{
connector->in_fd = fd;
+ connector->fd_is_socket = ssh_connector_fd_is_socket(fd);
connector->in_channel = NULL;
}
void ssh_connector_set_out_fd(ssh_connector connector, socket_t fd)
{
connector->out_fd = fd;
+ connector->fd_is_socket = ssh_connector_fd_is_socket(fd);
connector->out_channel = NULL;
}
@@ -238,8 +250,8 @@ static void ssh_connector_fd_in_cb(ssh_connector connector)
/* Don't attempt reading more than the window */
toread = MIN(size, CHUNKSIZE);
}
-
- r = read(connector->in_fd, buffer, toread);
+
+ r = ssh_connector_fd_read(connector, buffer, toread);
if (r < 0) {
ssh_connector_except(connector, connector->in_fd);
return;
@@ -277,7 +289,7 @@ static void ssh_connector_fd_in_cb(ssh_connector connector)
* bytes
*/
while (total != r) {
- w = write(connector->out_fd, buffer + total, r - total);
+ w = ssh_connector_fd_write(connector, buffer + total, r - total);
if (w < 0){
ssh_connector_except(connector, connector->out_fd);
return;
@@ -319,7 +331,7 @@ static void ssh_connector_fd_out_cb(ssh_connector connector){
} else if(r>0) {
/* loop around write in case the write blocks even for CHUNKSIZE bytes */
while (total != r){
- w = write(connector->out_fd, buffer + total, r - total);
+ w = ssh_connector_fd_write(connector, buffer + total, r - total);
if (w < 0){
ssh_connector_except(connector, connector->out_fd);
return;
@@ -451,7 +463,7 @@ static int ssh_connector_channel_data_cb(ssh_session session,
ssh_connector_except_channel(connector, connector->out_channel);
}
} else if (connector->out_fd != SSH_INVALID_SOCKET) {
- w = write(connector->out_fd, data, len);
+ w = ssh_connector_fd_write(connector, data, len);
if (w < 0)
ssh_connector_except(connector, connector->out_fd);
} else {
@@ -634,3 +646,64 @@ int ssh_connector_remove_event(ssh_connector connector) {
return SSH_OK;
}
+
+/**
+ * @internal
+ *
+ * @brief Check the file descriptor to check if it is a Windows socket handle or a named pipe handle.
+ *
+ */
+static bool ssh_connector_fd_is_socket(socket_t s)
+{
+#ifdef _WIN32
+ struct sockaddr_in addr_fd;
+ int len = sizeof(addr_fd);
+ int rc = getsockname(s,(struct sockaddr *)&addr_fd,&len);
+ if (rc < 0) {
+ SSH_LOG(SSH_LOG_TRACE, "error %i getsockname() for fd %d", WSAGetLastError(),s);
+ }
+ /* The descriptor is neither a socket nor a named pipe */
+ if(rc != ENOTSOCK && !GetNamedPipeInfo((HANDLE)_get_osfhandle(s), 0, 0, 0, 0))
+ return true;
+ else
+ return false;
+#else
+ return false;
+#endif
+}
+
+/**
+ * @internal
+ *
+ * @brief read len bytes from socket into buffer
+ *
+ */
+static int ssh_connector_fd_read(ssh_connector connector, void *buffer, uint32_t len) {
+ int rc = -1;
+
+ if(connector->fd_is_socket)
+ rc = recv(connector->in_fd,buffer, len, 0);
+ else
+ rc = read(connector->in_fd,buffer, len);
+
+ return rc;
+}
+
+/**
+ * @internal
+ *
+ * @brief brief writes len bytes from buffer to socket
+ *
+ */
+static int ssh_connector_fd_write(ssh_connector connector, const void *buffer,
+ uint32_t len) {
+ int w = -1;
+
+ if (connector->fd_is_socket)
+ w = send(connector->out_fd,buffer, len, 0);
+ else
+ w = write(connector->out_fd, buffer, len);
+
+ return w;
+}
+
--
2.19.0

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
42801

Event Timeline

asn added a subscriber: asn.Sep 25 2018, 8:15 AM

Thanks you very much for the patch!

I have some comments about the code:

  • For ssh_connector_fd_read() and ssh_connector_fd_write() Please use ssize_t as the return value and the variable inside.
  • Also check on unix using fstat and IS_SOCK() if it is a socket!
  • Use 4 spaces
  • Always use brackets for if-clauses, see CVE-2014-1266
  • Pass MSG_NOSIGNAL to send if the flag exists

I've did some cleanup to socket.c you can find it here:
https://git.libssh.org/users/asn/libssh.git/log/?h=master-connector

This should be a good reference how the code should look like.

Thanks!

Connector descriptions which are mention in coding. I will wait for the time because connector add checks all descriptions which are here with perfect essayshark. I start to do work for connector and make it more perfect.