From 8bcb14f79dc90d2b410f0d943e8f7a58cfecf5ec Mon Sep 17 00:00:00 2001
From: matt335672 <30179339+matt335672@users.noreply.github.com>
Date: Sat, 21 Jun 2025 16:26:48 +0100
Subject: [PATCH] Prefer SessionSockdirGroup to be set to 'root'

With recent changes to the SCP interface, the xrdp process no longer
needs read access to the user sockdir when sesman is in use.
---
 docs/man/sesman.ini.5.in      | 11 +++++--
 docs/man/xrdp.ini.5.in        |  4 +++
 sesman/sesman.ini.in          | 10 +++++--
 tools/chkpriv/xrdp-chkpriv.in | 56 +++++++++++++++++++++--------------
 xrdp/xrdp.ini.in              |  3 ++
 5 files changed, 57 insertions(+), 27 deletions(-)

diff --git a/docs/man/sesman.ini.5.in b/docs/man/sesman.ini.5.in
index b60ac0a1..caebd2cd 100644
--- a/docs/man/sesman.ini.5.in
+++ b/docs/man/sesman.ini.5.in
@@ -371,9 +371,14 @@ transitions between confinement domains.
 
 .TP
 \fBSessionSockdirGroup\fR=\fIgroup\fR
-Sets the group owner of the directories containing session sockets. This
-MUST be the same as runtime_group in xrdp.ini, or xrdp will not
-be able to connect to any sessions.
+Sets the group owner of the directories containing session sockets.
+
+For normal operation with sesman, set this to 'root' for maximum security.
+
+If you are using xrdp to connect to VNC sessions with X server
+sockets or chansrv sockets in the local sockets dir, set this to
+the runtime_group in xrdp.ini. If you do not do this, xrdp will not
+be able to connect to your sessions.
 
 .SH "X11 SERVER"
 Following parameters can be used in the \fB[Xvnc]\fR and
diff --git a/docs/man/xrdp.ini.5.in b/docs/man/xrdp.ini.5.in
index 048aaa9f..cd6f8c42 100644
--- a/docs/man/xrdp.ini.5.in
+++ b/docs/man/xrdp.ini.5.in
@@ -415,6 +415,10 @@ Either the first or second form of this setting is recommended. Replace
 required if \fBxrdp\fR is unable to determine the session uid from the
 other values in the connection block.
 
+If you use this setting, you must also set SessionSockdirGroup in
+\fBsesman.ini\fR to be the same as runtime_group in this file. This is
+necessary to give \fBxrdp\fR the privilege to connect to \fBxrdp\-chansrv\fR.
+
 .TP
 \fBkeycode_set\fR=\fI<string>\fR
 [Xorg only] Asks for the specified keycode set to be used by the X server.
diff --git a/sesman/sesman.ini.in b/sesman/sesman.ini.in
index e950570f..23e70a72 100644
--- a/sesman/sesman.ini.in
+++ b/sesman/sesman.ini.in
@@ -48,8 +48,14 @@ RestrictInboundClipboard=none
 #XorgNoNewPrivileges=true
 ; Specify the group which is to have read access to the directory where
 ; local sockets for the session are created.
-; This MUST be the same as runtime_group in xrdp.ini, or xrdp will not
-; be able to connect to your sessions.
+; This should take one of the following values:-
+; 1) For normal operation with sesman, set this to 'root' for
+;    maximum security
+; 2) If you are using xrdp to connect to VNC sessions with X server
+;    sockets or chansrv sockets in the local sockets dir, set this to
+;    the runtime_group in xrdp.ini. If you do not do this, xrdp will not
+;    be able to connect to your sessions.
+SessionSockdirGroup=root
 #SessionSockdirGroup=xrdp
 
 
diff --git a/tools/chkpriv/xrdp-chkpriv.in b/tools/chkpriv/xrdp-chkpriv.in
index 32580ce4..ed8c2285 100644
--- a/tools/chkpriv/xrdp-chkpriv.in
+++ b/tools/chkpriv/xrdp-chkpriv.in
@@ -28,32 +28,37 @@ DROPPRIV=@pkglibexecdir@/xrdp-droppriv
 
 # Helper functions to print colored tag like "[  OK  ]"
 
-print_ok()
-{
-    if [ -t 1 ]; then
+if [ -t 1 ]; then
+    print_ok()
+    {
         printf "\033[1m[  \033[1;32mOK\033[0m  ]\033[0m "
-    else
-        printf "[  OK  ] "
-    fi
-}
+    }
 
-print_warn()
-{
-    if [ -t 1 ]; then
+    print_warn()
+    {
         printf "\033[1m[ \033[1;33mWARN\033[0m ]\033[0m "
-    else
-        printf "[ WARN ] "
-    fi
-}
+    }
 
-print_ng()
-{
-    if [ -t 1 ]; then
+    print_ng()
+    {
         printf "\033[1m[  \033[1;31mNG\033[0m  ]\033[0m "
-    else
+    }
+else
+    print_ok()
+    {
+        printf "[  OK  ] "
+    }
+
+    print_warn()
+    {
+        printf "[ WARN ] "
+    }
+
+    print_ng()
+    {
         printf "[  NG  ] "
-    fi
-}
+    }
+fi
 
 # -----------------------------------------------------------------------------
 # G E T   I N I   V A L U E
@@ -155,9 +160,16 @@ else
 fi
 
 # Groups agree between sesman and xrdp?
-if [ "$runtime_user" = "$SessionSockdirGroup" ]; then
+if [ -z "$SessionSockdirGroup" ] || [ "$SessionSockdirGroup"  = "root" ]; then
+    print_ok
+    echo "sesman.ini is configured for secure connections to sesman sessions."
+
+elif [ "$SessionSockdirGroup"  = "$runtime_group" ]; then
     print_ok
     echo "xrdp.ini and sesman.ini agree on group ownership"
+    print_warn
+    echo "consider setting SessionSockdirGroup = root for maximum security"
+
 else
     print_ng
     echo "xrdp.ini and sesman.ini do not agree on group ownership"
@@ -214,7 +226,7 @@ fi
 # privileges.  On Debian for example, we might be using the 'ssl-cert'
 # group to obtain access to /etc/ssl/private/ssl-cert-snakeoil.key
 for file in "$certificate" "$key_file"; do
-    if ! [ -e $file ]; then
+    if ! [ -e "$file" ]; then
         print_ng
         echo "$file does not exist"
         errors=$(( errors + 1 ))
diff --git a/xrdp/xrdp.ini.in b/xrdp/xrdp.ini.in
index 512bf3ec..e7ea9823 100644
--- a/xrdp/xrdp.ini.in
+++ b/xrdp/xrdp.ini.in
@@ -307,6 +307,9 @@ password=ask
 ; display number of the session, and (if applicable) 'u' with the numeric
 ; UID of the session.
 ;
+; You will also need to change the value of SessionSockdirGroup in
+; sesman.ini to allow xrdp to reach the chansrv instance
+;
 ; If 'username' or 'pamusername' is set, you probably don't need to use
 ; the two parameter variant with 'u'.
 #chansrvport=DISPLAY(n)