simon-git: putty (master): Simon Tatham

Commits to Tartarus hosted VCS tartarus-commits at lists.tartarus.org
Fri May 18 07:55:38 BST 2018


TL;DR:
  3692c23 Remove unused params from console_get_userpass_input.
  a486318 Remove unused params from cmdline_get_passwd_input.
  e3bdd62 ssh.c: new data type 'struct PacketQueue'.
  9d96c3e bufchain: new combined fetch + consume functions.
  cfc3386 Add a reference count in 'struct Packet'.
  2ee07f8 Add a concept of 'idempotent callback'.
  7400653 New coroutine 'crMaybeWait' macros, which may not return.
  9d495b2 Make {term,}get_userpass_input take a bufchain.
  2b57b84 Make the rdpkt functions output to a PacketQueue.
  fe6caf5 Put all incoming SSH wire data into a bufchain.
  0ce9224 Factor out general processing for all packets.
  0a15a2c Unconditionally fill the SSH-1 dispatch table.
  5d9adc5 Stop passing incoming packets through ssh->protocol.
  bf62c85 Stop using ssh->protocol_initial_phase_done in SSH-1.
  f10a65d Put all user input into a bufchain.
  c3abc30 Remove the ssh*_protocol() functions completely.
  265365a do_ssh1_login: remove user input parameters.
  96d9d78 do_ssh1_login: change return type to void.
  6cfe0a2 do_ssh1_login: get packets from a PacketQueue.
  364b3a2 do_ssh2_authconn: remove user input parameters.
  e2b7f4f do_ssh2_authconn: get packets from a PacketQueue.
  a41eeff do_ssh2_authconn: replace s->gotit with pq_push_front.
  7c47a17 do_ssh1_connection: remove user input parameters.
  38fccbf do_ssh1_connection: get packets from a PacketQueue.
  322fb59 do_ssh2_transport: remove user input parameters.
  4e4a1a3 do_ssh2_transport: get packets from a PacketQueue.
  ea7f3c2 Separate the SSH-2 userauth and connection coroutines.

Repository:     https://git.tartarus.org/simon/putty.git
On the web:     https://git.tartarus.org/?p=simon/putty.git
Branch updated: master
Committer:      Simon Tatham <anakin at pobox.com>
Date:           2018-05-18 07:55:38

commit 3692c239d7c6a038bea4ef889ae5781be9eabe18
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=3692c239d7c6a038bea4ef889ae5781be9eabe18;hp=14a69dc632868b2bc053bf19c719b4f1d3153a97
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:56 2018 +0100

    Remove unused params from console_get_userpass_input.
    
    NFC: this is a preliminary refactoring, intended to make my life
    easier when I start changing around the APIs used to pass user
    keyboard input around. The fewer functions even _have_ such an API,
    the less I'll have to do at that point.

 cmdgen.c          | 6 +++---
 putty.h           | 3 +--
 unix/uxcons.c     | 3 +--
 unix/uxpgnt.c     | 2 +-
 unix/uxplink.c    | 2 +-
 unix/uxsftp.c     | 2 +-
 windows/wincons.c | 3 +--
 7 files changed, 9 insertions(+), 12 deletions(-)

commit a486318dadb3c46442a2c905fcaa04d17846dc1f
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=a486318dadb3c46442a2c905fcaa04d17846dc1f;hp=3692c239d7c6a038bea4ef889ae5781be9eabe18
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:56 2018 +0100

    Remove unused params from cmdline_get_passwd_input.
    
    NFC; I expect this to be a useful simplification for the same reasons
    as the previous commit.

 cmdline.c          | 4 ++--
 nocmdline.c        | 2 +-
 putty.h            | 2 +-
 unix/gtkwin.c      | 2 +-
 unix/uxplink.c     | 2 +-
 unix/uxsftp.c      | 2 +-
 windows/window.c   | 2 +-
 windows/winplink.c | 2 +-
 windows/winsftp.c  | 2 +-
 9 files changed, 10 insertions(+), 10 deletions(-)

