strcspn() returns a size_t, which is not safe to pass as the parameter
in a printf argument list corresponding to a "*" field width specifier
in the format string, because the latter should be int, which may not
be the same size as size_t.
We were calling Windows file-handling API functions GetFilesize and
SetFilePointer, each of which returns two halves of a large integer by
writing the high half through a pointer, with pointers to the wrong
integer types. Now we're always passing the exact type defined in the
API, and converting after the fact to our own uint64 type, so this
should avoid any risk of wrong-sized pointers.
These integer types are correct for the id/handle parameter to
AppendMenu / InsertMenu / DeleteMenu, and also for the return type of
dialog box procedures.
We also have the special-purpose -DUNPROTECT to disable just the ACL
changes, but if you want to compile without any Windows security API
support at all (e.g. experimentally building against winelib) then
it's easier not to have to specify both defines separately.
With all due respect to Microsoft, a cross-platform program simply
cannot switch to using MS's assorted 'secure' versions of standard C
functions if it wants to continue compiling on platforms other than
Windows. So I might as well squash the warnings, so that any other
more interesting compiler warnings can avoid being swamped in the
mess.
The UI now only has "1" and "2" options for SSH protocol version, which
behave like the old "1 only" and "2 only" options; old
SSH-N-with-fallback settings are interpreted as SSH-N-only.
This prevents any attempt at a protocol downgrade attack.
Most users should see no difference; those poor souls who still have to
work with SSH-1 equipment now have to explicitly opt in.
The old README.txt instructed you to manually update PATH if you
wanted to run pscp from a command prompt. But the MSI installer can do
that automatically, so the wording needs tweaks. And now that we're
actually launching README (at least optionally) from the installer UI,
it's more important to not make it look silly.
This is a thing that the Inno Setup installer did, and that I didn't
get round to replicating when I rushed out the initial MSI in a hurry.
I've checked that this doesn't prevent unattended installation by
administrators: running 'msiexec /q /i putty-whatever.msi' as
administrator still installs silently after this change, without
popping up the README unexpectedly on anyone's desktop as a side
effect.
(I _think_ - but I'm still a long way from an MSI expert - that that's
because /q turns off the whole UI part of the MSI system, and the
loading of README is actually triggered by the transition away from
the final UI dialog box, which we now never visit in the first place.)
I rushed out the MSI in too much of a hurry to sort out this kind of
thing, but now we've got leisure to reconsider, I think it's better
behaviour not to clutter everyone's desktops unless specifically asked
to.
It's only a warning; Windows PuTTYgen puts it up as a message box, and
will still generate the key if you click yes, and Unix PuTTYgen just
prints the warning and gets on with generation anyway. But it might
help encourage people to move away from 1024-bit keys, if they're
still using them.
Protecting our processes from outside interference need not be limited
to just PuTTY: there's no reason why the other SSH-speaking tools
shouldn't have the same treatment (PSFTP, PSCP, Plink), and PuTTYgen
and Pageant which handle private key material.
A compiler warning drew my attention to the fact that 'next' in
pinger_schedule() was an int, not the unsigned long it should have
been. And looking at the code that handles it, it was also taking no
care with integer wraparound when checking whether an existing
scheduled ping should be moved forward.
So now I do something a bit more robust, by remembering what time it
_was_ when we set pinger->next, and checking if the new time value
falls in the interval between those two times.
This replicates the existing error message in Windows PuTTYgen, on the
basis that keys smaller than that will actually fail to work with our
signing code.
I generated these as a by-product of using afl-fuzz on the testbn
executable. I'd hoped it would find interesting bugs in the bignum
code itself, but in fact it mostly found bugs in the parsing code in
the test main(). However, even so, it did produce a list of tests
which reach as much of the code as afl-fuzz was able to, and that
seems like a useful thing to save.
Also, of course, just having a place to put explicitly specified
bignum tests is a good thing anyway, because then we can put
regression tests on the end of it every time we fix bugs.
cmdgen.c has contained code for ages to build a test main() if you
compile with -DTEST_CMDGEN. But it's painful to do so manually, since
you've still got to link in all the same supporting objects, and also
nobody can have actually done that for a while because the stub test
code hasn't been kept up to date with changes in the internal APIs
(specifically prompt_t).
Now we have the ability to include our test programs in Recipe as [UT]
or [XT] so as to leave them out of 'make install', that seems like a
useful thing to do with cmdgen's test suite. So here's a Recipe change
that builds it as 'cgtest', plus fixes for compiler warnings and bit
rot. Pleasantly, the test suite still _passes_ after those are fixed.
E.g. you might pass '--random-device=/dev/urandom'.
Mostly because I got sick of waiting for /dev/random to finish
blocking while I was trying to generate throwaway keys for testing bug
fixes in cmdgen itself. But it might also be useful on systems that
call their random device by a different name that we haven't
encountered.
(Since cmdgen also reads the saved PuTTY random seed file, setting
this option to /dev/zero will not render key generation deterministic.
It's tempting to provide _some_ way to do that, for testing purposes
and clearly marked as dangerous of course, but I think it would take
more faff than this.)
Partly this is because the geometry_widget functionality is going away
in a later version of GTK3, so sooner or later we'll need not to be
using it anyway. But also, it turns out that GTK 3's geometry
calculations have the unfortunate effect of setting the window's base
and min heights to values that are not congruent mod height_increment
(because the former is the value we gave, but the latter is based on
the minimum height of the scrollbar), which confuses at least one
window manager (xfwm4) and causes the window to be created one row too
small.
So I've redone all the geometry computations my own way, based on the
knowledge that the only widgets visible in the top-level window are
the drawing area and the scrollbar and I know how both of those
behave, and taking care to keep base_height and min_height congruent
to avoid that xfwm4 bug.
Both in a new section about reporting vulnerabilities, and in the
section about large attachments (since some large attachments will
surely contain confidential information from the sender).
I think the deterministic DSA system we've been using for ages can now
be considered proven in use, not to mention the fact that RFC 6979 and
the Ed25519 spec both give variants on the same idea. So I've removed
the 'don't use DSA if you can avoid it' warning.
Previously, due to confusion, it was placed either at the end of the
list or at the start, depending on whether the user had any saved
configuration at all. Now we get to choose a sensible place for it in
the list, and for the moment I think second place behind AES is
reasonable.
It was only prepared to position a new element relative to an existing
one if the latter was specified in the saved configuration, not if the
latter was another element new to this run of gprefs(). This wasn't
deliberate at all: it was just due to me failing to update the 'seen'
bitmap in the loop adding new elements.
If you're connecting to a new server and it _only_ provides host key
types you've configured to be below the warning threshold, it's OK to
give the standard askalg() message. But if you've newly demoted a host
key type and now reconnect to some server for which that type was the
best key you had cached, the askalg() wording isn't really appropriate
(it's not that the key we've settled on is the first type _supported
by the server_, it's that it's the first type _cached by us_), and
also it's potentially helpful to list the better algorithms so that
the user can pick one to cross-certify.
When Jacob introduced this message in d0d3c47a0, he was right to
assume that hostkey_algs[] and ssh->uncert_hostkeys[] were sorted in
the same order. Unfortunately, he became wrong less than an hour later
when I committed d06098622. Now we avoid making any such assumption.
It won't return true, because pterm's use of conf is a bit nonstandard
(it doesn't really bother about the protocol field, and has no use for
either host names _or_ serial port filenames). Was affecting both
gtkapp and gtkmain based builds.
Since we got a dynamic preference order, it's been bailing out at a
random point, and listing keys we wouldn't use.
(It would still be nice to only mention keys that we'd actually use, but
that's now quite fiddly.)
I noticed this in passing while tinkering with the hostkey_algs array:
these arrays are full of pointers-to-const, but are not also
themselves declared const, which they should have been all along.
Now we actually have enough of them to worry about, and especially
since some of the types we support are approved by organisations that
people might make their own decisions about whether to trust, it seems
worth having a config list for host keys the same way we have one for
kex types and ciphers.
To make room for this, I've created an SSH > Host Keys config panel,
and moved the existing host-key related configuration (manually
specified fingerprints) into there from the Kex panel.
It's too esoteric to be the first thing on the Auth panel; I've never
heard of any SSH server that supports it in the decade since I
implemented it. The only Google hits are lost souls mistakenly believing
they need it for passwordless public-key login and the like.
