From 61b509f1d5d9b85128504c7b752e6e36d7b60b15 Mon Sep 17 00:00:00 2001
From: matt335672 <30179339+matt335672@users.noreply.github.com>
Date: Thu, 27 Jun 2024 11:39:21 +0100
Subject: [PATCH] Enforce no login screen if require_credentials is set

If the setting require_credentials is true, there should be no way
for the user to get to a login screen.

This commit makes the following changes if this flag is active:-
- Makes the checks around TS_INFO_PACKET more explicit.
- Closes the connection if the first login attempt fails.

(cherry picked from commit 8ac2f6db34649a93d3c9c4fe8fda61203702e615)
---
 docs/man/xrdp.ini.5.in |  9 ++++++---
 libxrdp/xrdp_sec.c     | 27 ++++++++++++++++++++-------
 xrdp/xrdp.ini.in       |  3 ++-
 xrdp/xrdp_mm.c         |  8 ++++++++
 4 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/docs/man/xrdp.ini.5.in b/docs/man/xrdp.ini.5.in
index e44327ce..5f8a00e9 100644
--- a/docs/man/xrdp.ini.5.in
+++ b/docs/man/xrdp.ini.5.in
@@ -133,9 +133,12 @@ by \fBseparator\fP.
 
 .TP
 \fBrequire_credentials\fP=\fI[true|false]\fP
-If set to \fB1\fP, \fBtrue\fP or \fByes\fP, \fBxrdp\fP requires clients to include username and
-password initial connection phase. In other words, xrdp doesn't allow clients to show login
-screen if set to true. If not specified, defaults to \fBfalse\fP.
+If set to \fB1\fP, \fBtrue\fP or \fByes\fP, \fBxrdp\fP requires clients
+to include username and password initial connection phase. In other
+words, xrdp doesn't allow clients to show login screen if set to true.
+It follows that an incorrect password will cause the login to immediately
+fail without displaying the login screen.  If not specified, defaults
+to \fBfalse\fP.
 
 .TP
 \fBsecurity_layer\fP=\fI[tls|rdp|negotiate]\fP
diff --git a/libxrdp/xrdp_sec.c b/libxrdp/xrdp_sec.c
index 892af7b5..e7cd8326 100644
--- a/libxrdp/xrdp_sec.c
+++ b/libxrdp/xrdp_sec.c
@@ -1012,6 +1012,25 @@ xrdp_sec_process_logon_info(struct xrdp_sec *self, struct stream *s)
         return 1;
     }
 
+    // If we require credentials, don't continue if they're not provided
+    if (self->rdp_layer->client_info.require_credentials)
+    {
+        if ((flags & RDP_LOGON_AUTO) == 0)
+        {
+            LOG(LOG_LEVEL_ERROR, "Server is configured to require that the "
+                "client enable auto logon with credentials, but the client did "
+                "not request auto logon.");
+            return 1;
+        }
+        if (len_user == 0 || len_password == 0)
+        {
+            LOG(LOG_LEVEL_ERROR, "Server is configured to require that the "
+                "client enable auto logon with credentials, but the client did "
+                "not supply both a username and password.");
+            return 1;
+        }
+    }
+
     if (flags & RDP_LOGON_AUTO)
     {
         if (ts_info_utf16_in(s, len_password, self->rdp_layer->client_info.password, sizeof(self->rdp_layer->client_info.password)) != 0)
@@ -1033,18 +1052,12 @@ xrdp_sec_process_logon_info(struct xrdp_sec *self, struct stream *s)
     }
     else
     {
+        // Skip the password
         if (!s_check_rem_and_log(s, len_password + 2, "Parsing [MS-RDPBCGR] TS_INFO_PACKET Password"))
         {
             return 1;
         }
         in_uint8s(s, len_password + 2);
-        if (self->rdp_layer->client_info.require_credentials)
-        {
-            LOG(LOG_LEVEL_ERROR, "Server is configured to require that the "
-                "client enable auto logon with credentials, but the client did "
-                "not request auto logon.");
-            return 1; /* credentials on cmd line is mandatory */
-        }
     }
     if (self->rdp_layer->client_info.domain_user_separator[0] != '\0'
             && self->rdp_layer->client_info.domain[0] != '\0')
diff --git a/xrdp/xrdp.ini.in b/xrdp/xrdp.ini.in
index 11ecada6..c240ebfd 100644
--- a/xrdp/xrdp.ini.in
+++ b/xrdp/xrdp.ini.in
@@ -88,7 +88,8 @@ max_bpp=32
 new_cursors=true
 ; fastpath - can be 'input', 'output', 'both', 'none'
 use_fastpath=both
-; when true, userid/password *must* be passed on cmd line
+; when true, userid/password *must* be passed on cmd line. If the password
+; is incorrect, the login will fail
 #require_credentials=true
 ; when true, the userid will be used to try to authenticate
 #enable_token_login=true
diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c
index 986c1cb5..6b29640b 100644
--- a/xrdp/xrdp_mm.c
+++ b/xrdp/xrdp_mm.c
@@ -2687,6 +2687,14 @@ xrdp_mm_process_login_response(struct xrdp_mm *self)
                                 self->wm->pamerrortxt);
             }
 
+            if (self->wm->client_info->require_credentials)
+            {
+                /* Credentials had to be specified, but were invalid */
+                g_set_wait_obj(self->wm->pro_layer->self_term_event);
+                LOG(LOG_LEVEL_ERROR, "require_credentials is set, "
+                    "but the user could not be logged in");
+            }
+
             if (server_closed)
             {
                 if (login_result == E_SCP_LOGIN_NOT_AUTHENTICATED)