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

69 Commits

Author SHA1 Message Date
Simon Tatham
5dfd92b6ed Merge global request queue fix from 'pre-0.77'. 2022-05-04 12:53:03 +01:00
Simon Tatham
03e71efcc5 Fix linked-list mismanagement in global request queue.
When we linked a new entry on to the global request queue, we forgot
to set its next pointer to NULL, so that when it was removed again,
s->globreq_head could end up pointing to nonsense.

In addition, even if the next pointer happened to be NULL by luck, we
also did not notice that s->globreq_head had become NULL and respond
by nulling out s->globreq_tail, which would leave s->globreq_tail as a
stale pointer to the just-freed list element, causing a memory access
error on the next attempt to link something on to the list.

This could come up in the situation where you open Change Settings and
configure a remote port forwarding, close it (so that the global
request is sent, queued, replied to, and unqueued again), and then
reopen Change Settings and configure a second one (so that the linked
list in the confused state actually gets used).
2022-05-04 12:49:02 +01:00
Simon Tatham
8c4524aa91 Fix null-pointer dereferences in CA config.
Introduced in dc7ba12253 earlier today. On GTK these caused no
problems worse than a GTK warning, but I'd better fix them before they
(potentially) do worse on Windows!
2022-05-02 19:01:03 +01:00
Simon Tatham
c6e40f6785 Add some blank lines in setup_ca_config_box.
It's becoming hard to see what's going on in all that control setup.
2022-05-02 11:17:58 +01:00
Simon Tatham
dc7ba12253 Permit configuring RSA signature types in certificates.
As distinct from the type of signature generated by the SSH server
itself from the host key, this lets you exclude (and by default does
exclude) the old "ssh-rsa" SHA-1 signature type from the signature of
the CA on the certificate.
2022-05-02 11:17:58 +01:00
Simon Tatham
8d2c643fcb CA config: protect against saving a key with no wildcards. 2022-05-01 11:29:54 +01:00
Simon Tatham
6472b5ded7 CA config: permit pasting a whole OpenSSH public key.
Now, we try putting the contents of the public-key edit box through
ppk_load_s if it isn't a plain base64-encoded string.
2022-05-01 11:27:46 +01:00
Simon Tatham
2a44b6354f CA config: make the 'Done' button cancel and not default.
This means that, on the one hand, an absentminded press of Return
doesn't dismiss the entire CA config box, which would be pretty
annoying if you were half way through entering a load of fiddly stuff.

And on the other hand, you _can_ press Escape to dismiss the box,
which is less likely to happen by accident.
2022-05-01 10:38:50 +01:00
Simon Tatham
ddcd93ab12 CA config box: add a 'Read from file' button.
This allows you to load a CA public key from a disk file (in any
format acceptable to ppk_load_pub, which means OpenSSH one-line public
keys and also RFC4716 ones).
2022-05-01 10:16:19 +01:00
Simon Tatham
4fcb3bbe81 Move host CA config box out into its own source file.
In the course of polishing up this dialog box, I'm going to want it to
actually do cryptographic things (such as checking validity of a
public key blob and printing its fingerprint), which means it will
need to link against SSH utility functions.

So I've moved the dialog-box setup and handling code out of config.c
into a new file in the ssh subdirectory and in the ssh library, where
those facilities will be conveniently available.

This also means that dialog-box setup code _won't_ be linked into
PuTTYtel or pterm (on either platform), so I've added a stub source
file to provide its entry-point function in those tools. Also,
provided a const bool to indicate whether that dialog is available,
which we use to decide whether to recognise that command-line option.
2022-05-01 10:16:19 +01:00
Simon Tatham
958304897d Fix rekeying when using a certified host key.
In a rekey, we expect to see the same host key again, which we enforce
by comparing its cache string, which we happened to have handy. But
certified host keys don't have cache strings, so this no longer works
reliably - the 'assert(s->keystr)' fails.

(This is what I get for making a zillion short-lived test connections
and not leaving any of them running for more than 2 minutes!)

