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.
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.
Unlike ssh-add, we can identify the key by its comment or by a prefix
of its fingerprint as well as using a public key file on disk. The
string given as an argument to -d is interpreted as whichever of those
things matches; disambiguating prefixes are available if needed.
Obviously PuTTY can't actually do public-key authentication itself, if
you give it a public rather than private key file. But it can still
match the supplied public key file against the list of keys in the
agent, and narrow down to that. So if for some reason you're
forwarding an agent to a machine you don't want to trust with your
_private_ key file (even encrypted), you can still use the '-i' option
to select which key from the agent to use, by uploading just the
public key file to that machine.
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.
It's just ssh_pkt_addstring_data but using strlen to get the length of
string to add, so make that explicit by having it call
ssh_pkt_addstring_data. Good compilers should be unaffected by this
change.
You can now load keys at Pageant init time, by putting the key file
names as bare arguments on the command line, e.g. 'pageant -T key.ppk'
or 'pageant key.ppk --exec some command'; also, 'pageant -a key.ppk'
behaves more or less like ssh-add, contacting an existing agent to add
the key.
The askpass() function currently supports terminal-based prompting
only. X11 askpass is yet to be implemented.
This brings in the code we'll need to request passphrases from the
terminal, and to talk to an existing SSH agent as a client.
Adding uxcons.c required adjusting the set of stub functions in
uxpgnt.c: uxcons.c removed the need for several, but added one of its
own (log_eventlog). A net win, though.
I've moved the setup and running of the actual agent server into
run_agent(), so that main() is now only command-line parsing and
validation. We recognise a collection of new command-line options for
talking to an existing agent as a client (analogous to ssh-add), which
go to a new run_client() function, but I haven't filled in that
function itself yet.
Those must have been wrong _forever_, but because Windows Pageant
doesn't mind if the message length is longer than it should be, I've
never noticed before. How embarrassing.
This is only intended for use in Unix Pageant; for any application
that's actually trying to get something else useful done at the same
time as the agent request is pending, it's much more sensible to use
the more rigorous existing approach of requesting a callback once the
agent request is answered.
Adding this mode is the easiest way to allow Unix Pageant's
command-line key loading to work, but it doesn't solve the underlying
problem that the supposedly cross-platform pageant_add_keyfile will
not work on a platform where we really _are_ constrained to do agent
requests asynchronously (perhaps because we're a GUI app in some
system that doesn't let us control our own top-level event loop).
If and when that situation arises, I'll have no choice but to turn
pageant_add_keyfile and friends (specifically, any function in
pageant.c that calls agent_query) into coroutine-structured functions,
and have clients call them repeatedly until they return 'finished'.
But for now, this is a lot easier!
Now --exec instantly terminates option processing, by treating
everything after it as the command. This means it doesn't matter if
the --exec command word looks like another option, and it also means
we can simplify the handling of real non-option argument words, when I
get round to adding some for loading keys.
I've now centralised into pageant.c all the logic about trying to load
keys of any type, with no passphrase or with the passphrases used in
previous key-loading actions or with a new user-supplied passphrase,
whether we're the main Pageant process ourself or are talking to
another one as a client. The only part of that code remaining in
winpgnt.c is the user interaction via dialog boxes, which of course is
the part that will need to be done differently on other platforms.
This is intended to be a useful mode when you want to run an ssh agent
in a terminal session with no X11 available. You just execute a
command along the lines of eval $(pageant -T), and then Pageant will
run in the background for the rest of that terminal session - and when
the terminal session ends, so that Pageant loses its controlling tty,
it will take that as the signal to shut down. So, no need to manually
kill it, and unlike 'pageant --exec $SHELL', you can also do this half
way through a session if you don't realise until later that you need
an SSH agent, without losing any shell command history or other shell
context that you've accumulated so far in the session.
Unfortunately, I haven't been able to find any reliable way to
actually implement this -T mode, short of having Pageant wake up at
regular intervals and try to open /dev/tty to see if it's still there.
I had hoped that I could arrange to reliably get SIGHUP, or select on
/dev/tty for exceptional conditions, or some such, but nothing I've
tried along those lines seems to work.
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!
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'.
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.
I'm finding missing constifications all over the place this week.
Turns out that dmemdump() has been taking a non-const memory pointer
ever since the beginning, and it's never come up until now. How silly.
The last use of it, to store the contents of the saved session name
edit box, was removed nearly two years ago in svn r9923 and replaced
by ctrl_alloc_with_free. The mechanism has been unused ever since
then, and I suspect any further uses of it would be a bad idea for the
same reasons, so let's get rid of it.
The memory dangling off ssd->sesslist should be freed when ssd itself
goes away, and the font settings ctrlset we delete in gtkcfg.c should
be freed as well once it's been removed from its containing array.
Thanks to Ranjini Aravind for pointing these out.
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!
I've moved the listening socket setup back to before the lifetime
preparations, so in particular we find out that we couldn't bind to
the socket _before_ we fork. The only part that really needed to come
after lifetime setup was the logging setup, so that's now a separate
function called later.
Also, the random exit(0)s in silly places like x11_closing have turned
into setting a time_to_die flag, so that all clean exits funnel back
to the end of main() which at least tries to tidy up a bit afterwards.
(Finally, fixed a small bug in testing the return value of waitpid(),
which only showed up once we didn't exit(0) after the first wait.
Ahem.)
This would have caused intermittent use-after-free crashes in Windows
Pageant, but only with keys added via the primary Pageant's own UI or
command line - not keys submitted from another process, because those
don't go through the same function.
The auxiliary values (the two primes and the inverse of one mod the
other) were being read into the key structure wrongly, causing
crt_modpow() in sshrsa.c to give the wrong answers where straight
modpow would not have.
This must have been broken ever since I implemented the RSA CRT
optimisation in 2011. And nobody has noticed, which is a good sign for
the phasing out of SSH-1 :-) I only spotted it myself because I was
testing all the Pageant message types in the course of implementing
the new logging.
Now it actually logs all its requests and responses, the fingerprints
of keys mentioned in all messages, and so on.
I've also added the -v option, which causes Pageant in any mode to
direct that logging information to standard error. In --debug mode,
however, the logging output goes to standard output instead (because
when debugging, that information changes from a side effect to the
thing you actually wanted in the first place :-).
An internal tweak: the logging functions now take a va_list rather
than an actual variadic argument list, so that I can pass it through
several functions.
LIFE_EXEC is already dealt with, and I forgot to take out the comment
reminding me to do it, ahem.
The LIFE_PARENT mentioned in the same comment was an idea I had but
couldn't think of a way to make it work: if you have a terminal-only
shell session in which you want to eval $(ssh-agent), then it's
annoying and fragile to have to remember to kill the agent when you
log out, so you'd like it to automatically tie its lifetime to that of
the shell from which you invoked it. Unfortunately, I don't know of
any way to do that without race conditions. (E.g. if only pageant
didn't fork, then it could poll its own ppid until it became 1 - but
the child process would find it was 1 already.)
This is much more like ssh-agent than the Windows version is - it sets
SSH_AUTH_SOCK and SSH_AGENT_PID as its means of being found by other
processes, rather than Windows Pageant's approach of establishing
itself in a well-known location. But the actual agent code is the same
as Windows Pageant.
For the moment, this is an experimental utility and I don't expect it
to be useful to many people; its immediate use to me is that it
provides a way to test and debug the agent code on Unix, and also to
use the agent interface as a convenient way to exercise public key
functions I want to debug. And of course it means I can be constantly
using and testing my own code, on whatever platform I happen to be
using. In the further future, I have a list of possible features I
might add to it, but I don't know which ones I'll decide are
worthwhile.
One feature I've already put in is a wider range of lifetime
management options than ssh-agent: the -X mode causes Pageant to make
a connection to your X display, and automatically terminate when that
connection closes, so that it has the same lifetime as your X session
without having to do the cumbersome trick of exec()ing the subsequent
session-management process.
The exact nature of the Socket is left up to the front end to decide,
so that we can use a Unix-domain socket on Unix and a Windows named
pipe on Windows. But the logic of how we receive data and what we send
in response is all cross-platform.
I'm aiming for windows/winpgnt.c to only contain the parts of Windows
Pageant that are actually to do with handling the Windows API, and for
all the actual agent logic to be cross-platform.
This commit is a start: I've moved every function and internal
variable that was easy to move. But it doesn't get all the way there -
there's still a lot of logic in add_keyfile() and get_keylist*() that
would be good to move out to cross-platform code, but it's harder
because that code is currently quite intertwined with details of
Windows OS interfacing such as printing message boxes and passphrase
prompts and calling back out to agent_query if the Pageant doing that
job isn't the primary one.
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().
This was a lot less work than the importer, partly because the bcrypt
primitive is already working now, and mostly because we don't have to
handle the possible cross product of ciphers and kdfs in full and
completely hypothetical generality - we can emit a fixed choice of
either nothing or our chosen pair.
I thought it would be a good idea to share the loading code on the
basis that the outer header line + base64 format isn't too different,
but in fact I ended up faffing endlessly with mode bits and unions and
constantly re-testing in every subfunction which kind of key it was,
so that small saving wasn't worth it.
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.
I'd somehow managed to declare an enum in cmdgen.c with key types
OPENSSH and SSHCOM, and use it interchangeably with the one in ssh.h
with SSH_KEYTYPE_OPENSSH and SSH_KEYTYPE_SSHCOM.
It so happened that the relevant two enum values matched up! So this
hasn't caused a bug yet, but it's an accident waiting to happen. Fix
it before it does.
This is import only, for the moment: I haven't written an exporter
yet. Also, we currently don't support the format's full generality - a
new-style OpenSSH key file can contain multiple keys, but this code
currently only handles files with one key in them. That should be easy
to change, though, given only a little UI.
I'm about to use these in a new piece of code, but they may come in
helpful elsewhere as well. match_ssh_id in particular wraps an idiom
that's quite common in the rest of the codebase.
Since I've recently published a program that can easily generate the
required digits of pi, and since I was messing around in sshblowf.c
already, it seemed like a good idea to provide a derivation of all
that hex data.
This isn't the same as the standard bcrypt; it's OpenSSH's
modification that they use for their new-style key format.
In order to implement this, I've broken up blowfish_setkey() into two
subfunctions, and provided one of them with an extra optional salt
parameter, which is NULL in ordinary Blowfish but used by bcrypt.
Also, I've exposed some of sshblowf.c's internal machinery for the new
sshbcrypt.c to use.
