1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00
Commit Graph

142 Commits

Author SHA1 Message Date
Simon Tatham
25b034ee39 Complete rewrite of PuTTY's bignum library.
The old 'Bignum' data type is gone completely, and so is sshbn.c. In
its place is a new thing called 'mp_int', handled by an entirely new
library module mpint.c, with API differences both large and small.

The main aim of this change is that the new library should be free of
timing- and cache-related side channels. I've written the code so that
it _should_ - assuming I haven't made any mistakes - do all of its
work without either control flow or memory addressing depending on the
data words of the input numbers. (Though, being an _arbitrary_
precision library, it does have to at least depend on the sizes of the
numbers - but there's a 'formal' size that can vary separately from
the actual magnitude of the represented integer, so if you want to
keep it secret that your number is actually small, it should work fine
to have a very long mp_int and just happen to store 23 in it.) So I've
done all my conditionalisation by means of computing both answers and
doing bit-masking to swap the right one into place, and all loops over
the words of an mp_int go up to the formal size rather than the actual
size.

I haven't actually tested the constant-time property in any rigorous
way yet (I'm still considering the best way to do it). But this code
is surely at the very least a big improvement on the old version, even
if I later find a few more things to fix.

I've also completely rewritten the low-level elliptic curve arithmetic
from sshecc.c; the new ecc.c is closer to being an adjunct of mpint.c
than it is to the SSH end of the code. The new elliptic curve code
keeps all coordinates in Montgomery-multiplication transformed form to
speed up all the multiplications mod the same prime, and only converts
them back when you ask for the affine coordinates. Also, I adopted
extended coordinates for the Edwards curve implementation.

sshecc.c has also had a near-total rewrite in the course of switching
it over to the new system. While I was there, I've separated ECDSA and
EdDSA more completely - they now have separate vtables, instead of a
single vtable in which nearly every function had a big if statement in
it - and also made the externally exposed types for an ECDSA key and
an ECDH context different.

A minor new feature: since the new arithmetic code includes a modular
square root function, we can now support the compressed point
representation for the NIST curves. We seem to have been getting along
fine without that so far, but it seemed a shame not to put it in,
since it was suddenly easy.

In sshrsa.c, one major change is that I've removed the RSA blinding
step in rsa_privkey_op, in which we randomise the ciphertext before
doing the decryption. The purpose of that was to avoid timing leaks
giving away the plaintext - but the new arithmetic code should take
that in its stride in the course of also being careful enough to avoid
leaking the _private key_, which RSA blinding had no way to do
anything about in any case.

Apart from those specific points, most of the rest of the changes are
more or less mechanical, just changing type names and translating code
into the new API.
2018-12-31 14:54:59 +00:00
Simon Tatham
3214563d8e Convert a lot of 'int' variables to 'bool'.
My normal habit these days, in new code, is to treat int and bool as
_almost_ completely separate types. I'm still willing to use C's
implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine,
no need to spell it out as blob.len != 0), but generally, if a
variable is going to be conceptually a boolean, I like to declare it
bool and assign to it using 'true' or 'false' rather than 0 or 1.

PuTTY is an exception, because it predates the C99 bool, and I've
stuck to its existing coding style even when adding new code to it.
But it's been annoying me more and more, so now that I've decided C99
bool is an acceptable thing to require from our toolchain in the first
place, here's a quite thorough trawl through the source doing
'boolification'. Many variables and function parameters are now typed
as bool rather than int; many assignments of 0 or 1 to those variables
are now spelled 'true' or 'false'.

I managed this thorough conversion with the help of a custom clang
plugin that I wrote to trawl the AST and apply heuristics to point out
where things might want changing. So I've even managed to do a decent
job on parts of the code I haven't looked at in years!

To make the plugin's work easier, I pushed platform front ends
generally in the direction of using standard 'bool' in preference to
platform-specific boolean types like Windows BOOL or GTK's gboolean;
I've left the platform booleans in places they _have_ to be for the
platform APIs to work right, but variables only used by my own code
have been converted wherever I found them.

In a few places there are int values that look very like booleans in
_most_ of the places they're used, but have a rarely-used third value,
or a distinction between different nonzero values that most users
don't care about. In these cases, I've _removed_ uses of 'true' and
'false' for the return values, to emphasise that there's something
more subtle going on than a simple boolean answer:
 - the 'multisel' field in dialog.h's list box structure, for which
   the GTK front end in particular recognises a difference between 1
   and 2 but nearly everything else treats as boolean
 - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you
   something about the specific location of the urgent pointer, but
   most clients only care about 0 vs 'something nonzero'
 - the return value of wc_match, where -1 indicates a syntax error in
   the wildcard.
 - the return values from SSH-1 RSA-key loading functions, which use
   -1 for 'wrong passphrase' and 0 for all other failures (so any
   caller which already knows it's not loading an _encrypted private_
   key can treat them as boolean)
 - term->esc_query, and the 'query' parameter in toggle_mode in
   terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h,
   but can also hold -1 for some other intervening character that we
   don't support.

