Merge pull request from GHSA-7w22-h4w7-8j5j

Enforce no login screen if require_credentials is set
neutrinolabs · Jul 11, 2024 · 19c111c · 19c111c
2 parents 10c4700 + 8ac2f6d
commit 19c111c
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 11 deletions.
diff --git a/docs/man/xrdp.ini.5.in b/docs/man/xrdp.ini.5.in
@@ -156,9 +156,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
@@ -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
@@ -94,7 +94,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
@@ -2803,6 +2803,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)