From 0ebf4cff13a4235ad990af4e89c66c0ef7150da9 Mon Sep 17 00:00:00 2001
From: matt335672 <30179339+matt335672@users.noreply.github.com>
Date: Mon, 1 Jul 2024 14:25:23 +0100
Subject: [PATCH] Check unprivileged user can't write TLS files

The unprivileged user needs to be able to read the certificate and
key files to offer TLS, but should not be able to write to then.

This commit checks the TLS files are read-only, rather than
simply readable
---
 tools/chkpriv/xrdp-chkpriv.in | 46 +++++++++++++++--------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/tools/chkpriv/xrdp-chkpriv.in b/tools/chkpriv/xrdp-chkpriv.in
index f78bb722..3193e0d3 100755
--- a/tools/chkpriv/xrdp-chkpriv.in
+++ b/tools/chkpriv/xrdp-chkpriv.in
@@ -120,9 +120,9 @@ fi
 
 # Groups agree between sesman and xrdp?
 if [ "$runtime_user" = "$SessionSockdirGroup" ]; then
-    echo "-Info- xrdp.ini and sesman.ini agree on group ownbership"
+    echo "-Info- xrdp.ini and sesman.ini agree on group ownership"
 else
-    echo "-Error- xrdp.ini and sesman.ini do not agree on group ownbership"
+    echo "-Error- xrdp.ini and sesman.ini do not agree on group ownership"
     errors=$(( errors + 1 ))
 fi
 
@@ -165,33 +165,27 @@ else
     errors=$(( errors + 1 ))
 fi
 
-# Are cert and key readable by the user?
+# Are cert and key readable (but NOT writeable) by the user?
 #
-# These aren't necessarily our files, so we can't be prescriptive about
+# These aren't necessarily our files, so we can't be too prescriptive about
 # privileges.  On Debian for example, we might be using the 'ssl-cert'
 # group to obtain access to /etc/ssl/private/ssl-cert-snakeoil.key
-if ! [ -e $certificate ]; then
-    echo "-Error- $certificate does not exist"
-    errors=$(( errors + 1 ))
-elif $DROPPRIV "$runtime_user" "$runtime_group" sh -c '[ -r '"$certificate"' ]'
-then
-    echo "-Info- $certificate is readable by $runtime_user:$runtime_group"
-else
-    echo "-Error- $certificate is not readable by $runtime_user:$runtime_group"
-    errors=$(( errors + 1 ))
-fi
-
-if ! [ -e $key_file ]; then
-    echo "-Error- $key_file does not exist"
-    errors=$(( errors + 1 ))
-elif $DROPPRIV "$runtime_user" "$runtime_group" sh -c '[ -r '"$key_file"' ]'
-    sh -c '[ -r '"$key_file"' ]'
-then
-    echo "-Info- $key_file is readable by $runtime_user:$runtime_group"
-else
-    echo "-Error- $key_file is not readable by $runtime_user:$runtime_group"
-    errors=$(( errors + 1 ))
-fi
+for file in "$certificate" "$key_file"; do
+    if ! [ -e $file ]; then
+        echo "-Error- $file does not exist"
+        errors=$(( errors + 1 ))
+    elif ! $DROPPRIV "$runtime_user" "$runtime_group" sh -c '[ -r '"$file"' ]'
+    then
+        echo "-Error- $file is not readable by $runtime_user:$runtime_group"
+        errors=$(( errors + 1 ))
+    elif $DROPPRIV "$runtime_user" "$runtime_group" sh -c '[ -w '"$file"' ]'
+    then
+        echo "-Error- $file is writeable by $runtime_user:$runtime_group"
+        errors=$(( errors + 1 ))
+    else
+        echo "-Info- $file is read-only for $runtime_user:$runtime_group"
+    fi
+done
 
 echo
 if [ $errors -eq 0 ]; then