In a few places there's an integer that I haven't turned into a bool
even though it really _can_ only take values 0 or 1 (and, as above,
tried to make the call sites consistent in not calling those values
true and false), on the grounds that I thought it would make it more
confusing to imply that the 0 value was in some sense 'negative' or
bad and the 1 positive or good:
 - the return value of plug_accepting uses the POSIXish convention of
   0=success and nonzero=error; I think if I made it bool then I'd
   also want to reverse its sense, and that's a job for a separate
   piece of work.
 - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1
   represent the default and alternate screens. There's no obvious
   reason why one of those should be considered 'true' or 'positive'
   or 'success' - they're just indices - so I've left it as int.

ssh_scp_recv had particularly confusing semantics for its previous int
return value: its call sites used '<= 0' to check for error, but it
never actually returned a negative number, just 0 or 1. Now the
function and its call sites agree that it's a bool.

In a couple of places I've renamed variables called 'ret', because I
don't like that name any more - it's unclear whether it means the
return value (in preparation) for the _containing_ function or the
return value received from a subroutine call, and occasionally I've
accidentally used the same variable for both and introduced a bug. So
where one of those got in my way, I've renamed it to 'toret' or 'retd'
(the latter short for 'returned') in line with my usual modern
practice, but I haven't done a thorough job of finding all of them.

Finally, one amusing side effect of doing this is that I've had to
separate quite a few chained assignments. It used to be perfectly fine
to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a
the 'true' defined by stdbool.h, that idiom provokes a warning from
gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-03 13:45:00 +00:00
Simon Tatham
a6f1709c2f Adopt C99 <stdbool.h>'s true/false.
This commit includes <stdbool.h> from defs.h and deletes my
traditional definitions of TRUE and FALSE, but other than that, it's a
100% mechanical search-and-replace transforming all uses of TRUE and
FALSE into the C99-standardised lowercase spellings.

No actual types are changed in this commit; that will come next. This
is just getting the noise out of the way, so that subsequent commits
can have a higher proportion of signal.
2018-11-03 13:45:00 +00:00
Simon Tatham
56096ba558 New utility functions to make ptrlens.
One to make one from a NUL-terminated string, and another to make one
from a strbuf. I've switched over all the obvious cases where I should
have been using these functions.
2018-10-15 22:48:58 +01:00
Simon Tatham
3f0f6d2013 Missing error message when loading a private key file.
If the file is empty, or otherwise fails to start with a recognised
'PuTTY-User-Key-File' header line, we forgot to fill in the error
message before returning failure, leading to a null pointer
dereference.
2018-10-09 18:52:11 +01:00
Simon Tatham
8b98fea4ae New BinarySink function 'put_padding'.
It is to put_data what memset is to memcpy. Several places
in the code wanted it already, but not _quite_ enough for me to
have written it with the rest of the BinarySink infrastructure
originally.
2018-06-09 14:20:33 +01:00
Simon Tatham
06a14fe8b8 Reorganise ssh_keyalg and use it as a vtable.
After Pavel Kryukov pointed out that I have to put _something_ in the
'ssh_key' structure, I thought of an actually useful thing to put
there: why not make it store a pointer to the ssh_keyalg structure?
Then ssh_key becomes a classoid - or perhaps 'traitoid' is a closer
analogy - in the same style as Socket and Plug. And just like Socket
and Plug, I've also arranged a system of wrapper macros that avoid the
need to mention the 'object' whose method you're invoking twice at
each call site.

The new vtable pointer directly replaces an existing field of struct
ec_key (which was usable by several different ssh_keyalgs, so it
already had to store a pointer to the currently active one), and also
replaces the 'alg' field of the ssh2_userkey structure that wraps up a
cryptographic key with its comment field.

I've also taken the opportunity to clean things up a bit in general:
most of the methods now have new and clearer names (e.g. you'd never
know that 'newkey' made a public-only key while 'createkey' made a
public+private key pair unless you went and looked it up, but now
they're called 'new_pub' and 'new_priv' you might be in with a
chance), and I've completely removed the openssh_private_npieces field
after realising that it was duplicating information that is actually
_more_ conveniently obtained by calling the new_priv_openssh method
(formerly openssh_createkey) and throwing away the result.
2018-06-03 15:15:51 +01:00
Simon Tatham
7f56e1e365 Remove 'keystr' parameter in get_rsa_ssh1_pub.
This parameter returned a substring of the input, which was used for
two purposes. Firstly, it was used to hash the host and server keys
during the initial SSH-1 key setup phase; secondly, it was used to
check the keys in Pageant against the public key blob of a key
specified on the command line.

Unfortunately, those two purposes didn't agree! The first one needs
just the bare key modulus bytes (without even the SSH-1 mpint length
header); the second needs the entire key blob. So, actually, it seems
to have never worked in SSH-1 to say 'putty -i keyfile' and have PuTTY
find that key in Pageant and not have to ask for the passphrase to
decrypt the version on disk.

