1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-10 09:58:01 +00:00
Commit Graph

678 Commits

Author SHA1 Message Date
Jacob Nevins
63dddfc00f Old Dropbear servers have the ssh-close-vs-request bug.
Add automatic bug detection. (Versions verified by Matt Johnston.)
2015-04-23 23:42:45 +01:00
Simon Tatham
808e414130 Merge branch 'pre-0.64' 2015-02-28 07:57:58 +00:00
Simon Tatham
174476813f Enforce acceptable range for Diffie-Hellman server value.
Florent Daigniere of Matta points out that RFC 4253 actually
_requires_ us to refuse to accept out-of-range values, though it isn't
completely clear to me why this should be a MUST on the receiving end.

Matta considers this to be a security vulnerability, on the grounds
that if a server should accidentally send an obviously useless value
such as 1 then we will fail to reject it and agree a key that an
eavesdropper could also figure out. Their id for this vulnerability is
MATTA-2015-002.
2015-02-28 07:57:35 +00:00
Simon Tatham
c269dd0135 Move echo/edit state change functionality out of ldisc_send.
I'm not actually sure why we've always had back ends notify ldisc of
changes to echo/edit settings by giving ldisc_send(ldisc,NULL,0,0) a
special meaning, instead of by having a separate dedicated notify
function with its own prototype and parameter set. Coverity's recent
observation that the two kinds of call don't even have the same
requirements on the ldisc (particularly, whether ldisc->term can be
NULL) makes me realise that it's really high time I separated the two
conceptually different operations into actually different functions.

While I'm here, I've renamed the confusing ldisc_update() function
which that special operation ends up feeding to, because it's not
actually a function applying to an ldisc - it applies to a front end.
So ldisc_send(ldisc,NULL,0,0) is now ldisc_echoedit_update(ldisc), and
that in turn figures out the current echo/edit settings before passing
them on to frontend_echoedit_update(). I think that should be clearer.
2014-11-22 16:18:00 +00:00
Simon Tatham
d870b5650e Merge branch 'pre-0.64' 2014-11-22 16:02:01 +00:00
Simon Tatham
f454235444 Add some missing initialisations.
Spotted by valgrind, after I was testing all the Coverity bug fixes :-)
2014-11-22 15:26:16 +00:00
Simon Tatham
90dcef3d9e Fix assorted memory leaks.
All spotted by Coverity.
2014-11-22 15:26:13 +00:00
Simon Tatham
a918c97dc9 Merge connection-sharing shutdown fix from pre-0.64. 2014-11-10 18:32:12 +00:00
Simon Tatham
063c438fec Shut down connshare upstream along with the SSH connection.
This ought to happen in ssh_do_close alongside the code that shuts
down other local listening things like port forwardings, for the same
obvious reason. In particular, we should get through this _before_ we
put up a modal dialog box telling the user what just went wrong with
the SSH connection, so that further sessions started while that box is
active don't try futilely to connect to the not-really-listening
zombie upstream.
2014-11-10 18:31:34 +00:00
Jacob Nevins
0ab2e03ef2 Merge reconfig fixes from branch 'pre-0.64'. 2014-11-09 00:12:55 +00:00
Jacob Nevins
f662ff790c Disable some mid-session configs for downstreams.
Compression, encryption, and key exchange settings are all meaningless
to reconfigure in connection-sharing downstreams.
2014-11-09 00:10:46 +00:00
Jacob Nevins
3b4f3266d8 Merge SSH-2-only default from 'pre-0.64'. 2014-11-08 19:35:57 +00:00
Jacob Nevins
efb6aa4642 Tweak SSH protocol version refusal messages.
"required by user" will grate if the user did not configure the
behaviour (and I'm about to change the default to `2 only').
2014-11-08 18:38:33 +00:00
Ben Harris
cdb80025b7 Factor out code for constructing lists of algorithms for SSH-2 KEXINIT. 2014-11-03 21:47:28 +00:00
Chris Staite
2bf8688355 Elliptic-curve cryptography support.
This provides support for ECDSA public keys, for both hosts and users,
and also ECDH key exchange. Supported curves are currently just the
three NIST curves required by RFC 5656.
2014-11-02 18:16:54 +00:00
Simon Tatham
24cd95b6f9 Change the naming policy for connection-sharing Unix sockets.
I had initially assumed that, since all of a user's per-connection
subdirectories live inside a top-level putty-connshare.$USER directory
that's not accessible to anyone else, there would be no need to
obfuscate the names of the internal directories for privacy, because
nobody would be able to look at them anyway.

Unfortunately, that's not true: 'netstat -ax' run by any user will
show up the full pathnames of Unix-domain sockets, including pathname
components that you wouldn't have had the access to go and look at
directly. So the Unix connection sharing socket names do need to be
obfuscated after all.

