Fix message ordering in devredir

Author: matt335672

common/ms-rdpefs.h

@@ -119,5 +119,10 @@ enum IRP_MN
     IRP_MN_NOTIFY_CHANGE_DIRECTORY  = 0x00000002
 };
 
+/*
+ * General Capability Set (2.2.2.7.1)
+ */
+/* extendedPDU fields */
+#define RDPDR_USER_LOGGEDON_PDU         0x00000004
 
 #endif /* MS_RDPEFS_H */

sesman/chansrv/devredir.c

@@ -98,6 +98,7 @@ enum COMPLETION_TYPE
 
 /* globals */
 extern int g_rdpdr_chan_id; /* in chansrv.c */
+int g_need_user_loggedon_pdu = 0;
 int g_is_printer_redir_supported = 0;
 int g_is_port_redir_supported = 0;
 int g_is_drive_redir_supported = 0;
@@ -370,26 +371,38 @@ devredir_data_in(struct stream *s, int chan_id, int chan_flags, int length,
                     case RDP_CLIENT_60_61:
                         break;
                 }
-                // LK_TODO devredir_send_server_clientID_confirm();
             }
             break;
 
         case PAKID_CORE_CLIENT_NAME:
             /* client is telling us its computer name; do we even care? */
 
-            /* let client know login was successful */
-            devredir_send_server_user_logged_on();
-            usleep(1000 * 100);
-
-            /* let client know our capabilities */
-            devredir_send_server_core_cap_req();
+            /* See 3.3.5.1.6 for sequencing rules */
+            if (g_client_rdp_version >= RDP_CLIENT_51)
+            {
+                /* let client know our capabilities */
+                devredir_send_server_core_cap_req();
+            }
 
             /* send confirm clientID */
             devredir_send_server_clientID_confirm();
             break;
 
         case PAKID_CORE_CLIENT_CAPABILITY:
             rv = devredir_proc_client_core_cap_resp(ls);
+            if (rv == 0)
+            {
+                if (g_need_user_loggedon_pdu)
+                {
+                    /* Tell client to announce remaining devices */
+                    devredir_send_server_user_logged_on();
+                }
+                else if (g_client_rdp_version >= RDP_CLIENT_51)
+                {
+                    /* See 3.3.5.1.7 */
+                    devredir_send_server_clientID_confirm();
+                }
+            }
             break;
 
         case PAKID_CORE_DEVICELIST_ANNOUNCE:
@@ -734,6 +747,51 @@ devredir_send_drive_dir_request(IRP *irp, tui32 device_id,
 **                   process data received from client                       **
 ******************************************************************************/
 
+/**
+ * Process a GENERAL_CAPS_SET packet from the client
+ * @param s Stream. CAPABILITY_HEADER is already read
+ * @param cap_len Amount of data left in stream for the packet
+ * @return 0 for success, -1 otherwise
+ *
+ * Globals are modified by this call.
+ * After this call, the capability is skipped in the stream
+ */
+static int
+process_client_general_caps_set(struct stream *s, unsigned int cap_len)
+{
+    int rv = -1;
+    char *exit_p = s->p + cap_len;
+
+    // Data we don't check at the start of the packet
+#define PACKET_SKIP_LENGTH ( \
+                             4 + /* osType */ \
+                             4 + /* osVersion */ \
+                             2 + /* protocolMajorVersion */ \
+                             2 + /* protocolMinorVersion */ \
+                             4 + /* ioCode1 */ \
+                             4) /* ioCode2 */
+
+    if (cap_len < (PACKET_SKIP_LENGTH + 4))
+    {
+        LOG(LOG_LEVEL_ERROR,
+            "[MS-RDPEFS] GENERAL_CAPS_SET: Short packet (%u bytes) encountered",
+            cap_len);
+    }
+    else
+    {
+        tui32 extended_pdu;
+        xstream_seek(s, PACKET_SKIP_LENGTH);
+        xstream_rd_u32_le(s, extended_pdu);
+
+        g_need_user_loggedon_pdu = (extended_pdu & RDPDR_USER_LOGGEDON_PDU);
+        rv = 0;
+    }
+
+    s->p = exit_p;
+    return rv;
+#undef PACKET_SKIP_LENGTH
+}
+
 /**
  * @brief process client's response to our core_capability_req() msg
  *
@@ -780,20 +838,27 @@ devredir_proc_client_core_cap_resp(struct stream *s)
         {
             case CAP_GENERAL_TYPE:
                 LOG_DEVEL(LOG_LEVEL_DEBUG, "got CAP_GENERAL_TYPE");
+                if (process_client_general_caps_set(s, cap_len) < 0)
+                {
+                    return -1;
+                }
                 break;
 
             case CAP_PRINTER_TYPE:
                 LOG_DEVEL(LOG_LEVEL_DEBUG, "got CAP_PRINTER_TYPE");
+                xstream_seek(s, cap_len);
                 g_is_printer_redir_supported = 1;
                 break;
 
             case CAP_PORT_TYPE:
                 LOG_DEVEL(LOG_LEVEL_DEBUG, "got CAP_PORT_TYPE");
+                xstream_seek(s, cap_len);
                 g_is_port_redir_supported = 1;
                 break;
 
             case CAP_DRIVE_TYPE:
                 LOG_DEVEL(LOG_LEVEL_DEBUG, "got CAP_DRIVE_TYPE");
+                xstream_seek(s, cap_len);
                 g_is_drive_redir_supported = 1;
                 if (cap_version == 2)
                 {
@@ -803,10 +868,10 @@ devredir_proc_client_core_cap_resp(struct stream *s)
 
             case CAP_SMARTCARD_TYPE:
                 LOG_DEVEL(LOG_LEVEL_DEBUG, "got CAP_SMARTCARD_TYPE");
+                xstream_seek(s, cap_len);
                 g_is_smartcard_redir_supported = (scard_init() == 0);
                 break;
         }
-        xstream_seek(s, cap_len);
     }
     return 0;
 }