commit e3bdd6231e233813ce8edc4aad98c24c402b4dac
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=e3bdd6231e233813ce8edc4aad98c24c402b4dac;hp=a486318dadb3c46442a2c905fcaa04d17846dc1f
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:56 2018 +0100

    ssh.c: new data type 'struct PacketQueue'.
    
    This is just a linked list of 'struct Packet' with a convenience API
    on the front. As yet it's unused, so ssh.c will currently not compile
    with gcc -Werror unless you also add -Wno-unused-function. But all the
    functions I've added here will be used in later commits in the current
    patch series, so that's only a temporary condition.

 ssh.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

commit 9d96c3eb02121156002e75fd516b47dcb4cc59cb
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=9d96c3eb02121156002e75fd516b47dcb4cc59cb;hp=e3bdd6231e233813ce8edc4aad98c24c402b4dac
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    bufchain: new combined fetch + consume functions.
    
    bufchain_fetch_consume is a one-stop function that moves a given
    number of bytes out of the head of a bufchain into an output buffer,
    removing them from the bufchain in the process.
    
    That function will fail an assertion (just like bufchain_fetch) if the
    bufchain doesn't actually _have_ at least that many bytes to read, so
    I also provide bufchain_try_fetch_consume which will return a success
    or failure status.
    
    Nothing uses these functions yet, but they will.

 misc.c | 16 ++++++++++++++++
 misc.h |  2 ++
 2 files changed, 18 insertions(+)

commit cfc3386a156f336c33dfe7ab7be4b64b8af57780
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=cfc3386a156f336c33dfe7ab7be4b64b8af57780;hp=9d96c3eb02121156002e75fd516b47dcb4cc59cb
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    Add a reference count in 'struct Packet'.
    
    This is another piece of not-yet-used infrastructure, which later on
    will simplify my life when I start processing PacketQueues and adding
    some of their packets to other PacketQueues, because this way the code
    can unref every packet removed from the source queue in the same way,
    whether or not the packet is actually finished with.

 ssh.c | 48 ++++++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

commit 2ee07f8c71aca0e9022f772901a10d1a52cf310a
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=2ee07f8c71aca0e9022f772901a10d1a52cf310a;hp=cfc3386a156f336c33dfe7ab7be4b64b8af57780
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    Add a concept of 'idempotent callback'.
    
    This is a set of convenience wrappers around the existing toplevel
    callback function, which arranges to avoid scheduling a second call to
    a callback function if one is already in the queue.
    
    Just like the last few commits, this is a piece of infrastructure that
    nothing is yet using. But it will.

 callback.c | 19 ++++++++++++++++++-
 putty.h    | 16 ++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

commit 7400653bc8321c59ee8a8e0d4132e45a45c1f7b3
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=7400653bc8321c59ee8a8e0d4132e45a45c1f7b3;hp=2ee07f8c71aca0e9022f772901a10d1a52cf310a
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    New coroutine 'crMaybeWait' macros, which may not return.
    
    The crWaitUntil macros have do-while type semantics, i.e. they always
    crReturn _at least_ once, and then perhaps more times if their
    termination condition is still not met. But sometimes a coroutine will
    want to wait for a condition that may _already_ be true - the key
    examples being non-emptiness of a bufchain or a PacketQueue, which may
    already be non-empty in spite of you having just removed something
    from its head.
    
    In that situation, it's obviously more convenient not to bother with a
    crReturn in the first place than to do one anyway and have to fiddle
    about with toplevel callbacks to make sure we resume later. So here's
    a new pair of macros crMaybeWaitUntil{,V}, which have the semantics of
    while rather than do-while, i.e. they test the condition _first_ and
    don't return at all if it's already met.

 ssh.c | 2 ++
 1 file changed, 2 insertions(+)