Fixed by removing that parameter completely, which simplifies all the
_other_ call sites, and replacing it by custom code in those two
places that each does the actually right thing.
2018-06-03 08:24:59 +01:00
Simon Tatham
ae3edcdfc0 Clean up ssh_keyalg APIs and implementations.
Quite a few of the function pointers in the ssh_keyalg vtable now take
ptrlen arguments in place of separate pointer and length pairs.
Meanwhile, the various key types' implementations of those functions
now work by initialising a BinarySource with the input ptrlen and
using the new decode functions to walk along it.

One exception is the openssh_createkey method which reads a private
key in the wire format used by OpenSSH's SSH-2 agent protocol, which
has to consume a prefix of a larger data stream, and tell the caller
how much of that data was the private key. That function now takes an
actual BinarySource, and passes that directly to the decode functions,
so that on return the caller finds that the BinarySource's read
pointer has been advanced exactly past the private key.

This let me throw away _several_ reimplementations of mpint-reading
functions, one in each of sshrsa, sshdss.c and sshecc.c. Worse still,
they didn't all have exactly the SSH-2 semantics, because the thing in
sshrsa.c whose name suggested it was an mpint-reading function
actually tolerated the wrong number of leading zero bytes, which it
had to be able to do to cope with the "ssh-rsa" signature format which
contains a thing that isn't quite an SSH-2 mpint. Now that deviation
is clearly commented!
2018-06-02 18:00:59 +01:00
Simon Tatham
28c086ca9a Rewrite key loading functions using BinarySource.
This does for sshpubk.c's handling of PuTTY's native key formats what
the previous commit did for the foreign formats handled by import.c.
2018-06-02 17:57:23 +01:00
Simon Tatham
9e96af59ce Introduce a new 'ptrlen' type.
This wraps up a (pointer, length) pair into a convenient struct that
lets me return it by value from a function, and also pass it through
to other functions in one go.

Ideally quite a lot of this code base could be switched over to using
ptrlen in place of separate pointer and length variables or function
parameters. (In fact, in my personal ideal conception of C, the usual
string type would be of this form, and all the string.h functions
would operate on ptrlens instead of zero-terminated 'char *'.)

For the moment, I'm just introducing it to make some upcoming
refactoring less inconvenient. Bulk migration of existing code to
ptrlen is a project for another time.

Along with the type itself, I've provided a convenient system of
including the contents of a ptrlen in a printf; a constructor function
that wraps up a pointer and length so you can make a ptrlen on the fly
in mid-expression; a function to compare a ptrlen against an ordinary
C string (which I mostly expect to use with string literals); and a
function 'mkstr' to make a dynamically allocated C string out of one.
That last function replaces a function of the same name in sftp.c,
which I'm promoting to a whole-codebase facility and adjusting its
API.
2018-06-02 17:33:23 +01:00
Simon Tatham
0fc2d3b455 Invent a struct type for polymorphic SSH key data.
During last week's work, I made a mistake in which I got the arguments
backwards in one of the key-blob-generating functions - mistakenly
swapped the 'void *' key instance with the 'BinarySink *' output
destination - and I didn't spot the mistake until run time, because in
C you can implicitly convert both to and from void * and so there was
no compile-time failure of type checking.

Now that I've introduced the FROMFIELD macro that downcasts a pointer
to one field of a structure to retrieve a pointer to the whole
structure, I think I might start using that more widely to indicate
this kind of polymorphic subtyping. So now all the public-key
functions in the struct ssh_signkey vtable handle their data instance
in the form of a pointer to a subfield of a new zero-sized structure
type 'ssh_key', which outside the key implementations indicates 'this
is some kind of key instance but it could be of any type'; they
downcast that pointer internally using FROMFIELD in place of the
previous ordinary C cast, and return one by returning &foo->sshk for
whatever foo they've just made up.

The sshk member is not at the beginning of the structure, which means
all those FROMFIELDs and &key->sshk are actually adding and
subtracting an offset. Of course I could have put the member at the
start anyway, but I had the idea that it's actually a feature _not_ to
have the two types start at the same address, because it means you
should notice earlier rather than later if you absentmindedly cast
from one to the other directly rather than by the approved method (in
particular, if you accidentally assign one through a void * and back
without even _noticing_ you perpetrated a cast). In particular, this
enforces that you can't sfree() the thing even once without realising
you should instead of called the right freekey function. (I found
several bugs by this method during initial testing, so I think it's
already proved its worth!)

While I'm here, I've also renamed the vtable structure ssh_signkey to
ssh_keyalg, because it was a confusing name anyway - it describes the
_algorithm_ for handling all keys of that type, not a specific key. So
ssh_keyalg is the collection of code, and ssh_key is one instance of
the data it handles.
2018-05-27 15:28:54 +01:00
Simon Tatham
8ce0a67028 Use BinarySink to tidy up key export code.
The output routines in import.c and sshpubk.c were further horrifying
hotbeds of manual length-counting. Reworked it all so that it builds
up key file components in strbufs, and uses the now boringly standard
put_* functions to write into those strbufs.

This removes the write_* functions in import.c, which I had to hastily
rename a few commits ago when I introduced the new marshalling system
in the first place.