Instead, we now keep the official public blob of the host key from the
first key exchange, and compare that to the public blob of the one in
the rekey.
2022-04-29 22:44:40 +01:00
Simon Tatham
f9bb1f4997 ssh/verstring.c: fix use of '\r' and '\n'.
It's a thoroughly pedantic point, but I just spotted that the
comparison of wire data against theoretically platform-dependent char
escapes is a violation of \k{udp-portability}.
2022-04-29 11:40:53 +01:00
Simon Tatham
42dcd465ab ssh2_scan_kexinits: dynamically allocate server_hostkeys[].
In commit 7d44e35bb3 I introduced a bug: we were providing an
array of MAXKEXLIST ints to ssh2_scan_kexinits() to write a list of
server-supplied host keys into, and when MAXKEXLIST stopped being a
thing, I mindlessly replaced it with an array dynamically allocated to
the number of host key types we'd offered the server.

But we return a list of host key types the _server_ offered _us_ (and
that we can speak at all), which isn't necessarily the same thing. In
particular, if you deliberately ask to cache a new host key type from
the specials menu, we send a KEXINIT offering just _one_ host key
type, namely the one you've asked for. But that loop still writes down
all the key types it gets back from the server, which is (almost
certainly) more than one. So the array overflows.

In that situation we don't really need the returned array of key types
at all, but it's easier to just make it work than to add conditionals.
Replaced it with a dynamically grown array in the usual sort of way.
2022-04-28 13:11:51 +01:00
Simon Tatham
21d4754b6a Initial support for host certificates.
Now we offer the OpenSSH certificate key types in our KEXINIT host key
algorithm list, so that if the server has a certificate, they can send
it to us.

There's a new storage.h abstraction for representing a list of trusted
host CAs, and which ones are trusted to certify hosts for what
domains. This is stored outside the normal saved session data, because
the whole point of host certificates is to avoid per-host faffing.

Configuring this set of trusted CAs is done via a new GUI dialog box,
separate from the main PuTTY config box (because it modifies a single
set of settings across all saved sessions), which you can launch by
clicking a button in the 'Host keys' pane. The GUI is pretty crude for
the moment, and very much at a 'just about usable' stage right now. It
will want some polishing.

If we have no CA configured that matches the hostname, we don't offer
to receive certified host keys in the first place. So for existing
users who haven't set any of this up yet, nothing will immediately
change.

Currently, if we do offer to receive certified host keys and the
server presents one signed by a CA we don't trust, PuTTY will bomb out
unconditionally with an error, instead of offering a confirmation box.
That's an unfinished part which I plan to fix before this goes into a
release.
2022-04-25 15:09:31 +01:00
Simon Tatham
df3a21d97b Support for detached certificates in userauth.
This is triggered by a new config option, or alternatively a -cert
command-line option. You provide a certificate file (i.e. a public key
containing one of the cert key formats), and then, whenever you
authenticate with a private key that matches the public key inside
that certificate, the certificate will be sent to the server in place
of whatever public key it would have used before.

I expect this to be more convenient for some users than the approach
of baking the certificate into a modified version of the PPK file -
especially users who want to use different certificates on the same
key, either in sequence (if a CA continually reissues certificates
with short lifetimes) or in parallel (if different hosts trust
different CAs).

In particular, this substitution is applied consistently, even when
doing authentication via an agent. So if your bare private key is held
in Pageant, you can _still_ specify a detached certificate, and PuTTY
will spot that the key it's picked from Pageant matches that
certificate, and do the same substitution.

