Simon Tatham [Sat, 27 Feb 2016 09:17:27 +0000 (09:17 +0000)]
Remove spurious -shareexists reference in Plink docs.
That option does exist, but only on master; it was not in the 0.66
release. It turned up by mistake when I updated the documentation copy
of the Plink online help while preparing the 0.66 release, because I
ran plink from the wrong branch.
The new release automation should stop that kind of mistake from
happening in future.
Simon Tatham [Sat, 27 Feb 2016 09:25:23 +0000 (09:25 +0000)]
Make get_user_sid() return the cached copy if one already exists.
A user reported in January that locking down our process ACL causes
get_user_sid's call to OpenProcessToken to fail with a permissions
error. This _shouldn't_ be important, because we'll already have found
and cached the user SID before getting that far - but unfortunately
the call to get_user_sid in winnpc.c was bypassing the cache and
trying the whole process again.
This fix changes the memory ownership semantics of get_user_sid():
it's now an error to free the value it gives you, or else the *next*
call to get_user_sid() will return a stale pointer. Hence, also
removed those frees everywhere they appear.
Simon Tatham [Sat, 27 Feb 2016 08:52:45 +0000 (08:52 +0000)]
Mention the new Secure Contact Key in the GPG docs appendix.
The reporter of vuln-pscp-sink-sscanf asked for a key to encrypt the
vulnerability report with, and having generated one, it seemed like a
good idea to make it part of the official PuTTY GPG key set and
publish it for the next person to use.
Simon Tatham [Mon, 29 Feb 2016 19:38:12 +0000 (19:38 +0000)]
Tighten up pointer handling after ssh_pkt_getstring.
ssh_pkt_getstring can return (NULL,0) if the input packet is too short
to contain a valid string.
In quite a few places we were passing the returned pointer,length pair
to a printf function with "%.*s" type format, which seems in practice
to have not been dereferencing the pointer but the C standard doesn't
actually guarantee that. In one place we were doing the same job by
hand with memcpy, and apparently that _can_ dereference the pointer in
practice (so a server could have caused a NULL-dereference crash by
sending an appropriately malformed "x11" type channel open request).
And also I spotted a logging call in the "forwarded-tcpip" channel
open handler which had forgotten the field width completely, so it was
erroneously relying on the string happening to be NUL-terminated in
the received packet.
I've tightened all of this up in general by normalising (NULL,0) to
("",0) before calling printf("%.*s"), and replacing the two even more
broken cases with the corrected version of that same idiom.
Simon Tatham [Wed, 24 Feb 2016 20:13:10 +0000 (20:13 +0000)]
Fix vulnerability CVE-2016-2563 in old scp protocol.
There was a rogue sscanf("%s") with no field width limit, targeting a
stack-based buffer, and scanning a string containing untrusted data.
It occurs in the 'sink' side of the protocol, i.e. when downloading
files *from* the server.
Our own bug id for this vulnerability is 'vuln-pscp-sink-sscanf'.
Colin Watson [Thu, 28 Jan 2016 21:22:07 +0000 (21:22 +0000)]
Fix strict-aliasing warnings in sk_tcp_peer_info.
GCC 6 emits strict-aliasing warnings here, so use the existing
sockaddr_union arrangements to avoid those. As a prerequisite for being
able to express sk_tcp_peer_info in terms of sockaddr_union, I fixed up
the union elements to be a bit less odd in the NO_IPV6 case.
Colin Watson [Thu, 28 Jan 2016 21:19:41 +0000 (21:19 +0000)]
Avoid -Wmisleading-indentation warnings with GCC 6.
GCC 6 warns about potentially misleading indentation, such as:
if (condition) stmt1; stmt2;
Chaining multiple ifs on a single line runs into this warning, even if
it's probably not actually misleading to a human eye, so just add a
couple of newlines to pacify the compiler.
Simon Tatham [Tue, 26 Jan 2016 22:09:01 +0000 (22:09 +0000)]
Fix a 64-bit-cleanness error in sshcrc's generator.
Not that anyone actually needs to use that conditioned-out main(),
since it only generates the table already present in the same source
file, but since @ch3root's unused-variable patch touched it I tried
compiling it and noticed in passing that I'd also got the wrong printf
format directive for an unsigned long.
Simon Tatham [Thu, 25 Feb 2016 20:45:27 +0000 (20:45 +0000)]
Autogenerate licence text in doc subdir from LICENCE.
Now we have licence.pl, it seems to me to make very good sense to have
it generate the Halibut form(s) of the licence and copyright year as
well as the source-code forms.
As a result, I believe _no_ copies of the licence text or copyright
date exist any more except for the master one in LICENCE - so I can
completely remove the checklist section about all the places to update
it, because there's only one. Hooray!
Simon Tatham [Thu, 25 Feb 2016 20:43:54 +0000 (20:43 +0000)]
Stop copying the licence text into C source code.
Now all the uses of the licence text or the short copyright notice get
it from a new header "licence.h", which in turn is built by a Perl
script licence.pl invoked by mkfiles.pl, using LICENCE itself as the
source.
Hence, I can completely remove a whole section from the list of
licence locations in CHECKLST.txt :-)
(cherry-picker's notes: one conflict was just changed context, the
other was deleting a copy of the licence that wasn't quite the same
between branches)
Simon Tatham [Thu, 25 Feb 2016 20:42:00 +0000 (20:42 +0000)]
Use readonly edit controls in some Windows dialogs.
This makes the About and Licence boxes copy-and-pasteable, similarly
to what I've just done on Unix.
(But unlike on the Unix side, here I haven't touched the host key
prompt dialog, because that's a standard Windows MessageBox and not
easy to mess around with. Plus, in any case, you can already hit ^C to
copy the whole text out of a MessageBox. Same goes for the PGP
fingerprints dialog.)
As a side effect, several copies of the copyright notice and licence
text have moved from .rc files into C source. I've updated
CHECKLST.txt, but they won't stay there for long.
(cherry-picker's notes: the conflict was just because several copies
of the licence text were deleted, and they weren't quite the same
between branches)
Simon Tatham [Thu, 25 Feb 2016 20:39:22 +0000 (20:39 +0000)]
Make some static text in GTK dialogs selectable.
I've made the licence text, the About box, and the host key dialog
into GTK selectable edit controls. (The former because it contains a
lot of text; the About box because pasting version numbers into bug
reports is obviously useful; the host key because of the fingerprint.)
Simon Tatham [Fri, 11 Dec 2015 06:47:20 +0000 (06:47 +0000)]
Code-sign the Windows PuTTY binaries and installer.
Or, at least, potentially do so. The build script now has a slot into
which code-signing can be dropped by setting a variable in the bob
configuration to specify an appropriate command line.
The variable will typically need to point at a script wrapping the
actual signing tool, since there are lots of fiddly details
(timestamping countersignature, certificate, private key, etc) not
given on the command lines in this build script, on the basis that
they're local configuration questions for whoever is _running_ this
build script.
Simon Tatham [Fri, 27 Nov 2015 23:55:16 +0000 (23:55 +0000)]
Fix a mistaken use of a format string in logevent().
logevent() doesn't do printf-style formatting (though the logeventf
wrapper in ssh.c does), so if you need to format a message, it has to
be done separately with dupprintf.
Simon Tatham [Thu, 25 Feb 2016 20:30:58 +0000 (20:30 +0000)]
Make our process's ACL more restrictive.
By default Windows processes have wide open ACLs which allow interference
by other processes running as the same user. Adjust our ACL to make this
a bit harder.
Because it's useful to protect PuTTYtel as well, carve winsecur.c into
advapi functions and wincapi.c for crypt32 functions.
Simon Tatham [Sun, 22 Nov 2015 15:02:14 +0000 (15:02 +0000)]
Fix a memory leak in uxproxy.c.
We set up a pair of bufchains for the standard input and output
exchanged with the proxy process, but forgot to clear them when the
Local_Proxy_Socket is cleaned up.
Owen Dunn [Sun, 22 Nov 2015 12:04:04 +0000 (12:04 +0000)]
Move SID-getting code into a separate function so it can be shared by
make_private_security_descriptor and a new function protectprocess().
protectprocess() opens the running PuTTY process and adjusts the
Everyone and user access control entries in its ACL to deny a
selection of permissions which malicious processes running as the same
user could use to hijack PuTTY.
Simon Tatham [Thu, 12 Nov 2015 19:11:07 +0000 (19:11 +0000)]
Big revision to CHECKLST.txt for release.pl and Mason.
Half the release checklist has changed recently, what with me
completely reworking the website and also writing all this release
automation. I think these are all the checklist changes needed now the
dust has settled, though of course when I do the next actual release I
expect there'll turn out to be something I missed...
Simon Tatham [Thu, 12 Nov 2015 19:09:36 +0000 (19:09 +0000)]
Further release automation.
I've added extra modes to release.pl which should automate the more
tedious parts of the deployment phase: uploading the release build to
all the places it needs to go, checking its integrity once it gets
there, verifying that everything can be downloaded again usefully,
checking content-types etc.
The new version should check more thoroughly (it checks the whole FTP
and HTTP download directories, so it will spot errors like failing to
update the FTP 'latest' symlink), and take fewer commands to run.
Simon Tatham [Thu, 25 Feb 2016 20:26:33 +0000 (20:26 +0000)]
Fix potential segfaults in reading OpenSSH's ASN.1 key format.
The length coming back from ber_read_id_len might have overflowed, so
treat it as potentially negative. Also, while I'm here, accumulate it
inside ber_read_id_len as an unsigned, so as to avoid undefined
behaviour on integer overflow, and toint() it before return.
Thanks to Hanno Böck for spotting this, with the aid of AFL.
(cherry-picker's note: resolving the conflict involved removing an
entire section of the original commit which fixed ECDSA code not
present on this branch)
Simon Tatham [Thu, 25 Feb 2016 20:22:23 +0000 (20:22 +0000)]
Fix a segfault in parsing OpenSSH private key files.
The initial test for a line ending with "PRIVATE KEY-----" failed to
take into account the possibility that the line might be shorter than
that. Fixed by introducing a new library function strendswith(), and
strstartswith() for good measure, and using that.
Thanks to Hanno Böck for spotting this, with the aid of AFL.
Simon Tatham [Sat, 7 Nov 2015 15:59:00 +0000 (15:59 +0000)]
More post-release checklist updates, and a new script.
I've added a few sample shell commands in the upload procedure (mostly
so that I don't have to faff about remembering how rsync trailing
slashes work every time), and also written a script called
'release.pl', which automates the updating of the version number in
all the various places it needs to be done and also ensures the PSCP
and Plink transcripts in the docs will match the release itself.
Simon Tatham [Sat, 7 Nov 2015 15:15:07 +0000 (15:15 +0000)]
One small post-release checklist tweak.
I spotted that I've been checking that old-style Windows Help files
were delivered with content-type "application/octet-stream", but not
also checking the same thing about the marginally newer .CHM ones. (Or
at least not writing it down in the wishlist; I think I did actually
check on at least one occasion.)
Simon Tatham [Fri, 25 Sep 2015 15:03:47 +0000 (16:03 +0100)]
Fix winhandl.c's failure to ever free a foreign handle.
Handles managed by winhandl.c have a 'busy' flag, which is used to
mean two things: (a) is a subthread currently blocked on this handle
so various operations in the main thread have to be deferred until it
finishes? And (b) is this handle currently one that should be returned
to the main loop to be waited for?
For HT_INPUT and HT_OUTPUT, those things are either both true or both
false, so a single flag covering both of them is fine. But HT_FOREIGN
handles have the property that they should always be waited for in the
main loop, but no subthread is blocked on them. The latter means that
operations done on them in the main thread should not be deferred; the
only such operation is cleaning them up in handle_free().
handle_free() was failing to spot this, and was deferring freeing
HT_FOREIGN handles until their subthread terminated - which of course
never happened. As a result, when a named pipe server was closed, its
actual Windows event object got destroyed, but winhandl.c still kept
passing it back to the main thread, leading to a tight loop because
MsgWaitForMultipleObjects would return ERROR_INVALID_HANDLE and never
block.
Simon Tatham [Sun, 9 Aug 2015 20:18:27 +0000 (21:18 +0100)]
Add a FAQ for 'checksum mismatch' reports.
The aim is to try to reduce the incidence of the two least helpful
classes of those reports: the ones which have just got mismatched
checksum files, and the ones which don't tell us the information that
would help.
Ben Harris [Tue, 13 Oct 2015 19:33:12 +0000 (20:33 +0100)]
Check the x argument to check_boundary() more carefully.
This is a minimal fix for CVE-2015-5309, and while it's probably
unnecessary now, it seems worth committing for defence in depth and to
give downstreams something reasonably non-intrusive to cherry-pick.
Ben Harris [Wed, 7 Oct 2015 22:54:39 +0000 (23:54 +0100)]
More robust control sequence parameter handling.
Parameters are now accumulated in unsigned integers and carefully checked
for overflow (which is turned into saturation). Things that consume them
now have explicit range checks (again, saturating) to ensure that their
inputs are sane. This should make it much harder to cause overflow by
supplying ludicrously large numbers.
Fixes two bugs found with the help of afl-fuzz. One of them may be
exploitable and is CVE-2015-5309.
Ben Harris [Sat, 17 Oct 2015 20:00:31 +0000 (21:00 +0100)]
Handle packets with no type byte by returning SSH_MSG_UNIMPLEMENTED.
The previous assertion failure is obviously wrong, but RFC 4253 doesn't
explicitly declare them to be a protocol error. Currently, the incoming
packet isn't logged, which might cause some confusion for log parsers.
Simon Tatham [Fri, 25 Sep 2015 08:23:26 +0000 (09:23 +0100)]
Sanitise bad characters in log file names.
On Windows, colons are illegal in filenames, because they're part of
the path syntax. But colons can appear in automatically constructed
log file names, if an IPv6 address is expanded from the &H placeholder.
Now we coerce any such illegal characters to '.', which is a bit of a
bodge but should at least cause a log file to be generated.
Simon Tatham [Fri, 25 Sep 2015 08:15:21 +0000 (09:15 +0100)]
Shout more loudly if we can't open a log file.
A user points out that logging fopen failures to the Event Log is a
bit obscure, and it's possible to proceed for months in the assumption
that your sessions are being correctly logged when in fact the
partition was full or you were aiming them at the wrong directory. Now
we produce output visibly in the PuTTY window.
Simon Tatham [Thu, 24 Sep 2015 16:30:04 +0000 (17:30 +0100)]
Command-line options to log sessions.
Log files, especially SSH packet logs, are often things you want to
generate in unusual circumstances, so it's good to have lots of ways
to ask for them. Particularly, it's especially painful to have to set
up a custom saved session to get diagnostics out of the command-line
tools.
I've added options '-sessionlog', '-sshlog' and '-sshrawlog', each of
which takes a filename argument. I think the fourth option (session
output but filtered down to the printable subset) is not really a
_debugging_ log in the same sense, so it's not as critical to have an
option for it.
Simon Tatham [Sat, 17 Oct 2015 16:30:53 +0000 (17:30 +0100)]
Fix spurious EAGAIN in Plink host key (and other) prompts.
Plink sets standard input into nonblocking mode, meaning that read()
from fd 0 in an interactive context will typically return -1 EAGAIN.
But the prompt functions in uxcons.c, used for verifying SSH host keys
and suchlike, were doing an unguarded read() from fd 0, and then
panicking and aborting the session when they got EAGAIN.
Fixed by inventing a wrapper around read(2) which handles EAGAIN but
passes all other errors back to the caller. (Seemed slightly less
dangerous than the stateful alternative of temporarily re-blockifying
the file descriptor.)
Cherry-picker's notes: the conflict was a trivial one. The new
function block_and_read() by this commit appears just before
verify_ssh_host_key(), which has a new prototype on the source branch,
close enough to disrupt the patch hunk's context. Easily fixed.
Simon Tatham [Thu, 3 Sep 2015 18:04:54 +0000 (19:04 +0100)]
Key rollover: fix the .htaccess files built by Buildscr.
The build script generates the .htaccess files that go in each
individual build and redirect generic names like 'putty.tar.gz' to the
real filenames including that build's version number. Those .htaccess
files redirect the corresponding signatures as well, so they need
updating now that we're generating signature files with a different
extension.
Simon Tatham [Wed, 2 Sep 2015 17:30:10 +0000 (18:30 +0100)]
Key rollover: switch to signing using the new keys.
sign.sh's command-line syntax has changed, so I've updated the sample
command line in CHECKLST as well. Also the file extensions of the
signatures have changed, so I've updated the pre-release verification
command line in CHECKLST too.
Simon Tatham [Tue, 1 Sep 2015 17:35:38 +0000 (18:35 +0100)]
pterm: set IUTF8 on pty devices depending on charset.
In a UTF-8 pterm, it makes sense to set the IUTF8 flag (on systems
that have one) on the pty device, so that line editing will take
account of UTF-8 multibyte characters.
Simon Tatham [Sun, 23 Aug 2015 13:13:30 +0000 (14:13 +0100)]
Performance: cache character widths returned from Pango.
Profiling reveals that pterm in Pango rendering mode uses an absurd
amount of CPU when it's not even actually _drawing_ the text, because
of all the calls to pango_layout_get_pixel_extents() while
pangofont_draw_text tries to work out which characters it can safely
draw as part of a long string. Caching the results speeds things up
greatly.
Simon Tatham [Sat, 22 Aug 2015 14:05:12 +0000 (15:05 +0100)]
'pterm --display' should set $DISPLAY inside the terminal.
If you open a pterm on a different display via the --display
command-line option rather than by setting $DISPLAY, I think (and
other terminals seem to agree) that it's sensible to set $DISPLAY
anyway for processes running inside the terminal.
Simon Tatham [Sat, 8 Aug 2015 12:35:44 +0000 (13:35 +0100)]
New formatting directive in logfile naming: &P for port number.
Users have requested this from time to time, for distinguishing log
file names when there's more than one SSH server running on different
ports of the same host. Since we do take account of that possibility
in other areas (e.g. we cache host keys indexed by (host,port) rather
than just host), it doesn't seem unreasonable to do so here too.
Simon Tatham [Thu, 6 Aug 2015 18:25:56 +0000 (19:25 +0100)]
Work around a failure in Windows 10 jump lists.
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.
Simon Tatham [Sat, 1 Aug 2015 21:11:16 +0000 (22:11 +0100)]
Don't try to load GSSAPI libs unless we'll use them.
A user reports that in a particular situation one of the calls to
LoadLibrary from wingss.c has unwanted side effects, and points out
that this happens even when the saved session has GSSAPI disabled. So
I've evaluated as much as possible of the condition under which we
check the results of GSS library loading, and deferred the library
loading itself until after that condition says we even care about the
results.
Simon Tatham [Mon, 27 Jul 2015 19:06:02 +0000 (20:06 +0100)]
Handle the VK_PACKET virtual key code.
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.)
Simon Tatham [Mon, 27 Jul 2015 19:06:02 +0000 (20:06 +0100)]
Turn the Windows PuTTY main window into a Unicode window.
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.
Simon Tatham [Mon, 27 Jul 2015 19:06:02 +0000 (20:06 +0100)]
New centralised helper function dup_mb_to_wc().
PuTTY's main mb_to_wc() function is all very well for embedding in
fiddly data pipelines, but for the simple job of turning a C string
into a C wide string, really I want something much more like
dupprintf. So here is one.
I've had to put it in a new separate source file miscucs.c rather than
throwing it into misc.c, because misc.c is linked into tools that
don't also include a module providing the internal Unicode API (winucs
or uxucs). The new miscucs.c appears only in Unicode-using tools.
Simon Tatham [Sat, 25 Jul 2015 10:28:32 +0000 (11:28 +0100)]
Post-0.65 release checklist updates.
The -F option is no longer needed to bob in this situation; that
hasn't been the directory I keep release announcements in for a long
time; the Docs page needs adjusting for pre-release retirement as well
as the Downloads page.
Simon Tatham [Mon, 22 Jun 2015 18:37:27 +0000 (19:37 +0100)]
Fix a crash when connection-sharing during userauth.
If a sharing downstream disconnected while we were still in userauth
(probably by deliberate user action, since such a downstream would
have just been sitting there waiting for upstream to be ready for it)
then we could crash by attempting to count234(ssh->channels) before
the ssh->channels tree had been set up in the first place.
A simple null-pointer check fixes it. Thanks to Antti Seppanen for the
report.
Simon Tatham [Mon, 22 Jun 2015 18:36:57 +0000 (19:36 +0100)]
Fix a mismerge in kex null-pointer checks.
I removed a vital line of code while fixing the merge conflicts when
cherry-picking 1eb578a488a71284d6b18e46df301e54805f2c35 as 26fe1e26c0f7ab42440332882295667d4a0ac500, causing Diffie-Hellman key
exchange to be completely broken because the server's host key was
never constructed to verify the signature with. Reinstate it.
Simon Tatham [Thu, 18 Jun 2015 06:05:19 +0000 (07:05 +0100)]
Fix accidental dependence on Windows API quirk in config box.
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.
Simon Tatham [Sat, 13 Jun 2015 14:22:03 +0000 (15:22 +0100)]
Add missing null-pointer checks in key exchange.
Assorted calls to ssh_pkt_getstring in handling the later parts of key
exchange (post-KEXINIT) were not checked for NULL afterwards, so that
a variety of badly formatted key exchange packets would cause a crash
rather than a sensible error message.
None of these is an exploitable vulnerability - the server can only
force a clean null-deref crash, not an access to actually interesting
memory.
Thanks to '3unnym00n' for pointing out one of these, causing me to
find all the rest of them too.
Cherry-picker's notes: the main conflict arose because the original
commit also made fixes to the ECDH branch of the big key exchange if
statement, which doesn't exist on this branch. Also there was a minor
and purely textual conflict, when an error check was added right next
to a function call that had acquired an extra parameter on master.
Simon Tatham [Mon, 8 Jun 2015 18:24:58 +0000 (19:24 +0100)]
Use 64-bit BignumInt wherever __uint128_t is available.
gcc and clang both provide a type called __uint128_t when compiling
for 64-bit targets, code-generated more or less similarly to the way
64-bit long longs are handled on 32-bit targets (spanning two
registers, using ADD/ADC, that sort of thing). Where this is available
(and they also provide a handy macro to make it easy to detect), we
should obviously use it, so that we can handle bignums a larger chunk
at a time and make use of the full width of the hardware's multiplier.
Preliminary benchmarking using 'testbn' suggests a factor of about 2.5
improvement.
I've added the new possibility to the ifdefs in sshbn.h, and also
re-run contrib/make1305.py to generate a set of variants of the
poly1305 arithmetic for the new size of BignumInt.
Cherry-picker's notes: the conflict arose because the original commit
also added new 64-bit autogenerated forms of dedicated Poly1305
arithmetic, which doesn't exist on this branch.
Simon Tatham [Mon, 8 Jun 2015 18:23:48 +0000 (19:23 +0100)]
Improve integer-type hygiene in bignum code.
In many places I was using an 'unsigned int', or an implicit int by
virtue of writing an undecorated integer literal, where what was
really wanted was a BignumInt. In particular, this substitution breaks
in any situation where BignumInt is _larger_ than unsigned - which it
is shortly about to be.
Cherry-picker's notes: the conflicts were because the original commit
also modified new code (sshccp.c for dedicated Poly1305 arithmetic
routines, sshbn.c for a few bignum functions introduced on trunk for
various pieces of new crypto) which doesn't exist on this branch.
Simon Tatham [Sun, 7 Jun 2015 20:14:09 +0000 (21:14 +0100)]
Don't try sending on sharing channels.
The final main loop in do_ssh2_authconn will sometimes loop over all
currently open channels calling ssh2_try_send_and_unthrottle. If the
channel is a sharing one, however, that will reference fields of the
channel structure like 'remwindow', which were never initialised in
the first place (thanks, valgrind). Fix by excluding CHAN_SHARING
channels from that loop.
Simon Tatham [Sun, 7 Jun 2015 20:09:41 +0000 (21:09 +0100)]
Clean up downstream sockets when upstream loses its SSH connection.
If the real SSH connection goes away and we call sharestate_free with
downstreams still active, then that in turn calls share_connstate_free
on all those downstreams, freeing the things their sockets are using
as Plugs but not actually closing the sockets, so further data coming
in from downstream gives rise to a use-after-free bug.
(Thanks to Timothe Litt for a great deal of help debugging this.)
Simon Tatham [Sat, 6 Jun 2015 13:52:29 +0000 (14:52 +0100)]
Move BignumInt definitions into a header file.
This allows files other than sshbn.c to work with the primitives
necessary to build multi-word arithmetic functions satisfying all of
PuTTY's portability constraints.
Simon Tatham [Thu, 28 May 2015 17:14:14 +0000 (18:14 +0100)]
Commit my replacement Windows I-beam mouse pointer.
Installing this systemwide as the Windows text selection cursor is a
workaround for 'black-pointer'. It's a white I-beam with a one-pixel
black outline around it, so it should be visible on any background
colour. (I suppose that a backdrop of tightly packed I-beams looking
just like it might successfully hide it, but that's unlikely :-)
I constructed this some years ago for personal use; I needed it again
this week and had to go and recover it from a backup of a defunct
system, which made me think I really ought to check it in somewhere,
and this 'contrib' directory seems like the ideal place.
Simon Tatham [Mon, 18 May 2015 20:17:21 +0000 (21:17 +0100)]
Fix a compile warning with -DDEBUG.
An unguarded write() in the dputs function caused gcc -Werror to fail
to compile. I'm confused that this hasn't bitten me before, though -
obviously normal builds of PuTTY condition out the faulty code, but
_surely_ this can't be the first time I've enabled the developer
diagnostics since gcc started complaining about unchecked syscall
returns!
Simon Tatham [Mon, 18 May 2015 12:57:45 +0000 (13:57 +0100)]
Log identifying information for the other end of connections.
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-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.
Simon Tatham [Fri, 8 May 2015 18:04:16 +0000 (19:04 +0100)]
Completely remove the privdata mechanism in dialog.h.
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.
Simon Tatham [Fri, 8 May 2015 17:57:18 +0000 (18:57 +0100)]
Fix two small memory leaks in config mechanism.
The memory dangling off ssd->sesslist should be freed when ssd itself
goes away, and the font settings ctrlset we delete in gtkcfg.c should
be freed as well once it's been removed from its containing array.
Simon Tatham [Mon, 27 Apr 2015 19:48:29 +0000 (20:48 +0100)]
Provide a script to regenerate the Blowfish init tables.
Since I've recently published a program that can easily generate the
required digits of pi, and since I was messing around in sshblowf.c
already, it seemed like a good idea to provide a derivation of all
that hex data.
Simon Tatham [Mon, 27 Apr 2015 05:54:21 +0000 (06:54 +0100)]
Paste error in comment.
SSH2_MSG_KEX_DH_GEX_REQUEST_OLD and SSH2_MSG_KEX_DH_GEX_REQUEST were
correctly _defined_ as different numbers, but the comments to the
right containing the hex representations of their values were
accidentally the same.
Cherry-picker's notes: the conflict was because the original commit
also added smemclrs to SHA384_Simple and the ssh_hash structures for
SHA-384 and SHA-512, none of which exists on this branch so those
changes are irrelevant.
Simon Tatham [Sun, 26 Apr 2015 22:31:11 +0000 (23:31 +0100)]
Use a timing-safe memory compare to verify MACs.
Now that we have modes in which the MAC verification happens before
any other crypto operation and hence will be the only thing seen by an
attacker, it seems like about time we got round to doing it in a
cautious way that tries to prevent the attacker from using our memcmp
as a timing oracle.
So, here's an smemeq() function which has the semantics of !memcmp but
attempts to run in time dependent only on the length parameter. All
the MAC implementations now use this in place of !memcmp to verify the
MAC on input data.
Cherry-picker's notes: the above commit comment isn't really true on
this branch, since the ETM packet protocol changes haven't been
cherry-picked. But it seemed silly to deliberately leave out even a
small safety measure.
Simon Tatham [Sat, 25 Apr 2015 09:46:53 +0000 (10:46 +0100)]
Support RFC 4419.
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.
Simon Tatham [Tue, 7 Apr 2015 21:17:08 +0000 (22:17 +0100)]
Fix a dangerous cross-thread memory access.
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.
Simon Tatham [Tue, 7 Apr 2015 20:54:41 +0000 (21:54 +0100)]
Clean up a stale foreign handle in winnps.c.
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.
Simon Tatham [Sat, 7 Mar 2015 20:57:26 +0000 (20:57 +0000)]
Don't output negative numbers in the ESC[13t report.
A minus sign is illegal at that position in a control sequence, so if
ESC[13t should report something like ESC[3;-123;234t then we won't
accept it as input. Switch to printing the numbers as unsigned, so
that negative window coordinates are output as their 32-bit two's
complement; experimentation suggests that PuTTY does accept that on
input.
Simon Tatham [Sat, 7 Mar 2015 17:10:36 +0000 (17:10 +0000)]
Stop Windows PuTTY becoming unresponsive if server floods us.
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.
projects / PuTTY.git / log