Hi,

while this does not fix guacamole sending events too early I´ve created

https://github.com/FreeRDP/FreeRDP/pull/6742 to just discard such events before sending (and being noisy about it)


regards

Armin

On 21.01.21 20:59, George Krajcsovits via FreeRDP-devel wrote:
Hi,

for now I'll be using this patch to Guacamole, which is ugly AF, but works
around the problem:

diff --git a/src/protocols/rdp/input.c b/src/protocols/rdp/input.c
index cb9bb10e..b221c1ee 100644
--- a/src/protocols/rdp/input.c
+++ b/src/protocols/rdp/input.c
@@ -33,6 +33,8 @@

  #include <stdlib.h>

+#define CONNECTION_IS_ACTIVE(rdpinstance)
(*((int*)(rdpinstance->context->rdp))==14)
+
  int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) {

      guac_client* client = user->client;
@@ -52,6 +54,11 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x,
int y, int mask) {
      if (rdp_client->recording != NULL)
          guac_common_recording_report_mouse(rdp_client->recording, x, y,
mask);

+    if (!CONNECTION_IS_ACTIVE(rdp_inst)) {
+        rdp_client->mouse_button_mask = mask;
+        goto complete;
+    }
+
      /* If button mask unchanged, just send move event */
      if (mask == rdp_client->mouse_button_mask)
          rdp_inst->input->MouseEvent(rdp_inst->input, PTR_FLAGS_MOVE, x, y);
@@ -135,8 +142,9 @@ int guac_rdp_user_key_handler(guac_user* user, int
keysym, int pressed) {
          guac_common_recording_report_key(rdp_client->recording,
                  keysym, pressed);

+    freerdp* rdp_inst = rdp_client->rdp_inst;
      /* Skip if keyboard not yet ready */
-    if (rdp_client->keyboard == NULL)
+    if (rdp_client->keyboard == NULL || rdp_inst == NULL ||
!CONNECTION_IS_ACTIVE(rdp_inst))
          goto complete;

      /* Update keysym state */
@@ -161,8 +169,10 @@ int guac_rdp_user_size_handler(guac_user* user, int
width, int height) {
      width  = width  * settings->resolution / user->info.optimal_resolution;
      height = height * settings->resolution / user->info.optimal_resolution;

-    /* Send display update */
-    guac_rdp_disp_set_size(rdp_client->disp, settings, rdp_inst, width,
height);
+    if (rdp_inst != NULL || CONNECTION_IS_ACTIVE(rdp_inst)) {
+        /* Send display update */
+        guac_rdp_disp_set_size(rdp_client->disp, settings, rdp_inst,
width, height);
+    }

      return 0;


Obviously won't be sending this upstream.

regards, krajo

On Wed, Jan 20, 2021 at 5:36 PM George Krajcsovits <
george.krajcsov...@gmail.com> wrote:

Hi,

so it seems like the "Activated" event would be perfect to restart sending
input messages to freerdp, and I can wait for it by:
PubSub_SubscribeActivated(context->pubSub, my_callback);

However I don't see what event / callback to use for sensing when to stop
sending the input messages. I'd rather not modify libfreerdp and solve this
in Guacamole, but haven't found a good way. The events I tried were
Terminate, ErrorInfo, GraphicsReset which did not  happen.
ChannelDisconnect does happen, but those channels aren't mandatory so
that's a no go. Also I'm not sure if an event is fast enough to stop
messages anyway - are events real time or come from some queue?

Also PreConnect callback does not happen on redirect, so that doesn't work
either.

regards, krajo

On Wed, Jan 20, 2021 at 2:16 PM George Krajcsovits <
george.krajcsov...@gmail.com> wrote:

Hi,

thank you, I'll take a look. In the meantime I did find an interesting
commit in FreeRDP that seems related:

https://github.com/FreeRDP/FreeRDP/commit/2ae7c2a6d480a647059e85c6cee2a493641137e3

I'll see if guacamole does something like this and if it can be fixed if
not.

regards, krajo

On Wed, Jan 20, 2021 at 1:54 PM Darren DeHaven <clearmin...@gmail.com>
wrote:

Hi George,

(Note: I'm just a user.)

Your issue sounds similar to either of these bugs:

guacd, Release 1.2.0 segfaults in connection with RDP-Sessions
   https://issues.apache.org/jira/browse/GUACAMOLE-1203

Tolerate RDP protocol violations where possible:
https://issues.apache.org/jira/browse/GUACAMOLE-1059
(this bug shows as fixed in 1.2.0.  I wonder if this "fix" is causing
the 1203 bug?)

* Additional reference:
rdpbcgr connection sequence:

https://securitylab.github.com/static/09944e314cb4d4cf299530f0d9383846/179f0/image5.png

https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/023f1e69-cfe8-4ee6-9ee0-7e759fb4e4ee

Other rdp details:
https://securitylab.github.com/research/fuzzing-sockets-FreeRDP


On Wed, Jan 20, 2021 at 3:35 AM George Krajcsovits via FreeRDP-devel <
freerdp-devel@lists.sourceforge.net> wrote:

Hi,

I'm using freerdp 2.2.0 via Guacamole 1.3.0 on Ubuntu 18.04 LTS.
In my use case, first there is a TLS security connection to the server,
which then redirects to the final connection , which is using NLA.

If I move the mouse during the redirection (or do any other input), then
NLA connection fails because the server receives INPUT PDUs during the
connection phase and aborts due to protocol violation. If I don't make
inputs, then everything is fine.

Weirdly xfreerdp, remmina work ok, I just cannot figure out what they do
differently to I guess inhibit sending input events while the new
connection is made.

Any tips? Is there some callback about this (I don't see PreConnect
callback)? I just started to go through the libfreerdp, xfreerdp and
guacamole code to figure out a solution.

thanks & regards, krajo


--
Learn to separate truth from illusion, because in this world, it's the
hardest thing to do.

_______________________________________________
FreeRDP-devel mailing list
FreeRDP-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/freerdp-devel

--
Learn to separate truth from illusion, because in this world, it's the
hardest thing to do.


--
Learn to separate truth from illusion, because in this world, it's the
hardest thing to do.




_______________________________________________
FreeRDP-devel mailing list
FreeRDP-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/freerdp-devel

Reply via email to