However, I wasn't quite able to get rid of _all_ of import.c's local
formatting functions; there are a couple still there (but now with new
BinarySink-style API) which output multiprecision integers in a couple
of different formats starting from an existing big-endian binary
representation, as opposed to starting from an internal Bignum.
2018-05-25 14:36:16 +01:00
Simon Tatham
67de463cca Change ssh.h crypto APIs to output to BinarySink.
This affects all the functions that generate public and private key
and signature blobs of all kinds, plus ssh_ecdhkex_getpublic. Instead
of returning a bare block of memory and taking an extra 'int *length'
parameter, all these functions now write to a BinarySink, and it's the
caller's job to have prepared an appropriate one where they want the
output to go (usually a strbuf).

The main value of this change is that those blob-generation functions
were chock full of ad-hoc length-counting and data marshalling. You
have only to look at rsa2_{public,private}_blob, for example, to see
the kind of thing I was keen to get rid of!
2018-05-25 14:36:16 +01:00
Simon Tatham
4988fd410c Replace all uses of SHA*_Bytes / MD5Update.
In fact, those functions don't even exist any more. The only way to
get data into a primitive hash state is via the new put_* system. Of
course, that means put_data() is a viable replacement for every
previous call to one of the per-hash update functions - but just
mechanically doing that would have missed the opportunity to simplify
a lot of the call sites.
2018-05-25 14:36:16 +01:00
Simon Tatham
2259f3d335 Fix null deref on writing OpenSSH pubkey with no comment.
If we're called on to generate the one-line OpenSSH public key format
for a key that we don't have a comment field for, we were mistakenly
testing this by checking if '*comment' rather than 'comment' was zero,
i.e. if comment was NULL we'd dereference it by mistake.
2018-05-25 14:08:42 +01:00
Simon Tatham
7e8ae41a3f Clean up the crufty old SSH-1 RSA API.
Lots of functions had really generic names (like 'makekey'), or names
that missed out an important concept (like 'rsakey_pubblob', which
loads a public blob from a _file_ and doesn't generate it from an
in-memory representation at all). Also, the opaque 'int order' that
distinguishes the two formats of public key blob is now a mnemonic
enumeration, and while I'm at it, rsa_ssh1_public_blob takes one of
those as an extra argument.
2018-05-25 14:08:24 +01:00
Simon Tatham
12a080874f Add an assortment of missing frees and closes.
Coverity's resource-leak checker is on the ball as usual.
2017-02-14 22:14:25 +00:00
Tim Kosse
737cb2d24e Fix a memory leak in openssh_loadpub
The line read from the input file was not freed after successfully
loading an OpenSSH key.
2017-01-28 14:03:09 +00:00
Tim Kosse
29d805a49e Fix a memory leak in rsakey_pubblob
The line read from the input file was not freed after successfully
loading an SSH1 key.
2017-01-28 14:03:09 +00:00
Tim Kosse
8afae2a5f8 Fix resource leak in rsakey_pubblob
The input file was not closed after successfully loading an SSH1 key.
2017-01-28 14:03:09 +00:00
Simon Tatham
89da2ddf56 Giant const-correctness patch of doom!
Having found a lot of unfixed constness issues in recent development,
I thought perhaps it was time to get proactive, so I compiled the
whole codebase with -Wwrite-strings. That turned up a huge load of
const problems, which I've fixed in this commit: the Unix build now
goes cleanly through with -Wwrite-strings, and the Windows build is as
close as I could get it (there are some lingering issues due to
occasional Windows API functions like AcquireCredentialsHandle not
having the right constness).

Notable fallout beyond the purely mechanical changing of types:
 - the stuff saved by cmdline_save_param() is now explicitly
   dupstr()ed, and freed in cmdline_run_saved.
 - I couldn't make both string arguments to cmdline_process_param()
   const, because it intentionally writes to one of them in the case
   where it's the argument to -pw (in the vain hope of being at least
   slightly friendly to 'ps'), so elsewhere I had to temporarily
   dupstr() something for the sake of passing it to that function
 - I had to invent a silly parallel version of const_cmp() so I could
   pass const string literals in to lookup functions.
 - stripslashes() in pscp.c and psftp.c has the annoying strchr nature
2015-05-15 12:47:44 +01:00
Simon Tatham
870ad6ab07 Pass the ssh_signkey structure itself to public key methods.
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.
2015-05-15 10:12:07 +01:00
Simon Tatham
a5fc95b715 Const-correctness of name fields in struct ssh_*.
All the name strings in ssh_cipher, ssh_mac, ssh_hash, ssh_signkey
point to compile-time string literals, hence should obviously be const
char *.

