Now you can see exactly what pathname the backend tried to open for
the serial port, and what error code it got back from the OS when it
tried. That should help users distinguish between (for example) a
permissions problem and a typo in the filename.
Now, instead of a 'const char *' in the static data segment, error
messages returned from backend setup are dynamically allocated and
freed by the caller.
This will allow me to make the messages much more specific (including
errno values and the like). However, this commit is pure refactoring:
I've _just_ changed the allocation policy, and left all the messages
alone.
If a terminal window closed with a popup (due to a network error,
for instance) while the mouse pointer was hidden by 'Hide mouse
pointer when typing in window', the mouse pointer could remain hidden
while over the terminal window, making it hard to navigate to the
popup.
This fills in the missing piece of Windows Pageant's story on deferred
decryption: we now actually know how to put up a dialog box asking for
the passphrase, when a not-yet-decrypted key is used.
This is quite a rough implementation so far, but it's a start. Known
issues:
- these new non-modal dialog boxes are serialised with respect to
each other by the Pageant core, but they can run in parallel with a
passphrase prompt popping up from the ordinary GUI 'Add Key'
operation. That may be too confusing; perhaps I should fix it.
- I'm not confident that the passphrase dialog box gets the keyboard
focus in all situations where I'd like it to (or what I can do
about it if not).
- the text in the non-modal box has two copies of the instruction
'enter passphrase for key'.
In commit 1f399bec58 I had the idea of generating the protocol radio
buttons in the GUI configurer by looping over the backends[] array,
which gets the reliably correct list of available backends for a given
binary rather than having to second-guess. That's given me an idea: we
can do the same for the per-backend config panels too.
Now the GUI config panel for every backend is guarded by a check of
backend_vt_from_proto, and we won't display the config for that
backend unless it's present.
In particular, this allows me to move the serial-port configuration
back into config.c from the separate file sercfg.c: we find out
whether to apply it by querying backend_vt_from_proto(PROT_SERIAL),
the same as any other backend.
In _particular_ particular, that also makes it much easier for me to
move the serial config up the pecking order, so that it's now second
only to SSH in the list of per-protocol config panes, which I think is
now where it deserves to be.
(A side effect of that is that I now have to come up with a different
method of having each serial backend specify the subset of parity and
flow control schemes it supports. I've done it by adding an extra pair
of serial-port specific bitmask fields to BackendVtable, taking
advantage of the new vtable definition idiom to avoid having to
boringly declare them as zero in all the other backends.)
This is a sweeping change applied across the whole code base by a spot
of Emacs Lisp. Now, everywhere I declare a vtable filled with function
pointers (and the occasional const data member), all the members of
the vtable structure are initialised by name using the '.fieldname =
value' syntax introduced in C99.
We were already using this syntax for a handful of things in the new
key-generation progress report system, so it's not new to the code
base as a whole.
The advantage is that now, when a vtable only declares a subset of the
available fields, I can initialise the rest to NULL or zero just by
leaving them out. This is most dramatic in a couple of the outlying
vtables in things like psocks (which has a ConnectionLayerVtable
containing only one non-NULL method), but less dramatically, it means
that the new 'flags' field in BackendVtable can be completely left out
of every backend definition except for the SUPDUP one which defines it
to a nonzero value. Similarly, the test_for_upstream method only used
by SSH doesn't have to be mentioned in the rest of the backends;
network Plugs for listening sockets don't have to explicitly null out
'receive' and 'sent', and vice versa for 'accepting', and so on.
While I'm at it, I've normalised the declarations so they don't use
the unnecessarily verbose 'struct' keyword. Also a handful of them
weren't const; now they are.
The motivation is for the SUPDUP protocol. The server may send a
signal for the terminal to reset any input buffers. After this, the
server will not know the state of the terminal, so it is required to
send its cursor position back.
A 'strong' prime, as defined by the Handbook of Applied Cryptography,
is a prime p such that each of p-1 and p+1 has a large prime factor,
and that the large factor q of p-1 is such that q-1 in turn _also_ has
a large prime factor.
HoAC says that making your RSA key using primes of this form defeats
some factoring algorithms - but there are other faster algorithms to
which it makes no difference. So this is probably not a useful
precaution in practice. However, it has been recommended in the past
by some official standards, and it's easy to implement given the new
general facility in PrimeCandidateSource that lets you ask for your
prime to satisfy an arbitrary modular congruence. (And HoAC also says
there's no particular reason _not_ to use strong primes.) So I provide
it as an option, just in case anyone wants to select it.
The change to the key generation algorithm is entirely in sshrsag.c,
and is neatly independent of the prime-generation system in use. If
you're using Maurer provable prime generation, then the known factor q
of p-1 can be used to help certify p, and the one for q-1 to help with
q in turn; if you switch to probabilistic prime generation then you
still get an RSA key with the right structure, except that every time
the definition says 'prime factor' you just append '(probably)'.
(The probabilistic version of this procedure is described as 'Gordon's
algorithm' in HoAC section 4.4.2.)
Most of them are now _mandatory_ P3 scripts, because I'm tired of
maintaining everything to be compatible with both versions.
The current exceptions are gdb.py (which has to live with whatever gdb
gives it), and kh2reg.py (which is actually designed for other people
to use, and some of them might still be stuck on P2 for the moment).
In the Windows GUI, all the controls that were previously named or
labelled Ed25519 are now labelled EdDSA, and when you select that
top-level key type, there's a dropdown for the specific curve (just
like for ECDSA), whose only current value is Ed25519.
In command-line PuTTYgen, you can say '-t eddsa' and give a number of
bits, just like '-t ecdsa'. You can also still say '-t ed25519', for
backwards compatibility.
Also in command-line PuTTYgen, I've reworked the error messages if you
give a number of bits that doesn't correspond to a known elliptic
curve. Now the messages are generated by consulting the list of
curves, so that that list has to be updated by hand in one fewer
place.
In Windows PuTTYgen, this is selected by an extra set of radio-button
style menu options in the Key menu. In the command-line version,
there's a new --primes=provable option.
This whole system is new, so I'm not enabling it by default just yet.
I may in future, though: it's running faster than I expected (in
particular, a lot faster than any previous prototype of the same
algorithm I attempted in standalone Python).
The old system I removed in commit 79d3c1783b had both linear and
exponential phase types, but the new one only had exponential, because
at that point I'd just thrown away all the clients of the linear phase
type. But I'm going to add another one shortly, so I have to put it
back in.
The functions primegen() and primegen_add_progress_phase() are gone.
In their place is a small vtable system with two methods corresponding
to them, plus the usual admin of allocating and freeing contexts.
This API change is the starting point for being able to drop in
different prime generation algorithms at run time in response to user
configuration.
The old API was one of those horrible things I used to do when I was
young and foolish, in which you have just one function, and indicate
which of lots of things it's doing by passing in flags. It was crying
out to be replaced with a vtable.
While I'm at it, I've reworked the code on the Windows side that
decides what to do with the progress bar, so that it's based on
actually justifiable estimates of probability rather than magic
integer constants.
Since computers are generally faster now than they were at the start
of this project, I've also decided there's no longer any point in
making the fixed final part of RSA key generation bother to report
progress at all. So the progress bars are now only for the variable
part, i.e. the actual prime generations.
(This is a reapplication of commit a7bdefb39, without the Miller-Rabin
refactoring accidentally folded into it. Also this time I've added -lm
to the link options, which for some reason _didn't_ cause me a link
failure last time round. No idea why not.)
This reverts commit a7bdefb394.
I had accidentally mashed it together with another commit. I did
actually want to push both of them, but I'd rather push them
separately! So I'm backing out the combined blob, and I'll re-push
them with their proper comments and explanations.
The old API was one of those horrible things I used to do when I was
young and foolish, in which you have just one function, and indicate
which of lots of things it's doing by passing in flags. It was crying
out to be replaced with a vtable.
While I'm at it, I've reworked the code on the Windows side that
decides what to do with the progress bar, so that it's based on
actually justifiable estimates of probability rather than magic
integer constants.
Since computers are generally faster now than they were at the start
of this project, I've also decided there's no longer any point in
making the fixed final part of RSA key generation bother to report
progress at all. So the progress bars are now only for the variable
part, i.e. the actual prime generations.
This is built more or less entirely out of pieces I already had. The
SOCKS server code is provided by the dynamic forwarding code in
portfwd.c. When that accepts a connection request, it wants to talk to
an SSH ConnectionLayer, which is already a trait with interchangeable
implementations - so I just provide one of my own which only supports
the lportfwd_open() method. And that in turn returns an SshChannel
object, with a special trait implementation all of whose methods
just funnel back to an ordinary Socket.
Result: you get a Socket-to-Socket SOCKS implementation with no SSH
anywhere, and even a minimal amount of need to _pretend_ internally to
be an SSH implementation.
Additional features include the ability to log all the traffic in the
form of diagnostics to standard error, or log each direction of each
connection separately to a file, or for anything more general, to log
each direction of each connection through a pipe to a subcommand that
can filter out whatever you think are the interesting parts. Also, you
can spawn a subcommand after the SOCKS server is set up, and terminate
automatically when that subcommand does - e.g. you might use this to
wrap the execution of a single SOCKS-using program.
This is a modernisation of a diagnostic utility I've had kicking
around out-of-tree for a long time. With all of last year's
refactorings, it now becomes feasible to keep it in-tree without
needing huge amounts of scaffolding. Also, this version runs on
Windows, which is more than the old one did. (On Windows I haven't
implemented the subprocess parts, although there's no reason I
_couldn't_.)
As well as diagnostic uses, this may also be useful in some situations
as a thing to forward ports to: PuTTY doesn't currently support
reverse dynamic port forwarding (in which the remote listening port
acts as a SOCKS server), but you could get the same effect by
forwarding a remote port to a local instance of this. (Although, of
course, that's nothing you couldn't achieve using any other SOCKS
server.)
The previous 'name' field was awkwardly serving both purposes: it was
a machine-readable identifier for the backend used in the saved
session format, and it was also used in error messages when Plink
wanted to complain that it didn't support a particular backend. Now
there are two separate name fields for those purposes.
Sometimes, within a switch statement, you want to declare local
variables specific to the handler for one particular case. Until now
I've mostly been writing this in the form
switch (discriminant) {
case SIMPLE:
do stuff;
break;
case COMPLICATED:
{
declare variables;
do stuff;
}
break;
}
which is ugly because the two pieces of essentially similar code
appear at different indent levels, and also inconvenient because you
have less horizontal space available to write the complicated case
handler in - particuarly undesirable because _complicated_ case
handlers are the ones most likely to need all the space they can get!
After encountering a rather nicer idiom in the LLVM source code, and
after a bit of hackery this morning figuring out how to persuade
Emacs's auto-indent to do what I wanted with it, I've decided to move
to an idiom in which the open brace comes right after the case
statement, and the code within it is indented the same as it would
have been without the brace. Then the whole case handler (including
the break) lives inside those braces, and you get something that looks
more like this:
switch (discriminant) {
case SIMPLE:
do stuff;
break;
case COMPLICATED: {
declare variables;
do stuff;
break;
}
}
This commit is a big-bang change that reformats all the complicated
case handlers I could find into the new layout. This is particularly
nice in the Pageant main function, in which almost _every_ case
handler had a bundle of variables and was long and complicated. (In
fact that's what motivated me to get round to this.) Some of the
innermost parts of the terminal escape-sequence handling are also
breathing a bit easier now the horizontal pressure on them is
relieved.
(Also, in a few cases, I was able to remove the extra braces
completely, because the only variable local to the case handler was a
loop variable which our new C99 policy allows me to move into the
initialiser clause of its for statement.)
Viewed with whitespace ignored, this is not too disruptive a change.
Downstream patches that conflict with it may need to be reapplied
using --ignore-whitespace or similar.
We received a report that if you enable Windows 10's high-contrast
mode, the text in PuTTY's installer UI becomes invisible, because it's
displayed in the system default foreground colour against a background
of the white right-hand side of our 'msidialog.bmp' image. That's fine
when the system default fg is black, but high-contrast mode flips it
to white, and now you have white on white text, oops.
Some research in the WiX bug tracker suggests that in Windows 10 you
don't actually have to use BMP files for your installer images any
more: you can use PNG, and PNGs can be transparent. However, someone
else reported that that only works in up-to-date versions of Windows.
And in fact there's no need to go that far. A more elegant answer is
to simply not cover the whole dialog box with our background image in
the first place. I've reduced the size of the background image so that
it _only_ contains the pretty picture on the left-hand side, and omits
the big white rectangle that used to sit under the text. So now the
RHS of the dialog is not covered by any image at all, which has the
same effect as it being covered with a transparent image, except that
it doesn't require transparency support from msiexec. Either way, the
background for the text ends up being the system's default dialog-box
background, in the absence of any images or controls placed on top of
it - so when the high-contrast mode is enabled, it flips to black at
the same time as the text flips to white, and everything works as it
should.
The slight snag is that the pre-cooked WiX UI dialog specifications
let you override the background image itself, but not the Width and
Height fields in the control specifications that refer to them. So if
you just try to drop in a narrow image in the most obvious way, it
gets stretched across the whole window.
But that's not a show-stopper, because we're not 100% dependent on
getting WiX to produce exactly the right output. We already have the
technology to postprocess the MSI _after_ it comes out of WiX: we're
using it to fiddle the target-platform field for the Windows on Arm
installers. So all I had to do was to turn msiplatform.py into a more
general msifixup.py, add a second option to change the width of the
dialog background image, and run it on the x86 installers as well as
the Arm ones.
This applies to both server modes ('pageant -E key.ppk [lifetime]')
and client mode ('pageant -a -E key.ppk').
I'm not completely confident that the CLI syntax is actually right
yet, but for the moment, it's enough that it _exists_. Now I don't
have to test the encrypted-key loading via manually mocked-up agent
requests.
On Windows, due to a copy-paste goof, the message that should have
read "Configuring n stop bits" instead ended with "data bits".
While I'm here, I've arranged that the "1 stop bit" case of that
message is in the singular. And then I've done the same thing again on
Unix, because I noticed that message was unconditionally plural too.
Now I've got an enum for PlugLogType, it's easier to add things to it.
We were giving a blow-by-blow account of each connection attempt, and
when it failed, saying what went wrong before we moved on to the next
candidate address, but when one finally succeeded, we never logged
_that_. Now we do.
There aren't quite as many of these as there are on Unix, but Windows
Plink and PSFTP still share some suspiciously similar-looking code.
Now they're both clients of wincliloop.c.
This begins to head towards the goal of storing a key file encrypted
in Pageant, and decrypting it on demand via a GUI prompt the first
time a client requests a signature from it. That won't be a facility
available in all situations, so we have to be able to return failure
from the prompt.
More precisely, there are two versions of this API, one in
PageantClient and one in PageantListenerClient: the stream
implementation of PageantClient implements the former API and hands it
off to the latter. Windows Pageant has to directly implement both (but
they will end up funnelling to the same function within winpgnt.c).
NFC: for the moment, the new API functions are never called, and every
implementation of them returns failure.
In the previous trawl of this, I didn't bother with the statics in
main-program modules, on the grounds that my main aim was to avoid
'library' objects (shared between multiple programs) from polluting
the global namespace. But I think it's worth being more strict after
all, so this commit adds 'static' to a lot more file-scope variables
that aren't needed outside their own module.
Now it's no longer used, we can get rid of it, and better still, get
rid of every #define PUTTY_DO_GLOBALS in the many source files that
previously had them.
We now have no remaining things in header files that switch from being
a declaration to a definition depending on an awkward #define at the
point of including that header. There are still a few mutable
variables with external linkage, but at least now each one is defined
in a specific source file file appropriate to its purpose and context.
The remaining globals as of this commit were:
- 'logctx' and 'term', which never needed to be globals in the first
place, because they were never actually shared between source
files. Now 'term' is just a static in window.c, and 'logctx' is a
static in each of that and winplink.c.
- 'hinst', which still has external linkage, but is now defined
separately in each source file that sets it up (i.e. those with a
WinMain)
- osMajorVersion, osMinorVersion and osPlatformId, whose definitions
now live in winmisc.c alongside the code which sets them up.
(Actually they were defined there all along, it turns out, but
every toolchain I've built with has commoned them together with the
version defined by the GLOBAL in the header.)
- 'hwnd', which nothing was actually _using_ any more after previous
commits, so all this commit had to do was delete it.
The declarations in header files now use ordinary 'extern'. That means
I have to arrange to put definitions matching those declarations in
the appropriate modules; so I've made a macro DEFINE_WINDOWS_FUNCTION
which performs a definition matching a prior DECLARE_WINDOWS_FUNCTION
(and reusing the typedef made by the latter).
This applies not only to the batch of functions that were marked
GLOBAL in winstuff.h, but also the auxiliary sets marked
WINCAPI_GLOBAL and WINSECUR_GLOBAL in wincapi.h and winsecur.h
respectively.
This was the difficult part of cleaning up that global variable. The
main Windows PuTTY GUI is split between source files, so that _does_
actually need to refer to the main window from multiple places.
But all the places where windlg.c needed to use 'hwnd' are seat
methods, so they were already receiving a Seat pointer as a parameter.
In other words, the methods of the Windows GUI Seat were already split
between source files. So it seems only fair that they should be able
to share knowledge of the seat's data as well.
Hence, I've created a small 'WinGuiSeat' structure which both window.c
and windlg.c can see the layout of, and put the main terminal window
handle in there. Then the seat methods implemented in windlg.c, like
win_seat_verify_ssh_host_key, can use container_of to turn the Seat
pointer parameter back into the address of that structure, just as the
methods in window.c can do (even though they currently don't need to).
(Who knows: now that it _exists_, perhaps that structure can be
gradually expanded in future to turn it into a proper encapsulation of
all the Windows frontend's state, like we should have had all
along...)
I've also moved the Windows GUI LogPolicy implementation into the same
object (i.e. WinGuiSeat implements both traits at once). That allows
win_gui_logging_error to recover the same WinGuiSeat from its input
LogPolicy pointer, which means it can get from there to the Seat facet
of the same object, so that I don't need the extern variable
'win_seat' any more either.
Windows Pageant doesn't really have a 'main window' any more, ever
since I separated the roles of system-tray management and IPC receiver
into two different hidden windows managed by different threads. So it
was already silly to be storing one of them in the global 'HWND hwnd'
variable, because it's no longer obvious which it should be.
So there's now a static variable 'traywindow' within winpgnt.c which
it uses in place of the global 'hwnd'.
The GUI version of pgp_fingerprints() is now a differently named
function that takes a parent HWND as a parameter, and so does my
help-enabled wrapper around MessageBox.
It's now a static in the main source file of each application that
uses it, and isn't accessible from any other source file unless the
main one passes it by reference.
In fact, there were almost no instances of the latter: only the
config-box functions in windlg.c were using 'conf' by virtue of its
globalness, and it's easy to make those take it as a parameter.
It's silly to set it at each call site of restrict_process_acl() if
that function returns success! More sensible to have it be a flag in
the same source file as restrict_process_acl(), set as an automatic
_side effect_ of success.
I've renamed the variable itself, and the global name 'restricted_acl'
is now a query function that asks winsecur.c whether that operation
has been (successfully) performed.
These global variables are only ever used by load_settings, which uses
them to vary the default protocol and port number in the absence of
any specification elsewhere. So there's no real need for them to be
universally accessible via the awkward GLOBAL mechanism: they can be
statics inside settings.c, with accessor functions that can set them.
That was the last GLOBAL in putty.h, so I've removed the definition of
the macro GLOBAL itself as well. There are still some GLOBALs in the
Windows subdirectory, though.
I haven't managed to make this one _not_ be a mutable variable, but at
least it's not global across all tools any more: it lives in cmdline.c
along with the code that decides what to set it to, and cmdline.c
exports a query method to ask for its value.
Another ugly mutable global variable gone: now, instead of this
variable being defined in cmdline.c and written to by everyone's
main(), it's defined _alongside_ everyone's main() as a constant, and
cmdline.c just refers to it.
A bonus is that now nocmdline.c doesn't have to define it anyway for
tools that don't use cmdline.c. But mostly, it didn't need to be
mutable, so better for it not to be.
While I'm at it, I've also fiddled with the bit flags that go in it,
to define their values automatically using a list macro instead of
manually specifying each one to be a different power of 2.
This is another piece of the old 2003 attempt at async agent requests.
Nothing ever calls this function (in particular, the new working
version of async-agent doesn't need it). Remove it completely, and all
its special-window-message implementations too.
(If we _were_ still using this function, then it would surely be
possible to fold it into the more recently introduced general
toplevel-callback system, and get rid of all this single-use special
code. But we're not, so removing it completely is even easier.)
In particular, this system was the only reason why Windows Plink paid
any attention to its message queue. So now I can make it call plain
WaitForMultipleObjects instead of MsgWaitForMultipleObjects.
This was the easiest flag to remove: nothing ever checks it at all!
It was part of an abandoned early attempt to make Pageant requests
asynchronous. The flag was added in commit 135abf244 (April 2003); the
code that used it was #ifdef-ed out in commit 98d735fde (January 2004),
and removed completely in commit f864265e3 (January 2017).
We now have an actually working system for async agent requests on
Windows, via the new named-pipe IPC. And we also have a perfectly good
way to force a particular agent request to work synchronously: just
pass NULL as the callback function pointer. All of that works just
fine, without ever using this flag. So begone!
The global 'int flags' has always been an ugly feature of this code
base, and I suddenly thought that perhaps it's time to start throwing
it out, one flag at a time, until it's totally unused.
My first target is FLAG_VERBOSE. This was usually set by cmdline.c
when it saw a -v option on the program's command line, except that GUI
PuTTY itself sets it unconditionally on startup. And then various bits
of the code would check it in order to decide whether to print a given
message.
In the current system of front-end abstraction traits, there's no
_one_ place that I can move it to. But there are two: every place that
checked FLAG_VERBOSE has access to either a Seat or a LogPolicy. So
now each of those traits has a query method for 'do I want verbose
messages?'.
A good effect of this is that subsidiary Seats, like the ones used in
Uppity for the main SSH server module itself and the server end of
shell channels, now get to have their own verbosity setting instead of
inheriting the one global one. In fact I don't expect any code using
those Seats to be generating any messages at all, but if that changes
later, we'll have a way to control it. (Who knows, perhaps logging in
Uppity might become a thing.)
As part of this cleanup, I've added a new flag to cmdline_tooltype,
called TOOLTYPE_NO_VERBOSE_OPTION. The unconditionally-verbose tools
now set that, and it has the effect of making cmdline.c disallow -v
completely. So where 'putty -v' would previously have been silently
ignored ("I was already verbose"), it's now an error, reminding you
that that option doesn't actually do anything.
Finally, the 'default_logpolicy' provided by uxcons.c and wincons.c
(with identical definitions) has had to move into a new file of its
own, because now it has to ask cmdline.c for the verbosity setting as
well as asking console.c for the rest of its methods. So there's a new
file clicons.c which can only be included by programs that link
against both cmdline.c _and_ one of the *cons.c, and I've renamed the
logpolicy to reflect that.
These were just too footling for even me to bother splitting up into
multiple commits:
- a couple of int -> size_t changes left out of the big-bang commit
0cda34c6f
- a few 'const' added to pointer-type casts that are only going to be
read from (leaving out the const provokes a warning if the pointer
was const _before_ the cast)
- a couple of 'return' statements trying to pass the void return of
one function through to another.
- another missing (void) in a declaration in putty.h (but this one
didn't cause any knock-on confusion).
- a few tweaks to macros, to arrange that they eat a semicolon after
the macro call (extra do ... while (0) wrappers, mostly, and one
case where I had to do it another way because the macro included a
variable declaration intended to remain in scope)
- reworked key_type_to_str to stop putting an unreachable 'break'
statement after every 'return'
- removed yet another type-check of a function loaded from a Windows
system DLL
- and finally, a totally spurious semicolon right after an open brace
in mainchan.c.
