The low-level functions to handle a single atom of base64 at a time
have been in 'utils' / misc.h for ages, but the higher-level family of
base64_encode functions that handle a whole data block were hidden
away in sshpubk.c, and there was no higher-level decode function at
all.
Now moved both into 'utils' modules and declared them in misc.h rather
than ssh.h. Also, improved the APIs: they all take ptrlen in place of
separate data and length arguments, their naming is more consistent
and more explicit (the previous base64_encode which didn't name its
destination is now base64_encode_fp), and the encode functions now
accept cpl == 0 as a special case meaning that the output base64 data
is wanted in the form of an unbroken single-line string with no
trailing \n.
I'm about to want to create a second entirely different dialog box
whose contents are described using the same dialog.h API as the main
config box. So I'm starting by moving as much handler code as possible
out of GenericMainDlgProc and its callers, and into a set of reusable
subroutines.
In particular, this gets rid of the disgusting static variables that
stored all the config-box state. Now they're stored in a more sensible
struct, which lives in the new context-pointer field provided by the
reworked ShinyDialogBox.
It's self-contained enough not to really need to live in dialog.c as a
set of static functions. Also, moving it means we can isolate the
implementation details - which also makes it easy to change them.
One such change is that I've added the ability to bake a context
pointer into the dialog - unused so far, but it will be shortly.
(Also, while I'm here, renamed the functions so they sound more as if
they're adding features than working around bugs - not to mention not
imputing mental illness to the usual versions.)
This makes a second independent copy of an existing ssh_key, for
situations where one piece of code is going to want to keep it after
its current owner frees it.
In order to have it work on an arbitrary ssh_key, whether public-only
or a full public+private key pair, I've had to add an ssh_key query
method to ask whether a private key is known. I'm surprised I haven't
found a need for that before! But I suppose in most situations in an
SSH client you statically know which kind of key you're dealing with.
In this commit, I provide further functions which generate the
existing set of data types:
- key_components_add_text_pl() adds a text component, but takes a
ptrlen rather than a const char *, in case that was what you
happened to have already.
- key_components_add_uint() ends up adding an mp_int to the
structure, but takes it as input in the form of an ordinary C
integer, for the convenience of call sites which will want to do
that a lot and don't enjoy repeating the mp_int construction
boilerplate
- key_components_add_copy() takes a pointer to one of the
key_component sub-structs in an existing key_components, and copies
it into the output key_components under a new name, handling
whatever type it turns out to have.
This stores its data in the same format as the existing KCT_TEXT, but
it displays differently in puttygen --dump, expecting that the data
will be full of horrible control characters, invalid UTF-8, etc.
The displayed data is of the form b64("..."), so you get a hint about
what the encoding is, and can still paste into Python by defining the
identifier 'b64' to be base64.b64decode or equivalent.
Having recently pulled it out into its own file, I think it could also
do with a bit of tidying. In this rework:
- the substructure for a single component now has a globally visible
struct tag, so you can make a variable pointing at it, saving
verbiage in every piece of code looping over a key_components
- the 'is_mp_int' flag has been replaced with a type enum, so that
more types can be added without further upheaval
- the printing loop in cmdgen.c for puttygen --dump has factored out
the initial 'name=' prefix on each line so that it isn't repeated
per component type
- the storage format for text components is now a strbuf rather than
a plain char *, which I think is generally more useful.
Previously, the fact that "ssh-rsa" sometimes comes with two subtypes
"rsa-sha2-256" and "rsa-sha2-512" was known to three different parts
of the code - two in userauth and one in transport. Now the knowledge
of what those ids are, which one goes with which signing flags, and
which key types have subtypes at all, is centralised into a method of
the key algorithm, and all those locations just query it.
This will enable the introduction of further key algorithms that have
a parallel upgrade system.
It's a class method rather than an object method, so it doesn't allow
keys with the same algorithm to make different choices about what
flags they support. But that's not what I wanted it for: the real
purpose is to allow one key algorithm to delegate supported_flags to
another, by having its method implementation call the one from the
delegate class.
(If only C's compile/link model permitted me to initialise a field of
one global const struct variable to be a copy of that of another, I
wouldn't need the runtime overhead of this method! But object file
formats don't let you even specify that.)
Most key algorithms support no flags at all, so they all want to use
the same implementation of this method. So I've started a file of
stubs utils/nullkey.c to contain the common stub version.
All the fiddly business where you have to check that a thing exists,
make sure of its type, find its size, allocate some memory, and then
read it again properly (or, alternatively, loop round dealing with
ERROR_MORE_DATA) just doesn't belong at every call site. It's crying
out to be moved out into some separate utility functions that present
a more ergonomic API, so that the code that decides _which_ Registry
entries to read and what to do with them can concentrate on that.
So I've written a fresh set of registry API wrappers in windows/utils,
and simplified windows/storage.c as a result. The jump-list handling
code in particular is almost legible now!
The function will accept a public key file or a PPK, but if it fails
to parse as any of those, the error message says "not a PuTTY SSH-2
private key", which is particularly incongruous in situations where
you're specifically _not_ after the private half of the key.
Now says "not a public key or a PuTTY SSH-2 private key".
Every time I've had to do this before, I've always done the three-line
dance of initialising a BinarySource and calling get_string on it.
It's long past time I wrapped that up into a convenient subroutine.
If you already have a string (of potentially-binary data) in the form
of a ptrlen reference to somewhere else, and you want to keep a copy
somewhere, it's useful to copy it into a strbuf. But it takes a couple
of lines of faff to do that, and it's nicer to wrap that up into a
tiny helper function.
This commit adds that helper function strbuf_dup, and its non-movable
sibling strbuf_dup_nm for secret data. Also, gone through the existing
code and found a bunch of cases where this makes things less verbose.
These days, the base64 module has 'b64decode', which can tolerate a
str or a bytes as input. Switched to using that, and also, imported it
under a nice short name 'b64'.
In the process, removed the obsolete equivocation between
base64.decodebytes and base64.decodestring. That was there to cope
with Python 2 - but the assert statement right next to it has been
enforcing P3 since commit 2ec2b796ed two years ago!
When a subsidiary part of the SSH system wants to abort the whole
connection, it's supposed to call ssh_sw_abort_deferred, on pain of
free-order confusion. Elsewhere in mainchan.c I was getting this
right, but I missed a couple.
We call setlocale() at the start of the function to get the current
LC_CTYPE locale, then set it to what we need during the function, and
then call setlocale() at the end to put it back again. But the middle
call is allowed to invalidate the pointer returned from the first, so
we have to save it in our own allocated storage until the end of the
function.
This bit me during development just now, and I was surprised that it
hadn't come up before! But I suppose this is one of those things
that's only _allowed_ to fail, and need not in all circumstances -
perhaps it depends on what your LC_CTYPE was set to before.
The list of kex methods recently ran out of space due to the addition
of NTRU (at least, if you have GSSAPI enabled). It's time to stop
having an arbitrary limit on those arrays and switch to doing it
properly.
I wasn't really satisfied with the previous version, but it was
easiest to get Stein's algorithm working on polynomials by doing it
exactly how I already knew to do it for integers. But now I've
improved it in two ways.
The first improvement I got from another implementation: instead of
transforming A into A - kB for some k that makes the constant term
zero, you can scale _both_ inputs, replacing A with mA - kB for some
k,m. The advantage is that you can calculate m and k very easily, by
making each one the constant term of the other polynomial, which means
you don't need to invert something mod q in every step. (Rather like
the projective-coordinates optimisations in elliptic curves, where
instead of inverting in every step you accumulate the product of all
the factors that need to be inverted, and invert the whole product
once at the very end.)
The second improvement is to abandon my cumbersome unwinding loop that
builds up the output coefficients by reversing the steps in the
original gcd-finding loop. Instead, I do the thing you do in normal
Euclid's algorithm: keep track of the coefficients as you go through
the original loop. I had wanted to do this before, but hadn't figured
out how you could deal with dividing a coefficient by x when (unlike
the associated real value) the coefficient isn't a multiple of x. But
the answer is very simple: x is invertible in the ring we're working
in (its inverse mod x^p-x-1 is just x^{p-1}-1), so you _can_ just
divide your coefficient by x, and moreover, very easily!
Together, these changes speed up the NTRU key generation by about a
factor of 1.5. And they remove lots of complicated code as well, so
everybody wins.
This consists of DJB's 'Streamlined NTRU Prime' quantum-resistant
cryptosystem, currently in round 3 of the NIST post-quantum key
exchange competition; it's run in parallel with ordinary Curve25519,
and generates a shared secret combining the output of both systems.
(Hence, even if you don't trust this newfangled NTRU Prime thing at
all, it's at least no _less_ secure than the kex you were using
already.)
As the OpenSSH developers point out, key exchange is the most urgent
thing to make quantum-resistant, even before working quantum computers
big enough to break crypto become available, because a break of the
kex algorithm can be applied retroactively to recordings of your past
sessions. By contrast, authentication is a real-time protocol, and can
only be broken by a quantum computer if there's one available to
attack you _already_.
I've implemented both sides of the mechanism, so that PuTTY and Uppity
both support it. In my initial testing, the two sides can both
interoperate with the appropriate half of OpenSSH, and also (of
course, but it would be embarrassing to mess it up) with each other.
This is already slightly nice because it lets me separate the
Weierstrass and Montgomery code more completely, without having to
have a vtable tucked into dh->extra. But more to the point, it will
allow completely different kex methods to fit into the same framework
later.
To that end, I've moved more of the descriptive message generation
into the vtable, and also provided the constructor with a flag that
will let it do different things in client and server.
Also, following on from a previous commit, I've arranged that the new
API returns arbitrary binary data for the exchange hash, rather than
an mp_int. An upcoming implementation of this interface will want to
return an encoded string instead of an encoded mp_int.
Until now, every kex method has represented the output as an mp_int.
So we were storing it in the mp_int field s->K, and adding it to the
exchange hash and key derivation hashes via put_mp_ssh2.
But there's now going to be the first kex method that represents the
output as a string (so that it might have the top bit set, or multiple
leading zero bytes, without its length varying). So we now need to be
more general.
The most general thing it's sensible to do is to replace s->K with a
strbuf containing _already-encoded_ data to become part of the hash,
including length fields if necessary. So every existing kex method
still derives an mp_int, but then immediately puts it into that strbuf
using put_mp_ssh2 and frees it.
This means if I have functions like foo_subfoo_bar and foo_baz that
both operate on a foo, the Python testcrypt system can translate both
into .bar() and .baz() methods on the object, even though they don't
start with the same prefix.
bool is dangerous in a time-safe context, because C compilers might
insert a control flow divergence to implement the implicit
normalisation of nonzero integers to 1 when you assign to a bool.
Everywhere else time-safe, I avoid using it; but smemeq has been an
exception until now, because the response to smemeq returning failure
was to do an obvious protocol-level divergence _anyway_ (like
disconnecting due to MAC mismatch).
But I'm about to want to use smemeq in a context where I use the
result _subtly_ and don't want to give away what it is, so now it's
time to get rid of that bool and have smemeq return unsigned.
This reallocs an existing mp_int to have a different physical size,
e.g. to make sure there's enough space at the top of it.
Trivial, but I'm a little surprised I haven't needed it until now!
In test_primegen, we loop round retrieving random data until we find
some that will permit a successful prime generation, so that we can
log only the successful attempts, and not the failures (which don't
have to be time-safe). But this itself introduces a potential mismatch
between logs, because the simplistic RNG used in testsc will have
different control flow depending on how far through a buffer of hash
data it is at the start of a given run.
random_advance_counter() gives it a fresh buffer, so calling that at
the start of a run should normalise this out. The code to do that was
already in the middle of random_read(); I've just pulled it out into a
separately callable function.
This hasn't _actually_ caused failures in test_primegen, but I'm not
sure why not. (Perhaps just luck.) But it did cause a failure in
another test of a similar nature, so before I commit _that_ test (and
the thing it's testing), I'd better fix this.
They're pretty much self-contained, and don't really need to be in the
same module as sshpubk.c (which has other dependencies). Move them out
into a utils module, where pulling them in won't pull in anything else
unwanted.
Using a new screenshot-taking module I just added in windows/utils,
these new options allow me to start up one of the tools with
demonstration window contents and automatically save a .BMP screenshot
to disk. This will allow me to keep essentially the same set of demo
images and update them easily to keep pace with the current appearance
of the real tools as PuTTY - and Windows itself - both evolve.
Once we've actually loaded a key file, the job of updating the UI
fields is now done by a subroutine update_ui_after_load(), so that I
can call it from a different context in an upcoming commit.
I've been keeping them up to date with API changes as far as making
sure they still _compile_, but today I tried to actually run them, and
found that they were making a couple of segfault-inducing mistakes:
not filling in their vtable pointer, and not returning a 'realhost'
string. Now fixed.
If an SSH proxy socket is frozen for long enough, and the SSH server
continues to send, then sooner or later the proxy SSH connection will
end up having to freeze its underlying physical socket too. When the
proxy socket is later unfrozen, it needs to pass that unfreezing on in
turn.
The way this should happen is that when the SshProxy begins to clear
the backlog of data passed to it from the proxy SSH connection via
seat_stdout, it should call backend_unthrottle to inform that proxy
connection that the backlog is clearing.
But there was no backlog_unthrottle call in the whole of sshproxy.c.
Now there is.
I got a pterm into a stuck state this morning by an accidental mouse
action. I'd intended to press Ctrl + right-click to pop up the context
menu, but I accidentally pressed down the left button first, starting
a selection drag, and then while the left button was still held down,
pressed down the right button as well, triggering the menu.
The effect was that the context menu appeared while term->selstate was
set to DRAGGING, in which state terminal output is suppressed, and
which is only unset by a mouse-button release event. But then that
release event went to the popup menu, and the terminal window never
got it. So the terminal stayed stuck forever - or rather, until I
guessed the cause and did another selection drag to reset it.
This happened to me on GTK, but once I knew how I'd done it, I found I
could reproduce the same misbehaviour on Windows by the same method.
Added a simplistic fix, on both platforms, that cancels a selection
drag if the popup menu is summoned part way through it.
This gets us scalable icons that will go to extremely large sizes
without the problems that arise from scaling up the output of
mkicon.py, in which outlines become too thin because the script was
mostly concerned with trying to squeeze all the desired detail into
_tiny_ sizes.
The SVG icons are generated by mksvg.py, which is a conversion of the
existing mkicon.py. So the SVG files themselves are not committed in
this repo; 'make svg' in the icons subdir will generate them.
(I haven't decided yet whether this state of affairs should be
permanent. Perhaps _having_ generated the SVGs via a similar program
to the bitmap icons, we should regard the script as a discardable
booster stage and redesignate the SVGs themselves as the source format
for future modifications, so that they can be edited in Inkscape or
similar rather than by tinkering with Python. On the other hand,
perhaps keeping the script will make it easier to keep the icon family
consistent, e.g. if changing the style of one of the shared visual
components.)
My plan is that we should stick with the output of the previous
bitmap-generating script for all the _small_ icons, up to and
including 48 pixels, because it does a better job at low resolution.
(That was really what it was for in the first place: you can think of
it as an analogue of a scalable-font hinting system, to tune the
scaling for very low res so that all the important features are still
visible.)
I think probably I want to switch the 128-pixel icons used in the Mac
icon file over to being rendered from the SVG (though in this commit I
haven't gone that far, not least because I'll also need to prepare a
corresponding black and white version). I haven't done extensive
research yet to decide where I think the crossover point in between
is.
While testing the unrelated pile of commits just past, I accidentally
started a Cygwin saved session I hadn't run in ages which used the old
Telnet-based cygtermd as a local proxy command, and found that it
presented the Cygwin prompt with a trust sigil. Oops!
It turns out that this is because interactor_return_seat does two
things that can change the real seat's trust status, and it does them
in the wrong order: it defaults the status back to trusted (as if the
seat was brand new, because that's how they start out), and it calls
tempseat_flush which may have buffered a trust-status reset while the
seat was borrowed. The former should not override the latter!
This is another fallback needed on Win95, where the Win32 API
functions to convert between multibyte and wide strings exist, but
they haven't heard of the UTF-8 code page. PuTTY can't really do
without that these days.
(In particular, if a server sends a remote window-title escape
sequence while the terminal is in UTF-8 mode, then _something_ needs
to translate the UTF-8 data into Unicode for Windows to reconvert into
the character set used in window titles.)
This is a weird enough thing to be doing that I've put it under the
new #ifdef LEGACY_WINDOWS, so behaviour in the standard builds should
be unchanged.
This makes Pageant run on Win95 again. Previously (after fixing the
startup-time failures due to missing security APIs) it would go into
an uninterruptible CPU-consuming spin in the message loop when every
attempt to retrieve its messages failed because PeekMessageW doesn't
work at all on the 95 series.
Now it can be called from places other than Pageant's WinMain(). In
particular, the attempt to make a security descriptor in
lock_interprocess_mutex() is gated on it.
In return, however, I've tightened up the semantics. In normal PuTTY
builds that aren't trying to support pre-NT systems, the function
*unconditionally* returns true, on the grounds that we don't expect to
target any system that doesn't support the security APIs, and if
someone manages to contrive one anyway - or, more likely, if we some
day introduce a bug in our loading of the security API functions -
then this safety catch should make Pageant less likely to accidentally
fall back to 'never mind, just run in insecure mode'.
Turns out that PuTTY hasn't run successfully on legacy Windows since
0.66, in spite of an ongoing intention to keep it working. Among the
reasons for this is that CreateWindowExW simply fails with
ERROR_CALL_NOT_IMPLEMENTED: apparently Win95 and its ilk just didn't
have fully-Unicode windows as an option.
Fixed by resurrecting the previous code from the git history (in
particular, code removed by commit 67e5ceb9a8 was useful), and
including it as a runtime alternative.
One subtlety was that I found I had to name the A and W window classes
differently (by appending ".ansi" to the A one): apparently they
occupy the same namespace even though the names are in different
character sets, so if you somehow manage to register both classes,
they'll collide with each other without that tweak.