The detached certificate also overrides an existing certificate, if
there was one on the public key already.
2022-04-25 15:09:31 +01:00
Simon Tatham
cf36b9215f ssh_keyalg: new method 'alternate_ssh_id'.
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.
2022-04-24 08:39:04 +01:00
Simon Tatham
a5c0205b87 Utility functions to get the algorithm from a public 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.
2022-04-24 08:38:27 +01:00
Simon Tatham
e7d51505c7 Utility function strbuf_dup.
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.
2022-04-24 08:38:27 +01:00
Simon Tatham
e94097ccf6 Merge ssh_sw_abort_deferred fix from 'pre-0.77'. 2022-04-22 17:16:32 +01:00
Simon Tatham
38a5f59c75 mainchan.c: defer a couple of ssh_sw_abort.
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.
2022-04-22 17:15:34 +01:00
Simon Tatham
7d44e35bb3 transport2: make kexlists dynamically allocated.
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.
2022-04-21 08:13:38 +01:00
Simon Tatham
6a9e4ba24a kexinit_algorithm: switch to storing names as ptrlen.
They're now also compared as strings, fixing the slight fragility
where we depended on string-literal pointer equality.
2022-04-21 08:13:38 +01:00
Simon Tatham
3a54f28a4e Extra utility function add_to_commasep_pl.
Just like add_to_commasep, but takes a ptrlen.
2022-04-21 08:13:38 +01:00
Simon Tatham
faf1601a55 Implement OpenSSH 9.x's NTRU Prime / Curve25519 kex.
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.
2022-04-15 17:46:06 +01:00
Simon Tatham
e59ee96554 Refactor ecdh_kex into an organised vtable.
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.
2022-04-15 17:46:06 +01:00
Simon Tatham
e103ab1fb6 Refactor handling of SSH kex shared secret.
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.
2022-04-15 17:46:06 +01:00
Simon Tatham
5935c68288 Update source file names in comments and docs.
Correcting a source file name in the docs just now reminded me that
I've seen a lot of outdated source file names elsewhere in the code,
due to all the reorganisation since we moved to cmake. Here's a giant
pass of trying to make them all accurate again.
2022-01-22 15:51:31 +00:00
Simon Tatham
4ecb40a60d Fix a batch of typos in comments and docs. 2022-01-03 06:40:51 +00:00
Simon Tatham
a2ff884512 Richer data type for interactive prompt results.
All the seat functions that request an interactive prompt of some kind
to the user - both the main seat_get_userpass_input and the various
confirmation dialogs for things like host keys - were using a simple
int return value, with the general semantics of 0 = "fail", 1 =
"proceed" (and in the case of seat_get_userpass_input, answers to the
prompts were provided), and -1 = "request in progress, wait for a
callback".

In this commit I change all those functions' return types to a new
struct called SeatPromptResult, whose primary field is an enum
replacing those simple integer values.

The main purpose is that the enum has not three but _four_ values: the
"fail" result has been split into 'user abort' and 'software abort'.
The distinction is that a user abort occurs as a result of an
interactive UI action, such as the user clicking 'cancel' in a dialog
box or hitting ^D or ^C at a terminal password prompt - and therefore,
there's no need to display an error message telling the user that the
interactive operation has failed, because the user already knows,
because they _did_ it. 'Software abort' is from any other cause, where
PuTTY is the first to know there was a problem, and has to tell the
user.

We already had this 'user abort' vs 'software abort' distinction in
other parts of the code - the SSH backend has separate termination
functions which protocol layers can call. But we assumed that any
failure from an interactive prompt request fell into the 'user abort'
category, which is not true. A couple of examples: if you configure a
host key fingerprint in your saved session via the SSH > Host keys
pane, and the server presents a host key that doesn't match it, then
verify_ssh_host_key would report that the user had aborted the
connection, and feel no need to tell the user what had gone wrong!
Similarly, if a password provided on the command line was not
accepted, then (after I fixed the semantics of that in the previous
commit) the same wrong handling would occur.

So now, those Seat prompt functions too can communicate whether the
user or the software originated a connection abort. And in the latter
case, we also provide an error message to present to the user. Result:
in those two example cases (and others), error messages should no
longer go missing.

Implementation note: to avoid the hassle of having the error message
in a SeatPromptResult being a dynamically allocated string (and hence,
every recipient of one must always check whether it's non-NULL and
free it on every exit path, plus being careful about copying the
struct around), I've instead arranged that the structure contains a
function pointer and a couple of parameters, so that the string form
of the message can be constructed on demand. That way, the only users
who need to free it are the ones who actually _asked_ for it in the
first place, which is a much smaller set.

(This is one of the rare occasions that I regret not having C++'s
extra features available in this code base - a unique_ptr or
shared_ptr to a string would have been just the thing here, and the
compiler would have done all the hard work for me of remembering where
to insert the frees!)
2021-12-28 18:08:31 +00:00
Simon Tatham
88d5bb2a22 Cosmetic fix: double indentation in an if statement.
Not sure how that happened, but since I've spotted it, let's fix it.
2021-12-27 14:03:23 +00:00
Simon Tatham
60e557575e Cosmetic fix: silly parameter name caused by copy-paste.
In SSH-1 there's a function that takes a void * that it casts to the
state of the login layer. The corresponding function in SSH-2 casts it
to the state of a differently named layer, but I had still called the
parameter 'loginv'.
2021-12-27 14:02:26 +00:00
Simon Tatham
cd60a602f5 Stop using short exponents for Diffie-Hellman.
I recently encountered a paper [1] which catalogues all kinds of
things that can go wrong when one party in a discrete-log system
invents a prime and the other party chooses an exponent. In
particular, some choices of prime make it reasonable to use a short
exponent to save time, but others make that strategy very bad.