Since Unix doesn't have Windows's CryptProtectMemory, we have to do
this manually, by creating a file of random salt data inside the
top-level putty-connshare directory (if there isn't one there already)
and then hashing that salt with the "user@host" connection identifier
to get the socket directory name. What a pain.

[originally from svn r10222]
2014-09-09 12:47:39 +00:00
Simon Tatham
70ab076d83 New option to manually configure the expected host key(s).
This option is available from the command line as '-hostkey', and is
also configurable through the GUI. When enabled, it completely
replaces all of the automated host key management: the server's host
key will be checked against the manually configured list, and the
connection will be allowed or disconnected on that basis, and the host
key store in the registry will not be either consulted or updated.

The main aim is to provide a means of automatically running Plink,
PSCP or PSFTP deep inside Windows services where HKEY_CURRENT_USER
isn't available to have stored the right host key in. But it also
permits you to specify a list of multiple host keys, which means a
second use case for the same mechanism will probably be round-robin
DNS names that select one of several servers with different host keys.

Host keys can be specified as the standard MD5 fingerprint or as an
SSH-2 base64 blob, and are canonicalised on input. (The base64 blob is
more unwieldy, especially with Windows command-line length limits, but
provides a means of specifying the _whole_ public key in case you
don't trust MD5. I haven't bothered to provide an analogous mechanism
for SSH-1, on the basis that anyone worrying about MD5 should have
stopped using SSH-1 already!)

[originally from svn r10220]
2014-09-09 11:46:24 +00:00
Simon Tatham
4b2a2060bb Fix another crash at KEXINIT time, ahem.
This is the same code I previously fixed for failing to check NULL
pointers coming back from ssh_pkt_getstring if the server's KEXINIT
ended early, leading to an embarrassing segfault in place of a fatal
error message. But I've now also had it pointed out to me that the
fatal error message passes the string as %s, which is inappropriate
because (being read straight out of the middle of an SSH packet) it
isn't necessarily zero-terminated!

This is still just an embarrassing segfault in place of a fatal error
message, and not exploitable as far as I can see, because the string
is passed to a dupprintf, which will either read off the end of
allocated address space and segfault non-exploitably, or else it will
find a NUL after all and carefully allocate enough space to format an
error message containing all of the previous junk. But still, how
embarrassing to have messed up the same code _twice_.

[originally from svn r10211]
2014-07-28 17:47:36 +00:00
Simon Tatham
aaaf70a0fc Implement this year's consensus on CHANNEL_FAILURE vs CHANNEL_CLOSE.
We now expect that after the server has sent us CHANNEL_CLOSE, we
should not expect to see any replies to our outstanding channel
requests, and conversely after we have sent CHANNEL_CLOSE we avoid
sending any reply to channel requests from the server. This was the
consensus among implementors discussing the problem on ietf-ssh in
April 2014.

To cope with current OpenSSH's (and perhaps other servers we don't
know about yet) willingness to send request replies after
CHANNEL_CLOSE, I introduce a bug-compatibility flag which is detected
for every OpenSSH version up to and including the current 6.6 - but
not beyond, since https://bugzilla.mindrot.org/show_bug.cgi?id=1818
promises that 6.7 will also implement the new consensus behaviour.

[originally from svn r10200]
2014-07-06 14:05:39 +00:00
Simon Tatham
a44530bd98 Add auto-recognition of BUG_SSH2_RSA_PADDING for ProFTPD.
Martin Prikryl reports that it had the exact same bug as old OpenSSH
(insisting that RSA signature integers be padded with leading zero
bytes to the same length as the RSA modulus, where in fact RFC 4253
section 6.6 says it ought to have _no_ padding), but is recently
fixed. The first version string to not have the bug is reported to be
"mod_sftp/0.9.9", so here we recognise everything less than that as
requiring our existing workaround.

[originally from svn r10161]
2014-03-27 18:07:13 +00:00
Simon Tatham
ee83fb6fdb Fix a potential crash in ssh_setup_portfwd.
If we search for a colon by computing ptr + host_strcspn(ptr,":"),
then the resulting pointer is always non-NULL, and the 'not found'
condition is not !p but !*p.

This typo could have caused PuTTY to overrun a string, but not in a
security-bug sense because any such string would have to have been
loaded from the configuration rather than received from a hostile
source.

[originally from svn r10123]
2014-01-25 15:59:04 +00:00
Simon Tatham
2b70f39061 Avoid misidentifying unbracketed IPv6 literals as host:port.
Both GUI PuTTY front ends have a piece of logic whereby a string is
interpreted as host:port if there's _one_ colon in it, but if there's
more than one colon then it's assumed to be an IPv6 literal with no
trailing port number. This permits the PuTTY command line to take
strings such as 'host', 'host:22' or '[::1]:22', but also cope with a
bare v6 literal such as '::1'.

This logic is also required in the two Plink front ends and in the
processing of CONF_loghost for host key indexing in ssh.c, but was
missing in all those places. Add it.

[originally from svn r10121]
2014-01-25 15:58:57 +00:00
Simon Tatham
8da4fa5063 Use the new host_str* functions to improve IPv6 literal support.
I've gone through everywhere we handle host names / addresses (on
command lines, in PuTTY config, in port forwarding, in X display
names, in host key storage...) and tried to make them handle IPv6
literals sensibly, by using the host_str* functions I introduced in my
previous commit. Generally it's now OK to use a bracketed IPv6 literal
anywhere a hostname might have been valid; in a few cases where no
ambiguity exists (e.g. no :port suffix is permitted anyway)
unbracketed IPv6 literals are also acceptable.

[originally from svn r10120]
2014-01-25 15:58:54 +00:00
Simon Tatham
16e834a98a Fix breakage of SSH-2 packet decompression by r10070.
The line that resets st->pktin->length to cover only the semantic
payload of the SSH message was overwriting the modification to
st->pktin->length performed by the optional decompression step. I
didn't notice because I don't habitually enable compression.

