Unify connection fields for the connected client

The connected client is currently described in two places in
the xrdp_client_info structure:-

1) In the connection_description field. This was introduced as
   field client_ip by commit d797b2cf497587355bbf25cd27d59edd1c3f2915
   for xrdp v0.6.0

2) In the client_addr and client_port fields introduced by commit
   25369460a1b2f204d03a6bc4821251d7ef2d7adf for xrdp v0.8.0

This commit unifies these two sets of fields into a single
set of fields describing the connection IP and port (for
AF_INET/AF_INET6 connections only) and a connection description
for all connection types.

The code in os_calls to provide client logging has been simplified
somewhat which should make it easier to add new connection types (e.g.
AF_VSOCK).

The old connection_description field used to be passed to sesman to
inform sesman of the IP address of the client, and also to provide
a string for 'C' field session policy matching. 'C' field session policy
matching does not actually need this string (see #2239), and so now only
the IP field is passed to sesman.

6 files changed, 185 insertions, 302 deletions

diff --git a/common/os_calls.c b/common/os_calls.c
index 419a6bff..9f9c3f31 100644
--- a/common/os_calls.c
+++ b/common/os_calls.c
@@ -78,6 +78,7 @@
 #include "os_calls.h"
 #include "string_calls.h"
 #include "log.h"
+#include "xrdp_constants.h"
 
 /* for clearenv() */
 #if defined(_WIN32)
@@ -103,6 +104,22 @@ extern char **environ;
 #define INADDR_NONE ((unsigned long)-1)
 #endif
 
+/**
+ * Type big enough to hold socket address information for any connecting type
+ */
+union sock_info
+{
+    struct sockaddr sa;
+    struct sockaddr_in sa_in;
+#if defined(XRDP_ENABLE_IPV6)
+    struct sockaddr_in6 sa_in6;
+#endif
+    struct sockaddr_un sa_un;
+#if defined(XRDP_ENABLE_VSOCK)
+    struct sockaddr_vm sa_vm;
+#endif
+};
+
 /*****************************************************************************/
 int
 g_rm_temp_dir(void)
@@ -670,84 +687,99 @@ g_sck_get_peer_cred(int sck, int *pid, int *uid, int *gid)
 }
 
 /*****************************************************************************/