That paper is about the ElGamal encryption scheme used in OpenPGP,
which is basically integer Diffie-Hellman with one side's key being
persistent: a shared-secret integer is derived exactly as in DH, and
then it's used to communicate a message integer by simply multiplying
the shared secret by the message, mod p.

I don't _know_ that any problem of this kind arises in the SSH usage
of Diffie-Hellman: the standard integer DH groups in SSH are safe
primes, and as far as I know, the usual generation of prime moduli for
DH group exchange also picks safe primes. So the short exponents PuTTY
has been using _should_ be OK.

However, the range of imaginative other possibilities shown in that
paper make me nervous, even so! So I think I'm going to retire the
short exponent strategy, on general principles of overcaution.

This slows down 4096-bit integer DH by about a factor of 3-4 (which
would be worse if it weren't for the modpow speedup in the previous
commit). I think that's OK, because, firstly, computers are a lot
faster these days than when I originally chose to use short exponents,
and secondly, more and more implementations are now switching to
elliptic-curve DH, which is unaffected by this change (and with which
we've always been using maximum-length exponents).

[1] On the (in)security of ElGamal in OpenPGP. Luca De Feo, Bertram
Poettering, Alessandro Sorniotti. https://eprint.iacr.org/2021/923
2021-11-28 12:19:34 +00:00
Simon Tatham
1bf93289c9 Pull out SOCKS protocol constants into a header.
This seemed like a worthwhile cleanup to do while I was working on
this code anyway. Now all the magic numbers are defined in a header
file by macro names indicating their meaning, and used by both the
SOCKS client code in the proxy subdirectory and the SOCKS server code
in portfwd.c.
2021-11-19 15:09:17 +00:00
Simon Tatham
be8d3974ff Generalise strbuf_catf() into put_fmt().
marshal.h now provides a macro put_fmt() which allows you to write
arbitrary printf-formatted data to an arbitrary BinarySink.

We already had this facility for strbufs in particular, in the form of
strbuf_catf(). That was able to take advantage of knowing the inner
structure of a strbuf to minimise memory allocation (it would snprintf
directly into the strbuf's existing buffer if possible). For a general
black-box BinarySink we can't do that, so instead we dupvprintf into a
temporary buffer.

For consistency, I've removed strbuf_catf, and converted all uses of
it into the new put_fmt - and I've also added an extra vtable method
in the BinarySink API, so that put_fmt can still use strbuf_catf's
more efficient memory management when talking to a strbuf, and fall
back to the simpler strategy when that's not available.
2021-11-19 11:32:47 +00:00
Simon Tatham
7eb7d5e2e9 New Seat query, has_mixed_input_stream().
(TL;DR: to suppress redundant 'Press Return to begin session' prompts
in between hops of a jump-host configuration, in Plink.)

This new query method directly asks the Seat the question: is the same
stream of input used to provide responses to interactive login
prompts, and the session input provided after login concludes?

It's used to suppress the last-ditch anti-spoofing defence in Plink of
interactively asking 'Access granted. Press Return to begin session',
on the basis that any such spoofing attack works by confusing the user
about what's a legit login prompt before the session begins and what's
sent by the server after the main session begins - so if those two
things take input from different places, the user can't be confused.

This doesn't change the existing behaviour of Plink, which was already
suppressing the antispoof prompt in cases where its standard input was
redirected from something other than a terminal. But previously it was
doing it within the can_set_trust_status() seat query, and I've now
moved it out into a separate query function.

The reason why these need to be separate is for SshProxy, which needs
to give an unusual combination of answers when run inside Plink. For
can_set_trust_status(), it needs to return whatever the parent Seat
returns, so that all the login prompts for a string of proxy
connections in session will be antispoofed the same way. But you only
want that final 'Access granted' prompt to happen _once_, after all
the proxy connection setup phases are done, because up until then
you're still in the safe hands of PuTTY itself presenting an unbroken
sequence of legit login prompts (even if they come from a succession
of different servers). Hence, SshProxy unconditionally returns 'no' to
the query of whether it has a single mixed input stream, because
indeed, it never does - for purposes of session input it behaves like
an always-redirected Plink, no matter what kind of real Seat it ends
up sending its pre-session login prompts to.
2021-11-06 14:48:26 +00:00
Simon Tatham
2ae338b407 New plug_closing error type for 'user abort'.
This is generated when setup of a network connection is cancelled by
deliberate user action, namely, pressing ^C or ^D or the like at a
get_userpass_input prompt presented during proxy setup.

It's handled just like normal socket setup errors, except that it
omits the call to seat_connection_fatal, on the grounds that in this
one case of connection-setup failure, the user doesn't need to be
_informed_ that the connection failed - they already know, because
they failed it themself on purpose.
2021-11-06 14:48:26 +00:00
Simon Tatham
0fe41294e6 New API for plug_closing() with a custom type enum.
Passing an operating-system-specific error code to plug_closing(),
such as errno or GetLastError(), was always a bit weird, given that it
generally had to be handled by cross-platform receiving code in
backends. I had the platform.h implementations #define any error
values that the cross-platform code would have to handle specially,
but that's still not a great system, because it also doesn't leave
freedom to invent error representations of my own that don't
correspond to any OS code. (For example, the ones I just removed from
proxy.h.)

So now, the OS error code is gone from the plug_closing API, and in
its place is a custom enumeration of closure types: normal, error, and
the special case BROKEN_PIPE which is the only OS error code we have
so far needed to handle specially. (All others just mean 'abandon the
connection and print the textual message'.)

Having already centralised the handling of OS error codes in the
previous commit, we've now got a convenient place to add any further
type codes for errors needing special handling: each of Unix
plug_closing_errno(), Windows plug_closing_system_error(), and Windows
plug_closing_winsock_error() can easily grow extra special cases if
need be, and each one will only have to live in one place.
2021-11-06 14:48:26 +00:00
Simon Tatham
7460594433 Move TempSeat creation/destruction into Interactor.
Previously, SshProxy dealt with creating a TempSeat to wrap the one it
was borrowing from its client, and then each client in turn dealt with
detecting when it had had its seat borrowed and finishing up with the
TempSeat. The latter involved a lot of code duplication; the former
didn't involve code duplication _yet_ (since SshProxy was the only
thing doing this job), but would have once we started wanting to do
interactive password prompting for other types of network proxy.

Now all of that functionality is centralised into two new Interactor
helper functions: interactor_borrow_seat and interactor_return_seat.
2021-10-30 18:20:58 +01:00
Simon Tatham
f00c72cc2a Framework for announcing which Interactor is talking.
All this Interactor business has been gradually working towards being
able to inform the user _which_ network connection is currently
presenting them with a password prompt (or whatever), in situations
where more than one of them might be, such as an SSH connection being
used as a proxy for another SSH connection when neither one has
one-touch login configured.

At some point, we have to arrange that any attempt to do a user
interaction during connection setup - be it a password prompt, a host
key confirmation dialog, or just displaying an SSH login banner -
makes it clear which host it's come from. That's going to mean calling
some kind of announcement function before doing any of those things.

But there are several of those functions in the Seat API, and calls to
them are scattered far and wide across the SSH backend. (And not even
just there - the Rlogin backend also uses seat_get_userpass_input).
How can we possibly make sure we don't forget a vital call site on
some obscure little-tested code path, and leave the user confused in
just that one case which nobody might notice for years?

Today I thought of a trick to solve that problem. We can use the C
type system to enforce it for us!

The plan is: we invent a new struct type which contains nothing but a
'Seat *'. Then, for every Seat method which does a thing that ought to
be clearly identified as relating to a particular Interactor, we
adjust the API for that function to take the new struct type where it
previously took a plain 'Seat *'. Or rather - doing less violence to
the existing code - we only need to adjust the API of the dispatch
functions inline in putty.h.

How does that help? Because the way you _get_ one of these
struct-wrapped Seat pointers is by calling interactor_announce() on
your Interactor, which will in turn call interactor_get_seat(), and
wrap the returned pointer into one of these structs.

The effect is that whenever the SSH (or Rlogin) code wants to call one
of those particular Seat methods, it _has_ to call
interactor_announce() just beforehand, which (once I finish all of
this) will make sure the user is aware of who is presenting the prompt
or banner or whatever. And you can't forget to call it, because if you
don't call it, then you just don't have a struct of the right type to
give to the Seat method you wanted to call!

(Of course, there's nothing stopping code from _deliberately_ taking a
Seat * it already has and wrapping it into the new struct. In fact
SshProxy has to do that, in order to forward these requests up the
chain of Seats. But the point is that you can't do it _by accident_,
just by forgetting to make a vital function call - when you do that,
you _know_ you're doing it on purpose.)

No functional change: the new interactor_announce() function exists,
and the type-system trick ensures it's called in all the right places,
but it doesn't actually _do_ anything yet.
2021-10-30 18:20:33 +01:00
Simon Tatham
89a390bdeb Pass an Interactor to new_connection().
Thanks to the previous commit, this new parameter can replace two of
the existing ones: instead of passing a LogPolicy and a Seat, we now
pass just an Interactor, from which any proxy implementation can
extract the LogPolicy and the Seat anyway if they need it.
2021-10-30 18:19:56 +01:00
Simon Tatham
aac5e096fa Add Interactor methods to get/set LogPolicy and Seat.
Nothing uses this yet, but the next commit will.
2021-10-30 18:19:56 +01:00
Simon Tatham
44db74ec51 Introduce a new 'Interactor' trait.
This trait will be implemented by anything that wants to display
interactive prompts or notifications to the user in the course of
setting up a network connection, _or_ anything that wants to make a
network connection whose proxy setup might in turn need to do that.

To begin with, that means every Backend that makes network connections
at all must be an Interactor, because any of those network connections
might be proxied via an SSH jump host which might need to interact
with the user.

I'll fill in the contents of this trait over the next few commits, to
keep the patches comprehensible. For the moment, I've just introduced
the trait, set up implementations of it in the five network backends,
and given it a single 'description' method.

The previous 'description' methods of Backend and Plug are now
removed, and their work is done by the new Interactor method instead.
(I changed my mind since last week about where that should best live.)
This isn't too much of an upheaval, fortunately, because I hadn't got
round yet to committing anything that used those methods!
2021-10-30 18:19:54 +01:00
Simon Tatham
74a0be9c56 Split seat_banner from seat_output.
Previously, SSH authentication banners were displayed by calling the
ordinary seat_output function, and passing it a special value in the
SeatOutputType enumeration indicating an auth banner.

The awkwardness of this was already showing a little in SshProxy's
implementation of seat_output, where it had to check for that special
value and do totally different things for SEAT_OUTPUT_AUTH_BANNER and
everything else. Further work in that area is going to make it more
and more awkward if I keep the two output systems unified.

So let's split them up. Now, Seat has separate output() and banner()
methods, which each implementation can override differently if it
wants to.

All the 'end user' Seat implementations use the centralised
implementation function nullseat_banner_to_stderr(), which turns
banner text straight back into SEAT_OUTPUT_STDERR and passes it on to
seat_output. So I didn't have to tediously implement a boring version
of this function in GTK, Windows GUI, consoles, file transfer etc.
2021-10-30 17:37:09 +01:00
Simon Tatham
efa89573ae Reorganise host key checking and confirmation.
Previously, checking the host key against the persistent cache managed
by the storage.h API was done as part of the seat_verify_ssh_host_key
method, i.e. separately by each Seat.

Now that check is done by verify_ssh_host_key(), which is a new
function in ssh/common.c that centralises all the parts of host key
checking that don't need an interactive prompt. It subsumes the
previous verify_ssh_manual_host_key() that checked against the Conf,
and it does the check against the storage API that each Seat was
previously doing separately. If it can't confirm or definitively
reject the host key by itself, _then_ it calls out to the Seat, once
an interactive prompt is definitely needed.

The main point of doing this is so that when SshProxy forwards a Seat
call from the proxy SSH connection to the primary Seat, it won't print
an announcement of which connection is involved unless it's actually
going to do something interactive. (Not that we're printing those
announcements _yet_ anyway, but this is a piece of groundwork that
works towards doing so.)

But while I'm at it, I've also taken the opportunity to clean things
up a bit by renaming functions sensibly. Previously we had three very
similarly named functions verify_ssh_manual_host_key(), SeatVtable's
'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now
the Seat method is called 'confirm' rather than 'verify' (since its
job is now always to print an interactive prompt, so it looks more
like the other confirm_foo methods), and the storage.h function is
called check_stored_host_key(), which goes better with store_host_key
and avoids having too many functions with similar names. And the
'manual' function is subsumed into the new centralised code, so
there's now just *one* host key function with 'verify' in the name.

Several functions are reindented in this commit. Best viewed with
whitespace changes ignored.
2021-10-25 18:12:17 +01:00
Simon Tatham
f1746d69b1 Add 'description' methods for Backend and Plug.
These will typically be implemented by objects that are both a Backend
*and* a Plug, and the two methods will deliver the same results to any
caller, regardless of which facet of the object is known to that
caller.

Their purpose is to deliver a user-oriented natural-language
description of what network connection the object is handling, so that
it can appear in diagnostic messages.

The messages I specifically have in mind are going to appear in cases
where proxies require interactive authentication: when PuTTY prompts
interactively for a password, it will need to explain which *thing*
it's asking for the password for, and these descriptions are what it
will use to describe the thing in question.

Each backend is allowed to compose these messages however it thinks
best. In all cases at present, the description string is constructed
by the new centralised default_description() function, which takes a
host name and port number and combines them with the backend's display
name. But the SSH backend does things a bit differently, because it
uses the _logical_ host name (the one that goes with the SSH host key)
rather than the physical destination of the network connection. That
seems more appropriate when the question it's really helping the user
to answer is "What host am I supposed to be entering the password for?"

In this commit, no clients of the new methods are introduced. I have a
draft implementation of actually using it for the purpose I describe
above, but it needs polishing.
2021-10-24 10:48:25 +01:00
Simon Tatham
5374444879 Lowercase version of BackendVtable's displayname.
The current 'displayname' field is designed for presenting in the
config UI, so it starts with a capital letter even when it's not a
proper noun. If I want to name the backend in the middle of a
sentence, I'll need a version that starts with lowercase where
appropriate.

The old field is renamed displayname_tc, to avoid ambiguity.
2021-10-24 09:59:05 +01:00
Simon Tatham
d42f1fe96d Remove 'calling_back' parameter from plug_closing.
It was totally unused. No implementation of the 'closing' method in a
Plug vtable was checking it for any reason at all, except for
ProxySocket which captured it from its client in order to pass on to
its server (which, perhaps after further iterations of ProxySocket,
would have ended up ignoring it similarly). And every caller of
plug_closing set it to 0 (aka false), except for the one in sshproxy.c
which passed true (but it would have made no difference to anyone).

The comment in network.h refers to a FIXME comment which was in
try_send() when that code was written (see winnet.c in commit
7b0e082700). That FIXME is long gone, replaced by a use of a
toplevel callback. So I think the aim must have been to avoid
re-entrancy when sk_write called try_send which encountered a socket
error and called back to plug_closing - but that's long since fixed by
other means now.
2021-10-24 09:58:59 +01:00
Simon Tatham
a73aaf9457 Uppity: add command-line options to configure auth methods.
Now you can turn various authentication methods on and off, so that
the server won't even offer (say) k-i or publickey at all.

This subsumes the previous -allow-none-auth option; there's now a
general -{allow,deny}-auth=foo option schema, so -allow-auth=none is
the new spelling of -allow-none-auth. The former spelling is kept for
backwards compatibility, just in case.
2021-09-28 18:09:36 +01:00
Simon Tatham
fb663d4761 Promote ssh2_userauth_antispoof_msg into utils.
It doesn't actually do anything specific to the userauth layer; it's
just a helper function that deals with the mechanics of printing an
unspoofable message on various kinds of front end, and the only
parameters it needs are a Seat and a message.

Currently, it's used for 'here is the start/end of the server banner'
only. But it's also got all the right functionality to be used for the
(still missing) messages about which proxy SSH server the next set of
login prompts are going to refer to.
2021-09-16 17:49:31 +01:00
Simon Tatham
ac47e550c6 seat_output: add an output type for SSH banners. (NFC)
The jump host system ought really to be treating SSH authentication
banners as a distinct thing from the standard-error session output, so
that the former can be presented to the user in the same way as the
auth banner for the main session.

This change converts the 'bool is_stderr' parameter of seat_output()
into an enumerated type with three values. For the moment, stderr and
banners are treated the same, but the plan is for that to change.
2021-09-16 17:24:42 +01:00