[originally from svn r10103]
[r10070 == 9f5d51a4ac]
2013-12-02 19:26:36 +00:00
Simon Tatham
85d1e7608e Fix an assortment of dupprintf() format string bugs.
I've enabled gcc's format-string checking on dupprintf, by declaring
it in misc.h to have the appropriate GNU-specific attribute. This
pointed out a selection of warnings, which I've fixed.

[originally from svn r10084]
2013-11-17 14:05:44 +00:00
Simon Tatham
bb78583ad2 Implement connection sharing between instances of PuTTY.
The basic strategy is described at the top of the new source file
sshshare.c. In very brief: an 'upstream' PuTTY opens a Unix-domain
socket or Windows named pipe, and listens for connections from other
PuTTYs wanting to run sessions on the same server. The protocol spoken
down that socket/pipe is essentially the bare ssh-connection protocol,
using a trivial binary packet protocol with no encryption, and the
upstream has to do some fiddly transformations that I've been
referring to as 'channel-number NAT' to avoid resource clashes between
the sessions it's managing.

This is quite different from OpenSSH's approach of using the Unix-
domain socket as a means of passing file descriptors around; the main
reason for that is that fd-passing is Unix-specific but this system
has to work on Windows too. However, there are additional advantages,
such as making it easy for each downstream PuTTY to run its own
independent set of port and X11 forwardings (though the method for
making the latter work is quite painful).

Sharing is off by default, but configuration is intended to be very
easy in the normal case - just tick one box in the SSH config panel
and everything else happens automatically.

[originally from svn r10083]
2013-11-17 14:05:41 +00:00
Simon Tatham
e5a3e28eec Get rid of the error-return mechanism from x11_init.
Now that it doesn't actually make a network connection because that's
deferred until after the X authorisation exchange, there's no point in
having it return an error message and write the real output through a
pointer argument. Instead, we can just have it return xconn directly
and simplify the call sites.

[originally from svn r10081]
2013-11-17 14:05:23 +00:00
Simon Tatham
cc4fbe33bc Prepare to have multiple X11 auth cookies valid at once.
Rather than the top-level component of X forwarding being an
X11Display structure which owns some auth data, it's now a collection
of X11FakeAuth structures, each of which owns a display. The idea is
that when we receive an X connection, we wait to see which of our
available auth cookies it matches, and then connect to whatever X
display that auth cookie identifies. At present the tree will only
have one thing in it; this is all groundwork for later changes.

[originally from svn r10079]
2013-11-17 14:05:10 +00:00
Simon Tatham
01085358e4 Decouple X socket opening from x11_init().
Now we wait to open the socket to the X server until we've seen the
authorisation data. This prepares us to do something else with the
channel if we see different auth data, which will come up in
connection sharing.

[originally from svn r10078]
2013-11-17 14:05:04 +00:00
Simon Tatham
b71b443c7c Add a missing null pointer check in s_write.
I don't know that this can ever be triggered in the current state of
the code, but when I start mucking around with SSH session closing in
the near future, it may be handy to have it.

[originally from svn r10076]
2013-11-17 14:04:56 +00:00
Simon Tatham
9cbcd17651 Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.

However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.

[originally from svn r10074]
2013-11-17 14:04:41 +00:00
Simon Tatham
518facfeca Complete rewrite of the packet log censoring code.
Because the upcoming connection sharing changes are going to involve
us emitting outgoing SSH packets into our log file that we didn't
construct ourselves, we can no longer rely on metadata inserted at
packet construction time to tell us which parts of which packets have
to be blanked or omitted in the SSH packet log. Instead, we now have
functions that deal with constructing the blanks array just before
passing all kinds of packet (both SSH-1 and SSH-2, incoming and
outgoing) to logging.c; the blanks/nblanks fields in struct Packet are
therefore no longer needed.

[originally from svn r10071]
2013-11-17 14:04:25 +00:00
Simon Tatham
9f5d51a4ac Clean up the 'data' vs 'body' pointers in struct Packet.
There's always been some confusion over exactly what it all means. I
haven't cleaned it up to the point of complete sensibleness, but I've
got it to a point where I can at least understand and document the
remaining non-sensibleness.

[originally from svn r10070]
2013-11-17 14:04:18 +00:00
Simon Tatham
0bc76b8252 Move SSH protocol enumerations out into ssh.h.
This permits packet type codes and other magic numbers to be accessed
from modules other than ssh.c.

[originally from svn r10064]
2013-11-17 14:03:29 +00:00
Simon Tatham
5ecb7d7f1d Clean up the semantics of the ssh_rportfwd structure.
It's now indexed by source hostname as well as source port (so that
separate requests for the server to listen on addr1:1234 and
addr2:1234 can be disambiguated), and also its destination host name
is dynamically allocated rather than a fixed-size buffer.

[originally from svn r10062]
2013-11-17 14:03:21 +00:00
Simon Tatham
043a762b5f Handle socket errors on half-open channels.
Anthony Ho reports that this can occur naturally in some situation
involving Windows 8 + IE 11 and dynamic port forwarding: apparently we
get through the SOCKS negotiation, send our CHANNEL_OPEN, and then
*immediately* suffer a local WSAECONNABORTED error before the server
has sent back its OPEN_CONFIRMATION or OPEN_FAILURE. In this situation
ssh2_channel_check_close was failing to notice that the channel didn't
yet have a valid server id, and sending out a CHANNEL_CLOSE anyway
containing 32 bits of uninitialised nonsense.