Most of these const-correctness patches are just a mechanical job of
adding a 'const' in the one place you need it right now, and then
chasing the implications through the code adding further consts until
it compiles. But this one has actually shown up a bug: the 'algorithm'
output parameter in ssh2_userkey_loadpub was sometimes returning a
pointer to a string literal, and sometimes a pointer to dynamically
allocated memory, so callers were forced to either sometimes leak
memory or sometimes free a bad thing. Now it's consistently
dynamically allocated, and should be freed everywhere too.
2015-05-15 10:12:06 +01:00
Simon Tatham
8682246d33 Centralise SSH-2 key fingerprinting into sshpubk.c.
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.
2015-05-12 14:56:38 +01:00
Simon Tatham
eef0235a0f Centralise public-key output code into sshpubk.c.
There was a fair amount of duplication between Windows and Unix
PuTTYgen, and some confusion over writing things to FILE * and
formatting them internally into strings. I think all the public-key
output code now lives in sshpubk.c, and there's only one copy of the
code to generate each format.
2015-05-12 14:56:38 +01:00
Simon Tatham
f274b56a57 Const-correctness in the base64 functions. 2015-05-12 14:56:38 +01:00
Simon Tatham
3935cc3af1 Support loading public-key-only files in Unix PuTTYgen.
The rsakey_pubblob() and ssh2_userkey_loadpub() functions, which
expected to be given a private key file and load only the unencrypted
public half, now also cope with any of the public-only formats I know
about (SSH-1 only has one, whereas SSH-2 has the RFC 4716 format and
OpenSSH's one-line format) and return an appropriate public key blob
from each of those too.

cmdgen now supports this functionality, by permitting public key files
to be loaded and used by any operation that doesn't need the private
key: so you can convert back and forth between the SSH-2 public
formats, or list the file's fingerprint.
2015-05-12 12:19:57 +01:00
Simon Tatham
8c4ce6d8c6 Const-correctness in key-loading functions.
The passphrase parameter should be a const char *.
2015-05-11 15:49:09 +01:00
Simon Tatham
90af5bed04 Sort out the mess with OpenSSH key file formats.
When I implemented reading and writing of the new format a couple of
weeks ago, I kept them strictly separate in the UI, so you have to ask
for the format you want when exporting. But in fact this is silly,
because not every key type can be saved in both formats, and OpenSSH
itself has the policy of using the old format for key types it can
handle, unless specifically asked to use the new one.

So I've now arranged that the key file format enum has three values
for OpenSSH: PEM, NEW and AUTO. Files being loaded are identified as
either PEM or NEW, which describe the two physical file formats. But
exporting UIs present either AUTO or NEW, where AUTO is the virtual
format meaning 'save in the old format if possible, otherwise the new
one'.
2015-05-10 13:11:43 +01:00
Chris Staite
76a4b576e5 Support public keys using the "ssh-ed25519" method.
This introduces a third system of elliptic curve representation and
arithmetic, namely Edwards form.
2015-05-09 15:14:35 +01:00
Simon Tatham
1f4dc6faa7 Remove the list of key algorithms in pageant.c.
The only reason those couldn't be replaced with a call to the
centralised find_pubkey_alg is because that function takes a zero-
terminated string and instead we had a (length,pointer) string. Easily
fixed; there's now a find_pubkey_alg_len(), and we call that.

This also fixes a string-matching bug in which the sense of memcmp was
reversed by mistake for ECDSA keys!
2015-05-07 19:59:07 +01:00
Simon Tatham
79bbf37c9e Separate key-type enum values for old and new OpenSSH keys.
It's all very well for these two different formats to share a type
code as long as we're only loading them and not saving, but as soon as
we need to save one or the other, we'll need different type codes
after all.

This commit introduces the openssh_new_write() function, but for the
moment, it always returns failure.
2015-04-28 19:48:43 +01:00
Simon Tatham
78989c97c9 Fix a few memory leaks.
Patch due to Chris Staite.
2015-04-26 10:49:24 +01:00
Simon Tatham
808e414130 Merge branch 'pre-0.64' 2015-02-28 07:57:58 +00:00
Simon Tatham
65f69bca73 Add some missing smemclrs and sfrees.
The absence of these could have prevented sensitive private key
information from being properly cleared out of memory that PuTTY tools
had finished with.

Thanks to Patrick Coleman for spotting this and sending a patch.
2015-02-28 07:57:35 +00:00
Simon Tatham
1f75792805 Fix an erroneous length field in SSH-1 key load.
We incremented buf by a few bytes, so we must decrement the
corresponding length by the same amount, or else makekey() could
overrun.

Thanks to Patrick Coleman for the patch.
2015-02-28 07:57:35 +00:00
Chris Staite
2bf8688355 Elliptic-curve cryptography support.
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.
2014-11-02 18:16:54 +00:00
Simon Tatham
32adc1a79d Fix two double-frees in ssh2_load_userkey().
We should NULL out mac after freeing it, so that the cleanup code
doesn't try to free it again; also if the final key creation fails, we
should avoid freeing ret->comment when we're going to go to that same
cleanup code which will free 'comment' which contains the same pointer.

Thanks to Christopher Staite for pointing these out.
2014-10-28 18:40:43 +00:00
Simon Tatham
80a9a7918a Move base64_decode_atom into misc.c.
I'm about to need to refer to it from a source file that won't
necessarily always be linked against sshpubk.c, so it needs to live
somewhere less specialist. Now it sits alongside base64_encode_atom
(already in misc.c for another reason), which is neater anyway.

[originally from svn r10218]
2014-09-09 11:46:10 +00:00
Simon Tatham
16d62f6d94 Remove a redundant while-loop condition when reading RFC822-style
header text from a PuTTY key file.

(It's silly to have both while (len > 0) at the top of the loop _and_
an if (len == 0) return in the middle, and in fact the former was the
erroneous one since it would have prohibited a 39-character header,
which I intended to be permitted.)

[originally from svn r9926]
2013-07-14 10:46:55 +00:00
Simon Tatham
ea301bdd9b Fix another giant batch of resource leaks. (Mostly memory, but there's
one missing fclose too.)

[originally from svn r9919]
2013-07-14 10:46:07 +00:00
Simon Tatham
896bb7c74d Tighten up a lot of casts from unsigned to int which are read by one
of the GET_32BIT macros and then used as length fields. Missing bounds
checks against zero have been added, and also I've introduced a helper
function toint() which casts from unsigned to int in such a way as to
avoid C undefined behaviour, since I'm not sure I trust compilers any
more to do the obviously sensible thing.

[originally from svn r9918]
2013-07-14 10:45:54 +00:00
Simon Tatham
85300e0b15 Fix a mishandling of error returns from makekey() in the SSH-1 private
key loader.

[originally from svn r9905]
2013-07-11 17:24:04 +00:00
Simon Tatham
1dbdd2c43f Add another missing bounds check in the SSH-1 private key loader.
[originally from svn r9904]
2013-07-11 17:24:01 +00:00
Simon Tatham
bbc9709b48 A collection of small bug fixes from Chris West, apparently spotted by
Coverity: assorted language-use goofs like freeing the wrong thing or
forgetting to initialise a string on all code paths.

[originally from svn r9889]
2013-07-01 17:56:33 +00:00
Simon Tatham
40ce043a97 Give a better error message if a PuTTY private key file has a version
number we don't understand. It's nicer to report 'format too new' than
'not a PuTTY key file'.

[originally from svn r9763]
2013-02-22 21:39:02 +00:00
Simon Tatham
aa5bae8916 Introduce a new utility function smemclr(), which memsets things to
zero but does it in such a way that over-clever compilers hopefully
won't helpfully optimise the call away if you do it just before
freeing something or letting it go out of scope. Use this for
(hopefully) every memset whose job is to destroy sensitive data that
might otherwise be left lying around in the process's memory.

[originally from svn r9586]
2012-07-22 19:51:50 +00:00
Simon Tatham
62cbc7dc0b Turn 'Filename' into a dynamically allocated type with no arbitrary
length limit, just as I did to FontSpec yesterday.

[originally from svn r9316]
2011-10-02 11:01:57 +00:00
Jacob Nevins
c9326ad81b Make key file import robust against a PPK file that has somehow lost its
final newline. From Debian bug #414784, based on a patch by Justin Pryzby.

[originally from svn r8323]
2008-11-23 20:11:12 +00:00
Simon Tatham
4fa9564c90 Fix `puttygen-unix-perms': f_open(), PuTTY's wrapper on fopen, now
takes a third argument which is TRUE if the file is being opened for
writing and wants to be created in such a way that it's readable
only to the owner. This is used when saving private keys.

While I'm here, I also use this option when writing session logs, on
the general principle that they probably contain _something_
sensitive.

The new argument is only supported on Unix, for the moment. (I think
writing owner-accessible-only files is the default on Windows.)

[originally from svn r7084]
2007-01-09 18:14:30 +00:00
Jacob Nevins
cd94e3bc3c Patch from Colin Watson intended to give a clean Unix compile with GCC 4.
(Since we choose to compile with -Werror, this is particularly important.)

I haven't yet checked that the resulting source actually compiles cleanly with
GCC 4, hence not marking `gcc4-warnings' as fixed just yet.

[originally from svn r7041]
2006-12-30 23:00:14 +00:00
Jacob Nevins
ff05fb7ccc Fixes to make r6434 actually useful.
[originally from svn r6435]
[r6434 == c14f259ba2]
2005-10-30 16:28:45 +00:00
Jacob Nevins
c14f259ba2 Allow rsakey_pubblob() to return the key comment.
(like r6433 but for SSH-1)

[originally from svn r6434]
[r6433 == 49d2cf19ac]
2005-10-30 15:16:42 +00:00
Jacob Nevins
49d2cf19ac Add ability for ssh2_userkey_loadpub() to return the key comment.
(Not actually used currently, but it makes life easier for a patch I'm
working on.)

[originally from svn r6433]
2005-10-30 13:42:36 +00:00
Jacob Nevins
6eec320f0b Unify GET_32BIT()/PUT_32BIT() et al from numerous source files into misc.h.
I've done a bit of testing (not exhaustive), and I don't _think_ I've broken
anything...

[originally from svn r5632]
2005-04-12 20:04:56 +00:00
Jacob Nevins
5aa719d16e Consistently use a single notation to refer to SSH protocol versions, as
discussed. Use Barrett and Silverman's convention of "SSH-1" for SSH protocol
version 1 and "SSH-2" for protocol 2 ("SSH1"/"SSH2" refer to ssh.com
implementations in this scheme). <http://www.snailbook.com/terms.html>

[originally from svn r5480]
2005-03-10 16:36:05 +00:00
Jacob Nevins
6be9bb74bb rm incorrect comment
[originally from svn r5429]
2005-03-02 00:31:33 +00:00
Jacob Nevins
ec76c24084 Fix off-by-one in memory management of PPK reading routine, which could cause
1-byte buffer overflow when reading .PPK files with long lines (>=128 bytes
in header value -- probably only happened in practice in the comment field).

[originally from svn r5427]
2005-03-01 23:48:45 +00:00
Jacob Nevins
d88995b856 The comment that Private-Hash: was not allowed in DSS PuTTY-User-Key-File-1
keys was apparently incorrect; prior to r1413, it was both allowed, and
generated for passphraseless keys. Remove it and associated validation so that
people are able to load such keys into PuTTYgen to upgrade them, as suggested.

[originally from svn r5403]
[r1413 == 6608016fc2]
2005-02-26 15:50:29 +00:00
Simon Tatham
15c96004a7 Trivial bug fix pointed out by Paul Fox: potentially missing fclose().
[originally from svn r4877]
2004-11-22 11:02:44 +00:00
Simon Tatham
4217269931 Merged SSH1 robustness changes from 0.55 release branch on to trunk.
[originally from svn r4379]
2004-08-01 12:07:11 +00:00
Simon Tatham
72083d11ac Reasonably thorough test suite for command-line PuTTYgen, and
several bugs fixed in the process of constructing same.

[originally from svn r3767]
2004-01-24 17:16:37 +00:00
Simon Tatham
2d1287b9ca Added a command-line key generation tool. Currently builds and runs
on Linux, but the (very few) platform-specific bits are already
abstracted out of the main code, so it should port to other
platforms with a minimum of fuss.

[originally from svn r3762]
2004-01-22 19:15:32 +00:00
Simon Tatham
a06375ab96 I _think_ this should fix the problem reported by `Stacey': failure
to fclose a private key file.

[originally from svn r3552]
2003-11-19 17:30:16 +00:00
Jacob Nevins
2cde290c77 Oops. Fix a bug in my `keyfile-diagnostic' work which caused SSH-2 key loading
in pageant (and presumably puttygen) to crash.

[originally from svn r3450]
2003-09-10 12:30:10 +00:00
Jacob Nevins
3796897d7c Change an incorrect comment about "PuTTY-User-Key-File-2" private key format
after discussion with Simon.

[originally from svn r3440]
2003-09-02 19:02:06 +00:00
Jacob Nevins
15ac9c09eb When loading SSH-2 key, ignore passphrase argument if key is unencrypted.
This should get rid of a problem that three or four people reported where
PuTTY intermittently reports "Unable to load private key" (MAC failed).

(ssh.c:do_ssh2_authconn() should also initialise its passphrase so it's not
passing garbage passphrases around, of course, but I haven't yet worked out
where the best place in the auth loop to do that would be.)

[originally from svn r3439]
2003-09-02 19:00:17 +00:00
Jacob Nevins
eebc7529ed Work towards wish `keyfile-diagnostic'. Many sshpubk.c keyfile-loading
functions have sprouted `**errorstr' arguments, which if non-NULL can
return a textual error message. The interface additions are patchy and
ad-hoc since this seemed to suit the style of the existing interfaces.

I've since realised that most of this is masked by sanity-checking that
gets done before these functions are called, but it will at least report
MAC failures and the like (tested on Unix), which was the original point
of the exercise.

Note that not everyone who could be using this information is at the
moment.

[originally from svn r3430]
2003-08-29 22:52:57 +00:00
Simon Tatham
d36a4c3685 Introduced wrapper macros snew(), snewn() and sresize() for the
malloc functions, which automatically cast to the same type they're
allocating the size of. Should prevent any future errors involving
mallocing the size of the wrong structure type, and will also make
life easier if we ever need to turn the PuTTY core code from real C
into C++-friendly C. I haven't touched the Mac frontend in this
checkin because I couldn't compile or test it.

[originally from svn r3014]
2003-03-29 16:14:26 +00:00
Simon Tatham
8aa218fd1d Initialise a variable that might otherwise have caused trouble.
Thanks to Ross Younger.

[originally from svn r2769]
2003-02-01 17:25:06 +00:00
Simon Tatham
f26b7aa0d3 Created new data types Filename' and FontSpec', intended to be
opaque to all platform-independent modules and only handled within
per-platform code. `Filename' is there because the Mac has a magic
way to store filenames (though currently this checkin doesn't
support it!); `FontSpec' is there so that all the auxiliary stuff
such as font height and charset and so on which is needed under
Windows but not Unix can be kept where it belongs, and so that I can
have a hope in hell of dealing with a font chooser in the forthcoming
cross-platform config box code, and best of all it gets the horrid
font height wart out of settings.c and into the Windows code where
it should be.
The Mac part of this checkin is a bunch of random guesses which will
probably not quite compile, but which look roughly right to me.
Sorry if I screwed it up, Ben :-)

