From 79bec8110c0e491788f32bf1d9a3faab1ff9d224 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Fri, 22 Apr 2022 11:56:23 +0100 Subject: [PATCH 1/6] 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. --- common/os_calls.c | 416 +++++++++++---------------------- common/os_calls.h | 37 ++- common/trans.c | 8 +- common/trans.h | 2 - common/xrdp_client_info.h | 9 +- common/xrdp_constants.h | 15 ++ libipm/scp.c | 29 ++- libipm/scp.h | 16 +- libipm/scp_application_types.h | 2 +- libxrdp/xrdp_rdp.c | 29 +-- neutrinordp/xrdp-neutrinordp.c | 15 +- sesman/scp_process.c | 39 ++-- sesman/session.c | 80 +++---- sesman/session.h | 5 +- sesman/tools/sesadmin.c | 4 +- tools/devel/tcp_proxy/main.c | 2 +- xrdp/xrdp_mm.c | 4 +- xrdp/xrdp_wm.c | 5 +- xup/xup.c | 5 +- 19 files changed, 282 insertions(+), 440 deletions(-) diff --git a/common/os_calls.c b/common/os_calls.c index 419a6bff31..9f9c3f3177 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, ":%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, "[]:%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, ""); - } - 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 cfa57dbb9b..9c8a200e57 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 eb451073f2..6b39243b23 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 c6b6c4bbed..3d3991acb9 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 0cd844d46b..bf57de094e 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 95bb140783..7adce229fc 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 + * []: 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) diff --git a/libipm/scp.c b/libipm/scp.c index a6cfd8271c..45f35e4b61 100644 --- a/libipm/scp.c +++ b/libipm/scp.c @@ -217,7 +217,7 @@ int scp_send_gateway_request(struct trans *trans, const char *username, const char *password, - const char *connection_description) + const char *ip_addr) { int rv; @@ -227,7 +227,7 @@ scp_send_gateway_request(struct trans *trans, "sss", username, password, - connection_description); + ip_addr); /* Wipe the output buffer to remove the password */ libipm_msg_out_erase(trans); @@ -241,13 +241,13 @@ int scp_get_gateway_request(struct trans *trans, const char **username, const char **password, - const char **connection_description) + const char **ip_addr) { /* Make sure the buffer is cleared after processing this message */ libipm_set_flags(trans, LIBIPM_E_MSG_IN_ERASE_AFTER_USE); return libipm_msg_in_parse(trans, "sss", username, password, - connection_description); + ip_addr); } /*****************************************************************************/ @@ -290,7 +290,7 @@ scp_send_create_session_request(struct trans *trans, unsigned char bpp, const char *shell, const char *directory, - const char *connection_description) + const char *ip_addr) { int rv = libipm_msg_out_simple_send( trans, @@ -304,7 +304,7 @@ scp_send_create_session_request(struct trans *trans, bpp, shell, directory, - connection_description); + ip_addr); /* Wipe the output buffer to remove the password */ libipm_msg_out_erase(trans); @@ -324,7 +324,7 @@ scp_get_create_session_request(struct trans *trans, unsigned char *bpp, const char **shell, const char **directory, - const char **connection_description) + const char **ip_addr) { /* Intermediate values */ uint8_t i_type; @@ -346,7 +346,7 @@ scp_get_create_session_request(struct trans *trans, &i_bpp, shell, directory, - connection_description); + ip_addr); if (rv == 0) { @@ -475,7 +475,7 @@ scp_send_list_sessions_response( info->bpp, info->start_time, info->username, - info->connection_description); + info->start_ip_addr); } return rv; @@ -512,7 +512,7 @@ scp_get_list_sessions_response( uint8_t i_bpp; int64_t i_start_time; char *i_username; - char *i_connection_description; + char *i_start_ip_addr; rv = libipm_msg_in_parse( trans, @@ -525,7 +525,7 @@ scp_get_list_sessions_response( &i_bpp, &i_start_time, &i_username, - &i_connection_description); + &i_start_ip_addr); if (rv == 0) { @@ -533,7 +533,7 @@ scp_get_list_sessions_response( * structure result, and the strings it contains */ unsigned int len = sizeof(struct scp_session_info) + g_strlen(i_username) + 1 + - g_strlen(i_connection_description) + 1; + g_strlen(i_start_ip_addr) + 1; if ((p = (struct scp_session_info *)g_malloc(len, 1)) == NULL) { *status = E_SCP_LS_NO_MEMORY; @@ -543,7 +543,7 @@ scp_get_list_sessions_response( /* Set up the string pointers in the block to point * into the memory allocated after the block */ p->username = (char *)p + sizeof(struct scp_session_info); - p->connection_description = + p->start_ip_addr = p->username + g_strlen(i_username) + 1; /* Copy the data over */ @@ -555,8 +555,7 @@ scp_get_list_sessions_response( p->bpp = i_bpp; p->start_time = i_start_time; g_strcpy(p->username, i_username); - g_strcpy(p->connection_description, - i_connection_description); + g_strcpy(p->start_ip_addr, i_start_ip_addr); } } } diff --git a/libipm/scp.h b/libipm/scp.h index e2d87c3ff0..1409806f3a 100644 --- a/libipm/scp.h +++ b/libipm/scp.h @@ -177,7 +177,7 @@ scp_msg_in_reset(struct trans *trans); * @param trans SCP transport * @param username Username * @param password Password - * @param connection_description Description of the connection + * @param ip_addr IP address for the client (or "" if not known) * @return != 0 for error * * Server replies with E_SCP_GATEWAY_RESPONSE @@ -186,7 +186,7 @@ int scp_send_gateway_request(struct trans *trans, const char *username, const char *password, - const char *connection_description); + const char *ip_addr); /** * Parse an incoming E_SCP_GATEWAY_REQUEST message (SCP server) @@ -194,14 +194,14 @@ scp_send_gateway_request(struct trans *trans, * @param trans SCP transport * @param[out] username Username * @param[out] password Password - * @param[out] connection_description Description of the connection + * @param[out] ip_addr IP address for the client. May be "" * @return != 0 for error */ int scp_get_gateway_request(struct trans *trans, const char **username, const char **password, - const char **connection_description); + const char **ip_addr); /** * Send an E_SCP_GATEWAY_RESPONSE (SCP server) @@ -239,7 +239,7 @@ scp_get_gateway_response(struct trans *trans, * @param bpp Session bits-per-pixel (ignored for Xorg sessions) * @param shell User program to run. May be "" * @param directory Directory to run the program in. May be "" - * @param connection_description Description of the connection + * @param ip_addr IP address for the client (or "" if not known) * @return != 0 for error * * Server replies with E_SCP_CREATE_SESSION_RESPONSE @@ -254,7 +254,7 @@ scp_send_create_session_request(struct trans *trans, unsigned char bpp, const char *shell, const char *directory, - const char *connection_description); + const char *ip_addr); /** @@ -269,7 +269,7 @@ scp_send_create_session_request(struct trans *trans, * @param[out] bpp Session bits-per-pixel (ignored for Xorg sessions) * @param[out] shell User program to run. May be "" * @param[out] directory Directory to run the program in. May be "" - * @param[out] connection_description Description of the connection + * @param[out] ip_addr IP address for the client. May be "" * @return != 0 for error * * Returned string pointers are valid until scp_msg_in_reset() is @@ -285,7 +285,7 @@ scp_get_create_session_request(struct trans *trans, unsigned char *bpp, const char **shell, const char **directory, - const char **connection_description); + const char **ip_addr); /** * Send an E_SCP_CREATE_SESSION_RESPONSE (SCP server) diff --git a/libipm/scp_application_types.h b/libipm/scp_application_types.h index 007e86fa8d..cd21440e3d 100644 --- a/libipm/scp_application_types.h +++ b/libipm/scp_application_types.h @@ -58,7 +58,7 @@ struct scp_session_info unsigned char bpp; ///< Session bits-per-pixel time_t start_time; ///< When sesion was created char *username; ///< Username for session - char *connection_description; ///< Initial connection to session + char *start_ip_addr; ///< IP address of starting client }; diff --git a/libxrdp/xrdp_rdp.c b/libxrdp/xrdp_rdp.c index d08f068a10..f9196d4b18 100644 --- a/libxrdp/xrdp_rdp.c +++ b/libxrdp/xrdp_rdp.c @@ -360,7 +360,6 @@ struct xrdp_rdp * xrdp_rdp_create(struct xrdp_session *session, struct trans *trans) { struct xrdp_rdp *self = (struct xrdp_rdp *)NULL; - int bytes; LOG_DEVEL(LOG_LEVEL_TRACE, "in xrdp_rdp_create"); self = (struct xrdp_rdp *)g_malloc(sizeof(struct xrdp_rdp), 1); @@ -378,10 +377,13 @@ xrdp_rdp_create(struct xrdp_session *session, struct trans *trans) self->client_info.cache3_entries = 262; self->client_info.cache3_size = 4096; /* load client ip info */ - bytes = sizeof(self->client_info.connection_description) - 1; - g_write_connection_description(trans->sck, - self->client_info.connection_description, - bytes); + g_sck_get_peer_ip_address(trans->sck, + self->client_info.client_ip, + sizeof(self->client_info.client_ip), + NULL); + g_sck_get_peer_description(trans->sck, + self->client_info.client_description, + sizeof(self->client_info.client_description)); self->mppc_enc = mppc_enc_new(PROTO_RDP_50); #if defined(XRDP_NEUTRINORDP) self->rfx_enc = rfx_context_new(); @@ -914,6 +916,7 @@ int xrdp_rdp_incoming(struct xrdp_rdp *self) { struct xrdp_iso *iso; + iso = self->sec_layer->mcs_layer->iso_layer; if (xrdp_sec_incoming(self->sec_layer) != 0) @@ -924,18 +927,13 @@ xrdp_rdp_incoming(struct xrdp_rdp *self) self->mcs_channel = self->sec_layer->mcs_layer->userid + MCS_USERCHANNEL_BASE; LOG_DEVEL(LOG_LEVEL_DEBUG, "xrdp_rdp->mcs_channel %d", self->mcs_channel); - g_strncpy(self->client_info.client_addr, iso->trans->addr, - sizeof(self->client_info.client_addr) - 1); - g_strncpy(self->client_info.client_port, iso->trans->port, - sizeof(self->client_info.client_port) - 1); /* log TLS version and cipher of TLS connections */ if (iso->selectedProtocol > PROTOCOL_RDP) { LOG(LOG_LEVEL_INFO, - "TLS connection established from %s port %s: %s with cipher %s", - self->client_info.client_addr, - self->client_info.client_port, + "TLS connection established from %s %s with cipher %s", + self->client_info.client_description, iso->trans->ssl_protocol, iso->trans->cipher_name); } @@ -952,11 +950,8 @@ xrdp_rdp_incoming(struct xrdp_rdp *self) /* default */ "unknown"; LOG(LOG_LEVEL_INFO, - "Non-TLS connection established from %s port %s: " - "with security level : %s", - self->client_info.client_addr, - self->client_info.client_port, - security_level); + "Non-TLS connection established from %s with security level : %s", + self->client_info.client_description, security_level); } return 0; diff --git a/neutrinordp/xrdp-neutrinordp.c b/neutrinordp/xrdp-neutrinordp.c index 5c741f2069..0e59da1fdd 100644 --- a/neutrinordp/xrdp-neutrinordp.c +++ b/neutrinordp/xrdp-neutrinordp.c @@ -268,10 +268,9 @@ lxrdp_connect(struct mod *mod) } #endif LOG(LOG_LEVEL_ERROR, "NeutrinoRDP proxy connection: status [Failed]," - " RDP client [%s:%s], RDP server [%s:%d], RDP server username [%s]," + " RDP client [%s], RDP server [%s:%d], RDP server username [%s]," " xrdp pamusername [%s], xrdp process id [%d]", - mod->client_info.client_addr, - mod->client_info.client_port, + mod->client_info.client_description, mod->inst->settings->hostname, mod->inst->settings->port, mod->inst->settings->username, @@ -282,10 +281,9 @@ lxrdp_connect(struct mod *mod) else { LOG(LOG_LEVEL_INFO, "NeutrinoRDP proxy connection: status [Success]," - " RDP client [%s:%s], RDP server [%s:%d], RDP server username [%s]," + " RDP client [%s], RDP server [%s:%d], RDP server username [%s]," " xrdp pamusername [%s], xrdp process id [%d]", - mod->client_info.client_addr, - mod->client_info.client_port, + mod->client_info.client_description, mod->inst->settings->hostname, mod->inst->settings->port, mod->inst->settings->username, @@ -531,10 +529,9 @@ lxrdp_end(struct mod *mod) LOG_DEVEL(LOG_LEVEL_DEBUG, "lxrdp_end:"); LOG(LOG_LEVEL_INFO, "NeutrinoRDP proxy connection: status [Disconnect]," - " RDP client [%s:%s], RDP server [%s:%d], RDP server username [%s]," + " RDP client [%s], RDP server [%s:%d], RDP server username [%s]," " xrdp pamusername [%s], xrdp process id [%d]", - mod->client_info.client_addr, - mod->client_info.client_port, + mod->client_info.client_description, mod->inst->settings->hostname, mod->inst->settings->port, mod->inst->settings->username, diff --git a/sesman/scp_process.c b/sesman/scp_process.c index f4850a3bd2..822da41aee 100644 --- a/sesman/scp_process.c +++ b/sesman/scp_process.c @@ -41,27 +41,19 @@ * Logs an authentication failure message * * @param username Username - * @param connection_description Connection details + * @param ip_addr IP address, if known * * The message is intended for use by fail2ban. Make changes with care. */ static void -log_authfail_message(const char *username, const char *connection_description) +log_authfail_message(const char *username, const char *ip_addr) { - char ip[64]; - const char *ipp; - if (connection_description != NULL && - connection_description[0] != '\0') + if (ip_addr == NULL || ip_addr[0] == '\0') { - g_get_ip_from_description(connection_description, ip, sizeof(ip)); - ipp = ip; - } - else - { - ipp = "unknown"; + ip_addr = "unknown"; } LOG(LOG_LEVEL_INFO, "AUTHFAIL: user=%s ip=%s time=%d", - username, ipp, g_time1()); + username, ip_addr, g_time1()); } /******************************************************************************/ @@ -72,10 +64,9 @@ process_gateway_request(struct trans *trans) int rv; const char *username; const char *password; - const char *connection_description; + const char *ip_addr; - rv = scp_get_gateway_request(trans, &username, &password, - &connection_description); + rv = scp_get_gateway_request(trans, &username, &password, &ip_addr); if (rv == 0) { int errorcode = 0; @@ -103,7 +94,7 @@ process_gateway_request(struct trans *trans) } else { - log_authfail_message(username, connection_description); + log_authfail_message(username, ip_addr); } rv = scp_send_gateway_response(trans, errorcode); auth_end(data); @@ -128,7 +119,7 @@ process_create_session_request(struct trans *trans) trans, &sp.username, &password, &sp.type, &sp.width, &sp.height, &sp.bpp, - &sp.shell, &sp.directory, &sp.connection_description); + &sp.shell, &sp.directory, &sp.ip_addr); if (rv == 0) { @@ -150,12 +141,11 @@ process_create_session_request(struct trans *trans) { display = s_item->display; guid = s_item->guid; - if (sp.connection_description[0] != '\0') + if (sp.ip_addr[0] != '\0') { LOG( LOG_LEVEL_INFO, "++ reconnected session: username %s, " "display :%d.0, session_pid %d, ip %s", - sp.username, display, s_item->pid, - sp.connection_description); + sp.username, display, s_item->pid, sp.ip_addr); } else { @@ -172,12 +162,11 @@ process_create_session_request(struct trans *trans) if (1 == access_login_allowed(sp.username)) { - if (sp.connection_description[0] != '\0') + if (sp.ip_addr[0] != '\0') { LOG(LOG_LEVEL_INFO, "++ created session (access granted): " - "username %s, ip %s", sp.username, - sp.connection_description); + "username %s, ip %s", sp.username, sp.ip_addr); } else { @@ -196,7 +185,7 @@ process_create_session_request(struct trans *trans) } else { - log_authfail_message(sp.username, sp.connection_description); + log_authfail_message(sp.username, sp.ip_addr); } if (do_auth_end) diff --git a/sesman/session.c b/sesman/session.c index 00594d1d0f..b59be93d13 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -90,46 +90,33 @@ session_get_bydata(const struct session_parameters *sp) { struct session_chain *tmp; enum SESMAN_CFG_SESS_POLICY policy = g_cfg->sess.policy; - char ip[64]; - char tmp_ip[64]; - - if ((policy & SESMAN_CFG_SESS_POLICY_I) != 0) - { - /* We'll need to compare on IP addresses */ - g_get_ip_from_description(sp->connection_description, - ip, sizeof(ip)); - } - else - { - ip[0] = '\0'; - tmp_ip[0] = '\0'; - } LOG(LOG_LEVEL_DEBUG, "session_get_bydata: search policy %d U %s W %d H %d bpp %d T %d IP %s", policy, sp->username, sp->width, sp->height, sp->bpp, - sp->type, sp->connection_description); + sp->type, sp->ip_addr); - for (tmp = g_sessions ; tmp != 0 ; tmp = tmp->next) + if ((policy & SESMAN_CFG_SESS_POLICY_C) != 0) { + /* Never matches */ + return NULL; + } - if ((policy & SESMAN_CFG_SESS_POLICY_I) != 0) - { - g_get_ip_from_description(tmp->item->connection_description, - tmp_ip, sizeof (tmp_ip)); - } + for (tmp = g_sessions ; tmp != 0 ; tmp = tmp->next) + { + struct session_item *item = tmp->item; LOG(LOG_LEVEL_DEBUG, "session_get_bydata: try %p U %s W %d H %d bpp %d T %d IP %s", - tmp->item, - tmp->item->name, - tmp->item->width, tmp->item->height, - tmp->item->bpp, tmp->item->type, - tmp->item->connection_description); - - if (g_strncmp(sp->username, tmp->item->name, 255) != 0 || - tmp->item->bpp != sp->bpp || - tmp->item->type != sp->type) + item, + item->name, + item->width, item->height, + item->bpp, item->type, + item->start_ip_addr); + + if (g_strncmp(sp->username, item->name, 255) != 0 || + item->bpp != sp->bpp || + item->type != sp->type) { LOG(LOG_LEVEL_DEBUG, "session_get_bydata: Basic parameters don't match"); @@ -137,29 +124,23 @@ session_get_bydata(const struct session_parameters *sp) } if ((policy & SESMAN_CFG_SESS_POLICY_D) && - (tmp->item->width != sp->width || tmp->item->height != sp->height)) + (item->width != sp->width || item->height != sp->height)) { LOG(LOG_LEVEL_DEBUG, "session_get_bydata: Dimensions don't match for 'D' policy"); continue; } - if ((policy & SESMAN_CFG_SESS_POLICY_I) && g_strcmp(ip, tmp_ip) != 0) + if ((policy & SESMAN_CFG_SESS_POLICY_I) && + g_strcmp(item->start_ip_addr, sp->ip_addr) != 0) { LOG(LOG_LEVEL_DEBUG, "session_get_bydata: IPs don't match for 'I' policy"); continue; } - if ((policy & SESMAN_CFG_SESS_POLICY_C) && - g_strncmp(sp->connection_description, tmp->item->connection_description, 255) != 0) - { - LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: connections don't match for 'C' policy"); - } - LOG(LOG_LEVEL_DEBUG, "session_get_bydata: Got match"); - return tmp->item; + return item; } return 0; @@ -959,14 +940,15 @@ session_start(long data, LOG(LOG_LEVEL_INFO, "Starting session: session_pid %d, " "display :%d.0, width %d, height %d, bpp %d, client ip %s, " "user name %s", - pid, display, s->width, s->height, s->bpp, s->connection_description, s->username); + pid, display, s->width, s->height, s->bpp, s->ip_addr, s->username); temp->item->pid = pid; temp->item->display = display; temp->item->width = s->width; temp->item->height = s->height; temp->item->bpp = s->bpp; temp->item->data = data; - g_strncpy(temp->item->connection_description, s->connection_description, 255); /* store client ip data */ + g_strncpy(temp->item->start_ip_addr, s->ip_addr, + sizeof(temp->item->start_ip_addr) - 1); g_strncpy(temp->item->name, s->username, 255); temp->item->guid = *guid; @@ -1071,7 +1053,7 @@ session_kill(int pid) /* deleting the session */ LOG(LOG_LEVEL_INFO, "++ terminated session: username %s, display :%d.0, session_pid %d, ip %s", - tmp->item->name, tmp->item->display, tmp->item->pid, tmp->item->connection_description); + tmp->item->name, tmp->item->display, tmp->item->pid, tmp->item->start_ip_addr); g_free(tmp->item); if (prev == 0) @@ -1227,11 +1209,10 @@ session_get_byuser(const char *user, unsigned int *cnt, unsigned char flags) (sess[index]).bpp = tmp->item->bpp; (sess[index]).start_time = tmp->item->start_time; (sess[index]).username = g_strdup(tmp->item->name); - (sess[index]).connection_description = - g_strdup(tmp->item->connection_description); + (sess[index]).start_ip_addr = g_strdup(tmp->item->start_ip_addr); if ((sess[index]).username == NULL || - (sess[index]).connection_description == NULL) + (sess[index]).start_ip_addr == NULL) { free_session_info_list(sess, *cnt); (*cnt) = 0; @@ -1259,7 +1240,7 @@ free_session_info_list(struct scp_session_info *sesslist, unsigned int cnt) for (i = 0 ; i < cnt ; ++i) { g_free(sesslist[i].username); - g_free(sesslist[i].connection_description); + g_free(sesslist[i].start_ip_addr); } } @@ -1374,7 +1355,7 @@ clone_session_params(const struct session_parameters *sp) len += g_strlen(sp->username) + 1; len += g_strlen(sp->shell) + 1; len += g_strlen(sp->directory) + 1; - len += g_strlen(sp->connection_description) + 1; + len += g_strlen(sp->ip_addr) + 1; if ((result = (struct session_parameters *)g_malloc(len, 0)) != NULL) { @@ -1394,8 +1375,7 @@ clone_session_params(const struct session_parameters *sp) COPY_STRING_MEMBER(sp->username, result->username); COPY_STRING_MEMBER(sp->shell, result->shell); COPY_STRING_MEMBER(sp->directory, result->directory); - COPY_STRING_MEMBER(sp->connection_description, - result->connection_description); + COPY_STRING_MEMBER(sp->ip_addr, result->ip_addr); #undef COPY_STRING_MEMBER } diff --git a/sesman/session.h b/sesman/session.h index d8a9fb82b5..7bcec9571c 100644 --- a/sesman/session.h +++ b/sesman/session.h @@ -32,6 +32,7 @@ #include "guid.h" #include "scp_application_types.h" +#include "xrdp_constants.h" #define SESMAN_SESSION_STATUS_ACTIVE 0x01 #define SESMAN_SESSION_STATUS_IDLE 0x02 @@ -68,7 +69,7 @@ struct session_item time_t start_time; // struct session_date disconnect_time; // Currently unused // struct session_date idle_time; // Currently unused - char connection_description[256]; + char start_ip_addr[MAX_PEER_ADDRSTRLEN]; struct guid guid; }; @@ -91,7 +92,7 @@ struct session_parameters const char *username; const char *shell; const char *directory; - const char *connection_description; + const char *ip_addr; }; /** diff --git a/sesman/tools/sesadmin.c b/sesman/tools/sesadmin.c index 4ef56a0214..ba1cadafd4 100644 --- a/sesman/tools/sesadmin.c +++ b/sesman/tools/sesadmin.c @@ -153,9 +153,9 @@ print_session(const struct scp_session_info *s) printf("\tScreen size: %dx%d, color depth %d\n", s->width, s->height, s->bpp); printf("\tStarted: %s", ctime(&s->start_time)); - if (s->connection_description[0] != '\0') + if (s->start_ip_addr[0] != '\0') { - printf("\tConnection Description: %s\n", s->connection_description); + printf("\tStart IP address: %s\n", s->start_ip_addr); } } diff --git a/tools/devel/tcp_proxy/main.c b/tools/devel/tcp_proxy/main.c index 13319f2938..66dd9f316b 100644 --- a/tools/devel/tcp_proxy/main.c +++ b/tools/devel/tcp_proxy/main.c @@ -112,7 +112,7 @@ main_loop(char *local_port, char *remote_ip, char *remote_port, int hexdump) { while ((!g_terminated) && (error == 0)) { - acc_sck = g_tcp_accept(lis_sck); + acc_sck = g_sck_accept(lis_sck); if ((acc_sck == -1) && g_tcp_last_error_would_block(lis_sck)) { diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 16497a89a7..22993d01cc 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -232,7 +232,7 @@ xrdp_mm_send_gateway_login(struct xrdp_mm *self, const char *username, return scp_send_gateway_request( self->sesman_trans, username, password, - self->wm->client_info->connection_description); + self->wm->client_info->client_ip); } /*****************************************************************************/ @@ -301,7 +301,7 @@ xrdp_mm_send_login(struct xrdp_mm *self) xserverbpp, self->wm->client_info->program, self->wm->client_info->directory, - self->wm->client_info->connection_description); + self->wm->client_info->client_ip); } } diff --git a/xrdp/xrdp_wm.c b/xrdp/xrdp_wm.c index caa1160a0c..6d5f1b8936 100644 --- a/xrdp/xrdp_wm.c +++ b/xrdp/xrdp_wm.c @@ -683,12 +683,11 @@ xrdp_wm_init(struct xrdp_wm *self) if (file_read_section(fd, section_name, names, values) != 0) { LOG(LOG_LEVEL_INFO, - "Module \"%s\" specified by %s from %s port %s " + "Module \"%s\" specified by %s from %s " "is not configured. Using \"%s\" instead.", section_name, self->session->client_info->username, - self->session->client_info->client_addr, - self->session->client_info->client_port, + self->session->client_info->client_description, default_section_name); list_clear(names); list_clear(values); diff --git a/xup/xup.c b/xup/xup.c index 4ca395b92d..cae2910089 100644 --- a/xup/xup.c +++ b/xup/xup.c @@ -73,10 +73,9 @@ lib_mod_log_peer(struct mod *mod) { LOG(LOG_LEVEL_INFO, "lib_mod_log_peer: xrdp_pid=%d connected " "to X11rdp_pid=%d X11rdp_uid=%d X11rdp_gid=%d " - "client_ip=%s client_port=%s", + "client=%s", my_pid, pid, uid, gid, - mod->client_info.client_addr, - mod->client_info.client_port); + mod->client_info.client_description); } else { From a4c6c36cf26f923c20547a9bdd8cf1f3d85f104d Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Tue, 12 Apr 2022 12:37:30 +0100 Subject: [PATCH 2/6] Add PAM_RHOST support Supplies the IP address that an authentication event is received from as the PAM parameter PAM_RHOST for PAM-capable systems. --- sesman/auth.h | 4 +++- sesman/scp_process.c | 6 +++--- sesman/verify_user.c | 3 ++- sesman/verify_user_bsd.c | 3 ++- sesman/verify_user_kerberos.c | 5 +++-- sesman/verify_user_pam.c | 16 ++++++++++++++-- sesman/verify_user_pam_userpass.c | 5 +++-- 7 files changed, 30 insertions(+), 12 deletions(-) diff --git a/sesman/auth.h b/sesman/auth.h index 56f780902e..3ae0ea3c78 100644 --- a/sesman/auth.h +++ b/sesman/auth.h @@ -32,11 +32,13 @@ * @brief Validates user's password * @param user user's login name * @param pass user's password + * @param client_ip IP address of connecting client (or ""/NULL if not known) * @return non-zero handle on success, 0 on failure * */ long -auth_userpass(const char *user, const char *pass, int *errorcode); +auth_userpass(const char *user, const char *pass, + const char *client_ip, int *errorcode); /** * diff --git a/sesman/scp_process.c b/sesman/scp_process.c index 822da41aee..7acf61b46b 100644 --- a/sesman/scp_process.c +++ b/sesman/scp_process.c @@ -75,7 +75,7 @@ process_gateway_request(struct trans *trans) LOG(LOG_LEVEL_INFO, "Received authentication request for user: %s", username); - data = auth_userpass(username, password, &errorcode); + data = auth_userpass(username, password, ip_addr, &errorcode); if (data) { if (1 == access_login_allowed(username)) @@ -133,7 +133,7 @@ process_create_session_request(struct trans *trans) SCP_SESSION_TYPE_TO_STR(sp.type), sp.username); - data = auth_userpass(sp.username, password, &errorcode); + data = auth_userpass(sp.username, password, sp.ip_addr, &errorcode); if (data) { s_item = session_get_bydata(&sp); @@ -219,7 +219,7 @@ process_list_sessions_request(struct trans *trans) LOG(LOG_LEVEL_INFO, "Received request to list sessions for user %s", username); - data = auth_userpass(username, password, &errorcode); + data = auth_userpass(username, password, NULL, &errorcode); if (data) { struct scp_session_info *info = NULL; diff --git a/sesman/verify_user.c b/sesman/verify_user.c index 2c72d38e4f..0e48a48938 100644 --- a/sesman/verify_user.c +++ b/sesman/verify_user.c @@ -51,7 +51,8 @@ auth_account_disabled(struct spwd *stp); /******************************************************************************/ /* returns boolean */ long -auth_userpass(const char *user, const char *pass, int *errorcode) +auth_userpass(const char *user, const char *pass, + const char *client_ip, int *errorcode) { const char *encr; const char *epass; diff --git a/sesman/verify_user_bsd.c b/sesman/verify_user_bsd.c index 0648965309..8a34511937 100644 --- a/sesman/verify_user_bsd.c +++ b/sesman/verify_user_bsd.c @@ -46,7 +46,8 @@ /******************************************************************************/ /* returns boolean */ long -auth_userpass(const char *user, const char *pass, int *errorcode) +auth_userpass(const char *user, const char *pass, + const char *client_ip, int *errorcode) { int ret = auth_userokay(user, NULL, "auth-xrdp", pass); return ret; diff --git a/sesman/verify_user_kerberos.c b/sesman/verify_user_kerberos.c index 6ddc6a8077..de7fbd89df 100644 --- a/sesman/verify_user_kerberos.c +++ b/sesman/verify_user_kerberos.c @@ -400,8 +400,9 @@ k5_kinit(struct k_opts *opts, struct k5_data *k5, struct user_info *u_info) /******************************************************************************/ /* returns boolean */ -int -auth_userpass(const char *user, const char *pass, int *errorcode) +long +auth_userpass(const char *user, const char *pass, + const char *client_ip, int *errorcode) { struct k_opts opts; struct k5_data k5; diff --git a/sesman/verify_user_pam.c b/sesman/verify_user_pam.c index 0d384eaf9c..47e04e2a24 100644 --- a/sesman/verify_user_pam.c +++ b/sesman/verify_user_pam.c @@ -32,6 +32,7 @@ #include "os_calls.h" #include "log.h" #include "string_calls.h" +#include "auth.h" #include #include @@ -212,7 +213,8 @@ get_service_name(char *service_name) Stores the detailed error code in the errorcode variable*/ long -auth_userpass(const char *user, const char *pass, int *errorcode) +auth_userpass(const char *user, const char *pass, + const char *client_ip, int *errorcode) { int error; struct t_auth_info *auth_info; @@ -239,10 +241,20 @@ auth_userpass(const char *user, const char *pass, int *errorcode) return 0; } + if (client_ip != NULL && client_ip[0] != '\0') + { + error = pam_set_item(auth_info->ph, PAM_RHOST, client_ip); + if (error != PAM_SUCCESS) + { + LOG(LOG_LEVEL_ERROR, "pam_set_item(PAM_RHOST) failed: %s", + pam_strerror(auth_info->ph, error)); + } + } + error = pam_set_item(auth_info->ph, PAM_TTY, service_name); if (error != PAM_SUCCESS) { - LOG(LOG_LEVEL_ERROR, "pam_set_item failed: %s", + LOG(LOG_LEVEL_ERROR, "pam_set_item(PAM_TTY) failed: %s", pam_strerror(auth_info->ph, error)); } diff --git a/sesman/verify_user_pam_userpass.c b/sesman/verify_user_pam_userpass.c index ac5981f434..a2db011d38 100644 --- a/sesman/verify_user_pam_userpass.c +++ b/sesman/verify_user_pam_userpass.c @@ -38,8 +38,9 @@ /******************************************************************************/ /* returns boolean */ -int -auth_userpass(const char *user, const char *pass, int *errorcode) +long +auth_userpass(const char *user, const char *pass, + const char *client_ip, int *errorcode) { pam_handle_t *pamh; pam_userpass_t userpass; From c1d2dcfc7ffacfc4e68e2ee9d2a3df1eaad64925 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Tue, 3 May 2022 09:52:37 +0100 Subject: [PATCH 3/6] Cosmetic fixes to string_calls --- common/string_calls.c | 2 ++ common/string_calls.h | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/common/string_calls.c b/common/string_calls.c index 18f5451297..7a0a481735 100644 --- a/common/string_calls.c +++ b/common/string_calls.c @@ -921,6 +921,7 @@ g_strnjoin(char *dest, int dest_len, const char *joiner, const char *src[], int return dest; } +/*****************************************************************************/ int g_bitmask_to_str(int bitmask, const struct bitmask_string bitdefs[], char delim, char *buff, int bufflen) @@ -987,6 +988,7 @@ g_bitmask_to_str(int bitmask, const struct bitmask_string bitdefs[], return rlen; } +/*****************************************************************************/ int g_str_to_bitmask(const char *str, const struct bitmask_string bitdefs[], const char *delim, char *unrecognised, int unrecognised_len) diff --git a/common/string_calls.h b/common/string_calls.h index 87888c18b2..f2f635f306 100644 --- a/common/string_calls.h +++ b/common/string_calls.h @@ -171,7 +171,7 @@ g_get_display_num_from_display(const char *display_text); * a hexadecimal constant. */ int -g_bitmask_to_str(int bitmask, const struct bitmask_string[], +g_bitmask_to_str(int bitmask, const struct bitmask_string bitdefs[], char delim, char *buff, int bufflen); /*** @@ -184,7 +184,7 @@ g_bitmask_to_str(int bitmask, const struct bitmask_string[], * @return bitmask value for recognised tokens */ int -g_str_to_bitmask(const char *str, const struct bitmask_string[], +g_str_to_bitmask(const char *str, const struct bitmask_string bitdefs[], const char *delim, char *unrecognised, int unrecognised_len); From 61bfb264dedf39acdf5e17a3935494434c47c948 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Tue, 3 May 2022 09:53:24 +0100 Subject: [PATCH 4/6] Add bitmask to character string conversions --- common/string_calls.c | 85 +++++++++++++++++++++++++++++++++++++++++++ common/string_calls.h | 64 +++++++++++++++++++++++++++++++- 2 files changed, 148 insertions(+), 1 deletion(-) diff --git a/common/string_calls.c b/common/string_calls.c index 7a0a481735..20e2895955 100644 --- a/common/string_calls.c +++ b/common/string_calls.c @@ -1057,3 +1057,88 @@ g_str_to_bitmask(const char *str, const struct bitmask_string bitdefs[], return mask; } +/*****************************************************************************/ +int +g_bitmask_to_charstr(int bitmask, const struct bitmask_char bitdefs[], + char *buff, int bufflen, int *rest) +{ + int rlen = 0; /* Returned length */ + + if (bufflen <= 0) /* Caller error */ + { + rlen = -1; + } + else + { + char *p = buff; + /* Find the last writeable character in the buffer */ + const char *last = buff + (bufflen - 1); + + const struct bitmask_char *b; + + for (b = &bitdefs[0] ; b->c != '\0'; ++b) + { + if ((bitmask & b->mask) != 0) + { + if (p < last) + { + *p++ = b->c; + } + ++rlen; + + /* Remove the bit so we don't report it back */ + bitmask &= ~b->mask; + } + } + *p = '\0'; + + if (rest != NULL) + { + *rest = bitmask; + } + } + + return rlen; +} + +/*****************************************************************************/ +int +g_charstr_to_bitmask(const char *str, const struct bitmask_char bitdefs[], + char *unrecognised, int unrecognised_len) +{ + int bitmask = 0; + const char *cp; + int j = 0; + + if (str != NULL && bitdefs != NULL) + { + for (cp = str ; *cp != '\0' ; ++cp) + { + const struct bitmask_char *b; + char c = toupper(*cp); + + for (b = &bitdefs[0] ; b->c != '\0'; ++b) + { + if (toupper(b->c) == c) + { + bitmask |= b->mask; + break; + } + } + if (b->c == '\0') + { + if (unrecognised != NULL && j < (unrecognised_len - 1)) + { + unrecognised[j++] = *cp; + } + } + } + } + + if (unrecognised != NULL && j < unrecognised_len) + { + unrecognised[j] = '\0'; + } + + return bitmask; +} diff --git a/common/string_calls.h b/common/string_calls.h index f2f635f306..29c066e366 100644 --- a/common/string_calls.h +++ b/common/string_calls.h @@ -52,6 +52,21 @@ struct bitmask_string #define BITMASK_STRING_END_OF_LIST { 0, NULL } +/** + * Map a bitmask to a char value + * + * + * This structure is used by g_bitmask_to_charstr() to specify the + * char for each bit in the bitmask + */ +struct bitmask_char +{ + int mask; + char c; +}; + +#define BITMASK_CHAR_END_OF_LIST { 0, '\0' } + /** * Processes a format string for general info * @@ -158,6 +173,9 @@ g_get_display_num_from_display(const char *display_text); /** * Converts a bitmask into a string for output purposes * + * Similar to g_bitmask_to_charstr(), but tokens are strings, separated + * by delimiters. + * * @param bitmask Bitmask to convert * @param bitdefs Definitions for strings for bits * @param delim Delimiter to use between strings @@ -176,8 +194,12 @@ g_bitmask_to_str(int bitmask, const struct bitmask_string bitdefs[], /*** * Converts a string containing a series of tokens to a bitmask. + * + * Similar to g_charstr_to_bitmask(), but tokens are strings, separated + * by delimiters. + * * @param str Input string - * @param bitmask_string Array mapping tokens to bitmask values + * @param bitdefs Array mapping tokens to bitmask values * @param delim Delimiter for tokens in str * @param[out] unrecognised Buffer for any unrecognised tokens * @param unrecognised_len Length of unrecognised including '\0'; @@ -188,6 +210,46 @@ g_str_to_bitmask(const char *str, const struct bitmask_string bitdefs[], const char *delim, char *unrecognised, int unrecognised_len); +/** + * Converts a bitmask into a string for output purposes + * + * Similar to g_bitmask_to_str(), but tokens are individual characters, and + * there are no delimiters. + * + * @param bitmask Bitmask to convert + * @param bitdefs Definitions for strings for bits + * @param buff Output buff + * @param bufflen Length of buff, including terminator '`\0' + * @param[out] rest Any unused bits which weren't covered by bitdefs. + * May be NULL. + * + * @return Total length excluding terminator which would be written, as + * in snprintf(). Can be used to check for overflow + * + * @note Any undefined bits in the bitmask are appended to the output as + * a hexadecimal constant. + */ +int +g_bitmask_to_charstr(int bitmask, const struct bitmask_char bitdefs[], + char *buff, int bufflen, int *rest); + +/*** + * Converts a string containing a series of characters to a bitmask. + * + * Similar to g_str_to_bitmask(), but tokens are individual characters, and + * there are no delimiters. + * + * @param str Input string + * @param bitdefs Array mapping tokens to bitmask values + * @param delim Delimiter for tokens in str + * @param[out] unrecognised Buffer for any unrecognised tokens + * @param unrecognised_len Length of unrecognised including '\0'; + * @return bitmask value for recognised tokens + */ +int +g_charstr_to_bitmask(const char *str, const struct bitmask_char bitdefs[], + char *unrecognised, int unrecognised_len); + int g_strlen(const char *text); char *g_strchr(const char *text, int c); char *g_strrchr(const char *text, int c); From a16695efd49538f734b7e88d69f9d6cea3783848 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 4 May 2022 11:28:11 +0100 Subject: [PATCH 5/6] Added unit tests for bitmask<->charstr calls --- tests/common/test_string_calls.c | 353 +++++++++++++++++++++++++++++++ 1 file changed, 353 insertions(+) diff --git a/tests/common/test_string_calls.c b/tests/common/test_string_calls.c index 1380357c50..6dc14a7349 100644 --- a/tests/common/test_string_calls.c +++ b/tests/common/test_string_calls.c @@ -649,6 +649,336 @@ START_TEST(test_str2bm__empty_token) } END_TEST +/******************************************************************************/ +START_TEST(test_bm2char__no_bits_defined) +{ + int rv; + char buff[64]; + int rest; + + static const struct bitmask_char bits[] = + { + BITMASK_CHAR_END_OF_LIST + }; + + rv = g_bitmask_to_charstr(0xffff, bits, buff, sizeof(buff), &rest); + + ck_assert_str_eq(buff, ""); + ck_assert_int_eq(rv, 0); + ck_assert_int_eq(rest, 0xffff); +} +END_TEST + +START_TEST(test_bm2char__all_bits_defined) +{ + int rv; + char buff[64]; + int rest; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + {1 << 1, 'B'}, + {1 << 6, 'C'}, + {1 << 7, 'D'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 1 << 0 | 1 << 1 | 1 << 6 | 1 << 7; + + rv = g_bitmask_to_charstr(bitmask, bits, buff, sizeof(buff), &rest); + + ck_assert_str_eq(buff, "ABCD"); + ck_assert_int_eq(rv, 4); + ck_assert_int_eq(rest, 0); +} +END_TEST + +START_TEST(test_bm2char__some_bits_undefined) +{ + int rv; + char buff[64]; + int rest; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + {1 << 1, 'B'}, + {1 << 6, 'C'}, + {1 << 7, 'D'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 1 << 0 | 1 << 1 | 1 << 16; + + rv = g_bitmask_to_charstr(bitmask, bits, buff, sizeof(buff), &rest); + + ck_assert_str_eq(buff, "AB"); + ck_assert_int_eq(rv, 2); + ck_assert_int_eq(rest, (1 << 16)); +} +END_TEST + +START_TEST(test_bm2char__overflow_all_bits_defined) +{ + int rv; + char buff[3]; + int rest; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + {1 << 1, 'B'}, + {1 << 6, 'C'}, + {1 << 7, 'D'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 1 << 0 | 1 << 1 | 1 << 6 | 1 << 7; + + rv = g_bitmask_to_charstr(bitmask, bits, buff, sizeof(buff), &rest); + + ck_assert_str_eq(buff, "AB"); + ck_assert_int_eq(rv, 4); + ck_assert_int_eq(rest, 0); +} +END_TEST + +START_TEST(test_bm2char__overflow_some_bits_undefined) +{ + int rv; + char buff[2]; + int rest; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + {1 << 1, 'B'}, + {1 << 6, 'C'}, + {1 << 7, 'D'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 1 << 0 | 1 << 1 | 1 << 16; + + rv = g_bitmask_to_charstr(bitmask, bits, buff, sizeof(buff), &rest); + + ck_assert_str_eq(buff, "A"); + ck_assert_int_eq(rv, 2); + ck_assert_int_eq(rest, (1 << 16)); +} +END_TEST + +START_TEST(test_bm2char__null_rest_param) +{ + int rv; + char buff[10]; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + {1 << 1, 'B'}, + {1 << 6, 'C'}, + {1 << 7, 'D'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 1 << 0 | 1 << 1 | 1 << 16; + + rv = g_bitmask_to_charstr(bitmask, bits, buff, sizeof(buff), NULL); + + ck_assert_str_eq(buff, "AB"); + ck_assert_int_eq(rv, 2); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_char2bm__null_string) +{ + int rv; + char buff[16] = { 'd', 'u', 'm', 'm', 'y' }; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + BITMASK_CHAR_END_OF_LIST + }; + + rv = g_charstr_to_bitmask(NULL, bits, buff, sizeof(buff)); + + ck_assert_str_eq(buff, ""); + ck_assert_int_eq(rv, 0); +} +END_TEST + +START_TEST(test_char2bm__empty_string) +{ + int rv; + char buff[16] = { 'd', 'u', 'm', 'm', 'y' }; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + BITMASK_CHAR_END_OF_LIST + }; + + rv = g_charstr_to_bitmask("", bits, buff, sizeof(buff)); + + ck_assert_str_eq(buff, ""); + ck_assert_int_eq(rv, 0); +} +END_TEST + +START_TEST(test_char2bm__null_bitdefs) +{ + int rv; + char buff[16] = { 'd', 'u', 'm', 'm', 'y' }; + + rv = g_charstr_to_bitmask("A", NULL, buff, sizeof(buff)); + + ck_assert_str_eq(buff, ""); + ck_assert_int_eq(rv, 0); +} +END_TEST + +START_TEST(test_char2bm__null_buffer) +{ + int rv; + char buff[16] = { 'd', 'u', 'm', 'm', 'y' }; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + BITMASK_CHAR_END_OF_LIST + }; + + rv = g_charstr_to_bitmask("B", bits, NULL, sizeof(buff)); + + ck_assert_str_eq(buff, "dummy"); + ck_assert_int_eq(rv, 0); +} +END_TEST + +START_TEST(test_char2bm__zero_buffer) +{ + int rv; + char buff[1]; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + BITMASK_CHAR_END_OF_LIST + }; + + rv = g_charstr_to_bitmask("B", bits, buff, 0); + + ck_assert_int_eq(rv, 0); +} +END_TEST + +START_TEST(test_char2bm__zero_mask) +{ + int rv; + char buff[16] = { 'd', 'u', 'm', 'm', 'y' }; + + static const struct bitmask_char bits[] = + { + {0, 'A'}, /* mask 0 should not be detected as end of list */ + {1 << 0, 'B'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 1 << 0; + rv = g_charstr_to_bitmask("B", bits, buff, sizeof(buff)); + + ck_assert_str_eq(buff, ""); + ck_assert_int_eq(rv, bitmask); +} +END_TEST + +START_TEST(test_char2bm__all_defined) +{ + int rv; + char buff[16] = { 'd', 'u', 'm', 'm', 'y' }; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + {1 << 1, 'B'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 1 << 0 | 1 << 1; + rv = g_charstr_to_bitmask("AB", bits, buff, sizeof(buff)); + + ck_assert_str_eq(buff, ""); + ck_assert_int_eq(rv, bitmask); +} +END_TEST + +START_TEST(test_char2bm__no_defined) +{ + int rv; + char buff[16] = { 'd', 'u', 'm', 'm', 'y' }; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + {1 << 1, 'B'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 0; + rv = g_charstr_to_bitmask("CD", bits, buff, sizeof(buff)); + + ck_assert_str_eq(buff, "CD"); + ck_assert_int_eq(rv, bitmask); +} +END_TEST + +START_TEST(test_char2bm__some_defined) +{ + int rv; + char buff[16] = { 'd', 'u', 'm', 'm', 'y' }; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + {1 << 1, 'B'}, + {1 << 2, 'C'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 1 << 1; + rv = g_charstr_to_bitmask("0B1", bits, buff, sizeof(buff)); + + ck_assert_str_eq(buff, "01"); + ck_assert_int_eq(rv, bitmask); +} +END_TEST + +START_TEST(test_char2bm__overflow_undefined) +{ + int rv; + char buff[16] = { 'd', 'u', 'm', 'm', 'y' }; + + static const struct bitmask_char bits[] = + { + {1 << 0, 'A'}, + {1 << 1, 'B'}, + {1 << 2, 'C'}, + BITMASK_CHAR_END_OF_LIST + }; + + int bitmask = 1 << 1; + rv = g_charstr_to_bitmask("123456789Bvwxyz", bits, buff, 10); + + /* vwxyz is not filled */ + ck_assert_str_eq(buff, "123456789"); + ck_assert_int_eq(rv, bitmask); +} +END_TEST + /******************************************************************************/ START_TEST(test_strtrim__trim_left) @@ -712,6 +1042,8 @@ make_suite_test_string(void) TCase *tc_strnjoin; TCase *tc_bm2str; TCase *tc_str2bm; + TCase *tc_bm2char; + TCase *tc_char2bm; TCase *tc_strtrim; s = suite_create("String"); @@ -757,6 +1089,27 @@ make_suite_test_string(void) tcase_add_test(tc_str2bm, test_str2bm__first_delim_is_semicolon); tcase_add_test(tc_str2bm, test_str2bm__empty_token); + tc_bm2char = tcase_create("bm2char"); + suite_add_tcase(s, tc_bm2char); + tcase_add_test(tc_bm2char, test_bm2char__no_bits_defined); + tcase_add_test(tc_bm2char, test_bm2char__all_bits_defined); + tcase_add_test(tc_bm2char, test_bm2char__some_bits_undefined); + tcase_add_test(tc_bm2char, test_bm2char__overflow_all_bits_defined); + tcase_add_test(tc_bm2char, test_bm2char__overflow_some_bits_undefined); + tcase_add_test(tc_bm2char, test_bm2char__null_rest_param); + tc_char2bm = tcase_create("char2bm"); + suite_add_tcase(s, tc_char2bm); + tcase_add_test(tc_char2bm, test_char2bm__null_string); + tcase_add_test(tc_char2bm, test_char2bm__empty_string); + tcase_add_test(tc_char2bm, test_char2bm__null_bitdefs); + tcase_add_test(tc_char2bm, test_char2bm__null_buffer); + tcase_add_test(tc_char2bm, test_char2bm__zero_buffer); + tcase_add_test(tc_char2bm, test_char2bm__zero_mask); + tcase_add_test(tc_char2bm, test_char2bm__all_defined); + tcase_add_test(tc_char2bm, test_char2bm__no_defined); + tcase_add_test(tc_char2bm, test_char2bm__some_defined); + tcase_add_test(tc_char2bm, test_char2bm__overflow_undefined); + tc_strtrim = tcase_create("strtrim"); suite_add_tcase(s, tc_strtrim); tcase_add_test(tc_strtrim, test_strtrim__trim_left); From 3e488773d7d59e9aa94a0940130a032e631d53cd Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 2 May 2022 12:54:40 +0100 Subject: [PATCH 6/6] Updated session allocation policy for sesman Made session allocation policies more readable and maintainable. The 'C' policy which was confusing before has been replaced with the 'Separate' keyword. This is a public interface change, but is unlikely to affect many users. The logging in session_get_bydata() is substantially improved, making it far easier to spot why sessions are getting matched or not matched. --- docs/man/sesman.ini.5.in | 38 ++++++----- sesman/config.c | 138 +++++++++++++++++++++++++++------------ sesman/config.h | 41 ++++++------ sesman/sesman.ini.in | 21 ++++-- sesman/session.c | 62 +++++++++++++----- 5 files changed, 198 insertions(+), 102 deletions(-) diff --git a/docs/man/sesman.ini.5.in b/docs/man/sesman.ini.5.in index af3be2a726..366ecfadc6 100644 --- a/docs/man/sesman.ini.5.in +++ b/docs/man/sesman.ini.5.in @@ -176,30 +176,34 @@ If set to \fI0\fR, idle sessions will never be disconnected by timeout. This works only with xorgxrdp sessions. Moreover, xorgxrdp must be v0.2.9 or later. .TP -\fBPolicy\fR=\fI[Default|UBD|UBI|UBC|UBDI|UBDC]\fR +\fBPolicy\fR=\fI[Default|Separate|{UBDI}]\fR Session allocation policy. Used to decide when to allocate a new session. Set to one of the following values: .br -.br -\fBDefault\fR - session per -.br -\fBUBD\fR - session per -.br -\fBUBI\fR - session per -.br -\fBUBC\fR - session per -.br -\fBUBDI\fR - session per -.br -\fBUBDC\fR - session per -.br +.RS +.HP 12 +\fBDefault\fR - Currently the same as "UB" for all session types +.HP 12 +\fBSeparate\fR - All sessions are separate. Sessions can never be rejoined, +and will need to be cleaned up manually, or automatically by setting other +sesman options. +.P +Alternatively combine one-or-more of the following options +.HP 4 +\fBU\fR - Sessions are separated per user +.HP 4 +\fBB\fR - Sessions are separated by bits-per-pixel +.HP 4 +\fBD\fR - Sessions are separated by initial display size +.HP 4 +\fBI\fR - Sessions are separated by IP address +.RE -.br -Note that the \fBUser\fR and \fBBitPerPixel\fR criteria cannot be turned +.IP +Note that the \fBU\fR and \fBB\fR criteria cannot be turned off. \fBDisplaySize\fR refers to the initial geometry of a connection, as actual display sizes can change dynamically. -.br .SH "SECURITY" Following parameters can be used in the \fB[Security]\fR section. diff --git a/sesman/config.c b/sesman/config.c index 4eabdd4687..23e7ff3715 100644 --- a/sesman/config.c +++ b/sesman/config.c @@ -37,6 +37,81 @@ #include "chansrv/chansrv_common.h" #include "scp.h" +static const struct bitmask_char policy_bits[] = +{ + { SESMAN_CFG_SESS_POLICY_U, 'U' }, + { SESMAN_CFG_SESS_POLICY_B, 'B' }, + { SESMAN_CFG_SESS_POLICY_D, 'D' }, + { SESMAN_CFG_SESS_POLICY_I, 'I' }, + BITMASK_CHAR_END_OF_LIST +}; + +/***************************************************************************//** + * Parse a session allocation policy string + */ +static unsigned int +parse_policy_string(const char *value) +{ + unsigned int rv; + char unrecognised[16]; + + if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_DFLT_S)) + { + rv = SESMAN_CFG_SESS_POLICY_DEFAULT; + } + else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_SEP_S)) + { + rv = SESMAN_CFG_SESS_POLICY_SEPARATE; + } + else + { + unrecognised[0] = '\0'; + rv = g_charstr_to_bitmask(value, policy_bits, unrecognised, + sizeof(unrecognised)); + if (unrecognised[0] != '\0') + { + LOG(LOG_LEVEL_WARNING, "Character(s) '%s' in the session" + " allocation policy are not recognised", unrecognised); + + if (g_strchr(unrecognised, 'C') != NULL || + g_strchr(unrecognised, 'c') != NULL) + { + /* Change from xrdp v0.9.x */ + LOG(LOG_LEVEL_WARNING, "Character 'C' is no longer used" + " in session allocation policies - use '%s'", + SESMAN_CFG_SESS_POLICY_SEP_S); + } + } + } + + return rv; +} + +/******************************************************************************/ +int +config_output_policy_string(unsigned int value, + char *buff, unsigned int bufflen) +{ + int rv = 0; + if (bufflen > 0) + { + if (value & SESMAN_CFG_SESS_POLICY_DEFAULT) + { + rv = g_snprintf(buff, bufflen, "Default"); + } + else if (value & SESMAN_CFG_SESS_POLICY_SEPARATE) + { + rv = g_snprintf(buff, bufflen, "Separate"); + } + else + { + rv = g_bitmask_to_charstr(value, policy_bits, buff, bufflen, NULL); + } + } + + return rv; +} + /***************************************************************************//** * * @brief Reads sesman [global] configuration section @@ -295,7 +370,8 @@ config_read_sessions(int file, struct config_sessions *se, struct list *param_n, struct list *param_v) { int i; - char *buf; + const char *buf; + const char *value; list_clear(param_v); list_clear(param_n); @@ -306,70 +382,43 @@ config_read_sessions(int file, struct config_sessions *se, struct list *param_n, se->max_idle_time = 0; se->max_disc_time = 0; se->kill_disconnected = 0; - se->policy = SESMAN_CFG_SESS_POLICY_DFLT; + se->policy = SESMAN_CFG_SESS_POLICY_DEFAULT; file_read_section(file, SESMAN_CFG_SESSIONS, param_n, param_v); for (i = 0; i < param_n->count; i++) { - buf = (char *)list_get_item(param_n, i); + buf = (const char *)list_get_item(param_n, i); + value = (const char *)list_get_item(param_v, i); if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_X11DISPLAYOFFSET)) { - se->x11_display_offset = g_atoi((char *)list_get_item(param_v, i)); + se->x11_display_offset = g_atoi(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_MAX)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_MAX)) { - se->max_sessions = g_atoi((char *)list_get_item(param_v, i)); + se->max_sessions = g_atoi(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_KILL_DISC)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_KILL_DISC)) { - se->kill_disconnected = g_text2bool((char *)list_get_item(param_v, i)); + se->kill_disconnected = g_text2bool(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_IDLE_LIMIT)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_IDLE_LIMIT)) { - se->max_idle_time = g_atoi((char *)list_get_item(param_v, i)); + se->max_idle_time = g_atoi(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_DISC_LIMIT)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_DISC_LIMIT)) { - se->max_disc_time = g_atoi((char *)list_get_item(param_v, i)); + se->max_disc_time = g_atoi(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_POLICY_S)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_POLICY_S)) { - char *value = (char *)list_get_item(param_v, i); - if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_DFLT_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_DFLT; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBD_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBD; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBI_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBI; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBC_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBC; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBDI_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBDI; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBDC_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBDC; - } - else /* silently ignore typos */ - { - se->policy = SESMAN_CFG_SESS_POLICY_DFLT; - } + se->policy = parse_policy_string(value); } } @@ -570,6 +619,7 @@ config_dump(struct config_sesman *config) struct config_security *sc; se = &(config->sess); sc = &(config->sec); + char policy_s[64]; /* Global sesman configuration */ g_writeln("Filename: %s", config->sesman_ini); @@ -583,13 +633,15 @@ config_dump(struct config_sesman *config) (config->auth_file_path ? config->auth_file_path : "disabled")); /* Session configuration */ + config_output_policy_string(se->policy, policy_s, sizeof(policy_s)); + g_writeln("Session configuration:"); g_writeln(" MaxSessions: %d", se->max_sessions); g_writeln(" X11DisplayOffset: %d", se->x11_display_offset); g_writeln(" KillDisconnected: %d", se->kill_disconnected); g_writeln(" IdleTimeLimit: %d", se->max_idle_time); g_writeln(" DisconnectedTimeLimit: %d", se->max_disc_time); - g_writeln(" Policy: %d", se->policy); + g_writeln(" Policy: %s", policy_s); /* Security configuration */ g_writeln("Security configuration:"); diff --git a/sesman/config.h b/sesman/config.h index b73ff46c7a..87698c5520 100644 --- a/sesman/config.h +++ b/sesman/config.h @@ -72,27 +72,18 @@ #define SESMAN_CFG_SESS_POLICY_S "Policy" #define SESMAN_CFG_SESS_POLICY_DFLT_S "Default" -#define SESMAN_CFG_SESS_POLICY_UBD_S "UBD" -#define SESMAN_CFG_SESS_POLICY_UBI_S "UBI" -#define SESMAN_CFG_SESS_POLICY_UBC_S "UBC" -#define SESMAN_CFG_SESS_POLICY_UBDI_S "UBDI" -#define SESMAN_CFG_SESS_POLICY_UBDC_S "UBDC" +#define SESMAN_CFG_SESS_POLICY_SEP_S "Separate" enum SESMAN_CFG_SESS_POLICY_BITS { - SESMAN_CFG_SESS_POLICY_D = 0x01, - SESMAN_CFG_SESS_POLICY_I = 0x02, - SESMAN_CFG_SESS_POLICY_C = 0x04 -}; - -enum SESMAN_CFG_SESS_POLICY -{ - SESMAN_CFG_SESS_POLICY_DFLT = 0, - SESMAN_CFG_SESS_POLICY_UBD = SESMAN_CFG_SESS_POLICY_D, - SESMAN_CFG_SESS_POLICY_UBI = SESMAN_CFG_SESS_POLICY_I, - SESMAN_CFG_SESS_POLICY_UBC = SESMAN_CFG_SESS_POLICY_C, - SESMAN_CFG_SESS_POLICY_UBDI = SESMAN_CFG_SESS_POLICY_D | SESMAN_CFG_SESS_POLICY_I, - SESMAN_CFG_SESS_POLICY_UBDC = SESMAN_CFG_SESS_POLICY_D | SESMAN_CFG_SESS_POLICY_C + /* If these two are set, they override everything else */ + SESMAN_CFG_SESS_POLICY_DEFAULT = (1 << 0), + SESMAN_CFG_SESS_POLICY_SEPARATE = (1 << 1), + /* Configuration bits */ + SESMAN_CFG_SESS_POLICY_U = (1 << 2), + SESMAN_CFG_SESS_POLICY_B = (1 << 3), + SESMAN_CFG_SESS_POLICY_D = (1 << 4), + SESMAN_CFG_SESS_POLICY_I = (1 << 5) }; /** @@ -180,7 +171,7 @@ struct config_sessions * @var policy * @brief session allocation policy */ - enum SESMAN_CFG_SESS_POLICY policy; + unsigned int policy; }; /** @@ -304,4 +295,16 @@ config_dump(struct config_sesman *config); void config_free(struct config_sesman *cs); +/** + * Converts a session allocation Policy value to a strin + * @param value - Session allocation policy value + * @param buff - Buffer for result + * @param bufflen - Length of buffer + * @return Length of string that would be required without a terminator + * to write the whole output (like snprintf()) + */ +int +config_output_policy_string(unsigned int value, + char *buff, unsigned int bufflen); + #endif diff --git a/sesman/sesman.ini.in b/sesman/sesman.ini.in index 15ef50bad2..278224a19d 100644 --- a/sesman/sesman.ini.in +++ b/sesman/sesman.ini.in @@ -69,13 +69,20 @@ DisconnectedTimeLimit=0 IdleTimeLimit=0 ;; Policy - session allocation policy -; Type: enum [ "Default" | "UBD" | "UBI" | "UBC" | "UBDI" | "UBDC" ] -; "Default" session per -; "UBD" session per -; "UBI" session per -; "UBC" session per -; "UBDI" session per -; "UBDC" session per +; +; Type: enum [ "Default" | "Separate" | Combination from {UBDI} ] +; "Default" Currently same as "UB" +; "Separate" All sessions are separate. Sessions can never be rejoined, +; and will need to be cleaned up manually, or automatically +; by setting other sesman options. +; +; Combination options:- +; U Sessions are separated per user +; B Sessions are separated by bits-per-pixel +; D Sessions are separated by initial display size +; I Sessions are separated by IP address +; +; The options U and B are always active, and cannot be de-selected. Policy=Default [Logging] diff --git a/sesman/session.c b/sesman/session.c index b59be93d13..f95a5e234c 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -88,17 +88,31 @@ dumpItemsToString(struct list *self, char *outstr, int len) struct session_item * session_get_bydata(const struct session_parameters *sp) { + char policy_str[64]; struct session_chain *tmp; - enum SESMAN_CFG_SESS_POLICY policy = g_cfg->sess.policy; + int policy = g_cfg->sess.policy; - LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: search policy %d U %s W %d H %d bpp %d T %d IP %s", - policy, sp->username, sp->width, sp->height, sp->bpp, - sp->type, sp->ip_addr); + if ((policy & SESMAN_CFG_SESS_POLICY_DEFAULT) != 0) + { + /* In the past (i.e. xrdp before v0.9.14), the default + * session policy varied by sp->type. If this is needed again + * in the future, here is the place to add it */ + policy = SESMAN_CFG_SESS_POLICY_U | SESMAN_CFG_SESS_POLICY_B; + } + + config_output_policy_string(policy, policy_str, sizeof(policy_str)); - if ((policy & SESMAN_CFG_SESS_POLICY_C) != 0) + LOG(LOG_LEVEL_DEBUG, + "%s: search policy=%s type=%s U=%s B=%d D=(%dx%d) I=%s", + __func__, + policy_str, SCP_SESSION_TYPE_TO_STR(sp->type), + sp->username, sp->bpp, sp->width, sp->height, + sp->ip_addr); + + /* 'Separate' policy never matches */ + if (policy & SESMAN_CFG_SESS_POLICY_SEPARATE) { - /* Never matches */ + LOG(LOG_LEVEL_DEBUG, "%s: No matches possible", __func__); return NULL; } @@ -107,19 +121,33 @@ session_get_bydata(const struct session_parameters *sp) struct session_item *item = tmp->item; LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: try %p U %s W %d H %d bpp %d T %d IP %s", + "%s: try %p type=%s U=%s B=%d D=(%dx%d) I=%s", + __func__, item, + SCP_SESSION_TYPE_TO_STR(item->type), item->name, + item->bpp, item->width, item->height, - item->bpp, item->type, item->start_ip_addr); - if (g_strncmp(sp->username, item->name, 255) != 0 || - item->bpp != sp->bpp || - item->type != sp->type) + if (item->type != sp->type) + { + LOG(LOG_LEVEL_DEBUG, "%s: Type doesn't match", __func__); + continue; + } + + if ((policy & SESMAN_CFG_SESS_POLICY_U) && + g_strncmp(sp->username, item->name, sizeof(item->name) - 1) != 0) { LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: Basic parameters don't match"); + "%s: Username doesn't match for 'U' policy", __func__); + continue; + } + + if ((policy & SESMAN_CFG_SESS_POLICY_B) && item->bpp != sp->bpp) + { + LOG(LOG_LEVEL_DEBUG, + "%s: bpp doesn't match for 'B' policy", __func__); continue; } @@ -127,7 +155,7 @@ session_get_bydata(const struct session_parameters *sp) (item->width != sp->width || item->height != sp->height)) { LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: Dimensions don't match for 'D' policy"); + "%s: Dimensions don't match for 'D' policy", __func__); continue; } @@ -135,14 +163,16 @@ session_get_bydata(const struct session_parameters *sp) g_strcmp(item->start_ip_addr, sp->ip_addr) != 0) { LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: IPs don't match for 'I' policy"); + "%s: IPs don't match for 'I' policy", __func__); continue; } - LOG(LOG_LEVEL_DEBUG, "session_get_bydata: Got match"); + LOG(LOG_LEVEL_DEBUG, + "%s: Got match, display=%d", __func__, item->display); return item; } + LOG(LOG_LEVEL_DEBUG, "%s: No matches found", __func__); return 0; }