We now handle this by turning our half-open CHAN_SOCKDATA_DORMANT into
a half-open CHAN_ZOMBIE, which means in turn that our handler
functions for OPEN_CONFIRMATION and OPEN_FAILURE have to recognise and
handle that case, the former by immediately initiating channel closure
once we _do_ have the channel's server id to do it with.

[originally from svn r10039]
2013-09-08 13:20:49 +00:00
Simon Tatham
8e7b0d0e4b Pass an error message through to sshfwd_unclean_close.
We have access to one at every call site, so there's really no reason
not to send it through to ssh.c to be logged.

[originally from svn r10038]
2013-09-08 07:14:56 +00:00
Simon Tatham
7e515c4111 Fix free of an uninitialised pointer.
CHAN_AGENT channels need c->u.a.message to be either NULL or valid
dynamically allocated memory, because it'll be freed by
ssh_channel_destroy. This bug triggers if an agent forwarding channel
is opened and closed without having sent any queries.

[originally from svn r10032]
2013-08-26 11:55:56 +00:00
Simon Tatham
b8e668cd9b Sensibly enforce non-interactive rekeying.
We now only present the full set of host key algorithms we can handle
in the first key exchange. In subsequent rekeys, we present only the
host key algorithm that we agreed on the previous time, and then we
verify the host key by simply enforcing that it's exactly the same as
the one we saw at first and disconnecting rudely if it isn't.

[originally from svn r10027]
2013-08-18 06:48:20 +00:00
Simon Tatham
808df44e54 Add an assortment of missing consts I've just noticed.
[originally from svn r9972]
2013-07-27 18:35:48 +00:00
Simon Tatham
eaea69ef53 If the SSH server sends us CHANNEL_CLOSE for a channel on which we're
sitting on a pile of buffered data waiting for WINDOW_ADJUSTs, we
should throw away that buffered data, because the CHANNEL_CLOSE tells
us that we won't be receiving those WINDOW_ADJUSTs, and if we hang on
to the data and keep trying then it'll prevent ssh_channel_try_eof
from sending the CHANNEL_EOF which is a prerequisite of sending our
own CHANNEL_CLOSE.

[originally from svn r9953]
2013-07-21 10:12:58 +00:00
Simon Tatham
0d7f2fdabc In the various channel request mini-coroutines, replace
crWaitUntilV(pktin) with plain crReturnV, because those coroutines can
be called back either with a response packet from the channel request
_or_ with NULL by ssh_free meaning 'please just clean yourself up'.

[originally from svn r9927]
2013-07-14 17:08:35 +00:00
Simon Tatham
2f6d6a839d Move the calculation of the exchange hash to above the various
warnings about insecure crypto components. The latter may crReturn
(though not in any current implementation, I believe), which
invalidates pktin, which is used by the former.

[originally from svn r9921]
2013-07-14 10:46:27 +00:00
Simon Tatham
ea301bdd9b Fix another giant batch of resource leaks. (Mostly memory, but there's
one missing fclose too.)

[originally from svn r9919]
2013-07-14 10:46:07 +00:00
Simon Tatham
896bb7c74d Tighten up a lot of casts from unsigned to int which are read by one
of the GET_32BIT macros and then used as length fields. Missing bounds
checks against zero have been added, and also I've introduced a helper
function toint() which casts from unsigned to int in such a way as to
avoid C undefined behaviour, since I'm not sure I trust compilers any
more to do the obviously sensible thing.

[originally from svn r9918]
2013-07-14 10:45:54 +00:00
Simon Tatham
ac8baf4cac Move the SSH-1 servkey and hostkey variables into the coroutine state,
since there is a theoretical code path (via the crReturn loop after
asking an interactive question about a host key or crypto algorithm)
on which we can leave and return to do_ssh1_login between allocating
and freeing those keys.

(In practice it shouldn't come up anyway with any of the current
implementations of the interactive question functions, not to mention
the unlikelihood of anyone non-specialist still using SSH-1, but
better safe than sorry.)

[originally from svn r9895]
2013-07-07 14:34:37 +00:00
Simon Tatham
bbc9709b48 A collection of small bug fixes from Chris West, apparently spotted by
Coverity: assorted language-use goofs like freeing the wrong thing or
forgetting to initialise a string on all code paths.

[originally from svn r9889]
2013-07-01 17:56:33 +00:00
Ben Harris
8f3cc4a9bf Add support for HMAC-SHA-256 as an SSH-2 MAC algorithm ("hmac-sha2-256")
as specified in RFC 6668.  This is not so much because I think it's 
necessary, but because scrypt uses HMAC-SHA-256 and once we've got it we 
may as well use it.

Code very closely derived from the HMAC-SHA-1 code.

Tested against OpenSSH 5.9p1 Debian-5ubuntu1.

[originally from svn r9759]
2013-02-20 23:30:55 +00:00
Ben Harris
bc8f2193f6 Allow remote-to-local forwardings to use IPv6.
RFC 4245 section 7.1 specifies the meaning of the "address to bind"
parameter in a "tcpip-forward" request.  "0.0.0.0" and "127.0.0.1" are
specified to be all interfaces and the loopback interface respectively
in IPv4, while "" and "localhost" are the address-family-agnostic
equivalents.  Switch PuTTY to using the latter, since it doesn't seem
right to force IPv4.