commit 9d495b21764ec1251e250882a2ff4074196ed0a1
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=9d495b21764ec1251e250882a2ff4074196ed0a1;hp=7400653bc8321c59ee8a8e0d4132e45a45c1f7b3
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    Make {term,}get_userpass_input take a bufchain.
    
    NFC for the moment, because the bufchain is always specially
    constructed to hold exactly the same data that would have been passed
    in to the function as a (pointer,length) pair. But this API change
    allows get_userpass_input to express the idea that it consumed some
    but not all of the data in the bufchain, which means that later on
    I'll be able to point the same function at a longer-lived bufchain
    containing the full stream of keyboard input and avoid dropping
    keystrokes that arrive too quickly after the end of an interactive
    password prompt.

 putty.h            |  7 +++---
 rlogin.c           | 23 ++++++++++++++----
 ssh.c              | 69 ++++++++++++++++++++++++++++++++++++++++--------------
 terminal.c         | 11 ++++-----
 unix/gtkwin.c      |  4 ++--
 unix/uxplink.c     |  2 +-
 unix/uxsftp.c      |  2 +-
 windows/window.c   |  4 ++--
 windows/winplink.c |  4 ++--
 windows/winsftp.c  |  4 ++--
 10 files changed, 88 insertions(+), 42 deletions(-)

commit 2b57b84fa5e84ff3eb2b84b8561be9463e7802ff
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=2b57b84fa5e84ff3eb2b84b8561be9463e7802ff;hp=9d495b21764ec1251e250882a2ff4074196ed0a1
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    Make the rdpkt functions output to a PacketQueue.
    
    Each of the coroutines that parses the incoming wire data into a
    stream of 'struct Packet' now delivers those packets to a PacketQueue
    called ssh->pq_full (containing the full, unfiltered stream of all
    packets received on the SSH connection), replacing the old API in
    which each coroutine would directly return a 'struct Packet *' to its
    caller, or NULL if it didn't have one ready yet.
    
    This simplifies the function-call API of the rdpkt coroutines (they
    now return void). It increases the complexity at the other end,
    because we've now got a function ssh_process_pq_full (scheduled as an
    idempotent callback whenever rdpkt appends anything to the queue)
    which pulls things out of the queue and passes them to ssh->protocol.
    But that's only a temporary complexity increase; by the time I finish
    the upcoming stream of refactorings, there won't be two chained
    functions there any more.
    
    One small workaround I had to add in this commit is a flag called
    'pending_newkeys', which ssh2_rdpkt sets when it's just returned an
    SSH_MSG_NEWKEYS packet, and then waits for the transport layer to
    process the NEWKEYS and set up the new encryption context before
    processing any more wire data. This wasn't necessary before, because
    the old architecture was naturally synchronous - ssh2_rdpkt would
    return a NEWKEYS, which would be immediately passed to
    do_ssh2_transport, which would finish processing it immediately, and
    by the time ssh2_rdpkt was next called, the keys would already be in
    place.
    
    This change adds a big while loop around the whole of each rdpkt
    function, so it's easiest to read it as a whitespace-ignored diff.

 ssh.c | 849 +++++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 447 insertions(+), 402 deletions(-)

commit fe6caf563cfc3a119b1a79503705e7cd547868f8
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=fe6caf563cfc3a119b1a79503705e7cd547868f8;hp=2b57b84fa5e84ff3eb2b84b8561be9463e7802ff
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    Put all incoming SSH wire data into a bufchain.
    
    I've completely removed the top-level coroutine ssh_gotdata(), and
    replaced it with a system in which ssh_receive (which is a plug
    function, i.e. called directly from the network code) simply adds the
    incoming data to a new bufchain called ssh->incoming_data, and then
    queues an idempotent callback to ensure that whatever function is
    currently responsible for the top-level handling of wire data will be
    invoked in the near future.
    
    So the decisions that ssh_gotdata was previously making are now made
    by changing which function is invoked by that idempotent callback:
    when we finish doing SSH greeting exchange and move on to the packet-
    structured main phase of the protocol, we just change
    ssh->current_incoming_data_fn and ensure that the new function gets
    called to take over anything still outstanding in the queue.
    
    This simplifies the _other_ end of the API of the rdpkt functions. In
    the previous commit, they stopped returning their 'struct Packet'
    directly, and instead put it on a queue; in this commit, they're no
    longer receiving a (data, length) pair in their parameter list, and
    instead, they're just reading from ssh->incoming_data. So now, API-
    wise, they take no arguments at all except the main 'ssh' state
    structure.
    
    It's not just the rdpkt functions that needed to change, of course.
    The SSH greeting handlers have also had to switch to reading from
    ssh->incoming_data, and are quite substantially rewritten as a result.
    (I think they look simpler in the new style, personally.)
    
    This new bufchain takes over from the previous queued_incoming_data,
    which was only used at all in cases where we throttled the entire SSH
    connection. Now, data is unconditionally left on the new bufchain
    whether we're throttled or not, and the only question is whether we're
    currently bothering to read it; so all the special-purpose code to
    read data from a bufchain and pass it to rdpkt can go away, because
    rdpkt itself already knows how to do that job.
    
    One slightly fiddly point is that we now have to defer processing of
    EOF from the SSH server: if we have data already in the incoming
    bufchain and then the server slams the connection shut, we want to
    process the data we've got _before_ reacting to the remote EOF, just
    in case that data gives us some reason to change our mind about how we
    react to the EOF, or a last-minute important piece of data we might
    need to log.

 ssh.c | 541 +++++++++++++++++++++++++++++++-----------------------------------
 1 file changed, 257 insertions(+), 284 deletions(-)