[originally from svn r2765]
2003-02-01 12:54:40 +00:00
Ben Harris
2af5d85989 Fix more unsigned char * vs char * warnings, usually by making implicit
casts explicit.

[originally from svn r2464]
2003-01-05 14:11:14 +00:00
Simon Tatham
442a360fb2 Support username and password authentication when talking to HTTP
proxies.

[originally from svn r1971]
2002-09-21 14:03:05 +00:00
Simon Tatham
ff5241c1ed Added export of ssh.com key files.
[originally from svn r1682]
2002-05-15 19:16:45 +00:00
Simon Tatham
8c3a0eb50b Improved error messages if you use the wrong key type: you should
now be told that the key is the wrong type, _and_ what type it is,
rather than being given a blanket `unable to read key file' message.

[originally from svn r1662]
2002-05-11 12:13:42 +00:00
Simon Tatham
cf356a9a5f Pageant is now able to avoid asking for the passphrase when asked to
load a key that is already loaded. This makes command lines such as
`pageant mykey -c mycommand' almost infinitely more useful.

[originally from svn r1522]
2001-12-30 15:58:17 +00:00
Simon Tatham
6608016fc2 INCOMPATIBLE CHANGE to the SSH2 private key file format. There is
now a passphrase-keyed MAC covering _all_ important data in the
file, including the public blob and the key comment. Should
conclusively scupper any attacks based on nobbling the key file in
an attempt to sucker the machine that decrypts it. MACing the
comment field also protects against a key-substitution attack (if
someone's worked out a way past our DSA protections and can extract
the private key from a signature, swapping key files and
substituting comments might just enable them to get the signature
they need to do this. Paranoid, but might as well).