There's an argument that PuTTY should provide a means of configuring the
address family used for remote forwardings like it does for local ones.

[originally from svn r9668]
2012-09-18 21:50:47 +00:00
Ben Harris
d5836982e2 Two related changes to timing code:
First, make absolute times unsigned.  This means that it's safe to 
depend on their overflow behaviour (which is undefined for signed 
integers).  This requires a little extra care in handling comparisons, 
but I think I've correctly adjusted them all.

Second, functions registered with schedule_timer() are guaranteed to be 
called with precisely the time that was returned by schedule_timer().  
Thus, it's only necessary to check these values for equality rather than 
doing risky range checks, so do that.

The timing code still does lots that's undefined, unnecessary, or just
wrong, but this is a good start.

[originally from svn r9667]
2012-09-18 21:42:48 +00:00
Ben Harris
8c1d1be956 Most of the code for "nc" mode duplicated that for opening a session or
a fowarded port.  Arrange that this code is shared instead.  The main
visible change is a slight change of log messages.

[originally from svn r9655]
2012-09-08 19:46:07 +00:00
Simon Tatham
14539a7719 Hiroshi Oota points out that PuTTY's agent forwarding sockets can get
confused if they receive a request followed by immediate EOF, since we
currently send outgoing EOF as soon as we see the incoming one - and
then, when the response comes back from the real SSH agent, we send it
along anyway as channel data in spite of having sent EOF.

To fix this, I introduce a new field for each agent channel which
counts the number of calls to ssh_agentf_callback that are currently
expected, and we don't send EOF on an agent channel until we've both
received EOF and that value drops to zero.

[originally from svn r9651]
2012-09-08 10:40:36 +00:00
Ben Harris
33c58dd91b Factor out some common code for constructing SSH2_CHANNEL_OPEN.
[originally from svn r9649]
2012-09-01 12:28:38 +00:00
Ben Harris
de6d59b500 struct winadj is unused now. G/c it.
[originally from svn r9648]
2012-09-01 12:10:27 +00:00
Ben Harris
98e562b7f6 All of the initial CHANNEL_REQUESTs are conditional on (ssh->mainchan &&
!ssh->ncmode), so bundle them up in a big block conditional on this rather
than checking it five times.

[originally from svn r9647]
2012-09-01 12:03:12 +00:00
Ben Harris
54da9ee07a do_ssh2_authconn() now installs the standard handlers for CHANNEL_SUCCESS
and CHANNEL_FAILURE as soon as it's opened a channel, so there's no need
for it to set them again later.

[originally from svn r9646]
2012-09-01 11:32:53 +00:00
Simon Tatham
b631c1e18e Rename the various ssh2_maybe_setup_* functions to ssh2_setup_*, and
move the primary conditions out of them into their callers. Fixes a
crash in 'plink -N', since those functions would be called with a NULL
channel parameter and immediately dereference it to try to get c->ssh.

[originally from svn r9644]
2012-08-30 18:44:35 +00:00
Ben Harris
e27ce2017e Change return type of do_ssh2_transport() to void.
Nothing pays attention to it any more, anyway.

[originally from svn r9635]
2012-08-27 22:02:17 +00:00
Ben Harris
3cc03d85e7 Make bombout() less of a macro and more of a function.
This gives GCC slightly fewer opportunities to gratuitously inflate
its output.

[originally from svn r9634]
2012-08-27 21:55:45 +00:00
Ben Harris
df83634e21 Fix a memory leak in parse_ttymodes() (found by Memcheck/Valgrind).
[originally from svn r9633]
2012-08-27 19:11:39 +00:00
Ben Harris
475507e978 Slightly simplify crFinishFree{,V}.
They're only likely to be useful for freeing a coroutine state
structure, in which case there's no need to reset the line number
(since all such coroutines keep their line number in the state
structure) and the state structure pointer is always called "s".

[originally from svn r9632]
2012-08-27 18:44:06 +00:00
Ben Harris
0768c8557d Don't close SSH-2 channels with outstanding channel requests on local error.
In sshfwd_unclean_close(), get ssh2_check_close() to handle sending
SSH_MSG_CHANNEL_CLOSE.  That way, it can hold off doing so until any
outstanding channel requests are processed.

Also add event log message for unclean channel closures.

[originally from svn r9631]
2012-08-27 17:37:44 +00:00
Simon Tatham
3fa95b2a7f It's not legal to free a coroutine's state structure before invoking
crFinish or crFinishV, since they will attempt to write to the
coroutine state variable contained in that structure. Introduced some
new all-in-one macros crFinishFree and crFinishFreeV, and used those
instead. Should fix today's report of a crash just after authentication.

[originally from svn r9630]
2012-08-27 14:34:41 +00:00
Ben Harris
3d466aec90 Better handling of outstanding CHANNEL_REQUESTS on channel destruction.
Part the first: make sure that all structures describing channel
requests are freed when the SSH connection is freed.  This involves
adding a means to ask a response handler to free any memory it holds.

Part the second: in ssh_channel_try_eof(), call
ssh2_channel_check_close() rather than emitting an SSH_MSG_CHANNEL_EOF
directly.  This avoids the possibility of closing the channel while a
CHANNEL_REQUEST is outstanding.