-void
-g_sck_close(int sck)
-{
-#if defined(_WIN32)
-    closesocket(sck);
-#else
-    char sockname[128];
-    union
-    {
-        struct sockaddr sock_addr;
-        struct sockaddr_in sock_addr_in;
-#if defined(XRDP_ENABLE_IPV6)
-        struct sockaddr_in6 sock_addr_in6;
-#endif
-#if defined(XRDP_ENABLE_VSOCK)
-        struct sockaddr_vm sock_addr_vm;
-#endif
-    } sock_info;
-    socklen_t sock_len = sizeof(sock_info);
-
-    memset(&sock_info, 0, sizeof(sock_info));
 
-    if (getsockname(sck, &sock_info.sock_addr, &sock_len) == 0)
+static const char *
+get_peer_description(const union sock_info *sock_info,
+                     char *desc, unsigned int bytes)
+{
+    if (bytes > 0)
     {
-        switch (sock_info.sock_addr.sa_family)
+        int family = sock_info->sa.sa_family;
+        switch (family)
         {
             case AF_INET:
             {
-                struct sockaddr_in *sock_addr_in = &sock_info.sock_addr_in;
-
-                g_snprintf(sockname, sizeof(sockname), "AF_INET %s:%d",
-                           inet_ntoa(sock_addr_in->sin_addr),
-                           ntohs(sock_addr_in->sin_port));
+                char ip[INET_ADDRSTRLEN];
+                const struct sockaddr_in *sa_in = &sock_info->sa_in;
+                if (inet_ntop(family, &sa_in->sin_addr,
+                              ip, sizeof(ip)) != NULL)
+                {
+                    g_snprintf(desc, bytes, "%s:%d", ip,
+                               ntohs(sa_in->sin_port));
+                }
+                else
+                {
+                    g_snprintf(desc, bytes, "<unknown AF_INET>:%d",
+                               ntohs(sa_in->sin_port));
+                }
                 break;
             }
 
 #if defined(XRDP_ENABLE_IPV6)
-
             case AF_INET6:
             {
-                char addr[48];
-                struct sockaddr_in6 *sock_addr_in6 = &sock_info.sock_addr_in6;
-
-                g_snprintf(sockname, sizeof(sockname), "AF_INET6 %s port %d",
-                           inet_ntop(sock_addr_in6->sin6_family,
-                                     &sock_addr_in6->sin6_addr, addr, sizeof(addr)),
-                           ntohs(sock_addr_in6->sin6_port));
+                char ip[INET6_ADDRSTRLEN];
+                const struct sockaddr_in6 *sa_in6 = &sock_info->sa_in6;
+                if (inet_ntop(family, &sa_in6->sin6_addr,
+                              ip, sizeof(ip)) != NULL)
+                {
+                    g_snprintf(desc, bytes, "[%s]:%d", ip,
+                               ntohs(sa_in6->sin6_port));
+                }
+                else
+                {
+                    g_snprintf(desc, bytes, "[<unknown AF_INET6>]:%d",
+                               ntohs(sa_in6->sin6_port));
+                }
                 break;
             }
-
 #endif
 
             case AF_UNIX:
-                g_snprintf(sockname, sizeof(sockname), "AF_UNIX");
+            {
+                g_snprintf(desc, bytes, "AF_UNIX");
                 break;
+            }
 
 #if defined(XRDP_ENABLE_VSOCK)
 
             case AF_VSOCK:
             {
-                struct sockaddr_vm *sock_addr_vm = &sock_info.sock_addr_vm;
+                const struct sockaddr_vm *sa_vm = &sock_info->sa_vm;
+
+                g_snprintf(desc, bytes, "AF_VSOCK:cid=%u/port=%u",
+                           sa_vm->svm_cid, sa_vm->svm_port);
 
-                g_snprintf(sockname,
-                           sizeof(sockname),
-                           "AF_VSOCK cid %d port %d",
-                           sock_addr_vm->svm_cid,
-                           sock_addr_vm->svm_port);
                 break;
             }
 
 #endif
-
             default:
-                g_snprintf(sockname, sizeof(sockname), "unknown family %d",
-                           sock_info.sock_addr.sa_family);
+                g_snprintf(desc, bytes, "Unknown address family %d", family);
                 break;
         }
     }
+
+    return desc;
+}
+
+/*****************************************************************************/
+void
+g_sck_close(int sck)
+{
+#if defined(_WIN32)
+    closesocket(sck);
+#else
+    char sockname[MAX_PEER_DESCSTRLEN];
+
+    union sock_info sock_info;
+    socklen_t sock_len = sizeof(sock_info);
+    memset(&sock_info, 0, sizeof(sock_info));
+
+    if (getsockname(sck, &sock_info.sa, &sock_len) == 0)
+    {
+        get_peer_description(&sock_info, sockname, sizeof(sockname));
+    }
     else
     {
         LOG(LOG_LEVEL_WARNING, "getsockname() failed on socket %d: %s",
@@ -1238,83 +1270,10 @@ g_sck_listen(int sck)
 
 /*****************************************************************************/
 int
-g_tcp_accept(int sck)
-{
-    int ret;
-    char msg[256];
-    union
-    {
-        struct sockaddr sock_addr;
-        struct sockaddr_in sock_addr_in;
-#if defined(XRDP_ENABLE_IPV6)
-        struct sockaddr_in6 sock_addr_in6;
-#endif
-    } sock_info;
-
-    socklen_t sock_len = sizeof(sock_info);
-    memset(&sock_info, 0, sock_len);
-
-    ret = accept(sck, (struct sockaddr *)&sock_info, &sock_len);
-
-    if (ret > 0)
-    {
-        switch (sock_info.sock_addr.sa_family)
-        {
-            case AF_INET:
-            {
-                struct sockaddr_in *sock_addr_in = &sock_info.sock_addr_in;
-
-                g_snprintf(msg, sizeof(msg), "A connection received from %s port %d",
-                           inet_ntoa(sock_addr_in->sin_addr),
-                           ntohs(sock_addr_in->sin_port));
-                LOG(LOG_LEVEL_INFO, "%s", msg);
-
-                break;
-            }
-
-#if defined(XRDP_ENABLE_IPV6)
-
-            case AF_INET6:
-            {
-                struct sockaddr_in6 *sock_addr_in6 = &sock_info.sock_addr_in6;
-                char addr[256];
-
-                inet_ntop(sock_addr_in6->sin6_family,
-                          &sock_addr_in6->sin6_addr, addr, sizeof(addr));
-                g_snprintf(msg, sizeof(msg), "A connection received from %s port %d",
-                           addr, ntohs(sock_addr_in6->sin6_port));
-                LOG(LOG_LEVEL_INFO, "%s", msg);
-
-                break;
-
-            }
-
-#endif
-        }
-    }
-
-    return ret;
-}
-
-/*****************************************************************************/
-int
-g_sck_accept(int sck, char *addr, int addr_bytes, char *port, int port_bytes)
+g_sck_accept(int sck)
 {
     int ret;
-    char msg[256];
-    union
-    {
-        struct sockaddr sock_addr;
-        struct sockaddr_in sock_addr_in;
-#if defined(XRDP_ENABLE_IPV6)
-        struct sockaddr_in6 sock_addr_in6;
-#endif
-        struct sockaddr_un sock_addr_un;
-#if defined(XRDP_ENABLE_VSOCK)
-        struct sockaddr_vm sock_addr_vm;
-#endif
-    } sock_info;
-
+    union sock_info sock_info;
     socklen_t sock_len = sizeof(sock_info);
     memset(&sock_info, 0, sock_len);
 
@@ -1322,77 +1281,10 @@ g_sck_accept(int sck, char *addr, int addr_bytes, char *port, int port_bytes)
 
     if (ret > 0)
     {
-        switch (sock_info.sock_addr.sa_family)
-        {
-            case AF_INET:
-            {
-                struct sockaddr_in *sock_addr_in = &sock_info.sock_addr_in;
-
-                g_snprintf(addr, addr_bytes, "%s", inet_ntoa(sock_addr_in->sin_addr));
-                g_snprintf(port, port_bytes, "%d", ntohs(sock_addr_in->sin_port));
-                g_snprintf(msg, sizeof(msg),
-                           "AF_INET connection received from %s port %s",
-                           addr, port);
-                break;
-            }
-
-#if defined(XRDP_ENABLE_IPV6)
-
-            case AF_INET6:
-            {
-                struct sockaddr_in6 *sock_addr_in6 = &sock_info.sock_addr_in6;
-
-                inet_ntop(sock_addr_in6->sin6_family,
-                          &sock_addr_in6->sin6_addr, addr, addr_bytes);
-                g_snprintf(port, port_bytes, "%d", ntohs(sock_addr_in6->sin6_port));
-                g_snprintf(msg, sizeof(msg),
-                           "AF_INET6 connection received from %s port %s",
-                           addr, port);
-                break;
-            }
-
-#endif
-
-            case AF_UNIX:
-            {
-                g_strncpy(addr, "", addr_bytes - 1);
-                g_strncpy(port, "", port_bytes - 1);
-                g_snprintf(msg, sizeof(msg), "AF_UNIX connection received");
-                break;
-            }
-
-#if defined(XRDP_ENABLE_VSOCK)
-
-            case AF_VSOCK:
-            {
-                struct sockaddr_vm *sock_addr_vm = &sock_info.sock_addr_vm;
-
-                g_snprintf(addr, addr_bytes - 1, "%d", sock_addr_vm->svm_cid);
-                g_snprintf(port, addr_bytes - 1, "%d", sock_addr_vm->svm_port);
-
-                g_snprintf(msg,
-                           sizeof(msg),
-                           "AF_VSOCK connection received from cid: %s port: %s",
-                           addr,
-                           port);
-
-                break;
-            }
-
-#endif
-            default:
-            {
-                g_strncpy(addr, "", addr_bytes - 1);
-                g_strncpy(port, "", port_bytes - 1);
-                g_snprintf(msg, sizeof(msg),
-                           "connection received, unknown socket family %d",
-                           sock_info.sock_addr.sa_family);
-                break;
-            }
-        }
-
-
-        LOG(LOG_LEVEL_INFO, "Socket %d: %s", ret, msg);
+        char description[MAX_PEER_DESCSTRLEN];
+        get_peer_description(&sock_info, description, sizeof(description));
+        LOG(LOG_LEVEL_INFO, "Socket %d: connection accepted from %s",
+            ret, description);
 
     }
 
@@ -1401,117 +1293,85 @@ g_sck_accept(int sck, char *addr, int addr_bytes, char *port, int port_bytes)
 
 /*****************************************************************************/
 
-void
-g_write_connection_description(int rcv_sck, char *description, int bytes)
+const char *
+g_sck_get_peer_ip_address(int sck,
+                          char *ip, unsigned int bytes,
+                          unsigned short *port)
 {
-    char *addr;
-    int port;
-    int ok;
-
-    union
+    if (bytes > 0)
     {
-        struct sockaddr sock_addr;
-        struct sockaddr_in sock_addr_in;
-#if defined(XRDP_ENABLE_IPV6)
-        struct sockaddr_in6 sock_addr_in6;
-#endif
-        struct sockaddr_un sock_addr_un;
-    } sock_info;
+        int ok = 0;
+        union sock_info sock_info;
 
-    ok = 0;
-    socklen_t sock_len = sizeof(sock_info);
-    memset(&sock_info, 0, sock_len);
-#if defined(XRDP_ENABLE_IPV6)
-    addr = (char *)g_malloc(INET6_ADDRSTRLEN, 1);
-#else
-    addr = (char *)g_malloc(INET_ADDRSTRLEN, 1);
-#endif
+        socklen_t sock_len = sizeof(sock_info);
+        memset(&sock_info, 0, sock_len);
 
-    if (getpeername(rcv_sck, (struct sockaddr *)&sock_info, &sock_len) == 0)
-    {
-        switch (sock_info.sock_addr.sa_family)
+        if (getpeername(sck, (struct sockaddr *)&sock_info, &sock_len) == 0)
         {
-            case AF_INET:
+            int family = sock_info.sa.sa_family;
+            switch (family)
             {
-                struct sockaddr_in *sock_addr_in = &sock_info.sock_addr_in;
-                g_snprintf(addr, INET_ADDRSTRLEN, "%s", inet_ntoa(sock_addr_in->sin_addr));
-                port = ntohs(sock_addr_in->sin_port);
-                ok = 1;
-                break;
-            }
+                case AF_INET:
+                {
+                    struct sockaddr_in *sa_in = &sock_info.sa_in;
+                    if (inet_ntop(family, &sa_in->sin_addr, ip, bytes) != NULL)
+                    {
+                        ok = 1;
+                        if (port != NULL)
+                        {
+                            *port = ntohs(sa_in->sin_port);
+                        }
+                    }
+                    break;
+                }
 
 #if defined(XRDP_ENABLE_IPV6)
 
-            case AF_INET6:
-            {
-                struct sockaddr_in6 *sock_addr_in6 = &sock_info.sock_addr_in6;
-                inet_ntop(sock_addr_in6->sin6_family,
-                          &sock_addr_in6->sin6_addr, addr, INET6_ADDRSTRLEN);
-                port = ntohs(sock_addr_in6->sin6_port);
-                ok = 1;
-                break;
-            }
+                case AF_INET6:
+                {
+                    struct sockaddr_in6 *sa_in6 = &sock_info.sa_in6;
+                    if (inet_ntop(family, &sa_in6->sin6_addr, ip, bytes) != NULL)
+                    {
+                        ok = 1;
+                        if (port != NULL)
+                        {
+                            *port = ntohs(sa_in6->sin6_port);
+                        }
+                    }
+                    break;
+                }
 
 #endif
-
-            default:
-            {
-                break;
+                default:
+                    break;
             }
-
         }
 
-        if (ok)
+        if (!ok)
         {
-            g_snprintf(description, bytes, "%s:%d - socket: %d", addr, port, rcv_sck);
+            ip[0] = '\0';
         }
     }
 
-    if (!ok)
-    {
-        g_snprintf(description, bytes, "NULL:NULL - socket: %d", rcv_sck);
-    }
-
-    g_free(addr);
+    return ip;
 }
 
 /*****************************************************************************/
 
-const char *g_get_ip_from_description(const char *description,
-                                      char *ip, int bytes)
+const char *
+g_sck_get_peer_description(int sck,
+                           char *desc, unsigned int bytes)
 {
-    if (bytes > 0)
-    {
-        /* Look for the space after ip:port */
-        const char *end = g_strchr(description, ' ');
-        if (end == NULL)
-        {
-            end = description; /* Means we've failed */
-        }
-        else
-        {
-            /* Look back for the last ':' */
-            while (end > description && *end != ':')
-            {
-                --end;
-            }
-        }
+    union sock_info sock_info;
+    socklen_t sock_len = sizeof(sock_info);
+    memset(&sock_info, 0, sock_len);
 
-        if (end == description)
-        {
-            g_snprintf(ip, bytes, "<unknown>");
-        }
-        else if ((end - description) < (bytes - 1))
-        {
-            g_strncpy(ip, description, end - description);
-        }
-        else
-        {
-            g_strncpy(ip, description, bytes - 1);
-        }
+    if (getpeername(sck, (struct sockaddr *)&sock_info, &sock_len) == 0)
+    {
+        get_peer_description(&sock_info, desc, bytes);
     }
 
-    return ip;
+    return desc;
 }
 
 /*****************************************************************************/
diff --git a/common/os_calls.h b/common/os_calls.h
index cfa57dbb..9c8a200e 100644
--- a/common/os_calls.h
+++ b/common/os_calls.h
@@ -84,9 +84,7 @@ int      g_sck_vsock_bind(int sck, const char *port);
 int      g_sck_vsock_bind_address(int sck, const char *port, const char *address);
 int      g_tcp_bind_address(int sck, const char *port, const char *address);
 int      g_sck_listen(int sck);
-int      g_tcp_accept(int sck);
-int      g_sck_accept(int sck, char *addr, int addr_bytes,
-                      char *port, int port_bytes);
+int      g_sck_accept(int sck);
 int      g_sck_recv(int sck, void *ptr, int len, int flags);
 int      g_sck_send(int sck, const void *ptr, int len, int flags);
 int      g_sck_last_error_would_block(int sck);
@@ -94,18 +92,35 @@ int      g_sck_socket_ok(int sck);
 int      g_sck_can_send(int sck, int millis);
 int      g_sck_can_recv(int sck, int millis);
 int      g_sck_select(int sck1, int sck2);
-void     g_write_connection_description(int rcv_sck,
-                                        char *description, int bytes);
 /**
- * Extracts the IP address from the connection description
- * @param description Connection description (from
- *                    g_write_connection_description())
+ * Gets the IP address of a connected peer, if it has one
+ * @param sck File descriptor for peer
  * @param ip buffer to write IP address to
- * @param bytes Size of ip buffer
+ * @param bytes Size of ip buffer. Should be at least MAX_IP_ADDRSTRLEN
+ * @param[out] portptr Optional variable to receive the port number
  * @return Pointer to IP for convenience
+ *
+ * If the peer has no IP address (for example, it is a Unix Domain Socket),
+ * or the specified buffer is too small, the returned string is ""
+ */
+const char *
+g_sck_get_peer_ip_address(int sck,
+                          char *ip, unsigned int bytes,
+                          unsigned short *port);
+/**
+ * Gets a description for a connected peer
+ * @param sck File descriptor for peer
+ * @param desc buffer to write description to
+ * @param bytes Size of description buffer. Should be at least
+ *              MAX_PEER_DESCSTRLEN
+ * @return Pointer to desc for convenience
+ *
+ * Unlike g_sck_get_peer_ip_address(), this will return a
+ * description of some sort for any socket type.
  */
-const char *g_get_ip_from_description(const char *description,
-                                      char *ip, int bytes);
+const char *
+g_sck_get_peer_description(int sck,
+                           char *desc, unsigned int bytes);
 void     g_sleep(int msecs);
 tintptr  g_create_wait_obj(const char *name);
 tintptr  g_create_wait_obj_from_socket(tintptr socket, int write);
diff --git a/common/trans.c b/common/trans.c
index eb451073..6b39243b 100644
--- a/common/trans.c
+++ b/common/trans.c
@@ -330,9 +330,7 @@ trans_check_wait_objs(struct trans *self)
     {
         if (g_sck_can_recv(self->sck, 0))
         {
-            in_sck = g_sck_accept(self->sck, self->addr, sizeof(self->addr),
-                                  self->port, sizeof(self->port));
-
+            in_sck = g_sck_accept(self->sck);
             if (in_sck == -1)
             {
                 if (g_tcp_last_error_would_block(self->sck))
@@ -357,10 +355,6 @@ trans_check_wait_objs(struct trans *self)
                     in_trans->type1 = TRANS_TYPE_SERVER;
                     in_trans->status = TRANS_STATUS_UP;
                     in_trans->is_term = self->is_term;
-                    g_strncpy(in_trans->addr, self->addr,
-                              sizeof(self->addr) - 1);
-                    g_strncpy(in_trans->port, self->port,
-                              sizeof(self->port) - 1);
                     g_sck_set_non_blocking(in_sck);
                     if (self->trans_conn_in(self, in_trans) != 0)
                     {
diff --git a/common/trans.h b/common/trans.h
index c6b6c4bb..3d3991ac 100644
--- a/common/trans.h
+++ b/common/trans.h
@@ -104,8 +104,6 @@ struct trans
     char *listen_filename;
     tis_term is_term; /* used to test for exit */
     struct stream *wait_s;
-    char addr[256];
-    char port[256];
     int no_stream_init_on_data_in;
     int extra_flags; /* user defined */
     void *extra_data; /* user defined */
diff --git a/common/xrdp_client_info.h b/common/xrdp_client_info.h
index 0cd844d4..bf57de09 100644
--- a/common/xrdp_client_info.h
+++ b/common/xrdp_client_info.h
@@ -114,7 +114,7 @@ struct xrdp_client_info
     int rdp5_performanceflags;
     int brush_cache_code; /* 0 = no cache 1 = 8x8 standard cache
                            2 = arbitrary dimensions */
-    char connection_description[256];
+
     int max_bpp;
     int jpeg; /* non standard bitmap cache v2 cap */
     int offscreen_support_level;
@@ -146,8 +146,6 @@ struct xrdp_client_info
     int pointer_flags; /* 0 color, 1 new, 2 no new */
     int use_fast_path;
     int require_credentials; /* when true, credentials *must* be passed on cmd line */
-    char client_addr[256];
-    char client_port[256];
 
     int security_layer; /* 0 = rdp, 1 = tls , 2 = hybrid */
     int multimon; /* 0 = deny , 1 = allow */
@@ -191,6 +189,9 @@ struct xrdp_client_info
     long ssl_protocols;
     char *tls_ciphers;
 
+    char client_ip[MAX_PEER_ADDRSTRLEN];
+    char client_description[MAX_PEER_DESCSTRLEN];
+
     int client_os_major;
     int client_os_minor;
 
@@ -207,6 +208,6 @@ struct xrdp_client_info
 };
 
 /* yyyymmdd of last incompatible change to xrdp_client_info */
-#define CLIENT_INFO_CURRENT_VERSION 20220320
+#define CLIENT_INFO_CURRENT_VERSION 20220428
 
 #endif
diff --git a/common/xrdp_constants.h b/common/xrdp_constants.h
index 95bb1407..7adce229 100644
--- a/common/xrdp_constants.h
+++ b/common/xrdp_constants.h
@@ -37,7 +37,22 @@
  * ms-erref.h
  ******************************************************************************/
 
+/**
+ * Size of buffer including terminator for an IP address as returned
+ * by g_sck_get_peer_ip_address(). See POSIX INET6_ADDRSTRLEN
+ */
+#define MAX_PEER_ADDRSTRLEN 46
+
+/**
+ * Size of buffer including terminator for a socket description, as
+ * returned by g_sck_get_peer_description()
+ * Currently the largest is an IPv6 address (INET6_ADDRSTRLEN), plus
+ * []:<port> characters
+ */
+#define MAX_PEER_DESCSTRLEN (46 + 2 + 1 + 5)
+
 #define INFO_CLIENT_NAME_BYTES  32
+
 /**
  * Maximum length of a string including the mandatory null terminator
  * [MS-RDPBCGR] TS_INFO_PACKET(2.2.1.11.1.1)