simon-git: putty (pre-0.77): Simon Tatham

Commits to Tartarus hosted VCS tartarus-commits at lists.tartarus.org
Wed May 18 13:08:12 BST 2022


TL;DR:
  81dcbd62 Proxy: discard buffered input data on reconnection.
  787c358d Fix command-line password handling in Restart Session.

Repository:     https://git.tartarus.org/simon/putty.git
On the web:     https://git.tartarus.org/?p=simon/putty.git
Branch updated: pre-0.77
Committer:      Simon Tatham <anakin at pobox.com>
Date:           2022-05-18 13:08:12

commit 81dcbd6267db063367056cb0cf94eca4eb161991
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=81dcbd6267db063367056cb0cf94eca4eb161991;hp=0a877e9df5def14da30ab00046b501da20c9112a
Author: Simon Tatham <anakin at pobox.com>
Date:   Wed May 18 12:41:44 2022 +0100

    Proxy: discard buffered input data on reconnection.
    
    When talking to a web proxy which requires a password, our HTTP proxy
    code sends an initial attempt to connect without authentication,
    receives the 407 status indicating that authentication was required
    (and which kind), and then closes and reopens the connection (if given
    "Connection: close"). Then, on the next attempt, we try again with
    authentication, and expect the first thing in the input bufchain to be
    the HTTP status response to the revised request.
    
    But what happened about the error document that followed those HTTP
    headers? It - or at least some of it - would already have been in the
    input bufchain.
    
    With an HTTP/1.1 proxy, we already read it and discarded it (either
    via a Content-length header or chunked transfer encoding), before we
    set the 'reconnect' flag. So, assuming the proxy HTTP server is
    behaving sensibly, our input bufchain ought to be empty at the point
    when we start the fresh connection.
    
    But if the proxy only speaks HTTP/1.0 (which does still happen -
    'tinyproxy' is a still-current example), then we didn't get a
    Content-length header either, so we didn't run any of the code that
    skips over the error document. (And HTTP/1.0 implicitly has
    "Connection: close" semantics, which is why that doesn't matter.) As a
    result, some of it would still be in the input bufchain, and never got
    cleared out, and we'd try to parse _that_ as if it was the HTTP
    response from the second network connection.
    
    The simple solution is that when we close and reopen the proxy network
    connection, we also clear the input bufchain, so that the fresh
    connection starts from a clean slate.

 proxy/proxy.c | 7 +++++++
 1 file changed, 7 insertions(+)

commit 787c358d373bbef8383b385770747b1a6a4ab3ca
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=787c358d373bbef8383b385770747b1a6a4ab3ca;hp=81dcbd6267db063367056cb0cf94eca4eb161991
Author: Simon Tatham <anakin at pobox.com>
Date:   Wed May 18 13:04:56 2022 +0100

    Fix command-line password handling in Restart Session.
    
    When the user provides a password on the PuTTY command line, via -pw
    or -pwfile, the flag 'tried_once' inside cmdline_get_passwd_input() is
    intended to arrange that we only try sending that password once, and
    after we've sent it, we don't try again.
    
    But this plays badly with the 'Restart Session' operation. If the
    connection is lost and then restarted at user request, we _do_ want to
    send that password again!
    
    So this commit moves that static variable out into a small state
    structure held by the client of cmdline_get_passwd_input. Each client
    can decide how to manage that state itself.
    
    Clients that support 'Restart Session' - i.e. just GUI PuTTY itself -
    will initialise the state at the same time as instantiating the
    backend, so that every time the session is restarted, we return
    to (correctly) believing that we _haven't_ yet tried the password
    provided on the command line.
    
    But clients that don't support 'Restart Session' - i.e. Plink and file
    transfer tools - can do the same thing that cmdline.c was doing
    before: just keep the state in a static variable.
    
    This also means that the GUI login tools will now retain the
    command-line password in memory, whereas previously they'd have wiped
    it out once it was used. But the other tools will still wipe and free
    the password, because I've also added a 'bool restartable' flag to
    cmdline_get_passwd_input to let it know when it _is_ allowed to do
    that.
    
    In the GUI tools, I don't see any way to get round that, because if
    the session is restarted you _have_ to still have the password to use
    again. (And you can't infer that that will never happen from the
    CONF_close_on_exit setting, because that too could be changed in
    mid-session.) On the other hand, I think it's not all that worrying,
    because the use of either -pw or -pwfile means that a persistent copy
    of your password is *already* stored somewhere, so another one isn't
    too big a stretch.
    
    (Due to the change of -pw policy in 0.77, the effect of this bug was
    that an attempt to reconnect in a session set up this way would lead
    to "Configured password was not accepted". In 0.76, the failure mode
    was different: PuTTY would interactively prompt for the password,
    having wiped it out of memory after it was used the first time round.)

 cmdline.c                                  | 28 ++++++++++++++++++----------
 defs.h                                     |  2 ++
 putty.h                                    |  7 ++++++-
 stubs/nocmdline.c                          |  3 ++-
 unix/plink.c                               |  7 ++++++-
 unix/sftp.c                                |  8 +++++++-
 unix/window.c                              |  5 ++++-
 utils/CMakeLists.txt                       |  1 +
 utils/cmdline_get_passwd_input_state_new.c |  9 +++++++++
 windows/plink.c                            |  7 ++++++-
 windows/sftp.c                             |  8 +++++++-
 windows/window.c                           |  6 +++++-
 12 files changed, 73 insertions(+), 18 deletions(-)



More information about the tartarus-commits mailing list