simon-git: putty (main): Simon Tatham

Mon May 24 15:29:44 BST 2021

TL;DR:
  415927e9 Make SSH proxying conditional on CONF_proxy_type!
  02aca17f Reorder proxy-type enum for backwards compatibility.
  17c57e10 Reorganise Windows HANDLE management.
  9851d37c Add test script for simultaneous agent connections.

Repository:     https://git.tartarus.org/simon/putty.git
On the web:     https://git.tartarus.org/?p=simon/putty.git
Branch updated: main
Committer:      Simon Tatham <anakin at pobox.com>
Date:           2021-05-24 15:29:44

commit 415927e9b0ad6aee6b9a0ba184fc420c51e05b94
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=415927e9b0ad6aee6b9a0ba184fc420c51e05b94;hp=d008d235f3841139daca39efee25cd5423ce31b8
Author: Simon Tatham <anakin at pobox.com>
Date:   Mon May 24 13:52:51 2021 +0100

    Make SSH proxying conditional on CONF_proxy_type!

    In commit 0d3bb73608f1a5d, I introduced the new SSH / jump-host proxy
    type, which should be invoked by proxy.c when CONF_proxy_type is set
    to PROXY_SSH. In fact, I left out the check, so it's invoked by
    proxy.c _unconditionally_, after the check to see whether proxying is
    required at all. So any saved session configured with any other proxy
    type (other than PROXY_NONE) would be treated as PROXY_SSH by mistake.

    How embarrassing. I did remember at one point that I needed to fix
    this, but it fell out of my head before I pushed!

 proxy.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

commit 02aca17f360bbd8409c761af3713b11936f3af78
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=02aca17f360bbd8409c761af3713b11936f3af78;hp=415927e9b0ad6aee6b9a0ba184fc420c51e05b94
Author: Simon Tatham <anakin at pobox.com>
Date:   Mon May 24 13:57:39 2021 +0100

    Reorder proxy-type enum for backwards compatibility.

    Commit 0d3bb73608f1a5d inserted PROXY_SSH just before PROXY_CMD, so
    that it got the numerical value that PROXY_CMD had before. But that
    meant that any saved session configured as PROXY_CMD by an older build
    of PuTTY would be regarded as PROXY_SSH by the new version - even
    after the previous commit fixed the unconditional use of SSH proxying
    regardless of the type setting.

    Now PROXY_CMD has its old value back, and PROXY_SSH is inserted at the
    _end_ of the enum. (Or rather, just before the extra-weird PROXY_FUZZ,
    which is allowed to have an unstable numerical value because it's
    never stored in a saved session at all.)

    This is all rather unsatisfactory, and makes me wish I'd got round to
    reworking the saved data format to use keywords in place of integers.

 putty.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

commit 17c57e1078c8fabd5417175247b38ce7b1643ca4
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=17c57e1078c8fabd5417175247b38ce7b1643ca4;hp=02aca17f360bbd8409c761af3713b11936f3af78
Author: Simon Tatham <anakin at pobox.com>
Date:   Mon May 24 13:06:10 2021 +0100

    Reorganise Windows HANDLE management.

    Before commit 6e69223dc262755, Pageant would stop working after a
    certain number of PuTTYs were active at the same time. (At most about
    60, but maybe fewer - see below.)

    This was because of two separate bugs. The easy one, fixed in
    6e69223dc262755 itself, was that PuTTY left each named-pipe connection
    to Pageant open for the rest of its lifetime. So the real problem was
    that Pageant had too many active connections at once. (And since a
    given PuTTY might make multiple connections during userauth - one to
    list keys, and maybe another to actually make a signature - that was
    why the number of _PuTTYs_ might vary.)

    It was clearly a bug that PuTTY was leaving connections to Pageant
    needlessly open. But it was _also_ a bug that Pageant couldn't handle
    more than about 60 at once. In this commit, I fix that secondary bug.

    The cause of the bug is that the WaitForMultipleObjects function
    family in the Windows API have a limit on the number of HANDLE objects
    they can select between. The limit is MAXIMUM_WAIT_OBJECTS, defined to
    be 64. And handle-io.c was using a separate event object for each I/O
    subthread to communicate back to the main thread, so as soon as all
    those event objects (plus a handful of other HANDLEs) added up to more
    than 64, we'd start passing an overlarge handle array to
    WaitForMultipleObjects, and it would start not doing what we wanted.

    To fix this, I've reorganised handle-io.c so that all its subthreads
    share just _one_ event object to signal readiness back to the main
    thread. There's now a linked list of 'struct handle' objects that are
    ready to be processed, protected by a CRITICAL_SECTION. Each subthread
    signals readiness by adding itself to the linked list, and setting the
    event object to indicate that the list is now non-empty. When the main
    thread receives the event, it iterates over the whole list processing
    all the ready handles.

    (Each 'struct handle' still has a separate event object for the main
    thread to use to communicate _to_ the subthread. That's OK, because no
    thread is ever waiting on all those events at once: each subthread
    only waits on its own.)

    The previous HT_FOREIGN system didn't really fit into this framework.
    So I've moved it out into its own system. There's now a handle-wait.c
    which deals with the relatively simple job of managing a list of
    handles that need to be waited for, each with a callback function;
    that's what communicates a list of HANDLEs to event loops, and
    receives the notification when the event loop notices that one of them
    has done something. And handle-io.c is now just one client of
    handle-wait.c, providing a single HANDLE to the event loop, and
    dealing internally with everything that needs to be done when that
    handle fires.

    The new top-level handle-wait.c system *still* can't deal with more
    than MAXIMUM_WAIT_OBJECTS. At the moment, I'm reasonably convinced it
    doesn't need to: the only kind of HANDLE that any of our tools could
    previously have needed to wait on more than one of was the one in
    handle-io.c that I've just removed. But I've left some assertions and
    a TODO comment in there just in case we need to change that in future.

 windows/CMakeLists.txt      |  10 +-
 windows/cliloop.c           |  22 ++--
 windows/conpty.c            |   9 +-
 windows/handle-io.c         | 240 ++++++++++++++++++--------------------------
 windows/handle-wait.c       | 144 ++++++++++++++++++++++++++
 windows/named-pipe-server.c |   9 +-
 windows/pageant.c           |  17 ++--
 windows/platform.h          |  21 +++-
 windows/window.c            |  15 ++-
 9 files changed, 299 insertions(+), 188 deletions(-)

commit 9851d37ccbded022ef542f5e65c47cb560a6b6da
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=9851d37ccbded022ef542f5e65c47cb560a6b6da;hp=17c57e1078c8fabd5417175247b38ce7b1643ca4
Author: Simon Tatham <anakin at pobox.com>
Date:   Mon May 24 15:04:35 2021 +0100

    Add test script for simultaneous agent connections.

    This script makes 128 connections to your SSH agent at once, and then
    sends requests down them in random order to check that the agent is
    correctly selecting between all its incoming sockets / named pipes /
    whatever.

    128 is bigger than MAXIMUM_WAIT_OBJECTS, so a successful run of this
    script inside a Windows PuTTY agent-forwarding to a Pageant indicates
    that both the PuTTY and the Pageant are managing to handle >64 I/O
    subthreads without overloading their event loop.

 test/agentmulti.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/ssh.py        |  1 +
 2 files changed, 57 insertions(+)