[originally from svn r1413]
2001-11-25 14:31:46 +00:00
Simon Tatham
d345ebc2a5 Add support for DSA authentication in SSH2, following clever ideas
on how to get round the problem of generating a good k.

[originally from svn r1284]
2001-09-22 20:52:21 +00:00
Simon Tatham
3730ada5ce Run entire source base through GNU indent to tidy up the varying
coding styles of the various contributors! Woohoo!

[originally from svn r1098]
2001-05-06 14:35:20 +00:00
Simon Tatham
522f130391 Pageant interface changes. You can now do `pageant -c command' to
spawn another command after starting Pageant. Also, if Pageant is
already running, `pageant keyfile' and `pageant -c command' will do
the Right Thing, that is, add the key to the _first_ Pageant and/or
run a command and then exit. The only time you now get the `Pageant
is already running' error is if you try to start the second copy
with no arguments.
NB the affected files in this checkin are rather wide-ranging
because I renamed the not really SSH1-specific
`ssh1_bignum_bitcount' function to just `bignum_bitcount'.

[originally from svn r1044]
2001-04-16 11:16:58 +00:00
Simon Tatham
e3bcd35a37 Following the recent advisory about attacks on PGP keys based on
tampering with the unencrypted public part of the key but leaving
the private part intact ... we are now ultra-paranoid about RSA key
files, and we check that the public part matches the private part
_before_ we generate any signatures with them.