Also add some assertions that helped with tracking down the latter
problem.

[originally from svn r9623]
2012-08-25 21:06:48 +00:00
Ben Harris
4e623f5b23 Factor out common code to construct CHANNEL_REQUESTS.
This reduces code size a little and also makes it harder to
accidentally request a reply without putting in place a handler for
it or vice versa.

[originally from svn r9620]
2012-08-25 15:57:05 +00:00
Ben Harris
ab6d966f91 Simplify handling of responses to channel requests.
The various setup routines can only receive CHANNEL_SUCCESS or
CHANNEL_FAILURE, so there's no need for the to worry about receiving
anything else.  Strange packets will end up in do_ssh2_authconn
instead.

[originally from svn r9619]
2012-08-25 15:04:29 +00:00
Ben Harris
30e43ba2a3 Handle all replies to CHANNEL_REQUESTs through the per-channel queue.
Each of the minor start-of-session requests is now dealt with by its own 
little co-routine, while the shell/command is done in do_ssh2_authconn() 
itself.  This eliminates one more round-trip in session setup: PuTTY gets
all the way up to sending a shell request before worrying about any
replies.

[originally from svn r9616]
2012-08-25 13:39:32 +00:00
Ben Harris
57945a753f Generalise SSH_MSG_CHANNEL_{SUCCESS,FAILURE} handling.
Now each channel has a queue of arbitrary handlers for those messages, 
with anything that sends a CHANNEL_REQUEST with want_reply true pushing 
a new entry onto the queue, and a shared handler that dispatches 
responses appropriately.

Currently, this is only used for winadj@putty.projects.tartarus.org, but 
extending it to cover the initial requests as well shouldn't be too 
painful.

[originally from svn r9615]
2012-08-25 11:12:14 +00:00
Ben Harris
f071feb9a6 Fix recently-introduced memory leak in ssh2_msg_unexpected().
[originally from svn r9611]
2012-08-21 23:04:22 +00:00
Ben Harris
e148dd97e3 De-duplicate code in KEXINIT generation.
There's no need to have identical code generating server-to-client and
client-to-server versions of the cipher and MAC lists; a couple of
twice-around loops will do fine.

[originally from svn r9610]
2012-08-21 22:33:31 +00:00
Ben Harris
8e0ab8be59 Reworking of packet delivery to coroutines in SSH-2.
Before, NULL in the dispatch table meant "send to the appropriate one of 
do_ssh2_transport() and do_ssh2_authconn()".  Now those (via small 
shims) are specified directly in the dispatch table, so ssh2_protocol() 
is much simpler.

In the process, this has somewhat centralised the handling of gross 
server protocol violations.  PuTTY will now disconnect with a rude 
message when (e.g.) OpenSSH sends us an SSH_MSG_UNIMPLEMENTED when we 
try to KEXINIT during authentication.

[originally from svn r9609]
2012-08-21 22:04:56 +00:00
Ben Harris
4e5012aeda Fix an erroneous "case" fallthrough in ssh1_msg_channel_close, which was
causing assertion failures when closing X11 channels in SSH-1.  Also fix 
another pasto.

[originally from svn r9608]
2012-08-19 11:35:26 +00:00
Ben Harris
e570820c74 Tweak to SSH coroutine code: put line number in the coroutine state
structure, which is consistent with Simon's canonical version of the
macros.

[originally from svn r9607]
2012-08-18 09:30:01 +00:00
Ben Harris
37ea0f4541 Reduce the number of round-trips involved in opening an SSH-2 session
by sending most of the initial SSH_MSG_CHANNEL_REQUEST messages before
waiting for any replies.  The initial version of this code was a clever
thing with a two-pass loop, but that got hairy so I went for the simpler
approach of separating the request and reply code and having flags to
keep track of which requests have been sent.

[originally from svn r9599]
2012-08-02 22:18:18 +00:00
Simon Tatham
71f0c7546d Add a bug-compatibility flag to disable the
winadj@putty.projects.tartarus.org request. Not currently enabled
automatically, but should be usable as a manual workaround.

[originally from svn r9592]
2012-07-28 19:30:12 +00:00
Simon Tatham
aa5bae8916 Introduce a new utility function smemclr(), which memsets things to
zero but does it in such a way that over-clever compilers hopefully
won't helpfully optimise the call away if you do it just before
freeing something or letting it go out of scope. Use this for
(hopefully) every memset whose job is to destroy sensitive data that
might otherwise be left lying around in the process's memory.

[originally from svn r9586]
2012-07-22 19:51:50 +00:00
Simon Tatham
a46aac386b Add missing check for failure to agree a host key algorithm.
[originally from svn r9557]
2012-06-04 23:32:36 +00:00
Simon Tatham
b73527e641 When we are asked to unthrottle an SSH connection (by the front end
calling back->unthrottle), we should immediately call
ssh_process_queued_incoming_data to handle the SSH packets that have
been saved for later functioning while we were throttled. Otherwise,
they'll sit there unhandled until the next call to ssh_gotdata, which
might not be for ages if the server thinks it's waiting for us.

[originally from svn r9523]
2012-05-12 17:00:54 +00:00
Simon Tatham
7c61b3c733 If we're called on to uncleanly close an SSH channel for which we've
already sent SSH2_MSG_CHANNEL_CLOSE, we should not skip the _whole_ of
sshfwd_unclean_close(), only the part about sending
SSH2_MSG_CHANNEL_CLOSE. It's still important to retag the SSH channel
as CHAN_ZOMBIE and clean up its previous data provider.