commit 0ce92248a08ced3bc33b212d8841b9a8eb322d38
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=0ce92248a08ced3bc33b212d8841b9a8eb322d38;hp=fe6caf563cfc3a119b1a79503705e7cd547868f8
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    Factor out general processing for all packets.
    
    NFC: I'm just moving a small piece of code out into a separate
    function, which does processing on incoming SSH-2 packets that is
    completely independent of the packet type. (Specifically, we count up
    the total amount of data so far transferred, and use it to trigger a
    rekey when we get over the per-session-key data limit.)
    
    The aim is that I'll be able to call this function from a central
    location that's not SSH-2 specific, by using a function pointer that
    points to this function in SSH-2 mode or is null in SSH-1 mode.

 ssh.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

commit 0a15a2c47192e33f4babdd2a9cf7c4648858f987
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=0a15a2c47192e33f4babdd2a9cf7c4648858f987;hp=0ce92248a08ced3bc33b212d8841b9a8eb322d38
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    Unconditionally fill the SSH-1 dispatch table.
    
    In SSH-2, every possible packet type code has a non-NULL entry in the
    dispatch table, even if most of them are just ssh2_msg_unimplemented.
    In SSH-1, some dispatch table entries are NULL, which means that the
    code processing the dispatch table has to have some SSH-1 specific
    fallback logic.
    
    Now I've put the fallback logic in a separate function, and replaced
    the NULL table entries with pointers to that function, so that another
    pointless difference between the SSH-1 and SSH-2 code is removed.

 ssh.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

commit 5d9adc5c936229c24a240d2f7c44c48c0f00c7cf
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=5d9adc5c936229c24a240d2f7c44c48c0f00c7cf;hp=0a15a2c47192e33f4babdd2a9cf7c4648858f987
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    Stop passing incoming packets through ssh->protocol.
    
    After the previous two refactorings, there's no longer any need to
    pass packets to ssh1_protocol or ssh2_protocol so that each one can do
    its own thing with them, because now the handling is the same in both
    cases: first call the general type-independent packet processing code
    (if any), and then call the dispatch table entry for the packet type
    (which now always exists).

 ssh.c | 52 +++++++++++++++++++---------------------------------
 1 file changed, 19 insertions(+), 33 deletions(-)

commit bf62c850510d26e0653335c10aabb427d634ee8f
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=bf62c850510d26e0653335c10aabb427d634ee8f;hp=5d9adc5c936229c24a240d2f7c44c48c0f00c7cf
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:57 2018 +0100

    Stop using ssh->protocol_initial_phase_done in SSH-1.
    
    This flag was used to indicate that ssh1_protocol (or, as of the
    previous commit, ssh1_coro_wrapper) should stop passing packets to
    do_ssh1_login and start passing them to do_ssh1_connection.
    
    Now, instead of using a flag, we simply have two separate versions of
    ssh1_coro_wrapper for the two phases, and indicate the change by
    rewriting all the entries in the dispatch table. So now we _just_ have
    a function-pointer dereference per packet, rather than one of those
    and then a flag check.

 ssh.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