[originally from svn r1021]
2001-03-22 21:48:33 +00:00
Simon Tatham
6b58ab6ad4 Fix various trivial compiler warnings
[originally from svn r983]
2001-03-05 17:31:36 +00:00
Simon Tatham
28b1fc766c Preliminary support for RSA user authentication in SSH2! Most of the
error messages are currently wrong, and Pageant doesn't yet support
the new key type, and I haven't thoroughly tested that falling back
to password authentication and trying invalid keys etc all work. But
what I have here has successfully performed a public key
authentication, so it's working to at least some extent.

[originally from svn r973]
2001-03-03 11:54:34 +00:00
Simon Tatham
d5240d4157 Make memory management uniform: _everything_ now goes through the
smalloc() macros and thence to the safemalloc() functions in misc.c.
This should allow me to plug in a debugging allocator and track
memory leaks and segfaults and things.

[originally from svn r818]
2000-12-12 10:33:13 +00:00
Simon Tatham
384eeb3f76 Fix miscellaneous compiler warnings. Thanks to Jacob Nevins
[originally from svn r755]
2000-10-24 10:47:49 +00:00
Simon Tatham
964e6407e1 Remember to close key files when rsakey_encrypted meets a bogus one
[originally from svn r739]
2000-10-20 17:57:47 +00:00
Simon Tatham
8d0bee8629 PuTTYgen initial version. Still to do are basic user-friendliness
features (prompt for passphrase twice, prompt before overwriting a
file, check the key file was actually saved OK), testing of the
generated keys to make sure I got the file format right, and support
for a variable key size. I think what's already here is basically
sound though.

[originally from svn r715]
2000-10-19 15:43:08 +00:00
Simon Tatham
48795b5896 Remove unnecessary DSS bit
[originally from svn r641]
2000-09-27 16:44:00 +00:00
Simon Tatham
673f2e48a7 Rationalise ordering of authentication operations. Still some work to do,
but at least pscp no longer hangs when prompting for a passphrase

[originally from svn r621]
2000-09-25 10:14:53 +00:00
Simon Tatham
c366174cc2 Added Pageant, a first-attempt PuTTY authentication agent
[originally from svn r589]
2000-09-14 15:02:50 +00:00
Simon Tatham
1791da274c Add sshpubk.c to load authenticating keys from files
[originally from svn r573]
2000-09-07 16:40:50 +00:00