We are passing pointers as third argument to AppendMenu. Do not
truncate them to UINT, use UINT_PTR instead which has the required
size on 64bit Windows.
We're passing a pointer as 4th argument to WinHelp. Do not cast it to
DWORD which would truncate the pointer. Instead use UINT_PTR as that
is what WinHelp expects.
We've had several reports that launching saved sessions from the
Windows 10 jump list fails; Changyu Li reports that this is because we
create those IShellLink objects with a command line string starting
with @, and in Windows 10 that causes the SetArguments method to
silently do the wrong thing.
This is generated in response to the SendInput() Windows API call, if
that in turn is passed an KEYBDINPUT structure with KEYEVENTF_UNICODE
set. That method of input generation is used by programs such as
'WinCompose' to send an arbitrary Unicode character as if it had been
typed at the keyboard, even if the keyboard doesn't actually provide a
key for it.
Like VK_PROCESSKEY, this key code is an exception to our usual policy
of manually translating keystrokes: we handle it by calling
TranslateMessage, to get back the Unicode character it contains as a
WM_CHAR message.
(If that Unicode character in turn is outside the BMP, it may come
back as a pair of WM_CHARs in succession containing UTF-16 surrogates;
if so, that's OK, because the new Unicode WM_CHAR handler can cope.)
This causes WM_CHAR messages sent to us to have a wParam containing a
16-bit value encoded in UTF-16, rather than an 8-bit value encoded in
the system code page.
As far as I can tell, there aren't many other knock-on effects - e.g.
you can still interact with the window using ordinary char-based API
functions such as SetWindowText, and the Windows API will do the
necessary conversions behind the scenes. However, even so, I'm half
expecting some sort of unforeseen bug to show up as a result of this.
Coverity complained that some paths through the loop in the
WM_INITDIALOG handler might leave firstpath==NULL. In fact this can't
happen because the input data to that loop is largely static and we
know what it looks like, but it doesn't seem unreasonable to add an
assertion anyway, to keep static checkers happy and as an explanatory
quasi-comment for humans.
Our config boxes are constructed using the CreateDialog() API
function, rather than the modal DialogBox(). CreateDialog() is not
that different from CreateWindow(), so windows created with it don't
appear on the screen automatically; MSDN says that they must be shown
via ShowWindow(), just like non-dialog windows have to be. But we
weren't doing that at any point!
So how was our config box ever getting displayed at all? Apparently by
sheer chance, it turns out. The handler for a selection change in the
tree view, which has to delete a whole panel of controls and creates a
different set, surrounds that procedure with some WM_SETREDRAW calls
and an InvalidateRect(), to prevent flicker while lots of changes were
being made. And the creation of the _first_ panelful of controls, at
dialog box setup, was done by simply selecting an item in the treeview
and expecting that handler to be recursively called. And it appears
that calling WM_SETREDRAW(TRUE) and then InvalidateRect was
undocumentedly having an effect equivalent to the ShowWindow() we
should have called, so that we never noticed the latter was missing.
But a recent Vista update (all reports implicate KB3057839) has caused
that not to work any more: on an updated Vista machine, in some
desktop configurations, it seems that any attempt to fiddle with
WM_SETREDRAW during dialog setup can leave the dialog box in a really
unhelpful invisible state - the window is _physically there_ (you can
see its taskbar entry, and the mouse pointer changes as you move over
where its edit boxes are), but 100% transparent.
So now we're doing something a bit more sensible. The first panelful
of controls is created directly by the WM_INITDIALOG handler, rather
than recursing into code that wasn't really designed to run at setup
time. To be on the safe side, that handler for treeview selection
change is also disabled until the WM_INITDIALOG handler has finished
(like we already did with the WM_COMMAND handler), so that we can be
sure of not accidentally messing about with WM_SETREDRAW at all during
setup. And at the end of setup, we show the window in the sensible
way, by a docs-approved call to ShowWindow().
This appears (on the one machine I've so far tested it on) to fix the
Vista invisible-window issue, and also it should be more API-compliant
and hence safer in future.
(cherry picked from commit 6163710f04)
When anyone connects to a PuTTY tool's listening socket - whether it's
a user of a local->remote port forwarding, a connection-sharing
downstream or a client of Pageant - we'd like to log as much
information as we can find out about where the connection came from.
To that end, I've implemented a function sk_peer_info() in the socket
abstraction, which returns a freeform text string as best it can (or
NULL, if it can't get anything at all) describing the thing at the
other end of the connection. For TCP connections, this is done using
getpeername() to get an IP address and port in the obvious way; for
Unix-domain sockets, we attempt SO_PEERCRED (conditionalised on some
moderately hairy autoconfery) to get the pid and owner of the peer. I
haven't implemented anything for Windows named pipes, but I will if I
hear of anything useful.
(cherry picked from commit c8f83979a3)
Conflicts:
pageant.c
Cherry-picker's notes: the conflict was because the original commit
also added a use of the same feature in the centralised Pageant code,
which doesn't exist on this branch. Also I had to remove 'const' from
the type of the second parameter to wrap_send_port_open(), since this
branch hasn't had the same extensive const-fixing as master.
The last use of it, to store the contents of the saved session name
edit box, was removed nearly two years ago in svn r9923 and replaced
by ctrl_alloc_with_free. The mechanism has been unused ever since
then, and I suspect any further uses of it would be a bad idea for the
same reasons, so let's get rid of it.
(cherry picked from commit 42c592c4ef)
PuTTY now uses the updated version of Diffie-Hellman group exchange,
except for a few old OpenSSH versions which Darren Tucker reports only
support the old version.
FIXME: this needs further work because the Bugs config panel has now
overflowed.
(cherry picked from commit 62a1bce7cb)
When a winhandl.c input thread returns EOF to the main thread, the
latter might immediately delete the input thread's context. I
carefully wrote in a comment that in that case we had to not touch ctx
ever again after signalling to the main thread - but the test for
whether that was true, which also touched ctx, itself came _after_ the
SetEvent which sent that signal. Ahem.
Spotted by Minefield, which it looks as if I haven't run for a while.
(cherry picked from commit 9fec2e7738)
I had set up an event object for signalling incoming connections to
the named pipe, and then called handle_add_foreign_event to get that
event object watched for connections - but when I closed down the
listening pipe, I deleted the event object without also cancelling
that foreign-event handle, so that winhandl.c would potentially call
the callback for a destroyed object.
(cherry picked from commit 6f241cef2c)
This was an old bug, fixed around 0.59, which apparently regressed
when I rewrote the main event loop using the toplevel_callback
mechanism.
Investigation just now suggests that it has to do with my faulty
assumption that Windows PeekMessage would deliver messages in its
message queue in FIFO order (i.e. that the thing calling itself a
message queue is actually a _queue_). In fact my WM_NETEVENT seems to
like to jump the queue, so that once a steady stream of them starts
arriving, we never do anything else in the main event loop (except
deal with handles).
Worked around in a simple and slightly bodgy way, namely, we don't
stop looping on PeekMessage and run our toplevel callbacks until we've
either run out of messages completely or else seen at least one that
_isn't_ a WM_NETEVENT. That way we should reliably interleave NETEVENT
processing with processing of other stuff.
(cherry picked from commit 7d97c2a8fd)
To understand the handle leak bug that I fixed in git commit
7549f2da40, I had to think fairly hard
to remind myself what all this code was doing, which means the
comments weren't good enough. Expanded and rewritten some of them in
the hope that things will be clearer next time.
(cherry picked from commit a87a14ae0f)
Cherry-picker's notes: this apparently pointless commit is required on
this branch because it's a dependency of the rather less pointless
9fec2e7738.
Our config boxes are constructed using the CreateDialog() API
function, rather than the modal DialogBox(). CreateDialog() is not
that different from CreateWindow(), so windows created with it don't
appear on the screen automatically; MSDN says that they must be shown
via ShowWindow(), just like non-dialog windows have to be. But we
weren't doing that at any point!
So how was our config box ever getting displayed at all? Apparently by
sheer chance, it turns out. The handler for a selection change in the
tree view, which has to delete a whole panel of controls and creates a
different set, surrounds that procedure with some WM_SETREDRAW calls
and an InvalidateRect(), to prevent flicker while lots of changes were
being made. And the creation of the _first_ panelful of controls, at
dialog box setup, was done by simply selecting an item in the treeview
and expecting that handler to be recursively called. And it appears
that calling WM_SETREDRAW(TRUE) and then InvalidateRect was
undocumentedly having an effect equivalent to the ShowWindow() we
should have called, so that we never noticed the latter was missing.
But a recent Vista update (all reports implicate KB3057839) has caused
that not to work any more: on an updated Vista machine, in some
desktop configurations, it seems that any attempt to fiddle with
WM_SETREDRAW during dialog setup can leave the dialog box in a really
unhelpful invisible state - the window is _physically there_ (you can
see its taskbar entry, and the mouse pointer changes as you move over
where its edit boxes are), but 100% transparent.
So now we're doing something a bit more sensible. The first panelful
of controls is created directly by the WM_INITDIALOG handler, rather
than recursing into code that wasn't really designed to run at setup
time. To be on the safe side, that handler for treeview selection
change is also disabled until the WM_INITDIALOG handler has finished
(like we already did with the WM_COMMAND handler), so that we can be
sure of not accidentally messing about with WM_SETREDRAW at all during
setup. And at the end of setup, we show the window in the sensible
way, by a docs-approved call to ShowWindow().
This appears (on the one machine I've so far tested it on) to fix the
Vista invisible-window issue, and also it should be more API-compliant
and hence safer in future.
The general plan is that if PuTTY knows a host key for a server, it
should preferentially ask for the same type of key so that there's some
chance of actually getting the same key again. This should mean that
when a server (or PuTTY) adds a new host key type, PuTTY doesn't
gratuitously switch to that key type and then warn the user about an
unrecognised key.
When anyone connects to a PuTTY tool's listening socket - whether it's
a user of a local->remote port forwarding, a connection-sharing
downstream or a client of Pageant - we'd like to log as much
information as we can find out about where the connection came from.
To that end, I've implemented a function sk_peer_info() in the socket
abstraction, which returns a freeform text string as best it can (or
NULL, if it can't get anything at all) describing the thing at the
other end of the connection. For TCP connections, this is done using
getpeername() to get an IP address and port in the obvious way; for
Unix-domain sockets, we attempt SO_PEERCRED (conditionalised on some
moderately hairy autoconfery) to get the pid and owner of the peer. I
haven't implemented anything for Windows named pipes, but I will if I
hear of anything useful.
The menu options and radio buttons for key type were not consistently
setting each other when selected: in particular, selecting from the
menu did not cause the ED25519 radio button to be either set or unset
when that would have been appropriate.
Looks as if I failed to catch in code review the fact that we should
have _one_ call to each of CheckRadioButton and CheckMenuRadioItem,
and they should both have the right 'first' and 'last' parameters
Having found a lot of unfixed constness issues in recent development,
I thought perhaps it was time to get proactive, so I compiled the
whole codebase with -Wwrite-strings. That turned up a huge load of
const problems, which I've fixed in this commit: the Unix build now
goes cleanly through with -Wwrite-strings, and the Windows build is as
close as I could get it (there are some lingering issues due to
occasional Windows API functions like AcquireCredentialsHandle not
having the right constness).
Notable fallout beyond the purely mechanical changing of types:
- the stuff saved by cmdline_save_param() is now explicitly
dupstr()ed, and freed in cmdline_run_saved.
- I couldn't make both string arguments to cmdline_process_param()
const, because it intentionally writes to one of them in the case
where it's the argument to -pw (in the vain hope of being at least
slightly friendly to 'ps'), so elsewhere I had to temporarily
dupstr() something for the sake of passing it to that function
- I had to invent a silly parallel version of const_cmp() so I could
pass const string literals in to lookup functions.
- stripslashes() in pscp.c and psftp.c has the annoying strchr nature
The ec_name_to_curve and ec_curve_to_name functions shouldn't really
have had to exist at all: whenever any part of the PuTTY codebase
starts using sshecc.c, it's starting from an ssh_signkey or ssh_kex
pointer already found by some other means. So if we make sure not to
lose that pointer, we should never need to do any string-based lookups
to find the curve we want, and conversely, when we need to know the
name of our curve or our algorithm, we should be able to look it up as
a straightforward const char * starting from the algorithm pointer.
This commit cleans things up so that that is indeed what happens. The
ssh_signkey and ssh_kex structures defined in sshecc.c now have
'extra' fields containing pointers to all the necessary stuff;
ec_name_to_curve and ec_curve_to_name have been completely removed;
struct ec_curve has a string field giving the curve's name (but only
for those curves which _have_ a name exposed in the wire protocol,
i.e. the three NIST ones); struct ec_key keeps a pointer to the
ssh_signkey it started from, and uses that to remember the algorithm
name rather than reconstructing it from the curve. And I think I've
got rid of all the ad-hockery scattered around the code that switches
on curve->fieldBits or manually constructs curve names using stuff
like sprintf("nistp%d"); the only remaining switch on fieldBits
(necessary because that's the UI for choosing a curve in PuTTYgen) is
at least centralised into one place in sshecc.c.
One user-visible result is that the format of ed25519 host keys in the
registry has changed: there's now no curve name prefix on them,
because I think it's not really right to make up a name to use. So any
early adopters who've been using snapshot PuTTY in the last week will
be inconvenienced; sorry about that.
All the name strings in ssh_cipher, ssh_mac, ssh_hash, ssh_signkey
point to compile-time string literals, hence should obviously be const
char *.
Most of these const-correctness patches are just a mechanical job of
adding a 'const' in the one place you need it right now, and then
chasing the implications through the code adding further consts until
it compiles. But this one has actually shown up a bug: the 'algorithm'
output parameter in ssh2_userkey_loadpub was sometimes returning a
pointer to a string literal, and sometimes a pointer to dynamically
allocated memory, so callers were forced to either sometimes leak
memory or sometimes free a bad thing. Now it's consistently
dynamically allocated, and should be freed everywhere too.
Adding an extra radio button to the key-type selector caused it to
wrap on to another line and push the bottom of the containing control
box down off the bottom of the window.
In the long term, should more public key formats continue to appear,
we'll probably have to replace the radio buttons with something more
extensible like a drop-down list. For the moment, though, I've fixed
it by just reducing the space per radio button to bring all five
controls back on to the same line.
To fit the text into the smaller space, I also removed the 'SSH-2'
prefix on each key type, which ought to be unnecessary these days
since SSH-2 is a well established default. Only the SSH-1 RSA key type
is still labelled with an SSH version. (And I've moved it to the far
end rather than the start of the line, while I'm here.)
There were ad-hoc functions for fingerprinting a bare key blob in both
cmdgen.c and pageant.c, not quite doing the same thing. Also, every
SSH-2 public key algorithm in the code base included a dedicated
fingerprint() method, which is completely pointless since SSH-2 key
fingerprints are computed in an algorithm-independent way (just hash
the standard-format public key blob), so each of those methods was
just duplicating the work of the public_blob() method with a less
general output mechanism.
Now sshpubk.c centrally provides an ssh2_fingerprint_blob() function
that does all the real work, plus an ssh2_fingerprint() function that
wraps it and deals with calling public_blob() to get something to
fingerprint. And the fingerprint() method has been completely removed
from ssh_signkey and all its implementations, and good riddance.
There was a fair amount of duplication between Windows and Unix
PuTTYgen, and some confusion over writing things to FILE * and
formatting them internally into strings. I think all the public-key
output code now lives in sshpubk.c, and there's only one copy of the
code to generate each format.
I've now centralised into pageant.c all the logic about trying to load
keys of any type, with no passphrase or with the passphrases used in
previous key-loading actions or with a new user-supplied passphrase,
whether we're the main Pageant process ourself or are talking to
another one as a client. The only part of that code remaining in
winpgnt.c is the user interaction via dialog boxes, which of course is
the part that will need to be done differently on other platforms.
When I implemented reading and writing of the new format a couple of
weeks ago, I kept them strictly separate in the UI, so you have to ask
for the format you want when exporting. But in fact this is silly,
because not every key type can be saved in both formats, and OpenSSH
itself has the policy of using the old format for key types it can
handle, unless specifically asked to use the new one.
So I've now arranged that the key file format enum has three values
for OpenSSH: PEM, NEW and AUTO. Files being loaded are identified as
either PEM or NEW, which describe the two physical file formats. But
exporting UIs present either AUTO or NEW, where AUTO is the virtual
format meaning 'save in the old format if possible, otherwise the new
one'.
The last use of it, to store the contents of the saved session name
edit box, was removed nearly two years ago in svn r9923 and replaced
by ctrl_alloc_with_free. The mechanism has been unused ever since
then, and I suspect any further uses of it would be a bad idea for the
same reasons, so let's get rid of it.
Now it actually logs all its requests and responses, the fingerprints
of keys mentioned in all messages, and so on.
I've also added the -v option, which causes Pageant in any mode to
direct that logging information to standard error. In --debug mode,
however, the logging output goes to standard output instead (because
when debugging, that information changes from a side effect to the
thing you actually wanted in the first place :-).
An internal tweak: the logging functions now take a va_list rather
than an actual variadic argument list, so that I can pass it through
several functions.
I'm aiming for windows/winpgnt.c to only contain the parts of Windows
Pageant that are actually to do with handling the Windows API, and for
all the actual agent logic to be cross-platform.
This commit is a start: I've moved every function and internal
variable that was easy to move. But it doesn't get all the way there -
there's still a lot of logic in add_keyfile() and get_keylist*() that
would be good to move out to cross-platform code, but it's harder
because that code is currently quite intertwined with details of
Windows OS interfacing such as printing message boxes and passphrase
prompts and calling back out to agent_query if the Pageant doing that
job isn't the primary one.
It's all very well for these two different formats to share a type
code as long as we're only loading them and not saving, but as soon as
we need to save one or the other, we'll need different type codes
after all.
This commit introduces the openssh_new_write() function, but for the
moment, it always returns failure.
PuTTY now uses the updated version of Diffie-Hellman group exchange,
except for a few old OpenSSH versions which Darren Tucker reports only
support the old version.
FIXME: this needs further work because the Bugs config panel has now
overflowed.
When a winhandl.c input thread returns EOF to the main thread, the
latter might immediately delete the input thread's context. I
carefully wrote in a comment that in that case we had to not touch ctx
ever again after signalling to the main thread - but the test for
whether that was true, which also touched ctx, itself came _after_ the
SetEvent which sent that signal. Ahem.
Spotted by Minefield, which it looks as if I haven't run for a while.
I had set up an event object for signalling incoming connections to
the named pipe, and then called handle_add_foreign_event to get that
event object watched for connections - but when I closed down the
listening pipe, I deleted the event object without also cancelling
that foreign-event handle, so that winhandl.c would potentially call
the callback for a destroyed object.
This was an old bug, fixed around 0.59, which apparently regressed
when I rewrote the main event loop using the toplevel_callback
mechanism.
Investigation just now suggests that it has to do with my faulty
assumption that Windows PeekMessage would deliver messages in its
message queue in FIFO order (i.e. that the thing calling itself a
message queue is actually a _queue_). In fact my WM_NETEVENT seems to
like to jump the queue, so that once a steady stream of them starts
arriving, we never do anything else in the main event loop (except
deal with handles).
Worked around in a simple and slightly bodgy way, namely, we don't
stop looping on PeekMessage and run our toplevel callbacks until we've
either run out of messages completely or else seen at least one that
_isn't_ a WM_NETEVENT. That way we should reliably interleave NETEVENT
processing with processing of other stuff.
To understand the handle leak bug that I fixed in git commit
7549f2da40, I had to think fairly hard
to remind myself what all this code was doing, which means the
comments weren't good enough. Expanded and rewritten some of them in
the hope that things will be clearer next time.
If (say) a read handle returns EOF, and its gotdata function responds
by calling handle_free(), then we want the handle to have already had
its defunct flag set so that the handle can be destroyed. Otherwise
handle_free will set the 'done' flag to ask the subthread to
terminate, and then sit and wait for it to say it's done so -
forgetting that it signalled termination already by returning EOF, and
hence will not be responding to that signal.
Ditto for write errors on write handles, though that should happen
less often.
The code for cleaning up handle structures works by the main thread
asking the per-handle subthread to shut down by means of setting its
'done' flag, and then once the subthread signals back through its
event object that it's done so, the main thread frees all its
resources and removes the event object from the list of things being
checked in the program's event loop.
But read threads were not sending back that final event acknowledging
a request to shut down, so their event objects were never being
cleaned up.
Bug spotted by Ronald Weiss.
We were checking the return value of CreateThread for validity, but
not keeping it to free afterwards if it _was_ valid. Also, we weren't
closing ctx->event in the valid case either. Patch due to Tim Kosse.
I don't think anyone has ever actually called it that, colloquially
_or_ formally, and if anyone ever did (in a bug report, say) I'd
probably have to stop and think to work out what they meant. It's
universally called Plink, and should be officially so as well :-)
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.
If (Msg)WaitForMultipleObjects returns WAIT_TIMEOUT, we expect 'next'
to have been initialised. This can occur without having called
run_timers(), if a toplevel callback was pending, so we can't expect
run_timers to have reliably initialised 'next'.
I'm not actually convinced this could have come up in either of the
affected programs (Windows PSFTP and Plink), due to the list of things
toplevel callbacks are currently used for, but it certainly wants
fixing anyway for the future.
Spotted by Coverity.
Pageant's list box needs its tab stops reorganised a little for new
tendencies in string length, and also has to cope with there only
being one prefix space in the output of the new string fingerprint
function. PuTTYgen needs to squash more radio buttons on to one line.
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.
The OpenSSH key importer and exporter were structured in the
assumption that the strong commonality of format between OpenSSH RSA
and DSA keys would persist across all key types. Moved code around so
it's now clear that this is a peculiarity of those _particular_ two
key types which will not apply to others we add alongside them.
Also, a boolean 'is_dsa' in winpgen.c has been converted into a more
sensible key type enumeration, and the individually typed key pointers
have been piled on top of each other in a union.
This is a pure refactoring change which should have no functional
effect.
I've shifted away from using the SVN revision number as a monotonic
version identifier (replacing it in the Windows version resource with
a count of days since an arbitrary epoch), and I've removed all uses
of SVN keyword expansion (replacing them with version information
written out by Buildscr).
While I'm at it, I've done a major rewrite of the affected code which
centralises all the computation of the assorted version numbers and
strings into Buildscr, so that they're all more or less alongside each
other rather than scattered across multiple source files.
I've also retired the MD5-based manifest file system. A long time ago,
it seemed like a good idea to arrange that binaries of PuTTY would
automatically cease to identify themselves as a particular upstream
version number if any changes were made to the source code, so that if
someone made a local tweak and distributed the result then I wouldn't
get blamed for the results. Since then I've decided the whole idea is
more trouble than it's worth, so now distribution tarballs will have
version information baked in and people can just cope with that.
[originally from svn r10262]
The winegcc hack I use for my Coverity builds is currently using a
version of wincrypt.h that's missing a couple of constants I use.
Ensure they're defined by hand, but (just in case I defined them
_wrong_) also provide a command-line define so I can do that only in
the case of Coverity builds.
[originally from svn r10234]
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]
A user points out that the person who writes a REG_SZ into the
registry can choose whether or not to NUL-terminate it properly, and
if they don't, RegQueryValueEx will retrieve it without the NUL. So if
someone does that to PuTTY's saved session data, then PuTTY may
retrieve nonsense strings.
Arguably this is the fault of whoever tampered with the saved session
data without doing it the same way we would have, but even so, there
ought to be some handling at our end other than silently returning the
wrong data, and putting the NUL back on seems more sensible than
complaining loudly.
[originally from svn r10215]
The IDM_RECONF handler unconditionally calls ldisc_configure to
reconfigure the line discipline for the new echo/edit settings, but in
fact ldisc can be NULL if no session is currently active. (Indeed, the
very next line acknowledges this, by testing it for NULL before
calling ldisc_send!) Thanks to Alexander Wong for the report.
[originally from svn r10214]
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]
Philippe Maupertuis reports that on one particular machine, Windows
causes the named pipe created by upstream PuTTY to be owned by the
Administrators group SID rather than the user's SID, which defeats the
security check in the downstream PuTTY. No other machine has been
reported to do this, but nonetheless it's clearly a thing that can
sometimes happen, so we now work around it by specifying explicitly in
the security descriptor for the pipe that its owner should be the user
SID rather than any other SID we might have the right to use.
[originally from svn r10188]
winshare.c includes ssh.h, but if you defined NO_SECURITY it then
decides to fall back to including the stub noshare.c, which includes
ssh.h again. Fix by moving a block of includes inside the ifdef.
[originally from svn r10184]
On Windows (X mouse reporting of the mouse wheel isn't currently done
by the Unix front end, though I'm shortly about to fix that too) a
mouse wheel event is translated into a virtual button, and we send
both a press and a release of that button to terminal.c, which encodes
both in X mouse reporting escape sequences and passes them on to the
server. This isn't consistent with what xterm does - scroll-wheel
events are encoded _like_ button presses, but differ semantically in
that they don't have matching releases. So we're updating to match
xterm.
[originally from svn r10138]
There's been a long-standing FIXME in Windows's sk_newlistener which
says that in IPv6 mode, an explicit source address (e.g. from a
command-line option of the form -L srcaddr:12345:dest:22) is ignored.
Now it's honoured if possible.
[originally from svn r10122]
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]
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]
Mike Edenfield points out that modern versions of the Windows SDK have
decided that 'INPUT' is a sensible name for an OS data structure
(sigh), and provided a patch to add a disambiguating prefix to
winhandl.c's enum values INPUT, OUTPUT and FOREIGN.
[originally from svn r10109]
Daniel Meidlinger reports that at least one Windows machine which is
not obviously otherwise misconfigured will respond to our
SetEntriesInAcl call with odd errors like ERROR_NONE_MAPPED or
ERROR_TRUSTED_RELATIONSHIP_FAILURE. This is apparently to do with
failure to convert the names "EVERYONE" and "CURRENT_USER" used in the
ACL specification to SIDs. (Or perhaps only one of them is the problem
- I didn't investigate in that direction.)
If we instead construct a fully SID-based ACL, using the well-known
world SID in place of EVERYONE and calling our existing get_user_sid
routine in place of CURRENT_USER, he reports that the problem goes
away, so let's do that instead.
While I'm here, I've slightly simplified the function prototype of
make_private_security_descriptor(), by turning 'networksid' into an
internal static that we can reuse in subsequent calls once we've set
it up. (Mostly because I didn't fancy adding another two pointless
parameters at every call site for the two new SIDs.)
[originally from svn r10096]
This will be useful if someone gets a mysterious Windows error on a
system configured into a language we don't speak - if they cut and
paste the error message to send to us, then we won't have to try to
translate it.
[originally from svn r10092]
XP doesn't have it, and I think having connection sharing work without
its privacy enhancement is better than having it not work at all.
[originally from svn r10087]
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]