commit f10a65dfe85dc66ad0a47dbf24048a8d7f150b0b
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=f10a65dfe85dc66ad0a47dbf24048a8d7f150b0b;hp=bf62c850510d26e0653335c10aabb427d634ee8f
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    Put all user input into a bufchain.
    
    This change introduces a new bufchain ssh->user_input, into which we
    put every byte received via back->send() (i.e. keyboard input from the
    GUI PuTTY window, data read by Plink from standard input, or outgoing
    SCP/SFTP protocol data made up by the file transfer utilities).
    
    Just like ssh->incoming_data, there's also a function pointer
    ssh->current_user_input_fn which says who currently has responsibility
    for pulling data back off that bufchain and processing it. So that can
    be changed over when the connection moves into a different major phase.
    
    At the moment, nothing very interesting is being done with this
    bufchain: each phase of the connection has its own little function
    that pulls chunks back out of it with bufchain_prefix and passes them
    to the unchanged main protocol coroutines. But this is groundwork for
    being able to switch over each of those coroutines in turn to read
    directly from ssh->user_input, with the aim of eliminating a
    collection of annoying bugs in which typed-ahead data is accidentally
    discarded at an SSH phase transition.

 ssh.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 3 deletions(-)

commit c3abc304057bd489f640aeb2df824a496b3e0142
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=c3abc304057bd489f640aeb2df824a496b3e0142;hp=f10a65dfe85dc66ad0a47dbf24048a8d7f150b0b
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    Remove the ssh*_protocol() functions completely.
    
    After the last few commits, neither incoming SSH packets nor incoming
    user input goes through those functions any more - each of those
    directions of data goes into a queue and from there to a callback
    specifically processing that queue. So the centralised top-level
    protocol switching functions have nothing left to switch, and can go.

 ssh.c | 59 +++--------------------------------------------------------
 1 file changed, 3 insertions(+), 56 deletions(-)

commit 265365ab8080606ad82e83c335151df60477fa8b
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=265365ab8080606ad82e83c335151df60477fa8b;hp=c3abc304057bd489f640aeb2df824a496b3e0142
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh1_login: remove user input parameters.
    
    This is the first refactoring of a major coroutine enabled by adding
    the ssh->user_input queue. Now, instead of receiving a fixed block of
    parameter data, do_ssh1_login reads directly from the user input
    bufchain.
    
    In particular, I can get rid of all the temporary bufchains I
    constructed to pass to get_userpass_input (or rather, the ones in this
    particular function), because now we can let get_userpass_input read
    directly from the main user_input bufchain, and it will read only as
    much data as it has an interest in, and leave the rest as type-ahead
    for future prompts or the main session.

 ssh.c | 52 ++++++++++++++++++----------------------------------
 1 file changed, 18 insertions(+), 34 deletions(-)

commit 96d9d788f61b012bb2a7aee1a7d79d8ddea67ebd
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=96d9d788f61b012bb2a7aee1a7d79d8ddea67ebd;hp=265365ab8080606ad82e83c335151df60477fa8b
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh1_login: change return type to void.
    
    Now it does its post-completion work itself instead of telling the
    callee to do the same. So its caller, ssh1_coro_wrapper_initial, is
    now a _completely_ trivial wrapper - but I'm not taking the
    opportunity to fold the two functions together completely, because the
    wrapper is going to acquire a new purpose in the next commit :-)

 ssh.c | 105 +++++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 55 insertions(+), 50 deletions(-)

