openssh bugs - Jan 2024 - [Bug 3655] New: Default ObscureKeystrokeTiming makes X forwarding really slow

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

            Bug ID: 3655
           Summary: Default ObscureKeystrokeTiming makes X forwarding
                    really slow
           Product: Portable OpenSSH
           Version: 9.6p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: ole.laursen at gmail.com

This is easy to reproduce with something like xterm or emacs:

$ ssh -X localhost
$ xterm
$ emacs

They are both slow to display their windows, and interaction is laggy.
In Emacs, text selection has several seconds of lag.

Meanwhile

ssh -X -oObscureKeystrokeTiming=no localhost

works fine.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Andreas Gustafsson <gson at araneus.fi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gson at araneus.fi

--- Comment #1 from Andreas Gustafsson <gson at araneus.fi> ---
I am also affected by this bug.  An X11 application suddenly became
slow to the point of unusability, and it took quite a lot of detective
work to determine that the cause was neither the application itself nor
the X server but ssh.

To help quantify the worst-case slowdown, I am attaching a test program
that makes 1000 round-trip requests to the X server.  Running it via
the ssh included in macOS Sonoma 14.4.1 (OpenSSH_9.6p1) shows:

$ ssh -X -oObscureKeystrokeTiming=no localhost
$ time ./test
real    0m0.120s
user    0m0.007s
sys     0m0.018s
$ exit

$ ssh -X localhost
$ time ./test
real    0m24.095s
user    0m0.018s
sys     0m0.066s

That's a slowdown by a factor of 200.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

--- Comment #2 from Andreas Gustafsson <gson at araneus.fi> ---
Created attachment 3800
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3800&action=edit
Test program that performs 1000 round-trip X11 requests

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Laine Stump <laine at laine.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |laine at laine.org

--- Comment #3 from Laine Stump <laine at laine.org> ---
I created an account here just so that I could follow the bz, and also
to say that emacs lag when selecting text is *far* beyond several
seconds of lag for me - it essentially locks up the entire emacs
session and I have to kill it. I have no choice but to disable
ObscureKeystrokeTiming in my system config.

My opinion (for what it's worth, which is "not much" :-)) is that
this
new feature should be off by default until it is less disruptive,
otherwise you'll end up with everyone manually disabling it in their
local ssh_config to work around the current misbehavior, and then even
when it does get fixed to work acceptably nobody will know, because
they long ago forced the feature off, and they'll never go back to turn
it on (since everything works as-is).

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Martin Schr?der <martin at oneiros.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |martin at oneiros.de

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Antonio Larrosa <alarrosa at suse.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alarrosa at suse.com

--- Comment #4 from Antonio Larrosa <alarrosa at suse.com> ---
Created attachment 3837
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3837&action=edit
Patch that fixes the x11 forwarding regression

This patch fixes the issue by not quantising outbound packets as long
as X11 communication happened recently (less than a second ago). The
way I understood it, the purpose of sending chaff packets is to add
traffic when in low traffic situations (like when writing in a shell),
but if there's X11 communications happening, there's already some added
traffic and there's no need to quantise the data sent.

This was tested to fix the issue with the test program in #c2 as well
as when running firefox or other X11 applications which now happen near
instantly again.

This can maybe be improved in several ways, like:
1) adding a boolean variable to the Channel struct that specifies if
it's an x11-connection type that can be quickly tested instead of doing
so many strcmps on Channel's ctype.
2) making lastused a timespec so that subsecond intervals can be tested
correctly.
But I'd like to know the upstream developers opinion on those ideas
before working on that.

Btw, is there some way to submit a Merge/Pull Request to the git
repository? or is adding the patch as an attachment here the proper way
to submit patches?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

--- Comment #5 from Antonio Larrosa <alarrosa at suse.com> ---
> already some added traffic and there's no need to quantise the data
> sent.
... in order to obfuscate the traffic since the X11 protocol already
took care of "obfuscating" it.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
Thanks for the patch - it looks good, except for this bit
> +	for (i = 0; i < ssh->chanctxt->channels_alloc; i++) {
> +	    c = ssh->chanctxt->channels[i];
> +	    if (c == NULL || c->ctype == NULL || c->lastused == 0 ||
> +		strcmp(c->ctype, "x11-connection"))
> +		continue;
> +	    if (monotime() - c->lastused < 1)
> +		return 1;
> +	}
which will call monotime() a bunch.

IMO it would be better to do something like this:

time_t lastused = 0;
for (i = 0; i < ssh->chanctxt->channels_alloc; i++) {
        c = ssh->chanctxt->channels[i];
        if (c == NULL || c->ctype == NULL || c->lastused == 0 ||
            strcmp(c->ctype, "x11-connection"))
                continue;
        lastused = c->lastused;
}
return (lastused != 0 && monotime() > lastused + 1);

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Antonio Larrosa <alarrosa at suse.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3837|0                           |1
        is obsolete|                            |

--- Comment #7 from Antonio Larrosa <alarrosa at suse.com> ---
Created attachment 3838
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3838&action=edit
Patch that fixes the x11 forwarding regression

That's a very good point, I updated the patch with your suggestion.

Just fixed the inequality test since we have to test if "monotime() <
lastused + 1"

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Damien Miller <djm at mindrot.org> ---
Thanks - that's been applied and will be in OpenSSH 10.0, due in a
couple of months.

commit fe6c6330c1a94c7a537efe9069853ce7a275c50a (HEAD -> master,
origin/master, origin/HEAD)
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Sun Oct 13 22:20:06 2024 +0000

    upstream: don't start the ObscureKeystrokeTiming mitigations if

    there has been traffic on a X11 forwarding channel recently.

    Should fix X11 forwarding performance problems when this setting is
    enabled. Patch from Antonio Larrosa via bz3655

    OpenBSD-Commit-ID: 820284a92eb4592fcd3d181a62c1b86b08a4a7ab

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Ettore Bronzini <bronzini.ettore at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bronzini.ettore at gmail.com

--- Comment #9 from Ettore Bronzini <bronzini.ettore at gmail.com> ---
(In reply to Antonio Larrosa from comment #7)
> Created attachment 3838 [details]
> Patch that fixes the x11 forwarding regression
> 
> That's a very good point, I updated the patch with your suggestion.
> 
> Just fixed the inequality test since we have to test if "monotime()
> < lastused + 1"
Hi, thanks for the patch. Can you please tell me also how and where to
run this patch to fix this problem? I am not an expert.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Richard W.M. Jones <rjones at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rjones at redhat.com

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3655

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net

--- Comment #10 from Darren Tucker <dtucker at dtucker.net> ---
Note that if you are using or backporting this, you also want the
change mentioned in comment#7 and bug#3820
(https://github.com/openssh/openssh-portable/commit/203bb886)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.