[originally from svn r9389]
2012-01-26 18:22:28 +00:00
Simon Tatham
49927f6c4d Introduce a function sshfwd_unclean_close(), supplied by ssh.c to
subsidiary network modules like portfwd.c. To be called when the
subsidiary module experiences a socket error: it sends an emergency
CHANNEL_CLOSE (not just outgoing CHANNEL_EOF), and immediately deletes
the local side of the channel. (I've invented a new channel type in
ssh.c called CHAN_ZOMBIE, for channels whose original local side has
already been thrown away and they're just hanging around waiting to
receive the acknowledging CHANNEL_CLOSE.)

As a result of this and the last few commits, I can now run a port
forwarding session in which a local socket error occurs on a forwarded
port, and PuTTY now handles it apparently correctly, closing both the
SSH channel and the local socket and then actually recognising that
it's OK to terminate when all _other_ channels have been closed.
Previously the channel corresponding to the duff connection would
linger around (because of net_pending_errors never being called), and
keep being selected on (hence chewing CPU), and inhibit program
termination at the end of the session (because not all channels were
closed).

[originally from svn r9364]
2011-12-08 19:15:58 +00:00
Simon Tatham
8aa218e894 When we receive CHANNEL_CLOSE on an SSH-2 channel and haven't sent EOF
on it yet, we should send EOF on _that channel_, not the main session
channel! Oops.

[originally from svn r9362]
2011-12-08 19:15:55 +00:00
Simon Tatham
69a01afe48 Whitespace fix while I was passing.
[originally from svn r9361]
2011-12-08 19:15:54 +00:00
Simon Tatham
8e9868bda0 Make sure we never send window adjustments (or winadjes) on channels
for which we've already sent CHANNEL_CLOSE. It would be embarrassing
if the remote end had also sent CHANNEL_CLOSE in response and then
received our communication once it had forgotten about the channel.

[originally from svn r9360]
2011-12-08 19:15:53 +00:00
Simon Tatham
bb542eaeff Add a missing free_prompts() call in the keyboard-interactive code.
[originally from svn r9357]
2011-12-07 19:07:02 +00:00
Simon Tatham
da66c0656a While I'm crusading against arbitrary limits, here's a redesign of
prompt_t to arrange that the buffer in which each prompt is stored can
be reallocated larger during the input process.

[originally from svn r9317]
2011-10-02 11:50:45 +00:00
Simon Tatham
62cbc7dc0b Turn 'Filename' into a dynamically allocated type with no arbitrary
length limit, just as I did to FontSpec yesterday.

[originally from svn r9316]
2011-10-02 11:01:57 +00:00
Simon Tatham
aac16a33e8 No, I take that back: we _do_ have a mechanism for suppressing reads
from forwarding data sources which will be good enough to last until
we close the socket, in the form of the override_throttle() functions.
So this finishes up the work in r9283, by manufacturing outgoing EOF
in response to incoming CLOSE on all channel types.

[originally from svn r9284]
[r9283 == c54e228d04]
2011-09-14 09:49:00 +00:00
Simon Tatham
c54e228d04 Another tweak to EOF policy: invent an outgoing EOF on receipt of an
incoming CHANNEL_CLOSE, if it's the main session channel. The idea is
that invocations such as 'plink -T hostname sh' (running a shell
without a remote pty) can be exited by typing 'exit' to the remote
shell, without plink blocking forever waiting for outgoing EOF.

I think it would be better to do the same for all other channel types
too, but that would need an extra API call which I haven't
implemented yet.

[originally from svn r9283]
2011-09-14 09:09:35 +00:00
Simon Tatham
6e2bcd24a4 Changed my mind about the EOF policy in SSH mode: I think the SSH
backend should unilaterally assume outgoing EOF when it sees incoming
EOF, if and only if the main session channel is talking to a pty.
(Because ptys don't have a strong concept of EOF in the first place,
that seems like a sensible place to draw the line.) This fixes a bug
introduced by today's revamp in which if you used Unix Plink to run a
console session it would hang after you hit ^D - because the server
had sent EOF, but it was waiting for a client-side EOF too.

[originally from svn r9282]
2011-09-13 15:38:12 +00:00
Simon Tatham
5848c8cc3d Nearly forgot noting this down in the 'half-closed' bug entry: don't
send CHANNEL_CLOSE until we have acks for all our winadj requests.
Should work around https://bugzilla.mindrot.org/show_bug.cgi?id=1818 .

[originally from svn r9280]
2011-09-13 11:56:25 +00:00
Simon Tatham
947962e0b9 Revamp of EOF handling in all network connections, pipes and other
data channels. Should comprehensively fix 'half-closed', in principle,
though it's a big and complicated change and so there's a good chance
I've made at least one mistake somewhere.

All connections should now be rigorous about propagating end-of-file
(or end-of-data-stream, or socket shutdown, or whatever) independently
in both directions, except in frontends with no mechanism for sending
explicit EOF (e.g. interactive terminal windows) or backends which are
basically always used for interactive sessions so it's unlikely that
an application would be depending on independent EOF (telnet, rlogin).

