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

84 Commits

Author SHA1 Message Date
Simon Tatham
840043f06e Add 'next_message' methods to cipher and MAC vtables.
This provides a convenient hook to be called between SSH messages, for
the crypto components to do any per-message processing like
incrementing a sequence number.
2022-08-16 18:27:06 +01:00
Simon Tatham
dbc77dbd7a Change the rules for how we free a linked cipher and MAC.
In the situation where a MAC and cipher implementation are tied
together by being facets of the same underlying object (used by the
inseparable ChaCha20 + Poly1305 pair), previously we freed them by
having the cipher_free function actually do the freeing, having the
mac_free function do nothing, and taking great care to call those in
the right order. (Otherwise, the mac_free function dereferences a
no-longer-valid vtable pointer and doesn't get as far as _finding out_
that it doesn't have to do anything.)

That's a time bomb in general, and especially awkward in situations
like testcrypt where we don't get precise control over freeing order
in any case. So I've replaced that system with one in which there are
two flags in the ChaCha20-Poly1305 structure, saying whether each of
the cipher and MAC facets is currently considered to be allocated.
When the last of those flags is cleared, the object is actually freed.
So now they can be freed in either order.
2022-08-16 18:22:29 +01:00
Simon Tatham
e52087719c Documentation for OpenSSH certificates.
Also I've filled in the help contexts in all the new GUI controls.
2022-08-07 18:44:11 +01:00
Simon Tatham
cd7f6c4407 Certificate-aware handling of key fingerprints.
OpenSSH, when called on to give the fingerprint of a certified public
key, will in many circumstances generate the hash of the public blob
of the _underlying_ key, rather than the hash of the full certificate.

I think the hash of the certificate is also potentially useful (if
nothing else, it provides a way to tell apart multiple certificates on
the same key). But I can also see that it's useful to be able to
recognise a key as the same one 'really' (since all certificates on
the same key share a private key, so they're unavoidably related).

So I've dealt with this by introducing an extra pair of fingerprint
types, giving the cross product of {MD5, SHA-256} x {base key only,
full certificate}. You can manually select which one you want to see
in some circumstances (notably PuTTYgen), and in others (such as
diagnostics) both fingerprints will be emitted side by side via the
new functions ssh2_double_fingerprint[_blob].

The default, following OpenSSH, is to just fingerprint the base key.
2022-08-05 18:08:59 +01:00
Simon Tatham
9cac27946a Formatting: miscellaneous.
This patch fixes a few other whitespace and formatting issues which
were pointed out by the bulk-reindent or which I spotted in passing,
some involving manual editing to break lines more nicely.

I think the weirdest hunk in here is the one in windows/window.c
TranslateKey() where _half_ of an assignment statement inside an 'if'
was on the same line as the trailing paren of the if condition. No
idea at all how that one managed to happen!
2022-08-03 20:48:46 +01:00
Simon Tatham
4b8dc56284 Formatting: remove spurious spaces in 'type * var'.
I think a lot of these were inserted by a prior run through GNU indent
many years ago. I noticed in a more recent experiment that that tool
doesn't always correctly distinguish which instances of 'id * id' are
pointer variable declarations and which are multiplications, so it
spaces some of the former as if they were the latter.
2022-08-03 20:48:46 +01:00
Simon Tatham
14203bc54f Formatting: standardise on "func(\n", not "func\n(".
If the function name (or expression) in a function call or declaration
is itself so long that even the first argument doesn't fit after it on
the same line, or if that would leave so little space that it would be
silly to try to wrap all the run-on lines into a tall thin column,
then I used to do this

    ludicrously_long_function_name
        (arg1, arg2, arg3);

and now prefer this

    ludicrously_long_function_name(
        arg1, arg2, arg3);

I picked up the habit from Python, where the latter idiom is required
by Python's syntactic significance of newlines (you can write the
former if you use a backslash-continuation, but pretty much everyone
seems to agree that that's much uglier). But I've found it works well
in C as well: it makes it more obvious that the previous line is
incomplete, it gives you a tiny bit more space to wrap the following
lines into (the old idiom indents the _third_ line one space beyond
the second), and I generally turn out to agree with the knock-on
indentation decisions made by at least Emacs if you do it in the
middle of a complex expression. Plus, of course, using the _same_
idiom between C and Python means less state-switching.

So, while I'm making annoying indentation changes in general, this
seems like a good time to dig out all the cases of the old idiom in
this code, and switch them over to the new.
2022-08-03 20:48:46 +01:00
Simon Tatham
4fa3480444 Formatting: realign run-on parenthesised stuff.
My bulk indentation check also turned up a lot of cases where a run-on
function call or if statement didn't have its later lines aligned
correctly relative to the open paren.

I think this is quite easy to do by getting things out of
sync (editing the first line of the function call and forgetting to
update the rest, perhaps even because you never _saw_ the rest during
a search-replace). But a few didn't quite fit into that pattern, in
particular an outright misleading case in unix/askpass.c where the
second line of a call was aligned neatly below the _wrong_ one of the
open parens on the opening line.

Restored as many alignments as I could easily find.
2022-08-03 20:48:46 +01:00
Simon Tatham
3a42a09dad Formatting: normalise back to 4-space indentation.
In several pieces of development recently I've run across the
occasional code block in the middle of a function which suddenly
switched to 2-space indent from this code base's usual 4. I decided I
was tired of it, so I ran the whole code base through a re-indenter,
which made a huge mess, and then manually sifted out the changes that
actually made sense from that pass.

Indeed, this caught quite a few large sections with 2-space indent
level, a couple with 8, and a handful of even weirder things like 3
spaces or 12. This commit fixes them all.
2022-08-03 20:48:46 +01:00
Simon Tatham
42740a5455 Allow manually confirming and caching certified keys.
In the case where a server presents a host key signed by a different
certificate from the one you've configured, it need not _always_ be
evidence of wrongdoing. I can imagine situations in which two CAs
cover overlapping sets of things, and you don't want to blanket-trust
one of them, but you do want to connect to a specific host signed by
that one.

Accordingly, PuTTY's previous policy of unconditionally aborting the
connection if certificate validation fails (which was always intended
as a stopgap until I thought through what I wanted to replace it with)
is now replaced by fallback handling: we present the host key
fingerprint to the user and give them the option to accept and/or
cache it based on the public key itself.

This means that the certified key types have to have a representation
in the host key cache. So I've assigned each one a type id, and
generate the cache string itself by simply falling back to the base
key.

(Rationale for the latter: re-signing a public key with a different
certificate doesn't change the _private_ key, or the set of valid
signatures generated with it. So if you've been convinced for reasons
other than the certificate that a particular private key is in the
possession of $host, then proof of ownership of that private key
should be enough to convince you you're talking to $host no matter
what CA has signed the public half this week.)

We now offer to receive a given certified host key type if _either_ we
have at least one CA configured to trust that host, _or_ we have a
certified key of that type cached. (So once you've decided manually
that you trust a particular key, we can still receive that key and
authenticate the host with it, even if you later delete the CA record
that it didn't match anyway.)

One change from normal (uncertified) host key handling is that for
certified key types _all_ the host key prompts use the stronger
language, with "WARNING - POTENTIAL SECURITY BREACH!" rather than the
mild 'hmm, we haven't seen this host before'. Rationale: if you
expected this CA key and got that one, it _could_ be a bold-as-brass
MITM attempt in which someone hoped you'd accept their entire CA key.
The mild wording is only for the case where we had no previous
expectations _at all_ for the host to violate: not a CA _or_ a cached
key.
2022-07-17 14:11:38 +01:00
Simon Tatham
a50178eba7 Fix typo in #undef.
In the macro automation for ssh2_bpp_check_unimplemented, I #defined
SSH2_BITMAP_WORD, and 20 lines later, tried to #undef it by the wrong
spelling. Of course this gave no error, so I didn't notice! But I
spotted it just now, so let's fix it.
2022-07-16 11:56:23 +01:00
Simon Tatham
f1c8298000 Centralise most details of host-key prompting.
The text of the host key warnings was replicated in three places: the
Windows rc file, the GTK dialog setup function, and the console.c
shared between both platforms' CLI tools. Now it lives in just one
place, namely ssh/common.c where the rest of the centralised host-key
checking is done, so it'll be easier to adjust the wording in future.

This comes with some extra automation. Paragraph wrapping is no longer
done by hand in any version of these prompts. (Previously we let GTK
do the wrapping on GTK, but on Windows the resource file contained a
bunch of pre-wrapped LTEXT lines, and console.c had pre-wrapped
terminal messages.) And the dialog heights in Windows are determined
automatically based on the amount of stuff in the window.

The main idea of all this is that it'll be easier to set up more
elaborate kinds of host key prompt that deal with certificates (if,
e.g., a server sends us a certified host key which we don't trust the
CA for). But there are side benefits of this refactoring too: each
tool now reliably inserts its own appname in the prompts, and also, on
Windows the entire prompt text is copy-pastable.

Details of implementation: there's a new type SeatDialogText which
holds a set of (type, string) pairs describing the contents of a
prompt. Type codes distinguish ordinary text paragraphs, paragraphs to
be displayed prominently (like key fingerprints), the extra-bold scary
title at the top of the 'host key changed' version of the dialog, and
the various information that lives in the subsidiary 'more info' box.
ssh/common.c constructs this, and passes it to the Seat to present the
actual prompt.

In order to deal with the different UI for answering the prompt, I've
added an extra Seat method 'prompt_descriptions' which returns some
snippets of text to interpolate into the messages. ssh/common.c calls
that while it's still constructing the text, and incorporates the
resulting snippets into the SeatDialogText.

For the moment, this refactoring only affects the host key prompts.
The warnings about outmoded crypto are still done the old-fashioned
way; they probably ought to be similarly refactored to use this new
SeatDialogText system, but it's not immediately critical for the
purpose I have right now.
2022-07-07 18:05:32 +01:00
Simon Tatham
f579b3c01e Certificate trust scope: change to a boolean-expression system.
This replaces the previous placeholder scheme of having a list of
hostname wildcards with implicit logical-OR semantics (if any wildcard
matched then the certificate would be trusted to sign for that host).
That scheme didn't allow for exceptions within a domain ('everything
in example.com except extra-high-security-machine.example.com'), and
also had no way to specify port numbers.

In the new system, you can still write a hostname wildcard by itself
in the simple case, but now those are just atomic subexpressions in a
boolean-logic domain-specific language I've made up. So if you want
multiple wildcards, you can separate them with || in a single longer
expression, and also you can use && and ! to impose exceptions on top
of that.

Full details of the expression language are in the comment at the top
of utils/cert-expr.c. It'll need documenting properly before release,
of course.

For the sake of backwards compatibility for early adopters who've
already set up configuration in the old system, I've put in some code
that will read the old MatchHosts configuration and automatically
translate it into the equivalent boolean expression (by simply
stringing together the list of wildcards with || between them).
2022-06-25 14:32:23 +01:00
Simon Tatham
4b0e54c22a CA config box: fully validate the CA public key.
Now we check that we can actually make an ssh_key out of it, and
moreover, that the key is of a sensible kind (i.e. not a certificate
in turn). If that's not true, we report something about the problem in
a new CTRL_TEXT below the public key input box. If the key _is_ valid,
that same text control is used to show its type, length and
fingerprint.

On Windows, I've widened the dialog box a little to make fingerprints
fit sensibly in it.
2022-05-07 12:02:23 +01:00
Simon Tatham
5ca78237ed CA config box: add some align_next_to.
Now the RSA signature-type checkboxes should be aligned with their
label; the 'Add' and 'Remove' buttons for wildcards should align with
the edit box for entering a wildcard; and the 'Load from file' button
for a public key aligns with the edit box for that.
2022-05-05 19:04:34 +01:00
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