commit 6cfe0a212e8e47ac687fa6703c7f962b82cda099
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=6cfe0a212e8e47ac687fa6703c7f962b82cda099;hp=96d9d788f61b012bb2a7aee1a7d79d8ddea67ebd
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh1_login: get packets from a PacketQueue.
    
    This introduces the first of several filtered PacketQueues that
    receive subsets of pq_full destined for a particular coroutine. The
    wrapper function ssh1_coro_wrapper_initial, whose purpose I just
    removed in the previous commit, now gains the replacement purpose of
    accepting a packet as a function argument and putting it on the new
    queue for do_ssh1_login to handle when it's ready. That wrapper in
    turn is called from the packet-type dispatch table, meaning that the
    new pq_ssh1_login will be filtered down to only the packets destined
    for this coroutine.
    
    This is the point where I finally start using the reference counting
    system that I added to 'struct Packet' about a dozen commits ago. The
    general packet handling calls ssh_unref_packet for everything that
    it's just pulled off pq_full and handed to a dispatch-table function -
    so _this_ dispatch-table function, which needs the packet not to be
    freed because it's going to go on to another queue and wait to be
    handled there, can arrange that by incrementing its ref count.
    
    This completes the transformation of do_ssh1_login into a function
    with a trivial argument list, whose job is to read from a pair of
    input queues (one for user keyboard input and one for SSH packets) and
    respond by taking action directly rather than returning a value to its
    caller to request action.
    
    It also lets me get rid of a big pile of those really annoying
    bombout() calls that I used to work around the old coroutine system's
    inability to deal with receiving an SSH packet when the control flow
    was in the middle of waiting for some other kind of input. That was
    always the weakness of the coroutine structure of this code, which I
    accepted as the price for the convenience of coroutines the rest of
    the time - but now I think I've got the benefits without that cost :-)
    
    The one remaining argument to do_ssh1_login is the main Ssh structure,
    although I've had to turn it into a void * to make the function's type
    compatible with the idempotent callback mechanism, since that will be
    calling do_ssh1_login directly when its input queue needs looking at.

 ssh.c | 222 ++++++++++++++++++++++++++++++++----------------------------------
 1 file changed, 106 insertions(+), 116 deletions(-)

commit 364b3a283891dcdba1bea024d311bf93d7e2fcb5
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=364b3a283891dcdba1bea024d311bf93d7e2fcb5;hp=6cfe0a212e8e47ac687fa6703c7f962b82cda099
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh2_authconn: remove user input parameters.
    
    Just as I did to do_ssh1_login, I'm removing the 'in' and 'inlen'
    parameters from the combined SSH-2 userauth + connection coroutine, in
    favour of it reading directly from ssh->user_input, and in particular
    also allowing get_userpass_input to do so on its behalf.
    
    Similarly to the previous case, I've completely emptied the wrapper
    function ssh2_authconn_input(), and again, there is a reason I can't
    quite get away with removing it...

 ssh.c | 169 ++++++++++++++++++++++++++++++++----------------------------------
 1 file changed, 81 insertions(+), 88 deletions(-)

commit e2b7f4f374fd201a405d7785b15d8af9acbb278e
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=e2b7f4f374fd201a405d7785b15d8af9acbb278e;hp=364b3a283891dcdba1bea024d311bf93d7e2fcb5
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh2_authconn: get packets from a PacketQueue.
    
    Just like do_ssh1_login, do_ssh2_authconn is now no longer receiving
    its packets via function arguments; instead, it's pulling them off a
    dedicated PacketQueue populated by particular dispatch table entries,
    with the same reference-counting system to stop them being freed when
    the pq_full processing function regains control.
    
    This eliminates further ugly bombout()s from the code while waiting
    for authentication responses from the user or the SSH agent.
    
    As I mentioned in the previous commit, I've had to keep
    ssh2_authconn_input separate from do_ssh2_authconn, and this time the
    reason why is thoroughly annoying: it's because do_ssh2_authconn has
    changed its prototype to take the Ssh structure as a void * so that it
    can be called from the idempotent callback system, whereas
    ssh2_authconn_input has to have the type of ssh->current_user_input_fn
    which takes the more sensible pointer type 'Ssh'.

 ssh.c | 76 +++++++++++++++++++++++++++++++++----------------------------------
 1 file changed, 37 insertions(+), 39 deletions(-)

commit a41eefff41f994c5d01978f8b2dd10683403e9ba
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=a41eefff41f994c5d01978f8b2dd10683403e9ba;hp=e2b7f4f374fd201a405d7785b15d8af9acbb278e
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh2_authconn: replace s->gotit with pq_push_front.
    
    The userauth loop has always had a rather awkward feature whereby some
    of its branches have _already_ received an SSH_MSG_USERAUTH_SUCCESS or
    SSH_MSG_USERAUTH_FAILURE packet (e.g. because they have to wait for a
    packet that might be one of those _or_ a continuation packet of some
    kind), whereas other branches go back round to the top of the loop at
    the moment that they know one of those will be the next packet to
    arrive. So we had a flag 's->gotit' which tells the start of the loop
    whether it's already sitting on the success or failure message, or
    whether the first thing it should do is to crWait for one.
    
    Now that the packets are coming from a linked list, there's a simpler
    way to handle this: the top of the userauth loop _always_ expects to
    find a success/failure message as the first thing in the queue, and if
    any branch of the auth code finds it's already removed such a message
    from the queue, it can simply put it back on the front again before
    going back round.

 ssh.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

