The current state of the OS X GTK port is looking more or less
plausible - it's not finished, of course, but then neither was the old
native Cocoa port. So I'm inclined to advertise it as *the* unfinished
OS X port: it's the one I intend to keep working on, and it's the one
I'd prefer people offered us help with if they're going to offer.
Hence, leaving the old macosx directory around is just confusing; that
directory is long-unmaintained, probably doesn't even compile, and its
only effect will be to mislead people into thinking it's still
relevant. I'm unilaterally deleting it; of course we can always
recover it from source control history if it's ever necessary to do
so.
This is an absolutely horrible piece of code, relying not only on font
metrics but also on an observed correlation between the length of a
key algorithm name and whether or not it needs a separate key size
displayed. But it'll do for the moment, and it's less effort than
writing a custom piece of Windows API code to display the list box
entries in a properly robust way :-(
Jacob pointed out that a free-text field for entering a key size in
bits is all very well for key types where we actually _can_ generate a
key to a size of your choice, but less useful for key types where
there are only three (or one) legal values for the field, especially
if we don't _say_ what they are.
So I've revamped the UI a bit: now, in ECDSA mode, you get a dropdown
list selector showing the available elliptic curves (and they're even
named, rather than just given by bit count), and in ED25519 mode even
that disappears. The curve selector for ECDSA and the bits selector
for RSA/DSA are independent controls, so each one remembers its last
known value even while temporarily hidden in favour of the other.
The actual generation function still expects a bit count rather than
an actual curve or algorithm ID, so the easiest way to actually
arrange to populate the drop-down list was to have an array of bit
counts exposed by sshecc.c. That's a bit ugly, but there we go.
One small functional change: if you enter an absurdly low value into
the RSA/DSA bit count box (under 256), PuTTYgen used to give a warning
and reset it to 256. Now it resets it to the default key length of
2048, basically because I was touching that code anyway to change a
variable name and just couldn't bring myself to leave it in a state
where it intentionally chose such an utterly useless key size. Of
course this doesn't prevent generation of 256-bit keys if someone
still really wants one - it just means they don't get one selected as
the result of a typo.
The About box is where it showed up most obviously that I'd hastily
bunged a GtkBox inside another GtkBox without considering their
margins: the 'action area' had twice the margin it should have had and
the rightmost button didn't align with the right edge of the rest of
the window contents.
Easily fixed by giving the inner hbox margin 0 (fixing the right align
and the excessive space around all the buttons), and using the
'spacing' property of GtkBox to ensure multiple buttons in it are
nicely separated without having to take care over that in the client
code that adds them.
This is a file that generally seems to turn up when you start using OS
X Finder to interact with directories - which is more likely now that
we're building OS X app bundles into this source tree.
This commit adds two .plist files, which go in the app bundles; two
.bundle files, which are input to gtk-mac-bundler and explain to it
how to _create_ the bundles; and a piece of manual addition to
Makefile.am that actually runs gtk-mac-bundler after building the
gtkapp.c based binaries and the OSX launcher. The latter is
conditionalised on configuring --with-quartz (unlike the binaries
themselves, which you can build on other platforms too, though they
won't do much that's useful).
The big problem with making an OS X application out of a GTK program
is that it won't start unless DYLD_LIBRARY_PATH and several other
environment variables point at all the GTK machinery. So your app
bundle has to contain two programs: a launcher to set up that
environment, and then the real main program that the launcher execs
once it's done so.
But in our case, we also need pterm to start subprocesses _without_
all that stuff in the environment - so our launcher has to be more
complicated than the usual one, because it's also got to save every
detail of how the environment was when it started up. So this is the
launcher program I'm going to use. Comments in the header explain in
more detail how it'll work.
Also in this commit, I add the other end of the same machinery to
gtkapp.c and uxpty.c: the former catches an extra command-line
argument that the launcher used to indicate how it had munged the
environment, and stores it in a global variable where the latter can
pick it up after fork() and use to actually undo the munging.
When it's finished, this will be the backbone of the OS X GTK port:
using a GtkApplication automatically gives us a properly OS X
integrated menu bar.
Using this source file in place of gtkmain.c turns the usual Unix
single-session-per-process PuTTY or pterm into the multi-session-per-
process OS X style one.
Things like Duplicate Session can be done much more simply here - we
just grab the Conf * from the source window and launch a new window
using it, with no fiddly interprocess work needed.
This is still experimental and has a lot of holes, but it's usable
enough to test and improve.
This is a weird thing to have to do, but it is necessary: the OS X
PuTTY will need its top-level windows to be instances of a thing
called GtkApplicationWindow, rather than plain GtkWindow. Hence, the
actual creation of windows needs to be somewhere that isn't
centralised between the two kinds of front end.
This is to [X] what [UT] is to [U]: that is, it's a program linked
against the GTK libraries, but one which doesn't become part of the
'make install' set. I'll use this for the individual binaries that
will go in the OS X application bundles, and then have another
makefile rule pick those up in turn.
Instead of main() living in uxputty.c and uxpterm.c, and doing a
little bit of setup before calling the larger pt_main() in gtkmain.c,
I've now turned things backwards: the big function in gtkmain.c *is*
main(), and the small pieces of preliminary setup in uxputty.c and
uxpterm.c are now a function called setup() which is called from
there. This will allow me to reuse the rest of ux{putty,pterm}.c, i.e.
the assorted top-level bits and pieces that distinguish PuTTY from
pterm, in the upcoming OS X application that will have its own main().
mkicon.py now outputs .pam by hand, rather than using ImageMagick to
go straight to .png. For most purposes the main makefile then uses
ImageMagick anyway, to convert those .pams straight to the .pngs that
the rest of the scripts were expecting. But one script that doesn't do
that is macicon.py, which builds the MacOS .icns file by directly
reading those .pam files back in.
This allows the 'make icns' target in the icons directory to build
from a clean checkout on vanilla MacOS, without requiring a user to
install ImageMagick or any other non-core Python image handling
module.
(I could probably take this change at least a little bit further. I
don't see any reason why icon.pl - generating the Windows .ico files -
couldn't read the .pam files directly, about as easily as macicon.py
did, if anyone had a use case for building the Windows icons in the
presence of Python and Perl but in the absence of ImageMagick. But the
.png files are directly useful outputs for Unix, so _some_ PNG-writing
will have to remain here.)
This lays further groundwork for the OS X GTK3 port, which is going to
have to deal with multiple sessions sharing the same process. gtkwin.c
was a bit too monolithic for this, since it included some
process-global runtime state (timers, toplevel callbacks), some
process startup stuff (gtk_init, gtk_main, argv processing) and some
per-session-window stuff.
The per-session stuff remains in gtkwin.c, with the top-level function
now being new_session_window() taking a Conf. The new gtkmain.c
contains the outer skeleton of pt_main(), handling argv processing and
one-off startup stuff like setlocale; and the new gtkcomm.c contains
the pieces of PuTTY infrastructure like timers and uxsel that are
shared between multiple sessions rather than reinstantiated per
session, which have been rewritten to use global variables rather than
fields in 'inst' (since it's now clear to me that they'll have to
apply to all the insts in existence at once).
There are still some lurking assumptions of one-session-per-process,
e.g. the use of gtk_main_quit when a session finishes, and the fact
that the config box insists on running as a separate invocation of
gtk_main so that one session's preliminary config box can't coexist
with another session already active. But this should make it possible
to at least write an OS X app good enough to start testing with, even
if it doesn't get everything quite right yet.
This change is almost entirely rearranging existing code, so it
shouldn't be seriously destabilising. But two noticeable actual
changes have happened, both pleasantly simplifying:
Firstly, the global-variables rewrite of gtkcomm.c has allowed the
post_main edifice to become a great deal simpler. Most of its
complexity was about remembering what 'inst' it had to call back to,
and in fact the right answer is that it shouldn't be calling back to
one at all. So now the post_main() called by gtkdlg.c has become the
same function as the old inst_post_main() that actually did the work,
instead of the two having to be connected by a piece of ugly plumbing.
Secondly, a piece of code that's vanished completely in this
refactoring is the temporary blocking of SIGCHLD around most of the
session setup code. This turns out to have been introduced in 2002,
_before_ I switched to using the intra-process signal pipe strategy
for SIGCHLD handling in 2003. So I now expect that we should be robust
in any case against receiving SIGCHLD at an inconvenient moment, and
hence there's no need to block it.
I got momentarily confused between whether the special code
(TS_LOCALSTART+i) meant the ith entry in the variable
uncert_hostkeys[] array, or the ith entry in the fixed hostkey_algs[]
array. Now I think everything agrees on it being the latter.
If a server offers host key algorithms that we don't have a stored key
for, they will now appear in a submenu of the Special Commands menu.
Selecting one will force a repeat key exchange with that key, and if
it succeeds, will add the new host key to the cache. The idea is that
the new key sent by the server is protected by the crypto established
in the previous key exchange, so this is just as safe as typing some
command like 'ssh-keygen -lf /etc/ssh/ssh_host_ed25519_key.pub' at the
server prompt and transcribing the results manually.
This allows switching over to newer host key algorithms if the client
has begun to support them (e.g. people using PuTTY's new ECC
functionality for the first time), or if the server has acquired a new
key (e.g. due to a server OS upgrade).
At the moment, it's only available manually, for a single host key
type at a time. Automating it is potentially controversial for
security policy reasons (what if someone doesn't agree this is what
they want in their host key cache, or doesn't want to switch over to
using whichever of the keys PuTTY would now put top of the list?), for
code plumbing reasons (chaining several of these rekeys might be more
annoying than doing one at a time) and for CPU usage reasons (rekeys
are expensive), but even so, it might turn out to be a good idea in
future.
The last list we returned is now stored in the main Ssh structure
rather than being a static array in ssh_get_specials.
The main point of this is that I want to start adding more dynamic
things to it, for which I can't predict the array's max length in
advance.
But also this fixes a conceptual wrongness, in that if a process had
more than one Ssh instance in it then their specials arrays would have
taken turns occupying the old static array, and although the current
single-threaded client code in the GUI front ends wouldn't have minded
(it would have read out the contents just once immediately after
get_specials returned), it still feels as if it was a bug waiting to
happen.
When we're displaying bidirectionally active text (that is, text that
the Unicode bidi algorithm will fiddle with), we need to suppress
Pango's bidi because we've already done our own. We were doing this by
calling is_rtl() on each character, and if it returned true,
displaying just that character in a separate Pango call.
Except that, ahem, we were only doing this if the _first_ character
encountered during a scan of the display buffer was rtl-sensitive. If
the first one was fine but a subsequent one was rtl-sensitive, then
that one would just get shoved into the buffer we'd already started.
Running pterm -fn 'client:Monospace 12' and displaying
testdata/utf8.txt now works again.
When I cut it in half so I could fetch the XCharStruct for a given
character, I forgot that the remaining half should check whether it
had got NULL from the XCharStruct finder. Ahem.
If you run something like 'seq 2000000000' in a GTK3 pterm, the window
never actually updates, because pterm always considers reading more
data from the pty to have higher priority than delivering the "draw"
event. Using g_io_add_watch_full instead of g_io_add_watch allows us
to explicitly lower the priority of the I/O sources, so that window
redraws will take precedence.
This is quite a pain, since it involves inventing an entire new piece
of infrastructure to install a custom Xlib error handler and give it a
queue of things to do. But it fixes a bug in which Unix pterm/PuTTY
crash out at startup if one of the root window's CUT_BUFFERn
properties contains something of a type other than STRING - in
particular, UTF8_STRING is not unheard-of.
For example, run
xprop -root -format CUT_BUFFER3 8u -set CUT_BUFFER3 "thingy"
and then pterm without this fix would have crashed.
I had completely forgotten, when rendering each glyph to a server-side
pixmap and downloading its contents, to only look at the part of the
pixmap that XDrawImageString would have overwritten, as specified by
the metrics in the XCharStruct. Now 'pterm -fn server:variable'
doesn't randomly make up bitmap nonsense outside each character's
bounding rectangle.
It would be nicer if we could also make this show up as the icon for
the .msi file itself when viewed in Explorer, but apparently nothing
can change that. But at least this still gives us _some_ use for the
cardboard-box icon :-)
Patch due to Colin Watson.
Putting the passphrase in a file avoids exposing it to 'ps' which can
print out every process's command line, while at the same time not
being as platform-specific as the approach of providing an fd number
(since cmdgen.c is in principle a potential cross-platform PuTTYgen,
not just a Unix one, which is why it's not in the 'unix' directory).
Of course it introduces its own risks if someone can read the file
from your disk after you delete it; probably the best approach to
avoiding this, if possible, is to point the option at a file on an
in-memory tmpfs type file system. Or better still, use bash-style
/dev/fd options such as
puttygen --new-passphrase <(echo -n "my passphrase") [options]
Failing that, try a secure file-wipe utility, as the man page change
mentions.
(And a use case not to be overlooked, of course, is the one where you
actually want to generate an unprotected key - in which case, just
pass /dev/null as the filename.)
Mostly this is a reaction to the reports of Inno Setup having a DLL
hijacking vulnerability. But also, the new installer has several other
nice features that our Inno Setup one didn't provide: it can put the
PuTTY install directory on PATH automatically, and it supports
completely automatic and silent install/uninstall via 'msiexec /q'
which should make it easier for sysadmins to roll out installation in
large organisations. Also, it just seems like good sense to be using
Windows's own native packaging system (or closest equivalent) rather
than going it alone.
(And on the developer side, I have to say I like the fact that WiX
lets me pass in the version number as a set of command-line #define-
equivalents, whereas for Inno Setup I had to have Buildscr apply Perl
rewriting to the source file.)
For the moment, I'm still building the old Inno Setup installer
alongside this one, but I expect to retire it once the WiX one has
survived in the wild for a while and proven itself more or less
stable.
I've found both MSI and WiX to be confusing and difficult
technologies, so this installer has some noticeable pieces missing
(e.g. retrospective reconfiguration of the installed feature set, and
per-user vs systemwide installation) simply because I couldn't get
them to work. I've commented the new installer source code heavily, in
the hope that a passing WiX expert can give me a hand!
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.
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.
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.
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.
