simon-git: putty (master): Simon Tatham

Sun Jun 3 08:33:27 BST 2018

TL;DR:
  6cbca87 Try harder not to call connection_fatal twice.
  7079cf0 Outgoing packet logging: log the right amount of data.
  2b54c86 Stop calling ssh2_set_window in SSH-1!
  314c8f5 Connection sharing: handle reply to cancel-tcpip-forward.
  3f1f7c3 Remove downstream remote port forwardings in ssh.c too.
  ae38636 Give rsa_fingerprint() a new name and API.
  ff11e10 Rename rsa_public_blob_len to mention SSH-1.
  7f56e1e Remove 'keystr' parameter in get_rsa_ssh1_pub.

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-06-03 08:33:27

commit 6cbca87a62683a0342b3bf9265dc833cf7f3e918
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=6cbca87a62683a0342b3bf9265dc833cf7f3e918;hp=6dc63925965f34e0e898c2c5e465476c536a215f
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Jun 3 06:46:28 2018 +0100

    Try harder not to call connection_fatal twice.

    If the server sends an SSH_MSG_DISCONNECT, then we call
    connection_fatal(). But if the server closes the network connection,
    then we call connection_fatal(). In situations where the former
    happens, the latter happens too.

    Currently, calling connection_fatal twice is especially bad on GTK
    because all dialogs are now non-modal and an assertion fails in the
    GTK front end when two fatal message boxes try to exist at the same
    time (the register_dialog system finds that slot is already occupied).

    But regardless of that, we'd rather not even _try_ to print two fatal
    boxes, because even if the front end doesn't fail an assertion,
    there's no guarantee that the _more useful_ one of the messages will
    end up being displayed. So a better fix is to have ssh.c make a
    sensible decision about which message is the helpful one - in this
    case, the actual error message out of the SSH_MSG_DISCONNECT, rather
    than the predictable fact of the connection having been slammed shut
    immediately afterwards - and only pass that one to the front end in
    the first place.

 ssh.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

commit 7079cf06c8b814fb28d002247bcb22c478bc11a0
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=7079cf06c8b814fb28d002247bcb22c478bc11a0;hp=6cbca87a62683a0342b3bf9265dc833cf7f3e918
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Jun 3 07:08:47 2018 +0100

    Outgoing packet logging: log the right amount of data.

    I must have introduced this bug yesterday when I rewrote the packet
    censoring functions using BinarySource. The base pointer passed to
    log_packet was pointing at the right place, but the accompanying
    length was the gross rather than net one, as it were - it counted the
    extra header data we're about to insert at the _start_ of the packet,
    so log_packet() was trying to print that many extra bytes at the _end_
    and overrunning its buffer.

 ssh.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

commit 2b54c86e7e043ebae6f056943bdffd039bfd0ba7
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=2b54c86e7e043ebae6f056943bdffd039bfd0ba7;hp=7079cf06c8b814fb28d002247bcb22c478bc11a0
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Jun 3 07:11:10 2018 +0100

    Stop calling ssh2_set_window in SSH-1!

    This must have been a bug introduced during the SSH-2 connection
    sharing rework. Apparently nobody's ever re-tested SSH-1 X forwarding
    since then - until I did so yesterday in the course of testing my
    enormous refactor of the packet unmarshalling code.

 ssh.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

commit 314c8f5270a8a880faaf30bb11ad3130350e6f55
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=314c8f5270a8a880faaf30bb11ad3130350e6f55;hp=2b54c86e7e043ebae6f056943bdffd039bfd0ba7
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Jun 3 07:37:45 2018 +0100

    Connection sharing: handle reply to cancel-tcpip-forward.

    This is another bug that must have been around since connection
    sharing was introduced, and nobody noticed until I did some unusually
    thorough testing yesterday.

    When a sharing downstream asks to set up a remote port forwarding, we
    pass through the "tcpip-forward" global request, and we also intercept
    the reply so that we know that the forwarding has been set up (and
    hence that we should be passing "forwarded-tcpip" channel opens for
    that port to this downstream). To do that, we set the want-reply flag
    in the version of the packet we pass to the server, even if it was
    clear in downstream's version; and we also put an item on a queue
    local to sshshare.c which reminds us what to do about the reply when
    it comes back.

    But when the downstream _cancels_ one of those forwardings, I wrote
    the code for all parts of that process except adding that queue item.
    I even wrote the code to _consume_ the queue item, but somehow I
    completely forgot to generate one in the first place! So the enum
    value GLOBREQ_CANCEL_TCPIP_FORWARD was declared, tested for, but never
    actually assigned to anything.

 sshshare.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