commit 7c47a17b4e00da0a639e7ccf9421f093388d52bd
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=7c47a17b4e00da0a639e7ccf9421f093388d52bd;hp=a41eefff41f994c5d01978f8b2dd10683403e9ba
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh1_connection: remove user input parameters.
    
    The connection protocol didn't have to do anything too complicated
    with user input - just remember to check for it and turn it into
    SSH1_CMSG_STDIN_DATA - so this is a much less involved change than the
    previous user input conversions.

 ssh.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

commit 38fccbf4aa9d998b559a3d2b163c7ad42049df97
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=38fccbf4aa9d998b559a3d2b163c7ad42049df97;hp=7c47a17b4e00da0a639e7ccf9421f093388d52bd
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh1_connection: get packets from a PacketQueue.
    
    Even with my love of verbose commit messages, I don't think there's
    anything interesting to say about this commit that the previous few
    similar ones haven't mentioned already. This is just more of the same
    transformations.

 ssh.c | 63 ++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 34 insertions(+), 29 deletions(-)

commit 322fb59105a9029f1971c358e520004b20ecc73b
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=322fb59105a9029f1971c358e520004b20ecc73b;hp=38fccbf4aa9d998b559a3d2b163c7ad42049df97
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh2_transport: remove user input parameters.
    
    This is not as trivial as it sounds, because although
    do_ssh2_transport never uses user input, its in/inlen parameter pair
    were nonetheless actually doing something, because by setting inlen to
    special values -1 or -2 they doubled as a way to initiate a rekey with
    a stated textual reason. So _that_ use of in and inlen has to be
    replaced. (Good thing too - that was a horribly ugly API!)
    
    I've replaced them with a new pair of fields in the main ssh
    structure, a textual "rekey_reason" for the Event Log and an
    enumeration "rekey_class" for discrimination within the code. In
    particular, that allows me to get rid of the ugly strcmp that was
    checking the textual rekey reason to find out whether the rekey was
    due to a GSSAPI credentials update - now there's a separate enum value
    for that kind of rekey and we can test for that more sensibly.

 ssh.c | 172 +++++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 92 insertions(+), 80 deletions(-)

commit 4e4a1a3a6276e6437bb96cb8a4533aa34a90552b
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=4e4a1a3a6276e6437bb96cb8a4533aa34a90552b;hp=322fb59105a9029f1971c358e520004b20ecc73b
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:58 2018 +0100

    do_ssh2_transport: get packets from a PacketQueue.
    
    More of the same; no specially interesting features here.

 ssh.c | 86 +++++++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 50 insertions(+), 36 deletions(-)

commit ea7f3c2abc9a83b2ff779dccafa4f3004283befe
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=ea7f3c2abc9a83b2ff779dccafa4f3004283befe;hp=4e4a1a3a6276e6437bb96cb8a4533aa34a90552b
Author: Simon Tatham <anakin at pobox.com>
Date:   Fri May 18 07:22:59 2018 +0100

    Separate the SSH-2 userauth and connection coroutines.
    
    do_ssh2_authconn is no more! In its place are two separate functions
    do_ssh2_userauth and do_ssh2_connection, each handling just one of the
    three main protocol components of SSH-2, and each with its own
    separate packet queue feeding from pq_full via the dispatch table.
    
    This should be entirely NFC: no bugs are fixed as a side effect of
    this change. It's mostly done in the hope that further refactoring in
    the future might finally be able to split up ssh.c into smaller source
    files, for which it's useful to have these two routines separate,
    since each one will be talking to a completely different set of
    supporting machinery.

 ssh.c | 280 +++++++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 165 insertions(+), 115 deletions(-)



More information about the tartarus-commits mailing list