backend_socket_log was generating the IP address in its error messages
by means of calling sk_getaddr(). But sk_getaddr only gets a SockAddr,
which may contain a whole list of candidate addresses; it doesn't also
get the information stored in the 'step' field of the Socket that was
actually trying to make the connection, which says _which_ of those
addresses we were in the middle of trying to connect to.
So now we construct a temporary SockAddr that points at the
appropriate one of the addresses, and use that for calls to plug_log
during connection setup.
If connect() returns EINPROGRESS, then previously we would detect a
successful connection by the socket becoming selectable for writing,
and spot an unsuccessful one by an error code being returned on the
first attempt to read from it.
This isn't the right way to do it: the right way is to respond to the
initial writability notification by calling getsockopt(SO_ERROR) to
retrieve the error code (if any) from the completed connection
attempt. Doing it the old way had the problem that when the socket
became writable, we could sometimes already have written some of our
outgoing data to it before finding out that the connect attempt failed
- which meant we'd discard that data from the bufchain, and no longer
have it to send through a later successful connection to a different
candidate address.
If the length field in the input data was so large that adding 4 to it
caused wraparound, the error check could fail to trigger. Fortunately,
this praticular get_ssh_string function is only used during private
key import from foreign file formats, so it won't be facing hostile
data.
In case of connection errors before and during the handshake,
net_select_result is retrying with the next address of the server. It
however was immediately going to the last address as it was not
checking the return value of try_connect for all intermediate
addresses.
The .htaccess written by Buildscr into the w64 directory was applying
a redirect from 'putty-installer.msi', but in fact the name by which
the website links to snapshot and prerelease installers in that
directory is 'putty-64bit-installer.msi'.
The code that copies the link maps of the release Windows builds into
the place I store them for later debugging should now not
embarrassingly look in the wrong place when we make our first
post-VS2015 release.
I've now made a new section about how to turn on pre-release mode,
because although it's been carefully made trivial within the website
repo itself, there are still several things in other places I need to
edit.
Also added a note to myself to turn off nightly pre-release builds
after the release has gone out. I found on a previous occasion that my
build machine had been building them for months after they were
needed, which was harmless to the website but a waste of CPU!
This arranges that the mechanism from the previous commit
automatically turns itself on and off depending on whether a .git
directory even exists (so it won't try to do anything in distribution
tarballs), and also arranges that it can be manually turned off by a
configure option (in case someone who _is_ building from a git
checkout finds it inconvenient for some reason I haven't thought of,
which seems quite plausible to me).
This is perhaps the more useful end of the mechanism I added in the
previous commit: now, when a developer runs a configure+make build
from a git checkout (rather than from a bob-built source tarball), the
Makefile will automatically run 'git rev-parse HEAD' and embed the
result in the binaries.
So now when I want to deploy my own bleeding-edge code for day-to-day
use on my own machine, I can easily check whether I've done it right
(e.g. did I install to the right prefix?), and also easily check
whether any given PuTTY or pterm has been restarted since I rolled out
a new version.
In order to arrange this (and in particular to force version.o to be
rebuilt when _any_ source file changes), I've had to reintroduce some
of the slightly painful Makefile nastiness that I removed in 4d8782e74
when I retired the 'manifest' system, namely having version.o depend
on a file empty.h, which in turn is trivially rebuilt by a custom make
rule whose dependencies include $(allsources). That's a bit
unfortunate, but I think acceptable: the main horribleness of the
manifest system was not that part, but the actual _manifests_, which
were there to arrange that if you modified the sources in a
distribution tarball the binaries would automatically switch to
reporting themselves as local builds rather than the version baked
into the tarball. I haven't reintroduced that part of the system: if
you check out a given git commit, modify the checked-out sources, and
build the result, the Makefile won't make any inconvenient attempts to
detect that, and the resulting build will still announce itself as the
git commit you started from.
The Windows binaries, and both Windows and Unix source archives,
output from a bob build will now include the full SHA-1 of the source
git commit in their buildinfo (hence in all the About boxes and
command-line version output).
This will be occasionally useful to me at release time (there was that
one embarrassing incident where I managed not to notice that I'd made
a release build from entirely the wrong commit), but mostly, it just
seems like an obviously useful thing to put in a general buildinfo
section now that there is one.
This shows the build platform (32- vs 64-bit in particular, and also
whether Unix GTK builds were compiled with or without the X11 pieces),
what compiler was used to build the binary, and any interesting build
options that might have been set on the make command line (especially,
but not limited to, the security-damaging ones like NO_SECURITY or
UNPROTECT). This will probably be useful all over the place, but in
particular it should allow the different Windows binaries to be told
apart!
Commits 21101c739 and 2eb952ca3 laid the groundwork for this, by
allowing the various About boxes to contain free text and also
ensuring they could be copied and pasted easily as part of a bug
report.
I'm faintly surprised I haven't needed this before. Basically it's an
allocating string formatter, like dupprintf, except that it
concatenates on to the end of a previous string. You instantiate a
strbuf, then repeatedly call strbuf_catf to append pieces of formatted
output to it, and then you can extract the whole string and free it
(separately or both in one step).
By default the VS2015 linker produces binaries with the minimum
version fields in the PE header set to 06.00, which causes XP not to
recognise them as valid binaries at all. But there's no other reason
VS2015-built binaries _can't_ run on versions of Windows as old as XP;
so here I add the link option to set those fields to 05.01 which makes
XP like them again.
This only applies to the 32-bit build, because the VS2015 64-bit
linker refuses to lower the min version field to under 06.00.
Originally added in commit 0014ffb70, and promptly reverted in
6bea4b250 when we realised that VS2003 didn't actually understand
them. But now we're building with VS2015, which does understand them,
it's actually useful to put them back in again.
Looking more closely, it turns out that VS2003 didn't actually _fail
to build_ if you passed these flags on the linker command line - it
just printed a warning and ignored them. (So there was no actual need
to revert the original change, except that it would have caused
confusion.) But that means I can add them unconditionally now, without
breaking even the legacy VS2003 build.
It's a bit conceptually incoherent anyway - if you're uninstalling
PuTTY _systemwide_ across a multi-user system, it doesn't really make
sense that you'd also want to wipe the saved sessions for the
individual user running the uninstaller.
Also, making this change to the Inno Setup uninstaller opens up a
nicer migration path to MSI for people doing large corporate rollouts:
they can upgrade to this version of the Inno Setup package, then do a
silent uninstall of it (which should now _actually_ be silent, since
this cleanup step was the thing that interrupted it otherwise) and
then a silent install of the MSI.
Our Inno Setup installer is legacy as far as I'm concerned, so there's
no point in introducing a 64-bit version of it. 64-bit PuTTY users can
use the MSI from the start, and then there'll only ever have been one
kind of installer and they won't collide with one another.
The 32- and 64-bit installers may be distinguishable by their pathname
in the build output directory, but it's better to have their actual
filenames be different as well, so they don't collide in people's
download directories.
The MSI format has a fixed field for target architecture, so there's
no way to build a single MSI that can decide at install time whether
to install 32-bit or 64-bit (or both). The best you can do along those
lines, apparently, is to have two MSI files plus a bootstrap .EXE that
decides which of them to run, and as far as I'm concerned that would
just reintroduce all the same risks and annoyances that made us want
to migrate away from .EXE installers anyway.
The downside of moving to VS2015 is that its output won't run on very
old versions of Windows. It's not yet clear whether anyone still cares
about things before, say, Win2000 or WinXP, but since my build
environment still _has_ VS2003 available, it's easy enough to build
the extra set of binaries anyway just in case. (At least for now.)
The new binaries live in a build output directory 'w32old'. As with
w64, there is no installer for them; but unlike w64, I don't intend to
add one.
Now we've got VS2015 available (and thanks to a lot of 64-bit
cleanness fixing, in particular Tim Kosse's large patch series from
August 2015) we can do this fairly easily.
The new binaries are shipped in a 'w64' directory, alongside the
just-renamed 'w32'. There is no 64-bit installer (yet).
This is a big jump from the previous VS2003. It should add a little
performance (VS2015 is a better-optimising compiler), but mostly it's
groundwork for doing other useful things such as 64-bit builds and
security-related linker features.
The downside is reduced support for very old versions of Windows,
which I'll address shortly.
This change does affect the layout of the output build directory, and
will need corresponding website changes to avoid breaking links.
'x86' was a misnomer anyway, because it was really Windows-specific
rather than just x86-specific. Calling it 'w32' will make it look less
strange to add 'w64' alongside it.
Uses the BUILDDIR mechanism I added to Makefile.vc in commit
d3db17f3e.
This change is purely internal to Buildscr, and shouldn't affect the
output of a build. It paves the way to have Buildscr run multiple
Windows builds using different compilers, by putting each one in a
different subdirectory so that their outputs don't collide.
I had mistakenly pulled a 'char' value out of a string and passed it
to x11_font_has_glyph and x11_char_struct, each of which takes its two
index bytes as int-typed parameters. But if chars are signed, that
turns high-bit-set characters into out-of-range array indices. Oops.
The range checks in x11_char_struct prevented that from causing any
problem worse than refusal to display any affected glyph. Even so,
that's not particularly helpful. Fixed by changing the index byte
parameters to unsigned char type.
I noticed today that Unix Plink responds to SIGWINCH by accidentally
dying of EINTR having interrupted its main select loop, and when I
checked, there turn out to be a couple of other select loops with the
same bug.
This is a purely stylistic cleanup - no functional change intended -
after Tim Kosse's changes in commits e9a76883a and 6f871e3d2.
I think the underlying cause of the confusion whose functional effects
he was fixing there is that I have the bad habit of tending to call
variables 'ret' for two different reasons: one is because it's holding
the value returned from some subroutine I've just called, and the
other is because it's holding the value I'm preparing to return from
the routine _containing_ the variable.
The reason it's a bad habit is that I confuse the two purposes, and a
variable of one type ends up accidentally being treated as the other.
So while Tim's commits have already fixed the functional effects of
the error in this case, this change should help prevent a recurrence
because now there's no variable called 'ret' at all that's in scope
for the whole function.
random_add_noise calls SHATransform for every 64 octets of incoming noise,
yet instead of xor'ing the hashed noise into the pool it instead only xor'ed
20 octets of the raw noise in each iteration. This effectively reduced the
amount of new entropy entering the pool.
It is possible for SSH_FXP_CLOSE requests to fail. This can happen if the
server buffers writes and an error occurs flushing the data to disk while
processing the SSH_FXP_CLOSE request. If the close fails, sftp_put_file now
returns an error as well.
Due to a shadowed variable, transfer failures were not reflected in the return
code to sftp_put_file. Instead of tracking the return code, use the 'err'
variable to decide which return code to use.
Due to the return variable 'ret' being shadowed in the transfer loop, errors
in the transfer loop did not affect the final return value of sftp_get_file.
This particularly affects psftp's batch mode (without passing the -be
command-line argument), which would errorneously continue. The solution is
to simply remove the shadowing declaration.
A user points out that buf[] in sk_tcp_peer_info is only used in the
IPv6 branch of an ifdef, and is declared with a size of
INET6_ADDRSTRLEN, which won't be defined in NO_IPV6 mode. So moving
the definition inside another IPv6-only ifdef fixes the resulting
build failure.
The id member of the sftp_request structure is of type unsigned int.
This type is also used in the sftp_reqfind callback. In
sftp_find_request we thus need to pass a pointer to unsigned int to
find234. Before this commit, sftp_find_request was passing a pointer
to unsigned long to find234, which causes the lookup to fail on
big-endian platforms where sizeof(unsigned int) != sizeof(unsigned
long), e.g. ppc64.
The new font name configured by the keystrokes was missing its
"client:" or "server:" prefix, which could have led to the selection
of the wrong font in rare situations.
Each gtkfont back end now provides a routine that will return the name
of a similar font to the current one but one notch larger or smaller.
For Pango, this is just a matter of incrementing the font size field
in a standard way; for X11 server-side fonts, we have to go and do an
XListFonts query with a wildcard that requests fonts that vary only in
the size fields from the current one, and then iterate over the result
looking for the best one.
(I expect this will be more useful to Pango scalable-font users than
to X11 fonts, but it seemed a shame not to give the X11 side my best
shot while I was at it.)
Choice of hotkey: I know I'm being inconsistent with gnome-terminal's
use of Ctrl-plus and Ctrl-minus. I thought that was because I was
already using Ctrl-minus as a more convenient synonym for
Ctrl-underscore (which sends the actual control code 0x1F), but now I
actually try it, apparently I'm not. However, Ctrl-plus and Ctrl-minus
are quite horrible as a keystroke pair anyway (one has to be typed
with shift and one without!), and I feel as if the 'less' and
'greater' signs are more specific anyway, in that they specifically
indicate _size_ rather than just 'unspecified numerical value'.
There were already two places in the code (x11font_enum_fonts and
x11_guess_derived_font_name) where we retrieved an XLFD from the X
server, sawed it up ad-hoc into its '-'-separated parts and accessed
them by numeric index.
I'm about to add a third, so before I do, let's turn this into a
somewhat principled system where we get to do the decode/encode in
just one place and call all the individual fields by names that are
actually memorable.
No functional change intended by this commit.
The algorithm Windows uses to generate AppUserModelIDs "hangs on" to
removable media (CDs/DVDs) if PuTTY is launched with a CD/DVD in a drive.
Set the AppUserModelID explicitly to avoid using this algorithm.
The XDG configuration location ($XDG_CONFIG_HOME/putty, or
~/.config/putty) is now prefered over the old ~/.putty location, if the
XDG location already exists. If it doesn't exist, we try to use one of
the old locations ($HOME/.putty, [/etc/passwd home]/.putty, /.putty). If
none of the directories exist, we fall back to ~/.config/putty or
~/.putty, if the XDG_DEFAULT macro is defined or not, respectively. The
PUTTYDIR environment variable remains a definitive override of the
configuration location. This all ensures that the old location is still
used, unless the user explicitly requests otherwise.
The configuration directories are created using the make_dir_path()
function, to ensure that saving the configuration doesn't fail e.g.
because of a non-existent ~/.config directory.