commit 3f1f7c3ce765f3f60d23f7290910f8bfc5b05b0a
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=3f1f7c3ce765f3f60d23f7290910f8bfc5b05b0a;hp=314c8f5270a8a880faaf30bb11ad3130350e6f55
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Jun 3 07:54:00 2018 +0100

    Remove downstream remote port forwardings in ssh.c too.

    Another piece of half-finished machinery that I can't have tested
    properly when I set up connection sharing: I had the function
    ssh_alloc_sharing_rportfwd which is how sshshare.c asks ssh.c to start
    sending it channel-open requests for a given remote forwarded port,
    but I had no companion function that removes one of those requests
    again when a downstream remote port forwarding goes away (either by
    mid-session cancel-tcpip-forward or by the whole downstream
    disconnecting).

    As a result, the _second_ attempt to set up the same remote port
    forwarding, after a sharing downstream had done so once and then
    stopped, would quietly fail.

 ssh.c      | 15 +++++++++++++++
 ssh.h      |  2 ++
 sshshare.c |  8 ++++++++
 3 files changed, 25 insertions(+)

commit ae3863679d83910fd69fbdd1627d92a61ff112c5
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=ae3863679d83910fd69fbdd1627d92a61ff112c5;hp=3f1f7c3ce765f3f60d23f7290910f8bfc5b05b0a
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Jun 3 08:08:53 2018 +0100

    Give rsa_fingerprint() a new name and API.

    It's an SSH-1 specific function, so it should have a name reflecting
    that, and it didn't. Also it had one of those outdated APIs involving
    passing it a client-allocated buffer and size. Now it has a sensible
    name, and internally it constructs the output string using a strbuf
    and returns it dynamically allocated.

 cmdgen.c          |  3 +--
 pageant.c         | 22 ++++++++++++----------
 ssh.c             | 14 ++++++--------
 ssh.h             |  2 +-
 sshrsa.c          | 23 +++++++++--------------
 windows/winpgen.c | 25 ++++++++++---------------
 windows/winpgnt.c | 11 +++++++----
 7 files changed, 46 insertions(+), 54 deletions(-)

commit ff11e10d62ce4bd29f99015909f953adcb2abd43
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=ff11e10d62ce4bd29f99015909f953adcb2abd43;hp=ae3863679d83910fd69fbdd1627d92a61ff112c5
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Jun 3 08:12:57 2018 +0100

    Rename rsa_public_blob_len to mention SSH-1.

    It's yet another function with an outdatedly vague name.

 pageant.c | 2 +-
 ssh.h     | 2 +-
 sshrsa.c  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

commit 7f56e1e36527e8ca56706dd149e973fff83c0f8a
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=7f56e1e36527e8ca56706dd149e973fff83c0f8a;hp=ff11e10d62ce4bd29f99015909f953adcb2abd43
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Jun 3 08:23:07 2018 +0100

    Remove 'keystr' parameter in get_rsa_ssh1_pub.

    This parameter returned a substring of the input, which was used for
    two purposes. Firstly, it was used to hash the host and server keys
    during the initial SSH-1 key setup phase; secondly, it was used to
    check the keys in Pageant against the public key blob of a key
    specified on the command line.

    Unfortunately, those two purposes didn't agree! The first one needs
    just the bare key modulus bytes (without even the SSH-1 mpint length
    header); the second needs the entire key blob. So, actually, it seems
    to have never worked in SSH-1 to say 'putty -i keyfile' and have PuTTY
    find that key in Pageant and not have to ask for the passphrase to
    decrypt the version on disk.

    Fixed by removing that parameter completely, which simplifies all the
    _other_ call sites, and replacing it by custom code in those two
    places that each does the actually right thing.

 cmdgen.c      |  2 +-
 marshal.h     |  4 ++--
 pageant.c     |  8 ++++----
 ssh.c         | 23 ++++++++++++++++-------
 ssh.h         |  3 +--
 sshpubk.c     |  2 +-
 sshrsa.c      | 13 +------------
 unix/uxpgnt.c |  3 +--
 8 files changed, 27 insertions(+), 31 deletions(-)