It doesn't seem to be all that good a spec, in that it seems to be
specified in terms of functions in libssh and hence based on the
assumption that you already know exactly what those functions do. But
it's something, at least.
We were doing an endianness flip on the output elliptic-curve point.
Endianness flips of bignums, of course, have to specify how many bytes
they're imagining the value to have (that's how you decide whether to
convert 0xA1A2 into 0xA2A1 or 0xA2A10000 or 0xA2A1000000000000 etc),
and we had chosen our byte count based on the highest set bit in the
_output value_ - but in fact we should have chosen it based on the
size of the curve's modulus, leading to a failure about 1/256 of the
time when the MSB happened to come out zero so the two byte counts
differed.
(Also added a missing smemclr, while I was there.)
It seems like quite an important thing to mention in the event log!
Suppose there's a bug affecting only one curve, for example? Fixed-
group Diffie-Hellman has always logged the group, but the ECDH log
message just told you the hash and not also the curve.
To implement this, I've added a 'textname' field to all elliptic
curves, whether they're used for kex or signing or both, suitable for
use in this log message and any others we might find a need for in
future.
snew(), and most of the bignum functions, are deliberately written to
fail an assertion and terminate the program rather than return NULL,
so there's no point carefully checking their every return value for
NULL. This removes a huge amount of pointless error-checking code, and
makes the elliptic curve arithmetic almost legible in places :-)
I've kept error checks after modinv(), because that can return NULL if
asked to invert zero. bigsub() can also fail in principle, because our
bignums are non-negative only, but in the couple of cases where it's
used there's a preceding compare that should prevent it, so I've just
added assertions.
Removed another set of ad-hoc tests of the key size to decide which
hash to use for the signature system, and replaced them with a
straightforward pointer to an ssh_hash structure in the 'extra' area.
The ec_name_to_curve and ec_curve_to_name functions shouldn't really
have had to exist at all: whenever any part of the PuTTY codebase
starts using sshecc.c, it's starting from an ssh_signkey or ssh_kex
pointer already found by some other means. So if we make sure not to
lose that pointer, we should never need to do any string-based lookups
to find the curve we want, and conversely, when we need to know the
name of our curve or our algorithm, we should be able to look it up as
a straightforward const char * starting from the algorithm pointer.
This commit cleans things up so that that is indeed what happens. The
ssh_signkey and ssh_kex structures defined in sshecc.c now have
'extra' fields containing pointers to all the necessary stuff;
ec_name_to_curve and ec_curve_to_name have been completely removed;
struct ec_curve has a string field giving the curve's name (but only
for those curves which _have_ a name exposed in the wire protocol,
i.e. the three NIST ones); struct ec_key keeps a pointer to the
ssh_signkey it started from, and uses that to remember the algorithm
name rather than reconstructing it from the curve. And I think I've
got rid of all the ad-hockery scattered around the code that switches
on curve->fieldBits or manually constructs curve names using stuff
like sprintf("nistp%d"); the only remaining switch on fieldBits
(necessary because that's the UI for choosing a curve in PuTTYgen) is
at least centralised into one place in sshecc.c.
One user-visible result is that the format of ed25519 host keys in the
registry has changed: there's now no curve name prefix on them,
because I think it's not really right to make up a name to use. So any
early adopters who've been using snapshot PuTTY in the last week will
be inconvenienced; sorry about that.
This gives families of public key and kex functions (by which I mean
those sharing a set of methods) a place to store parameters that allow
the methods to vary depending on which exact algorithm is in use.
The ssh_kex structure already had a set of parameters specific to
Diffie-Hellman key exchange; I've moved those into sshdh.c and made
them part of the 'extra' structure for that family only, so that
unrelated kex methods don't have to faff about saying NULL,NULL,0,0.
(This required me to write an extra accessor function for ssh.c to ask
whether a DH method was group-exchange style or fixed-group style, but
that doesn't seem too silly.)
Not all of them, but the ones that don't get a 'void *key' parameter.
This means I can share methods between multiple ssh_signkey
structures, and still give those methods an easy way to find out which
public key method they're dealing with, by loading parameters from a
larger structure in which the ssh_signkey is the first element.
(In OO terms, I'm arranging that all static methods of my public key
classes get a pointer to the class vtable, to make up for not having a
pointer to the class instance.)
I haven't actually done anything with the new facility in this commit,
but it will shortly allow me to clean up the constant lookups by curve
name in the ECDSA code.
There were ad-hoc functions for fingerprinting a bare key blob in both
cmdgen.c and pageant.c, not quite doing the same thing. Also, every
SSH-2 public key algorithm in the code base included a dedicated
fingerprint() method, which is completely pointless since SSH-2 key
fingerprints are computed in an algorithm-independent way (just hash
the standard-format public key blob), so each of those methods was
just duplicating the work of the public_blob() method with a less
general output mechanism.
Now sshpubk.c centrally provides an ssh2_fingerprint_blob() function
that does all the real work, plus an ssh2_fingerprint() function that
wraps it and deals with calling public_blob() to get something to
fingerprint. And the fingerprint() method has been completely removed
from ssh_signkey and all its implementations, and good riddance.
The source paper, and OpenSSH, agree that the lowest bit index used
from the hash of the private key is bit 3, i.e. bits 0,1,2 at the
bottom are all zero. We were only clearing bits 0 and 1, which would
have worked for about half of keys. I must have got lucky during
testing!
This is the kex protocol id "curve25519-sha256@libssh.org", so called
because it's over the prime field of order 2^255 - 19.
Arithmetic in this curve is done using the Montgomery representation,
rather than the Weierstrass representation. So 'struct ec_curve' has
grown a discriminant field and a union of subtypes.
Several of the functions in ssh2_signkey, and one or two SSH-1 key
functions too, were still taking assorted non-const buffer parameters
that had never been properly constified. Sort them all out.
This is better than listing all the algorithm names in yet another
place that will then need updating when a new key format is added.
However, that also means I need to find a new place to put the
'npieces' value I was previously setting up differently per key type;
since that's a fundamental property of the key format, I've moved it
to a constant field in the ssh_signkey structure, and filled that
field in for all the existing key types with the values from the
replaced code in openssh_read_new().
There was an error-handling path testing the wrong variable; an
inappropriate call to ec_point_free in decodepoint() (in fact, that
function always gets passed a pointer to an ec_point structure that's
not a dynamically allocated block at all or not in its own right, so
we should have just cleared its contents without freeing the structure
itself); a missing return on an error path which would have caused the
same structure to be freed a second time; and two missing freebn in
ecdsa_sign.
Patch due to Tim Kosse.
This provides support for ECDSA public keys, for both hosts and users,
and also ECDH key exchange. Supported curves are currently just the
three NIST curves required by RFC 5656.
Simon Tatham
Chris Staite