EOF should now never accidentally be sent while there's still buffered
data to go out before it. (May help fix 'portfwd-corrupt', and also I
noticed recently that the ssh main session channel can accidentally
have MSG_EOF sent before the output bufchain is clear, leading to
embarrassment when it subsequently does send the output).

[originally from svn r9279]
2011-09-13 11:44:03 +00:00
Simon Tatham
363f57396f Minor amendment to r9226: don't log GSSAPI failure in the Event Log
twice. (Once in the GSSAPI code, once at the top of the main auth
loop. Removed the latter.)

[originally from svn r9232]
[r9226 == 0b79fe0bcb]
2011-07-18 18:09:36 +00:00
Simon Tatham
0b79fe0bcb Reorganise the logging and display of messages about SSH
authentication. We should now produce an Event Log entry for every
authentication attempted and every authentication failure; meanwhile,
messages in the PuTTY window will not be generated for the failure of
auth types unless we also announced in the PuTTY window that we were
trying them. (GSSAPI was getting the latter wrong, leading to spurious
'Access denied' for many users of 0.61.)

[originally from svn r9226]
2011-07-16 12:06:32 +00:00
Simon Tatham
a1f3b7a358 Post-release destabilisation! Completely remove the struct type
'Config' in putty.h, which stores all PuTTY's settings and includes an
arbitrary length limit on every single one of those settings which is
stored in string form. In place of it is 'Conf', an opaque data type
everywhere outside the new file conf.c, which stores a list of (key,
value) pairs in which every key contains an integer identifying a
configuration setting, and for some of those integers the key also
contains extra parts (so that, for instance, CONF_environmt is a
string-to-string mapping). Everywhere that a Config was previously
used, a Conf is now; everywhere there was a Config structure copy,
conf_copy() is called; every lookup, adjustment, load and save
operation on a Config has been rewritten; and there's a mechanism for
serialising a Conf into a binary blob and back for use with Duplicate
Session.

User-visible effects of this change _should_ be minimal, though I
don't doubt I've introduced one or two bugs here and there which will
eventually be found. The _intended_ visible effects of this change are
that all arbitrary limits on configuration strings and lists (e.g.
limit on number of port forwardings) should now disappear; that list
boxes in the configuration will now be displayed in a sorted order
rather than the arbitrary order in which they were added to the list
(since the underlying data structure is now a sorted tree234 rather
than an ad-hoc comma-separated string); and one more specific change,
which is that local and dynamic port forwardings on the same port
number are now mutually exclusive in the configuration (putting 'D' in
the key rather than the value was a mistake in the first place).

One other reorganisation as a result of this is that I've moved all
the dialog.c standard handlers (dlg_stdeditbox_handler and friends)
out into config.c, because I can't really justify calling them generic
any more. When they took a pointer to an arbitrary structure type and
the offset of a field within that structure, they were independent of
whether that structure was a Config or something completely different,
but now they really do expect to talk to a Conf, which can _only_ be
used for PuTTY configuration, so I've renamed them all things like
conf_editbox_handler and moved them out of the nominally independent
dialog-box management module into the PuTTY-specific config.c.

[originally from svn r9214]
2011-07-14 18:52:21 +00:00
Jacob Nevins
3a649ed4ed Fix 'winadj-success': since at least one server (boks_sshd) blithely responds
to "winadj@putty.projects.tartarus.org" with SSH_MSG_CHANNEL_SUCCESS despite
probably having no idea what it means, treat this just the same as
SSH_MSG_CHANNEL_FAILURE instead of killing the connection.
Tested only as far as making sure that winadj/FAILURE with a normal server
isn't _completely_ broken.

[originally from svn r9185]
[this svn revision also touched putty-wishlist]
2011-07-01 21:53:41 +00:00
Simon Tatham
6e40a0db57 When we fail to get a response from Pageant, we should log the fact.
Currently, if the IPC exchange goes wrong, the Event Log just prints
"Pageant is running. Requesting keys." and then goes on to the next
step without ever saying what happened.

[originally from svn r9177]
2011-06-08 20:39:06 +00:00
Simon Tatham
934a5ad6b2 Fixes (mostly from Colin Watson, a couple redone by me) to make Unix
PuTTY compile cleanly under gcc 4.6.0 without triggering any of its
new warnings.

[originally from svn r9169]
2011-05-07 10:57:19 +00:00
Jacob Nevins
74c5f7dda9 Implement zlib@openssh.com, using the rekey-after-userauth method suggested in
the wishlist entry.

[originally from svn r9120]
[this svn revision also touched putty-website,putty-wishlist]
2011-03-04 22:34:47 +00:00
Jacob Nevins
99db81f2e7 When iterating over all channels for a dead SSH connection, don't miss out
those in the CHAN_SOCKDATA_DORMANT state (i.e., local-to-remote forwardings
which the SSH server had not yet acknowledged).
Marcel Kilgus has been running with the ssh_do_close() patch for nearly two
years (*cough*) and reports that it has eliminated frequent
'unclean-close-crash' symptoms for him (due to the unclosed socket generating
a pfd_closing() which accessed freed memory), although I've not reproduced
that. The patch to ssh_free() is mine and not known to fix any symptoms.

[originally from svn r9069]
[this svn revision also touched putty-wishlist]
2011-01-03 16:50:41 +00:00
Jacob Nevins
af1060856e Add an option to disable SSH-2 banners.
[originally from svn r9055]
2010-12-27 01:19:13 +00:00