From f0bae0050c15c837ac2ce6e9548a637e33528395 Mon Sep 17 00:00:00 2001 From: gpotter2 <10530980+gpotter2@users.noreply.github.com> Date: Tue, 6 May 2025 19:58:34 +0200 Subject: [PATCH 1/5] vmconnect mode: support all security modes when used in Hyper-V environment --- common/ms-rdpbcgr.h | 7 ++++- common/xrdp_client_info.h | 2 ++ libxrdp/xrdp_iso.c | 58 ++++++++++++++++++++++++++++++++++++--- libxrdp/xrdp_rdp.c | 4 +++ libxrdp/xrdp_sec.c | 6 +++- xrdp/xrdp.ini.in | 4 +++ 6 files changed, 75 insertions(+), 6 deletions(-) diff --git a/common/ms-rdpbcgr.h b/common/ms-rdpbcgr.h index 5e85a5a171..38c5393fca 100644 --- a/common/ms-rdpbcgr.h +++ b/common/ms-rdpbcgr.h @@ -35,7 +35,12 @@ #define PROTOCOL_RDSTLS 0x00000004 #define PROTOCOL_HYBRID_EX 0x00000008 -/* Negotiation packet flags (2.2.1.2.1) */ +/* Negotiation request packet flags (2.2.1.1.1) */ +#define RESTRICTED_ADMIN_MODE_REQUIRED 0x00000001 +#define REDIRECTED_AUTHENTICATION_MODE_REQUIRED 0x00000002 +#define CORRELATION_INFO_PRESENT 0x00000008 + +/* Negotiation response packet flags (2.2.1.2.1) */ #define EXTENDED_CLIENT_DATA_SUPPORTED 0x01 #define DYNVC_GFX_PROTOCOL_SUPPORTED 0x02 #define NEGRSP_RESERVED 0x04 diff --git a/common/xrdp_client_info.h b/common/xrdp_client_info.h index fbb88af787..71ffca7db0 100644 --- a/common/xrdp_client_info.h +++ b/common/xrdp_client_info.h @@ -181,6 +181,8 @@ struct xrdp_client_info int require_credentials; /* when true, credentials *must* be passed on cmd line */ int security_layer; /* SECURITY_LAYER_* */ + int vmconnect; /* Used when used from inside Hyper-V */ + int multimon; /* 0 = deny , 1 = allow */ struct display_size_description display_sizes; diff --git a/libxrdp/xrdp_iso.c b/libxrdp/xrdp_iso.c index 713422afb8..f75c3fb7b2 100644 --- a/libxrdp/xrdp_iso.c +++ b/libxrdp/xrdp_iso.c @@ -116,6 +116,11 @@ xrdp_iso_negotiate_security(struct xrdp_iso *self) { security_type_mask = PROTOCOL_SSL; } + /* But VMConnect mode supports everything. */ + if (client_info->vmconnect) + { + security_type_mask |= PROTOCOL_HYBRID | PROTOCOL_HYBRID_EX; + } /* Logically 'and' this value with the mask requested by the client, and * see what's left */ @@ -124,8 +129,36 @@ xrdp_iso_negotiate_security(struct xrdp_iso *self) protostr); security_type_mask &= self->requestedProtocol; + /* In VMConnect mode, we support everything. */ + if (client_info->vmconnect && (self->requestedProtocol > PROTOCOL_RDP)) + { + if (security_type_mask & PROTOCOL_HYBRID_EX) + { + LOG(LOG_LEVEL_INFO, "Selected HYBRID_EX security"); + self->selectedProtocol = PROTOCOL_HYBRID_EX; + got_protocol = 1; + } + else if (security_type_mask & PROTOCOL_HYBRID) + { + LOG(LOG_LEVEL_INFO, "Selected HYBRID security"); + self->selectedProtocol = PROTOCOL_HYBRID; + got_protocol = 1; + } + else if (security_type_mask & PROTOCOL_SSL) + { + LOG(LOG_LEVEL_INFO, "Selected TLS security"); + self->selectedProtocol = PROTOCOL_SSL; + got_protocol = 1; + } + else + { + /* Impossible */ + LOG(LOG_LEVEL_ERROR, "Impossible case."); + rv = 1; + } + } /* Is there a match on SSL/TLS? */ - if ((security_type_mask & PROTOCOL_SSL) != 0) + else if ((security_type_mask & PROTOCOL_SSL) != 0) { /* Can we do TLS? (basic check) */ if (g_file_readable(client_info->certificate) && @@ -205,13 +238,20 @@ xrdp_iso_process_rdp_neg_req(struct xrdp_iso *self, struct stream *s) /* The type field has already been read to determine that this function should be called */ in_uint8(s, flags); /* flags */ - if (flags != 0x0 && flags != 0x8 && flags != 0x1) + if ((flags & 0x0000000b) != flags) { LOG(LOG_LEVEL_ERROR, "Unsupported [MS-RDPBCGR] RDP_NEG_REQ flags: 0x%2.2x", flags); return 1; } + /* If both flags are set, it means 'OR', so fail only if only the one is set. */ + if ((flags & REDIRECTED_AUTHENTICATION_MODE_REQUIRED) && !(flags & RESTRICTED_ADMIN_MODE_REQUIRED)) + { + LOG(LOG_LEVEL_ERROR, "[MS-RDPBCGR] RDP_NEG_REQ: RemoteGuard isn't supported !"); + return 1; + } + in_uint16_le(s, len); /* length */ if (len != 8) { @@ -374,9 +414,12 @@ xrdp_iso_send_cc(struct xrdp_iso *self) char *holdp; char *len_ptr; char *len_indicator_ptr; + char flags; int len; int len_indicator; + struct xrdp_client_info *client_info = &(self->mcs_layer->sec_layer->rdp_layer->client_info); + make_stream(s); init_stream(s, 8192); @@ -410,10 +453,17 @@ xrdp_iso_send_cc(struct xrdp_iso *self) } else { + flags = EXTENDED_CLIENT_DATA_SUPPORTED; + + if (client_info->vmconnect) + { + /* NLA is handled by the host and not us. */ + flags |= RESTRICTED_ADMIN_MODE_SUPPORTED; + } + /* [MS-RDPBCGR] RDP_NEG_RSP */ out_uint8(s, RDP_NEG_RSP); /* type*/ - //TODO: hardcoded flags - out_uint8(s, EXTENDED_CLIENT_DATA_SUPPORTED); /* flags */ + out_uint8(s, flags); /* flags */ out_uint16_le(s, 8); /* length (must be 8) */ out_uint32_le(s, self->selectedProtocol); /* selectedProtocol */ LOG_DEVEL(LOG_LEVEL_TRACE, "Adding structure [MS-RDPBCGR] RDP_NEG_RSP " diff --git a/libxrdp/xrdp_rdp.c b/libxrdp/xrdp_rdp.c index c8efca378c..bc18a9ffb8 100644 --- a/libxrdp/xrdp_rdp.c +++ b/libxrdp/xrdp_rdp.c @@ -211,6 +211,10 @@ xrdp_rdp_read_config(const char *xrdp_ini, struct xrdp_client_info *client_info) client_info->security_layer = SECURITY_LAYER_NEGOTIATE; } } + else if (g_strcasecmp(item, "vmconnect") == 0) + { + client_info->vmconnect = g_text2bool(value); + } else if (g_strcasecmp(item, "certificate") == 0) { g_memset(client_info->certificate, 0, sizeof(char) * 1024); diff --git a/libxrdp/xrdp_sec.c b/libxrdp/xrdp_sec.c index f060b8dfee..940d3feb59 100644 --- a/libxrdp/xrdp_sec.c +++ b/libxrdp/xrdp_sec.c @@ -2252,7 +2252,11 @@ xrdp_sec_incoming(struct xrdp_sec *self) } /* initialize selected security layer */ - if (iso->selectedProtocol > PROTOCOL_RDP) + if (self->rdp_layer->client_info.vmconnect && iso->selectedProtocol > PROTOCOL_RDP) + { + /* Security handled by host: do nothing. */ + } + else if (iso->selectedProtocol > PROTOCOL_RDP) { /* init tls security */ diff --git a/xrdp/xrdp.ini.in b/xrdp/xrdp.ini.in index d2bc713366..566ca9d050 100644 --- a/xrdp/xrdp.ini.in +++ b/xrdp/xrdp.ini.in @@ -27,6 +27,10 @@ port=3389 ; prefer use vsock://: above use_vsock=false +; if used inside a Hyper-V VM with vmconnect, turn this on to enable +; wider protocol support. +#vmconnect=true + ; Unprivileged User name and group to run the xrdp daemon. ; It is HIGHLY RECOMMENDED you set these values. See the xrdp.ini(5) ; manpage for more information on setting and checking these. From 09e117325915585296d9ce1f6fc557a9960fb63a Mon Sep 17 00:00:00 2001 From: gpotter2 <10530980+gpotter2@users.noreply.github.com> Date: Thu, 8 May 2025 06:45:05 +0200 Subject: [PATCH 2/5] Apply suggestions Co-Authored-By: matt335672 <30179339+matt335672@users.noreply.github.com> --- libxrdp/xrdp_iso.c | 45 +++++++++++++++------------------------------ xrdp/xrdp.ini.in | 4 ++-- 2 files changed, 17 insertions(+), 32 deletions(-) diff --git a/libxrdp/xrdp_iso.c b/libxrdp/xrdp_iso.c index f75c3fb7b2..5e2cae89b4 100644 --- a/libxrdp/xrdp_iso.c +++ b/libxrdp/xrdp_iso.c @@ -129,40 +129,25 @@ xrdp_iso_negotiate_security(struct xrdp_iso *self) protostr); security_type_mask &= self->requestedProtocol; - /* In VMConnect mode, we support everything. */ - if (client_info->vmconnect && (self->requestedProtocol > PROTOCOL_RDP)) + if (security_type_mask & PROTOCOL_HYBRID_EX) { - if (security_type_mask & PROTOCOL_HYBRID_EX) - { - LOG(LOG_LEVEL_INFO, "Selected HYBRID_EX security"); - self->selectedProtocol = PROTOCOL_HYBRID_EX; - got_protocol = 1; - } - else if (security_type_mask & PROTOCOL_HYBRID) - { - LOG(LOG_LEVEL_INFO, "Selected HYBRID security"); - self->selectedProtocol = PROTOCOL_HYBRID; - got_protocol = 1; - } - else if (security_type_mask & PROTOCOL_SSL) - { - LOG(LOG_LEVEL_INFO, "Selected TLS security"); - self->selectedProtocol = PROTOCOL_SSL; - got_protocol = 1; - } - else - { - /* Impossible */ - LOG(LOG_LEVEL_ERROR, "Impossible case."); - rv = 1; - } + /* Currently supported by VMConnect mode only */ + LOG(LOG_LEVEL_INFO, "Selected HYBRID_EX security"); + self->selectedProtocol = PROTOCOL_HYBRID_EX; + got_protocol = 1; + } + else if (security_type_mask & PROTOCOL_HYBRID) + { + /* Currently supported by VMConnect mode only */ + LOG(LOG_LEVEL_INFO, "Selected HYBRID security"); + self->selectedProtocol = PROTOCOL_HYBRID; + got_protocol = 1; } - /* Is there a match on SSL/TLS? */ else if ((security_type_mask & PROTOCOL_SSL) != 0) { - /* Can we do TLS? (basic check) */ - if (g_file_readable(client_info->certificate) && - g_file_readable(client_info->key_file)) + /* Can we do TLS? (basic check). VMConnect is exempt. */ + if ((g_file_readable(client_info->certificate) && + g_file_readable(client_info->key_file)) || client_info->vmconnect) { LOG(LOG_LEVEL_INFO, "Selected TLS security"); self->selectedProtocol = PROTOCOL_SSL; diff --git a/xrdp/xrdp.ini.in b/xrdp/xrdp.ini.in index 566ca9d050..0e26f96bad 100644 --- a/xrdp/xrdp.ini.in +++ b/xrdp/xrdp.ini.in @@ -27,8 +27,8 @@ port=3389 ; prefer use vsock://: above use_vsock=false -; if used inside a Hyper-V VM with vmconnect, turn this on to enable -; wider protocol support. +; if used inside a Hyper-V VM through vmconnect and bound on vsock, +; turn this on to enable wider security protocol support. #vmconnect=true ; Unprivileged User name and group to run the xrdp daemon. From 6ba1503b6aa36ac73f5e1b67e9c905e89c8e2955 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 8 May 2025 11:51:32 +0100 Subject: [PATCH 3/5] Disable vmconnect mode for non-vsock connections --- libxrdp/xrdp_iso.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/libxrdp/xrdp_iso.c b/libxrdp/xrdp_iso.c index 5e2cae89b4..c3c72b18a7 100644 --- a/libxrdp/xrdp_iso.c +++ b/libxrdp/xrdp_iso.c @@ -72,6 +72,17 @@ xrdp_iso_create(struct xrdp_mcs *owner, struct trans *trans) self = (struct xrdp_iso *) g_malloc(sizeof(struct xrdp_iso), 1); self->mcs_layer = owner; self->trans = trans; + + // See if we're running in vmconnect mode on this connection + struct xrdp_client_info *client_info = &(self->mcs_layer->sec_layer->rdp_layer->client_info); + if (client_info->vmconnect && trans->mode != TRANS_MODE_VSOCK) + { + char desc[MAX_PEER_DESCSTRLEN]; + g_sck_get_peer_description(trans->sck, desc, sizeof(desc)); + LOG(LOG_LEVEL_INFO, "Disabling vmconnect mode for connection from %s", + desc); + client_info->vmconnect = 0; + } return self; } From f5aa00be63a17a08ee3d820114ea484a3fa88935 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 8 May 2025 12:38:01 +0100 Subject: [PATCH 4/5] Updated manpage --- docs/man/xrdp.ini.5.in | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/man/xrdp.ini.5.in b/docs/man/xrdp.ini.5.in index 06c303d8e1..88602c0ea8 100644 --- a/docs/man/xrdp.ini.5.in +++ b/docs/man/xrdp.ini.5.in @@ -23,6 +23,25 @@ All options and values (except for file names and paths) are case insensitive, a .SH "GLOBALS" The options to be specified in the \fB[Globals]\fR section are the following: +.TP +\fBport\fR=\fIport_specification [ port_specification ... ]\fR +Specify the port(s) that xrdp should listen on. More instructions and +examples can be found within xrdp.ini itself. + +.TP +\fBvmconnect\fR=\fI[true|false]\fR +(Hyper-V VMs only). Allows the user to skip some unnecessary security +checks when a virtual machine running xrdp is hosted on Hyper-V, +and the user connects to it via the vmms service on TCP port 2179. +In this configuration, RDP security is handled by the vmms service, and +security features which are not yet added to xrdp itself can be supported. + +This parameter is required in environments which do not support 'classic' +RDP encryption. + +The parameter is ignored for connections which are not made to the +virtual machine over the vsock transport. + .TP \fBautorun\fP=\fIsession_name\fP Section name for automatic login. If set and the client supplies valid From 8547744dec813e5336568114573dd9af8315694a Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 8 May 2025 14:57:25 +0100 Subject: [PATCH 5/5] Address review comments --- docs/man/xrdp.ini.5.in | 10 +++++----- xrdp/xrdp.c | 3 +++ xrdp/xrdp.ini.in | 5 ----- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/docs/man/xrdp.ini.5.in b/docs/man/xrdp.ini.5.in index 88602c0ea8..048aaa9f7a 100644 --- a/docs/man/xrdp.ini.5.in +++ b/docs/man/xrdp.ini.5.in @@ -30,11 +30,11 @@ examples can be found within xrdp.ini itself. .TP \fBvmconnect\fR=\fI[true|false]\fR -(Hyper-V VMs only). Allows the user to skip some unnecessary security -checks when a virtual machine running xrdp is hosted on Hyper-V, -and the user connects to it via the vmms service on TCP port 2179. -In this configuration, RDP security is handled by the vmms service, and -security features which are not yet added to xrdp itself can be supported. +(Hyper-V VMs only). Enables a wider support of security protocols when a +virtual machine running xrdp is hosted on Hyper-V, and the user connects +to it via the vmms service on TCP port 2179. In this configuration, +RDP security is handled by the vmms service, and security features which +are not yet added to xrdp itself can be supported. This parameter is required in environments which do not support 'classic' RDP encryption. diff --git a/xrdp/xrdp.c b/xrdp/xrdp.c index 0b7a453d7e..e41be7d137 100644 --- a/xrdp/xrdp.c +++ b/xrdp/xrdp.c @@ -332,6 +332,9 @@ read_xrdp_ini_startup_params(struct xrdp_startup_params *startup_params) else if (g_strcasecmp(name, "use_vsock") == 0) { + LOG(LOG_LEVEL_WARNING, + "Config parameter 'use_vsock' is obsolete. " + "Use 'port=vsock://...' instead"); startup_params->use_vsock = g_text2bool(val); } diff --git a/xrdp/xrdp.ini.in b/xrdp/xrdp.ini.in index 0e26f96bad..512bf3ec95 100644 --- a/xrdp/xrdp.ini.in +++ b/xrdp/xrdp.ini.in @@ -22,11 +22,6 @@ fork=true ; port=vsock://: port=3389 -; 'port' above should be connected to with vsock instead of tcp -; use this only with number alone in port above -; prefer use vsock://: above -use_vsock=false - ; if used inside a Hyper-V VM through vmconnect and bound on vsock, ; turn this on to enable wider security protocol support. #vmconnect=true