2004-10-06 22:31:07 +00:00
|
|
|
/*
|
|
|
|
* Platform-independent bits of X11 forwarding.
|
|
|
|
*/
|
|
|
|
|
2001-01-22 11:34:52 +00:00
|
|
|
#include <stdio.h>
|
|
|
|
#include <stdlib.h>
|
2003-01-10 18:33:35 +00:00
|
|
|
#include <assert.h>
|
2003-01-11 09:31:54 +00:00
|
|
|
#include <time.h>
|
2001-01-22 11:34:52 +00:00
|
|
|
|
|
|
|
#include "putty.h"
|
|
|
|
#include "ssh.h"
|
2021-04-22 16:58:40 +00:00
|
|
|
#include "channel.h"
|
2005-02-02 23:51:58 +00:00
|
|
|
#include "tree234.h"
|
2001-01-22 11:34:52 +00:00
|
|
|
|
2005-02-02 23:51:58 +00:00
|
|
|
struct XDMSeen {
|
|
|
|
unsigned int time;
|
|
|
|
unsigned char clientid[6];
|
|
|
|
};
|
|
|
|
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
typedef struct X11Connection {
|
2019-09-08 19:29:00 +00:00
|
|
|
unsigned char firstpkt[12]; /* first X data packet */
|
2013-11-17 14:05:10 +00:00
|
|
|
tree234 *authtree;
|
2008-11-17 18:38:09 +00:00
|
|
|
struct X11Display *disp;
|
2001-01-22 11:34:52 +00:00
|
|
|
char *auth_protocol;
|
|
|
|
unsigned char *auth_data;
|
|
|
|
int data_read, auth_plen, auth_psize, auth_dlen, auth_dsize;
|
Convert a lot of 'int' variables to 'bool'.
My normal habit these days, in new code, is to treat int and bool as
_almost_ completely separate types. I'm still willing to use C's
implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine,
no need to spell it out as blob.len != 0), but generally, if a
variable is going to be conceptually a boolean, I like to declare it
bool and assign to it using 'true' or 'false' rather than 0 or 1.
PuTTY is an exception, because it predates the C99 bool, and I've
stuck to its existing coding style even when adding new code to it.
But it's been annoying me more and more, so now that I've decided C99
bool is an acceptable thing to require from our toolchain in the first
place, here's a quite thorough trawl through the source doing
'boolification'. Many variables and function parameters are now typed
as bool rather than int; many assignments of 0 or 1 to those variables
are now spelled 'true' or 'false'.
I managed this thorough conversion with the help of a custom clang
plugin that I wrote to trawl the AST and apply heuristics to point out
where things might want changing. So I've even managed to do a decent
job on parts of the code I haven't looked at in years!
To make the plugin's work easier, I pushed platform front ends
generally in the direction of using standard 'bool' in preference to
platform-specific boolean types like Windows BOOL or GTK's gboolean;
I've left the platform booleans in places they _have_ to be for the
platform APIs to work right, but variables only used by my own code
have been converted wherever I found them.
In a few places there are int values that look very like booleans in
_most_ of the places they're used, but have a rarely-used third value,
or a distinction between different nonzero values that most users
don't care about. In these cases, I've _removed_ uses of 'true' and
'false' for the return values, to emphasise that there's something
more subtle going on than a simple boolean answer:
- the 'multisel' field in dialog.h's list box structure, for which
the GTK front end in particular recognises a difference between 1
and 2 but nearly everything else treats as boolean
- the 'urgent' parameter to plug_receive, where 1 vs 2 tells you
something about the specific location of the urgent pointer, but
most clients only care about 0 vs 'something nonzero'
- the return value of wc_match, where -1 indicates a syntax error in
the wildcard.
- the return values from SSH-1 RSA-key loading functions, which use
-1 for 'wrong passphrase' and 0 for all other failures (so any
caller which already knows it's not loading an _encrypted private_
key can treat them as boolean)
- term->esc_query, and the 'query' parameter in toggle_mode in
terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h,
but can also hold -1 for some other intervening character that we
don't support.
In a few places there's an integer that I haven't turned into a bool
even though it really _can_ only take values 0 or 1 (and, as above,
tried to make the call sites consistent in not calling those values
true and false), on the grounds that I thought it would make it more
confusing to imply that the 0 value was in some sense 'negative' or
bad and the 1 positive or good:
- the return value of plug_accepting uses the POSIXish convention of
0=success and nonzero=error; I think if I made it bool then I'd
also want to reverse its sense, and that's a job for a separate
piece of work.
- the 'screen' parameter to lineptr() in terminal.c, where 0 and 1
represent the default and alternate screens. There's no obvious
reason why one of those should be considered 'true' or 'positive'
or 'success' - they're just indices - so I've left it as int.
ssh_scp_recv had particularly confusing semantics for its previous int
return value: its call sites used '<= 0' to check for error, but it
never actually returned a negative number, just 0 or 1. Now the
function and its call sites agree that it's a bool.
In a couple of places I've renamed variables called 'ret', because I
don't like that name any more - it's unclear whether it means the
return value (in preparation) for the _containing_ function or the
return value received from a subroutine call, and occasionally I've
accidentally used the same variable for both and introduced a bug. So
where one of those got in my way, I've renamed it to 'toret' or 'retd'
(the latter short for 'returned') in line with my usual modern
practice, but I haven't done a thorough job of finding all of them.
Finally, one amusing side effect of doing this is that I've had to
separate quite a few chained assignments. It used to be perfectly fine
to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a
the 'true' defined by stdbool.h, that idiom provokes a warning from
gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
|
|
|
bool verified;
|
|
|
|
bool input_wanted;
|
|
|
|
bool no_data_sent_to_x_client;
|
2013-11-17 14:05:17 +00:00
|
|
|
char *peer_addr;
|
2003-01-12 14:11:38 +00:00
|
|
|
int peer_port;
|
2018-09-14 12:47:13 +00:00
|
|
|
SshChannel *c; /* channel structure held by SSH backend */
|
Get rid of lots of implicit pointer types.
All the main backend structures - Ssh, Telnet, Pty, Serial etc - now
describe structure types themselves rather than pointers to them. The
same goes for the codebase-wide trait types Socket and Plug, and the
supporting types SockAddr and Pinger.
All those things that were typedefed as pointers are older types; the
newer ones have the explicit * at the point of use, because that's
what I now seem to be preferring. But whichever one of those is
better, inconsistently using a mixture of the two styles is worse, so
let's make everything consistent.
A few types are still implicitly pointers, such as Bignum and some of
the GSSAPI types; generally this is either because they have to be
void *, or because they're typedefed differently on different
platforms and aren't always pointers at all. Can't be helped. But I've
got rid of the main ones, at least.
2018-10-04 18:10:23 +00:00
|
|
|
Socket *s;
|
2018-05-27 08:29:33 +00:00
|
|
|
|
2018-10-05 06:24:16 +00:00
|
|
|
Plug plug;
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
Channel chan;
|
|
|
|
} X11Connection;
|
2001-01-22 11:34:52 +00:00
|
|
|
|
2005-02-02 23:51:58 +00:00
|
|
|
static int xdmseen_cmp(void *a, void *b)
|
|
|
|
{
|
|
|
|
struct XDMSeen *sa = a, *sb = b;
|
|
|
|
return sa->time > sb->time ? 1 :
|
2019-09-08 19:29:00 +00:00
|
|
|
sa->time < sb->time ? -1 :
|
2005-02-02 23:51:58 +00:00
|
|
|
memcmp(sa->clientid, sb->clientid, sizeof(sa->clientid));
|
|
|
|
}
|
|
|
|
|
2013-11-17 14:05:10 +00:00
|
|
|
struct X11FakeAuth *x11_invent_fake_auth(tree234 *authtree, int authtype)
|
|
|
|
{
|
|
|
|
struct X11FakeAuth *auth = snew(struct X11FakeAuth);
|
|
|
|
int i;
|
|
|
|
|
2013-11-17 14:05:41 +00:00
|
|
|
/*
|
|
|
|
* This function has the job of inventing a set of X11 fake auth
|
|
|
|
* data, and adding it to 'authtree'. We must preserve the
|
|
|
|
* property that for any given actual authorisation attempt, _at
|
|
|
|
* most one_ thing in the tree can possibly match it.
|
|
|
|
*
|
|
|
|
* For MIT-MAGIC-COOKIE-1, that's not too difficult: the match
|
|
|
|
* criterion is simply that the entire cookie is correct, so we
|
|
|
|
* just have to make sure we don't make up two cookies the same.
|
|
|
|
* (Vanishingly unlikely, but we check anyway to be sure, and go
|
|
|
|
* round again inventing a new cookie if add234 tells us the one
|
|
|
|
* we thought of is already in use.)
|
|
|
|
*
|
|
|
|
* For XDM-AUTHORIZATION-1, it's a little more fiddly. The setup
|
|
|
|
* with XA1 is that half the cookie is used as a DES key with
|
|
|
|
* which to CBC-encrypt an assortment of stuff. Happily, the stuff
|
|
|
|
* encrypted _begins_ with the other half of the cookie, and the
|
|
|
|
* IV is always zero, which means that any valid XA1 authorisation
|
|
|
|
* attempt for a given cookie must begin with the same cipher
|
|
|
|
* block, consisting of the DES ECB encryption of the first half
|
|
|
|
* of the cookie using the second half as a key. So we compute
|
|
|
|
* that cipher block here and now, and use it as the sorting key
|
|
|
|
* for distinguishing XA1 entries in the tree.
|
|
|
|
*/
|
|
|
|
|
2013-11-17 14:05:10 +00:00
|
|
|
if (authtype == X11_MIT) {
|
2019-09-08 19:29:00 +00:00
|
|
|
auth->proto = X11_MIT;
|
2013-11-17 14:05:10 +00:00
|
|
|
|
2019-09-08 19:29:00 +00:00
|
|
|
/* MIT-MAGIC-COOKIE-1. Cookie size is 128 bits (16 bytes). */
|
2013-11-17 14:05:10 +00:00
|
|
|
auth->datalen = 16;
|
2019-09-08 19:29:00 +00:00
|
|
|
auth->data = snewn(auth->datalen, unsigned char);
|
2013-11-17 14:05:10 +00:00
|
|
|
auth->xa1_firstblock = NULL;
|
|
|
|
|
|
|
|
while (1) {
|
Replace random_byte() with random_read().
This is in preparation for a PRNG revamp which will want to have a
well defined boundary for any given request-for-randomness, so that it
can destroy the evidence afterwards. So no more looping round calling
random_byte() and then stopping when we feel like it: now you say up
front how many random bytes you want, and call random_read() which
gives you that many in one go.
Most of the call sites that had to be fixed are fairly mechanical, and
quite a few ended up more concise afterwards. A few became more
cumbersome, such as mp_random_bits, in which the new API doesn't let
me load the random bytes directly into the target integer without
triggering undefined behaviour, so instead I have to allocate a
separate temporary buffer.
The _most_ interesting call site was in the PKCS#1 v1.5 padding code
in sshrsa.c (used in SSH-1), in which you need a stream of _nonzero_
random bytes. The previous code just looped on random_byte, retrying
if it got a zero. Now I'm doing a much more interesting thing with an
mpint, essentially scaling a binary fraction repeatedly to extract a
number in the range [0,255) and then adding 1 to it.
2019-01-22 19:43:27 +00:00
|
|
|
random_read(auth->data, auth->datalen);
|
2013-11-17 14:05:10 +00:00
|
|
|
if (add234(authtree, auth) == auth)
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
2019-09-08 19:29:00 +00:00
|
|
|
auth->xdmseen = NULL;
|
2013-11-17 14:05:10 +00:00
|
|
|
} else {
|
2019-09-08 19:29:00 +00:00
|
|
|
assert(authtype == X11_XDM);
|
|
|
|
auth->proto = X11_XDM;
|
2013-11-17 14:05:10 +00:00
|
|
|
|
2019-09-08 19:29:00 +00:00
|
|
|
/* XDM-AUTHORIZATION-1. Cookie size is 16 bytes; byte 8 is zero. */
|
|
|
|
auth->datalen = 16;
|
|
|
|
auth->data = snewn(auth->datalen, unsigned char);
|
2013-11-17 14:05:10 +00:00
|
|
|
auth->xa1_firstblock = snewn(8, unsigned char);
|
|
|
|
memset(auth->xa1_firstblock, 0, 8);
|
|
|
|
|
|
|
|
while (1) {
|
Replace random_byte() with random_read().
This is in preparation for a PRNG revamp which will want to have a
well defined boundary for any given request-for-randomness, so that it
can destroy the evidence afterwards. So no more looping round calling
random_byte() and then stopping when we feel like it: now you say up
front how many random bytes you want, and call random_read() which
gives you that many in one go.
Most of the call sites that had to be fixed are fairly mechanical, and
quite a few ended up more concise afterwards. A few became more
cumbersome, such as mp_random_bits, in which the new API doesn't let
me load the random bytes directly into the target integer without
triggering undefined behaviour, so instead I have to allocate a
separate temporary buffer.
The _most_ interesting call site was in the PKCS#1 v1.5 padding code
in sshrsa.c (used in SSH-1), in which you need a stream of _nonzero_
random bytes. The previous code just looped on random_byte, retrying
if it got a zero. Now I'm doing a much more interesting thing with an
mpint, essentially scaling a binary fraction repeatedly to extract a
number in the range [0,255) and then adding 1 to it.
2019-01-22 19:43:27 +00:00
|
|
|
random_read(auth->data, 15);
|
|
|
|
auth->data[15] = auth->data[8];
|
|
|
|
auth->data[8] = 0;
|
|
|
|
|
2013-11-17 14:05:10 +00:00
|
|
|
memcpy(auth->xa1_firstblock, auth->data, 8);
|
|
|
|
des_encrypt_xdmauth(auth->data + 9, auth->xa1_firstblock, 8);
|
|
|
|
if (add234(authtree, auth) == auth)
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
auth->xdmseen = newtree234(xdmseen_cmp);
|
|
|
|
}
|
|
|
|
auth->protoname = dupstr(x11_authnames[auth->proto]);
|
|
|
|
auth->datastring = snewn(auth->datalen * 2 + 1, char);
|
|
|
|
for (i = 0; i < auth->datalen; i++)
|
2019-09-08 19:29:00 +00:00
|
|
|
sprintf(auth->datastring + i*2, "%02x",
|
|
|
|
auth->data[i]);
|
2013-11-17 14:05:10 +00:00
|
|
|
|
2013-11-17 14:05:41 +00:00
|
|
|
auth->disp = NULL;
|
2018-09-13 08:09:10 +00:00
|
|
|
auth->share_cs = NULL;
|
|
|
|
auth->share_chan = NULL;
|
2013-11-17 14:05:41 +00:00
|
|
|
|
2013-11-17 14:05:10 +00:00
|
|
|
return auth;
|
|
|
|
}
|
|
|
|
|
|
|
|
void x11_free_fake_auth(struct X11FakeAuth *auth)
|
|
|
|
{
|
|
|
|
if (auth->data)
|
2019-09-08 19:29:00 +00:00
|
|
|
smemclr(auth->data, auth->datalen);
|
2013-11-17 14:05:10 +00:00
|
|
|
sfree(auth->data);
|
|
|
|
sfree(auth->protoname);
|
|
|
|
sfree(auth->datastring);
|
|
|
|
sfree(auth->xa1_firstblock);
|
|
|
|
if (auth->xdmseen != NULL) {
|
2019-09-08 19:29:00 +00:00
|
|
|
struct XDMSeen *seen;
|
|
|
|
while ((seen = delpos234(auth->xdmseen, 0)) != NULL)
|
|
|
|
sfree(seen);
|
|
|
|
freetree234(auth->xdmseen);
|
2013-11-17 14:05:10 +00:00
|
|
|
}
|
|
|
|
sfree(auth);
|
|
|
|
}
|
|
|
|
|
|
|
|
int x11_authcmp(void *av, void *bv)
|
|
|
|
{
|
|
|
|
struct X11FakeAuth *a = (struct X11FakeAuth *)av;
|
|
|
|
struct X11FakeAuth *b = (struct X11FakeAuth *)bv;
|
|
|
|
|
|
|
|
if (a->proto < b->proto)
|
|
|
|
return -1;
|
|
|
|
else if (a->proto > b->proto)
|
|
|
|
return +1;
|
|
|
|
|
|
|
|
if (a->proto == X11_MIT) {
|
|
|
|
if (a->datalen < b->datalen)
|
|
|
|
return -1;
|
|
|
|
else if (a->datalen > b->datalen)
|
|
|
|
return +1;
|
|
|
|
|
|
|
|
return memcmp(a->data, b->data, a->datalen);
|
|
|
|
} else {
|
|
|
|
assert(a->proto == X11_XDM);
|
|
|
|
|
|
|
|
return memcmp(a->xa1_firstblock, b->xa1_firstblock, 8);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2005-02-02 23:51:58 +00:00
|
|
|
#define XDM_MAXSKEW 20*60 /* 20 minute clock skew should be OK */
|
|
|
|
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
static char *x11_verify(unsigned long peer_ip, int peer_port,
|
|
|
|
tree234 *authtree, char *proto,
|
|
|
|
unsigned char *data, int dlen,
|
|
|
|
struct X11FakeAuth **auth_ret)
|
2001-05-06 14:35:20 +00:00
|
|
|
{
|
2013-11-17 14:05:10 +00:00
|
|
|
struct X11FakeAuth match_dummy; /* for passing to find234 */
|
|
|
|
struct X11FakeAuth *auth;
|
|
|
|
|
|
|
|
/*
|
|
|
|
* First, do a lookup in our tree to find the only authorisation
|
|
|
|
* record that _might_ match.
|
|
|
|
*/
|
|
|
|
if (!strcmp(proto, x11_authnames[X11_MIT])) {
|
2013-11-17 14:05:41 +00:00
|
|
|
/*
|
|
|
|
* Just look up the whole cookie that was presented to us,
|
|
|
|
* which x11_authcmp will compare against the cookies we
|
|
|
|
* currently believe in.
|
|
|
|
*/
|
2013-11-17 14:05:10 +00:00
|
|
|
match_dummy.proto = X11_MIT;
|
|
|
|
match_dummy.datalen = dlen;
|
|
|
|
match_dummy.data = data;
|
|
|
|
} else if (!strcmp(proto, x11_authnames[X11_XDM])) {
|
2013-11-17 14:05:41 +00:00
|
|
|
/*
|
|
|
|
* Look up the first cipher block, against the stored first
|
|
|
|
* cipher blocks for the XDM-AUTHORIZATION-1 cookies we
|
|
|
|
* currently know. (See comment in x11_invent_fake_auth.)
|
|
|
|
*/
|
2013-11-17 14:05:10 +00:00
|
|
|
match_dummy.proto = X11_XDM;
|
|
|
|
match_dummy.xa1_firstblock = data;
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
} else if (!proto[0]) {
|
|
|
|
/*
|
|
|
|
* If the user has attempted to connect to the forwarded X
|
|
|
|
* display with no authority at all, we can give a better
|
|
|
|
* error message than the generic "unsupported protocol". We
|
|
|
|
* at least _recognise_ the null auth protocol, even if we
|
|
|
|
* don't _accept_ it.
|
|
|
|
*/
|
|
|
|
return dupstr("No authorisation provided");
|
2013-11-17 14:05:10 +00:00
|
|
|
} else {
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
return dupprintf("Unsupported authorisation protocol '%s'", proto);
|
2003-01-12 14:11:38 +00:00
|
|
|
}
|
2013-11-17 14:05:10 +00:00
|
|
|
|
|
|
|
if ((auth = find234(authtree, &match_dummy, 0)) == NULL)
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
return dupstr("Authorisation not recognised");
|
2013-11-17 14:05:10 +00:00
|
|
|
|
|
|
|
/*
|
|
|
|
* If we're using MIT-MAGIC-COOKIE-1, that was all we needed. If
|
|
|
|
* we're doing XDM-AUTHORIZATION-1, though, we have to check the
|
|
|
|
* rest of the auth data.
|
|
|
|
*/
|
|
|
|
if (auth->proto == X11_XDM) {
|
2019-09-08 19:29:00 +00:00
|
|
|
unsigned long t;
|
|
|
|
time_t tim;
|
|
|
|
int i;
|
|
|
|
struct XDMSeen *seen, *ret;
|
2003-01-12 14:11:38 +00:00
|
|
|
|
|
|
|
if (dlen != 24)
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
return dupprintf("XDM-AUTHORIZATION-1 data was wrong length "
|
|
|
|
"(%d, expected 24)", dlen);
|
2019-09-08 19:29:00 +00:00
|
|
|
if (peer_port == -1)
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
return dupstr("cannot do XDM-AUTHORIZATION-1 without remote "
|
|
|
|
"address data");
|
2019-09-08 19:29:00 +00:00
|
|
|
des_decrypt_xdmauth(auth->data+9, data, 24);
|
2023-04-01 15:07:29 +00:00
|
|
|
|
|
|
|
/* Bitwise-OR together any mismatches in the fixed parts of
|
|
|
|
* the data, to allow checking it all at once */
|
|
|
|
uint32_t mismatches = 0;
|
|
|
|
/* Check non-key half of auth cookie */
|
|
|
|
for (i = 0; i < 8; i++)
|
|
|
|
mismatches |= auth->data[i] ^ data[i];
|
|
|
|
/* Check IP address and port */
|
|
|
|
mismatches |= GET_32BIT_MSB_FIRST(data+8) ^ peer_ip;
|
|
|
|
mismatches |= (unsigned short)(GET_16BIT_MSB_FIRST(data+12) ^
|
|
|
|
peer_port);
|
|
|
|
/* Check zero padding */
|
|
|
|
for (i = 18; i < 24; i++)
|
|
|
|
mismatches |= data[i];
|
|
|
|
if (mismatches)
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
return dupstr("XDM-AUTHORIZATION-1 data failed check");
|
2023-04-01 15:07:29 +00:00
|
|
|
|
2019-09-08 19:29:00 +00:00
|
|
|
t = GET_32BIT_MSB_FIRST(data+14);
|
|
|
|
tim = time(NULL);
|
|
|
|
if (((unsigned long)t - (unsigned long)tim
|
2015-08-27 17:39:36 +00:00
|
|
|
+ XDM_MAXSKEW) > 2*XDM_MAXSKEW)
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
return dupstr("XDM-AUTHORIZATION-1 time stamp was too far out");
|
2019-09-08 19:29:00 +00:00
|
|
|
seen = snew(struct XDMSeen);
|
|
|
|
seen->time = t;
|
|
|
|
memcpy(seen->clientid, data+8, 6);
|
|
|
|
assert(auth->xdmseen != NULL);
|
|
|
|
ret = add234(auth->xdmseen, seen);
|
|
|
|
if (ret != seen) {
|
|
|
|
sfree(seen);
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
return dupstr("XDM-AUTHORIZATION-1 data replayed");
|
2019-09-08 19:29:00 +00:00
|
|
|
}
|
|
|
|
/* While we're here, purge entries too old to be replayed. */
|
|
|
|
for (;;) {
|
|
|
|
seen = index234(auth->xdmseen, 0);
|
|
|
|
assert(seen != NULL);
|
|
|
|
if (t - seen->time <= XDM_MAXSKEW)
|
|
|
|
break;
|
|
|
|
sfree(delpos234(auth->xdmseen, 0));
|
|
|
|
}
|
2003-01-10 18:33:35 +00:00
|
|
|
}
|
|
|
|
/* implement other protocols here if ever required */
|
2013-11-17 14:05:10 +00:00
|
|
|
|
|
|
|
*auth_ret = auth;
|
2003-01-12 14:11:38 +00:00
|
|
|
return NULL;
|
2001-01-22 11:34:52 +00:00
|
|
|
}
|
|
|
|
|
2013-11-17 14:05:04 +00:00
|
|
|
static void x11_send_init_error(struct X11Connection *conn,
|
|
|
|
const char *err_message);
|
|
|
|
|
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 13:28:32 +00:00
|
|
|
static void x11_closing(Plug *plug, PlugCloseType type, const char *error_msg)
|
2001-05-06 14:35:20 +00:00
|
|
|
{
|
2018-10-05 22:49:08 +00:00
|
|
|
struct X11Connection *xconn = container_of(
|
2018-10-05 06:24:16 +00:00
|
|
|
plug, struct X11Connection, plug);
|
2001-03-13 10:22:45 +00:00
|
|
|
|
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 13:28:32 +00:00
|
|
|
if (type != PLUGCLOSE_NORMAL) {
|
2012-04-23 17:59:53 +00:00
|
|
|
/*
|
2013-11-17 14:05:04 +00:00
|
|
|
* Socket error. If we're still at the connection setup stage,
|
|
|
|
* construct an X11 error packet passing on the problem.
|
|
|
|
*/
|
|
|
|
if (xconn->no_data_sent_to_x_client) {
|
|
|
|
char *err_message = dupprintf("unable to connect to forwarded "
|
|
|
|
"X server: %s", error_msg);
|
|
|
|
x11_send_init_error(xconn, err_message);
|
|
|
|
sfree(err_message);
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Whether we did that or not, now we slam the connection
|
|
|
|
* shut.
|
2012-04-23 17:59:53 +00:00
|
|
|
*/
|
2018-10-13 09:30:03 +00:00
|
|
|
sshfwd_initiate_close(xconn->c, error_msg);
|
2012-04-23 17:59:53 +00:00
|
|
|
} else {
|
|
|
|
/*
|
|
|
|
* Ordinary EOF received on socket. Send an EOF on the SSH
|
|
|
|
* channel.
|
|
|
|
*/
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
if (xconn->c)
|
|
|
|
sshfwd_write_eof(xconn->c);
|
2012-04-23 17:59:53 +00:00
|
|
|
}
|
2001-03-13 10:22:45 +00:00
|
|
|
}
|
|
|
|
|
2019-02-06 20:42:44 +00:00
|
|
|
static void x11_receive(Plug *plug, int urgent, const char *data, size_t len)
|
2001-05-06 14:35:20 +00:00
|
|
|
{
|
2018-10-05 22:49:08 +00:00
|
|
|
struct X11Connection *xconn = container_of(
|
2018-10-05 06:24:16 +00:00
|
|
|
plug, struct X11Connection, plug);
|
2001-01-22 11:34:52 +00:00
|
|
|
|
2018-10-29 19:50:29 +00:00
|
|
|
xconn->no_data_sent_to_x_client = false;
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
sshfwd_write(xconn->c, data, len);
|
2001-01-22 11:34:52 +00:00
|
|
|
}
|
|
|
|
|
2019-02-06 20:42:44 +00:00
|
|
|
static void x11_sent(Plug *plug, size_t bufsize)
|
2001-08-25 17:09:23 +00:00
|
|
|
{
|
2018-10-05 22:49:08 +00:00
|
|
|
struct X11Connection *xconn = container_of(
|
2018-10-05 06:24:16 +00:00
|
|
|
plug, struct X11Connection, plug);
|
2001-08-25 17:09:23 +00:00
|
|
|
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
sshfwd_unthrottle(xconn->c, bufsize);
|
2001-08-25 17:09:23 +00:00
|
|
|
}
|
|
|
|
|
2018-10-05 06:03:46 +00:00
|
|
|
static const PlugVtable X11Connection_plugvt = {
|
2024-06-29 11:07:59 +00:00
|
|
|
.log = nullplug_log,
|
Change vtable defs to use C99 designated initialisers.
This is a sweeping change applied across the whole code base by a spot
of Emacs Lisp. Now, everywhere I declare a vtable filled with function
pointers (and the occasional const data member), all the members of
the vtable structure are initialised by name using the '.fieldname =
value' syntax introduced in C99.
We were already using this syntax for a handful of things in the new
key-generation progress report system, so it's not new to the code
base as a whole.
The advantage is that now, when a vtable only declares a subset of the
available fields, I can initialise the rest to NULL or zero just by
leaving them out. This is most dramatic in a couple of the outlying
vtables in things like psocks (which has a ConnectionLayerVtable
containing only one non-NULL method), but less dramatically, it means
that the new 'flags' field in BackendVtable can be completely left out
of every backend definition except for the SUPDUP one which defines it
to a nonzero value. Similarly, the test_for_upstream method only used
by SSH doesn't have to be mentioned in the rest of the backends;
network Plugs for listening sockets don't have to explicitly null out
'receive' and 'sent', and vice versa for 'accepting', and so on.
While I'm at it, I've normalised the declarations so they don't use
the unnecessarily verbose 'struct' keyword. Also a handful of them
weren't const; now they are.
2020-03-10 21:06:29 +00:00
|
|
|
.closing = x11_closing,
|
|
|
|
.receive = x11_receive,
|
|
|
|
.sent = x11_sent,
|
2018-05-27 08:29:33 +00:00
|
|
|
};
|
|
|
|
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
static void x11_chan_free(Channel *chan);
|
2019-02-06 20:42:44 +00:00
|
|
|
static size_t x11_send(
|
|
|
|
Channel *chan, bool is_stderr, const void *vdata, size_t len);
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
static void x11_send_eof(Channel *chan);
|
Convert a lot of 'int' variables to 'bool'.
My normal habit these days, in new code, is to treat int and bool as
_almost_ completely separate types. I'm still willing to use C's
implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine,
no need to spell it out as blob.len != 0), but generally, if a
variable is going to be conceptually a boolean, I like to declare it
bool and assign to it using 'true' or 'false' rather than 0 or 1.
PuTTY is an exception, because it predates the C99 bool, and I've
stuck to its existing coding style even when adding new code to it.
But it's been annoying me more and more, so now that I've decided C99
bool is an acceptable thing to require from our toolchain in the first
place, here's a quite thorough trawl through the source doing
'boolification'. Many variables and function parameters are now typed
as bool rather than int; many assignments of 0 or 1 to those variables
are now spelled 'true' or 'false'.
I managed this thorough conversion with the help of a custom clang
plugin that I wrote to trawl the AST and apply heuristics to point out
where things might want changing. So I've even managed to do a decent
job on parts of the code I haven't looked at in years!
To make the plugin's work easier, I pushed platform front ends
generally in the direction of using standard 'bool' in preference to
platform-specific boolean types like Windows BOOL or GTK's gboolean;
I've left the platform booleans in places they _have_ to be for the
platform APIs to work right, but variables only used by my own code
have been converted wherever I found them.
In a few places there are int values that look very like booleans in
_most_ of the places they're used, but have a rarely-used third value,
or a distinction between different nonzero values that most users
don't care about. In these cases, I've _removed_ uses of 'true' and
'false' for the return values, to emphasise that there's something
more subtle going on than a simple boolean answer:
- the 'multisel' field in dialog.h's list box structure, for which
the GTK front end in particular recognises a difference between 1
and 2 but nearly everything else treats as boolean
- the 'urgent' parameter to plug_receive, where 1 vs 2 tells you
something about the specific location of the urgent pointer, but
most clients only care about 0 vs 'something nonzero'
- the return value of wc_match, where -1 indicates a syntax error in
the wildcard.
- the return values from SSH-1 RSA-key loading functions, which use
-1 for 'wrong passphrase' and 0 for all other failures (so any
caller which already knows it's not loading an _encrypted private_
key can treat them as boolean)
- term->esc_query, and the 'query' parameter in toggle_mode in
terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h,
but can also hold -1 for some other intervening character that we
don't support.
In a few places there's an integer that I haven't turned into a bool
even though it really _can_ only take values 0 or 1 (and, as above,
tried to make the call sites consistent in not calling those values
true and false), on the grounds that I thought it would make it more
confusing to imply that the 0 value was in some sense 'negative' or
bad and the 1 positive or good:
- the return value of plug_accepting uses the POSIXish convention of
0=success and nonzero=error; I think if I made it bool then I'd
also want to reverse its sense, and that's a job for a separate
piece of work.
- the 'screen' parameter to lineptr() in terminal.c, where 0 and 1
represent the default and alternate screens. There's no obvious
reason why one of those should be considered 'true' or 'positive'
or 'success' - they're just indices - so I've left it as int.
ssh_scp_recv had particularly confusing semantics for its previous int
return value: its call sites used '<= 0' to check for error, but it
never actually returned a negative number, just 0 or 1. Now the
function and its call sites agree that it's a bool.
In a couple of places I've renamed variables called 'ret', because I
don't like that name any more - it's unclear whether it means the
return value (in preparation) for the _containing_ function or the
return value received from a subroutine call, and occasionally I've
accidentally used the same variable for both and introduced a bug. So
where one of those got in my way, I've renamed it to 'toret' or 'retd'
(the latter short for 'returned') in line with my usual modern
practice, but I haven't done a thorough job of finding all of them.
Finally, one amusing side effect of doing this is that I've had to
separate quite a few chained assignments. It used to be perfectly fine
to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a
the 'true' defined by stdbool.h, that idiom provokes a warning from
gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
|
|
|
static void x11_set_input_wanted(Channel *chan, bool wanted);
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
static char *x11_log_close_msg(Channel *chan);
|
|
|
|
|
Change vtable defs to use C99 designated initialisers.
This is a sweeping change applied across the whole code base by a spot
of Emacs Lisp. Now, everywhere I declare a vtable filled with function
pointers (and the occasional const data member), all the members of
the vtable structure are initialised by name using the '.fieldname =
value' syntax introduced in C99.
We were already using this syntax for a handful of things in the new
key-generation progress report system, so it's not new to the code
base as a whole.
The advantage is that now, when a vtable only declares a subset of the
available fields, I can initialise the rest to NULL or zero just by
leaving them out. This is most dramatic in a couple of the outlying
vtables in things like psocks (which has a ConnectionLayerVtable
containing only one non-NULL method), but less dramatically, it means
that the new 'flags' field in BackendVtable can be completely left out
of every backend definition except for the SUPDUP one which defines it
to a nonzero value. Similarly, the test_for_upstream method only used
by SSH doesn't have to be mentioned in the rest of the backends;
network Plugs for listening sockets don't have to explicitly null out
'receive' and 'sent', and vice versa for 'accepting', and so on.
While I'm at it, I've normalised the declarations so they don't use
the unnecessarily verbose 'struct' keyword. Also a handful of them
weren't const; now they are.
2020-03-10 21:06:29 +00:00
|
|
|
static const ChannelVtable X11Connection_channelvt = {
|
|
|
|
.free = x11_chan_free,
|
|
|
|
.open_confirmation = chan_remotely_opened_confirmation,
|
|
|
|
.open_failed = chan_remotely_opened_failure,
|
|
|
|
.send = x11_send,
|
|
|
|
.send_eof = x11_send_eof,
|
|
|
|
.set_input_wanted = x11_set_input_wanted,
|
|
|
|
.log_close_msg = x11_log_close_msg,
|
|
|
|
.want_close = chan_default_want_close,
|
|
|
|
.rcvd_exit_status = chan_no_exit_status,
|
|
|
|
.rcvd_exit_signal = chan_no_exit_signal,
|
|
|
|
.rcvd_exit_signal_numeric = chan_no_exit_signal_numeric,
|
|
|
|
.run_shell = chan_no_run_shell,
|
|
|
|
.run_command = chan_no_run_command,
|
|
|
|
.run_subsystem = chan_no_run_subsystem,
|
|
|
|
.enable_x11_forwarding = chan_no_enable_x11_forwarding,
|
|
|
|
.enable_agent_forwarding = chan_no_enable_agent_forwarding,
|
|
|
|
.allocate_pty = chan_no_allocate_pty,
|
|
|
|
.set_env = chan_no_set_env,
|
|
|
|
.send_break = chan_no_send_break,
|
|
|
|
.send_signal = chan_no_send_signal,
|
|
|
|
.change_window_size = chan_no_change_window_size,
|
|
|
|
.request_response = chan_no_request_response,
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
};
|
|
|
|
|
2001-01-22 11:34:52 +00:00
|
|
|
/*
|
2013-11-17 14:05:23 +00:00
|
|
|
* Called to set up the X11Connection structure, though this does not
|
|
|
|
* yet connect to an actual server.
|
2001-01-22 11:34:52 +00:00
|
|
|
*/
|
2018-09-14 12:47:13 +00:00
|
|
|
Channel *x11_new_channel(tree234 *authtree, SshChannel *c,
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
const char *peeraddr, int peerport,
|
Convert a lot of 'int' variables to 'bool'.
My normal habit these days, in new code, is to treat int and bool as
_almost_ completely separate types. I'm still willing to use C's
implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine,
no need to spell it out as blob.len != 0), but generally, if a
variable is going to be conceptually a boolean, I like to declare it
bool and assign to it using 'true' or 'false' rather than 0 or 1.
PuTTY is an exception, because it predates the C99 bool, and I've
stuck to its existing coding style even when adding new code to it.
But it's been annoying me more and more, so now that I've decided C99
bool is an acceptable thing to require from our toolchain in the first
place, here's a quite thorough trawl through the source doing
'boolification'. Many variables and function parameters are now typed
as bool rather than int; many assignments of 0 or 1 to those variables
are now spelled 'true' or 'false'.
I managed this thorough conversion with the help of a custom clang
plugin that I wrote to trawl the AST and apply heuristics to point out
where things might want changing. So I've even managed to do a decent
job on parts of the code I haven't looked at in years!
To make the plugin's work easier, I pushed platform front ends
generally in the direction of using standard 'bool' in preference to
platform-specific boolean types like Windows BOOL or GTK's gboolean;
I've left the platform booleans in places they _have_ to be for the
platform APIs to work right, but variables only used by my own code
have been converted wherever I found them.
In a few places there are int values that look very like booleans in
_most_ of the places they're used, but have a rarely-used third value,
or a distinction between different nonzero values that most users
don't care about. In these cases, I've _removed_ uses of 'true' and
'false' for the return values, to emphasise that there's something
more subtle going on than a simple boolean answer:
- the 'multisel' field in dialog.h's list box structure, for which
the GTK front end in particular recognises a difference between 1
and 2 but nearly everything else treats as boolean
- the 'urgent' parameter to plug_receive, where 1 vs 2 tells you
something about the specific location of the urgent pointer, but
most clients only care about 0 vs 'something nonzero'
- the return value of wc_match, where -1 indicates a syntax error in
the wildcard.
- the return values from SSH-1 RSA-key loading functions, which use
-1 for 'wrong passphrase' and 0 for all other failures (so any
caller which already knows it's not loading an _encrypted private_
key can treat them as boolean)
- term->esc_query, and the 'query' parameter in toggle_mode in
terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h,
but can also hold -1 for some other intervening character that we
don't support.
In a few places there's an integer that I haven't turned into a bool
even though it really _can_ only take values 0 or 1 (and, as above,
tried to make the call sites consistent in not calling those values
true and false), on the grounds that I thought it would make it more
confusing to imply that the 0 value was in some sense 'negative' or
bad and the 1 positive or good:
- the return value of plug_accepting uses the POSIXish convention of
0=success and nonzero=error; I think if I made it bool then I'd
also want to reverse its sense, and that's a job for a separate
piece of work.
- the 'screen' parameter to lineptr() in terminal.c, where 0 and 1
represent the default and alternate screens. There's no obvious
reason why one of those should be considered 'true' or 'positive'
or 'success' - they're just indices - so I've left it as int.
ssh_scp_recv had particularly confusing semantics for its previous int
return value: its call sites used '<= 0' to check for error, but it
never actually returned a negative number, just 0 or 1. Now the
function and its call sites agree that it's a bool.
In a couple of places I've renamed variables called 'ret', because I
don't like that name any more - it's unclear whether it means the
return value (in preparation) for the _containing_ function or the
return value received from a subroutine call, and occasionally I've
accidentally used the same variable for both and introduced a bug. So
where one of those got in my way, I've renamed it to 'toret' or 'retd'
(the latter short for 'returned') in line with my usual modern
practice, but I haven't done a thorough job of finding all of them.
Finally, one amusing side effect of doing this is that I've had to
separate quite a few chained assignments. It used to be perfectly fine
to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a
the 'true' defined by stdbool.h, that idiom provokes a warning from
gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
|
|
|
bool connection_sharing_possible)
|
2001-05-06 14:35:20 +00:00
|
|
|
{
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
struct X11Connection *xconn;
|
2001-01-22 11:34:52 +00:00
|
|
|
|
|
|
|
/*
|
|
|
|
* Open socket.
|
|
|
|
*/
|
2013-11-17 14:05:23 +00:00
|
|
|
xconn = snew(struct X11Connection);
|
2018-10-05 06:24:16 +00:00
|
|
|
xconn->plug.vt = &X11Connection_plugvt;
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
xconn->chan.vt = &X11Connection_channelvt;
|
|
|
|
xconn->chan.initial_fixed_window_size =
|
|
|
|
(connection_sharing_possible ? 128 : 0);
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
xconn->auth_protocol = NULL;
|
2013-11-17 14:05:10 +00:00
|
|
|
xconn->authtree = authtree;
|
Convert a lot of 'int' variables to 'bool'.
My normal habit these days, in new code, is to treat int and bool as
_almost_ completely separate types. I'm still willing to use C's
implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine,
no need to spell it out as blob.len != 0), but generally, if a
variable is going to be conceptually a boolean, I like to declare it
bool and assign to it using 'true' or 'false' rather than 0 or 1.
PuTTY is an exception, because it predates the C99 bool, and I've
stuck to its existing coding style even when adding new code to it.
But it's been annoying me more and more, so now that I've decided C99
bool is an acceptable thing to require from our toolchain in the first
place, here's a quite thorough trawl through the source doing
'boolification'. Many variables and function parameters are now typed
as bool rather than int; many assignments of 0 or 1 to those variables
are now spelled 'true' or 'false'.
I managed this thorough conversion with the help of a custom clang
plugin that I wrote to trawl the AST and apply heuristics to point out
where things might want changing. So I've even managed to do a decent
job on parts of the code I haven't looked at in years!
To make the plugin's work easier, I pushed platform front ends
generally in the direction of using standard 'bool' in preference to
platform-specific boolean types like Windows BOOL or GTK's gboolean;
I've left the platform booleans in places they _have_ to be for the
platform APIs to work right, but variables only used by my own code
have been converted wherever I found them.
In a few places there are int values that look very like booleans in
_most_ of the places they're used, but have a rarely-used third value,
or a distinction between different nonzero values that most users
don't care about. In these cases, I've _removed_ uses of 'true' and
'false' for the return values, to emphasise that there's something
more subtle going on than a simple boolean answer:
- the 'multisel' field in dialog.h's list box structure, for which
the GTK front end in particular recognises a difference between 1
and 2 but nearly everything else treats as boolean
- the 'urgent' parameter to plug_receive, where 1 vs 2 tells you
something about the specific location of the urgent pointer, but
most clients only care about 0 vs 'something nonzero'
- the return value of wc_match, where -1 indicates a syntax error in
the wildcard.
- the return values from SSH-1 RSA-key loading functions, which use
-1 for 'wrong passphrase' and 0 for all other failures (so any
caller which already knows it's not loading an _encrypted private_
key can treat them as boolean)
- term->esc_query, and the 'query' parameter in toggle_mode in
terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h,
but can also hold -1 for some other intervening character that we
don't support.
In a few places there's an integer that I haven't turned into a bool
even though it really _can_ only take values 0 or 1 (and, as above,
tried to make the call sites consistent in not calling those values
true and false), on the grounds that I thought it would make it more
confusing to imply that the 0 value was in some sense 'negative' or
bad and the 1 positive or good:
- the return value of plug_accepting uses the POSIXish convention of
0=success and nonzero=error; I think if I made it bool then I'd
also want to reverse its sense, and that's a job for a separate
piece of work.
- the 'screen' parameter to lineptr() in terminal.c, where 0 and 1
represent the default and alternate screens. There's no obvious
reason why one of those should be considered 'true' or 'positive'
or 'success' - they're just indices - so I've left it as int.
ssh_scp_recv had particularly confusing semantics for its previous int
return value: its call sites used '<= 0' to check for error, but it
never actually returned a negative number, just 0 or 1. Now the
function and its call sites agree that it's a bool.
In a couple of places I've renamed variables called 'ret', because I
don't like that name any more - it's unclear whether it means the
return value (in preparation) for the _containing_ function or the
return value received from a subroutine call, and occasionally I've
accidentally used the same variable for both and introduced a bug. So
where one of those got in my way, I've renamed it to 'toret' or 'retd'
(the latter short for 'returned') in line with my usual modern
practice, but I haven't done a thorough job of finding all of them.
Finally, one amusing side effect of doing this is that I've had to
separate quite a few chained assignments. It used to be perfectly fine
to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a
the 'true' defined by stdbool.h, that idiom provokes a warning from
gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
|
|
|
xconn->verified = false;
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
xconn->data_read = 0;
|
2018-10-29 19:50:29 +00:00
|
|
|
xconn->input_wanted = true;
|
|
|
|
xconn->no_data_sent_to_x_client = true;
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
xconn->c = c;
|
|
|
|
|
2013-11-17 14:05:04 +00:00
|
|
|
/*
|
2013-11-17 14:05:10 +00:00
|
|
|
* We don't actually open a local socket to the X server just yet,
|
|
|
|
* because we don't know which one it is. Instead, we'll wait
|
|
|
|
* until we see the incoming authentication data, which may tell
|
|
|
|
* us what display to connect to, or whether we have to divert
|
|
|
|
* this X forwarding channel to a connection-sharing downstream
|
|
|
|
* rather than handling it ourself.
|
2013-11-17 14:05:04 +00:00
|
|
|
*/
|
2013-11-17 14:05:10 +00:00
|
|
|
xconn->disp = NULL;
|
2013-11-17 14:05:04 +00:00
|
|
|
xconn->s = NULL;
|
2001-03-13 10:22:45 +00:00
|
|
|
|
2003-01-12 14:11:38 +00:00
|
|
|
/*
|
2013-11-17 14:05:17 +00:00
|
|
|
* Stash the peer address we were given in its original text form.
|
2003-01-12 14:11:38 +00:00
|
|
|
*/
|
2013-11-17 14:05:17 +00:00
|
|
|
xconn->peer_addr = peeraddr ? dupstr(peeraddr) : NULL;
|
|
|
|
xconn->peer_port = peerport;
|
2003-01-12 14:11:38 +00:00
|
|
|
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
return &xconn->chan;
|
2001-01-22 11:34:52 +00:00
|
|
|
}
|
|
|
|
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
static void x11_chan_free(Channel *chan)
|
2001-05-06 14:35:20 +00:00
|
|
|
{
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
assert(chan->vt == &X11Connection_channelvt);
|
2018-10-05 22:49:08 +00:00
|
|
|
X11Connection *xconn = container_of(chan, X11Connection, chan);
|
2001-01-22 11:34:52 +00:00
|
|
|
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
if (xconn->auth_protocol) {
|
2019-09-08 19:29:00 +00:00
|
|
|
sfree(xconn->auth_protocol);
|
|
|
|
sfree(xconn->auth_data);
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
}
|
2001-01-22 11:34:52 +00:00
|
|
|
|
2013-11-17 14:05:04 +00:00
|
|
|
if (xconn->s)
|
|
|
|
sk_close(xconn->s);
|
2013-11-17 14:05:17 +00:00
|
|
|
|
|
|
|
sfree(xconn->peer_addr);
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
sfree(xconn);
|
2001-01-22 11:34:52 +00:00
|
|
|
}
|
|
|
|
|
Convert a lot of 'int' variables to 'bool'.
My normal habit these days, in new code, is to treat int and bool as
_almost_ completely separate types. I'm still willing to use C's
implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine,
no need to spell it out as blob.len != 0), but generally, if a
variable is going to be conceptually a boolean, I like to declare it
bool and assign to it using 'true' or 'false' rather than 0 or 1.
PuTTY is an exception, because it predates the C99 bool, and I've
stuck to its existing coding style even when adding new code to it.
But it's been annoying me more and more, so now that I've decided C99
bool is an acceptable thing to require from our toolchain in the first
place, here's a quite thorough trawl through the source doing
'boolification'. Many variables and function parameters are now typed
as bool rather than int; many assignments of 0 or 1 to those variables
are now spelled 'true' or 'false'.
I managed this thorough conversion with the help of a custom clang
plugin that I wrote to trawl the AST and apply heuristics to point out
where things might want changing. So I've even managed to do a decent
job on parts of the code I haven't looked at in years!
To make the plugin's work easier, I pushed platform front ends
generally in the direction of using standard 'bool' in preference to
platform-specific boolean types like Windows BOOL or GTK's gboolean;
I've left the platform booleans in places they _have_ to be for the
platform APIs to work right, but variables only used by my own code
have been converted wherever I found them.
In a few places there are int values that look very like booleans in
_most_ of the places they're used, but have a rarely-used third value,
or a distinction between different nonzero values that most users
don't care about. In these cases, I've _removed_ uses of 'true' and
'false' for the return values, to emphasise that there's something
more subtle going on than a simple boolean answer:
- the 'multisel' field in dialog.h's list box structure, for which
the GTK front end in particular recognises a difference between 1
and 2 but nearly everything else treats as boolean
- the 'urgent' parameter to plug_receive, where 1 vs 2 tells you
something about the specific location of the urgent pointer, but
most clients only care about 0 vs 'something nonzero'
- the return value of wc_match, where -1 indicates a syntax error in
the wildcard.
- the return values from SSH-1 RSA-key loading functions, which use
-1 for 'wrong passphrase' and 0 for all other failures (so any
caller which already knows it's not loading an _encrypted private_
key can treat them as boolean)
- term->esc_query, and the 'query' parameter in toggle_mode in
terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h,
but can also hold -1 for some other intervening character that we
don't support.
In a few places there's an integer that I haven't turned into a bool
even though it really _can_ only take values 0 or 1 (and, as above,
tried to make the call sites consistent in not calling those values
true and false), on the grounds that I thought it would make it more
confusing to imply that the 0 value was in some sense 'negative' or
bad and the 1 positive or good:
- the return value of plug_accepting uses the POSIXish convention of
0=success and nonzero=error; I think if I made it bool then I'd
also want to reverse its sense, and that's a job for a separate
piece of work.
- the 'screen' parameter to lineptr() in terminal.c, where 0 and 1
represent the default and alternate screens. There's no obvious
reason why one of those should be considered 'true' or 'positive'
or 'success' - they're just indices - so I've left it as int.
ssh_scp_recv had particularly confusing semantics for its previous int
return value: its call sites used '<= 0' to check for error, but it
never actually returned a negative number, just 0 or 1. Now the
function and its call sites agree that it's a bool.
In a couple of places I've renamed variables called 'ret', because I
don't like that name any more - it's unclear whether it means the
return value (in preparation) for the _containing_ function or the
return value received from a subroutine call, and occasionally I've
accidentally used the same variable for both and introduced a bug. So
where one of those got in my way, I've renamed it to 'toret' or 'retd'
(the latter short for 'returned') in line with my usual modern
practice, but I haven't done a thorough job of finding all of them.
Finally, one amusing side effect of doing this is that I've had to
separate quite a few chained assignments. It used to be perfectly fine
to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a
the 'true' defined by stdbool.h, that idiom provokes a warning from
gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
|
|
|
static void x11_set_input_wanted(Channel *chan, bool wanted)
|
2001-08-25 17:09:23 +00:00
|
|
|
{
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
assert(chan->vt == &X11Connection_channelvt);
|
2018-10-05 22:49:08 +00:00
|
|
|
X11Connection *xconn = container_of(chan, X11Connection, chan);
|
2001-08-25 17:09:23 +00:00
|
|
|
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
xconn->input_wanted = wanted;
|
2013-11-17 14:05:04 +00:00
|
|
|
if (xconn->s)
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
sk_set_frozen(xconn->s, !xconn->input_wanted);
|
2013-11-17 14:05:04 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
static void x11_send_init_error(struct X11Connection *xconn,
|
|
|
|
const char *err_message)
|
|
|
|
{
|
|
|
|
char *full_message;
|
|
|
|
int msglen, msgsize;
|
|
|
|
unsigned char *reply;
|
|
|
|
|
|
|
|
full_message = dupprintf("%s X11 proxy: %s\n", appname, err_message);
|
|
|
|
|
|
|
|
msglen = strlen(full_message);
|
|
|
|
reply = snewn(8 + msglen+1 + 4, unsigned char); /* include zero */
|
|
|
|
msgsize = (msglen + 3) & ~3;
|
2019-09-08 19:29:00 +00:00
|
|
|
reply[0] = 0; /* failure */
|
|
|
|
reply[1] = msglen; /* length of reason string */
|
|
|
|
memcpy(reply + 2, xconn->firstpkt + 2, 4); /* major/minor proto vsn */
|
2019-02-04 07:39:03 +00:00
|
|
|
PUT_16BIT_X11(xconn->firstpkt[0], reply + 6, msgsize >> 2);/* data len */
|
2013-11-17 14:05:04 +00:00
|
|
|
memset(reply + 8, 0, msgsize);
|
|
|
|
memcpy(reply + 8, full_message, msglen);
|
2018-05-26 07:31:34 +00:00
|
|
|
sshfwd_write(xconn->c, reply, 8 + msgsize);
|
2013-11-17 14:05:04 +00:00
|
|
|
sshfwd_write_eof(xconn->c);
|
2018-10-29 19:50:29 +00:00
|
|
|
xconn->no_data_sent_to_x_client = false;
|
2013-11-17 14:05:04 +00:00
|
|
|
sfree(reply);
|
|
|
|
sfree(full_message);
|
2001-08-25 17:09:23 +00:00
|
|
|
}
|
|
|
|
|
2001-01-22 11:34:52 +00:00
|
|
|
/*
|
|
|
|
* Called to send data down the raw connection.
|
|
|
|
*/
|
2019-02-06 20:42:44 +00:00
|
|
|
static size_t x11_send(
|
|
|
|
Channel *chan, bool is_stderr, const void *vdata, size_t len)
|
2001-05-06 14:35:20 +00:00
|
|
|
{
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
assert(chan->vt == &X11Connection_channelvt);
|
2018-10-05 22:49:08 +00:00
|
|
|
X11Connection *xconn = container_of(chan, X11Connection, chan);
|
2018-06-02 06:52:26 +00:00
|
|
|
const char *data = (const char *)vdata;
|
|
|
|
|
2001-01-22 11:34:52 +00:00
|
|
|
/*
|
|
|
|
* Read the first packet.
|
|
|
|
*/
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
while (len > 0 && xconn->data_read < 12)
|
2019-09-08 19:29:00 +00:00
|
|
|
xconn->firstpkt[xconn->data_read++] = (unsigned char) (len--, *data++);
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
if (xconn->data_read < 12)
|
2019-09-08 19:29:00 +00:00
|
|
|
return 0;
|
2001-01-22 11:34:52 +00:00
|
|
|
|
|
|
|
/*
|
|
|
|
* If we have not allocated the auth_protocol and auth_data
|
|
|
|
* strings, do so now.
|
|
|
|
*/
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
if (!xconn->auth_protocol) {
|
2019-02-04 07:39:03 +00:00
|
|
|
char endian = xconn->firstpkt[0];
|
2019-09-08 19:29:00 +00:00
|
|
|
xconn->auth_plen = GET_16BIT_X11(endian, xconn->firstpkt + 6);
|
|
|
|
xconn->auth_dlen = GET_16BIT_X11(endian, xconn->firstpkt + 8);
|
|
|
|
xconn->auth_psize = (xconn->auth_plen + 3) & ~3;
|
|
|
|
xconn->auth_dsize = (xconn->auth_dlen + 3) & ~3;
|
|
|
|
/* Leave room for a terminating zero, to make our lives easier. */
|
|
|
|
xconn->auth_protocol = snewn(xconn->auth_psize + 1, char);
|
|
|
|
xconn->auth_data = snewn(xconn->auth_dsize, unsigned char);
|
2001-01-22 11:34:52 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Read the auth_protocol and auth_data strings.
|
|
|
|
*/
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
while (len > 0 &&
|
|
|
|
xconn->data_read < 12 + xconn->auth_psize)
|
2019-09-08 19:29:00 +00:00
|
|
|
xconn->auth_protocol[xconn->data_read++ - 12] = (len--, *data++);
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
while (len > 0 &&
|
|
|
|
xconn->data_read < 12 + xconn->auth_psize + xconn->auth_dsize)
|
2019-09-08 19:29:00 +00:00
|
|
|
xconn->auth_data[xconn->data_read++ - 12 -
|
2022-08-03 19:48:46 +00:00
|
|
|
xconn->auth_psize] = (unsigned char) (len--, *data++);
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
if (xconn->data_read < 12 + xconn->auth_psize + xconn->auth_dsize)
|
2019-09-08 19:29:00 +00:00
|
|
|
return 0;
|
2001-01-22 11:34:52 +00:00
|
|
|
|
|
|
|
/*
|
2008-11-17 18:38:09 +00:00
|
|
|
* If we haven't verified the authorisation, do so now.
|
2001-01-22 11:34:52 +00:00
|
|
|
*/
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
if (!xconn->verified) {
|
2019-09-08 19:29:00 +00:00
|
|
|
const char *err;
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
char *errmut;
|
2013-11-17 14:05:10 +00:00
|
|
|
struct X11FakeAuth *auth_matched = NULL;
|
2013-11-17 14:05:17 +00:00
|
|
|
unsigned long peer_ip;
|
|
|
|
int peer_port;
|
|
|
|
int protomajor, protominor;
|
|
|
|
void *greeting;
|
|
|
|
int greeting_len;
|
|
|
|
unsigned char *socketdata;
|
|
|
|
int socketdatalen;
|
|
|
|
char new_peer_addr[32];
|
|
|
|
int new_peer_port;
|
2019-02-04 07:39:03 +00:00
|
|
|
char endian = xconn->firstpkt[0];
|
2013-11-17 14:05:17 +00:00
|
|
|
|
2019-02-04 07:39:03 +00:00
|
|
|
protomajor = GET_16BIT_X11(endian, xconn->firstpkt + 2);
|
|
|
|
protominor = GET_16BIT_X11(endian, xconn->firstpkt + 4);
|
2013-11-17 14:05:04 +00:00
|
|
|
|
|
|
|
assert(!xconn->s);
|
2001-05-06 14:35:20 +00:00
|
|
|
|
2019-09-08 19:29:00 +00:00
|
|
|
xconn->auth_protocol[xconn->auth_plen] = '\0'; /* ASCIZ */
|
2013-11-17 14:05:17 +00:00
|
|
|
|
2013-11-17 14:05:41 +00:00
|
|
|
peer_ip = 0; /* placate optimiser */
|
2013-11-17 14:05:17 +00:00
|
|
|
if (x11_parse_ip(xconn->peer_addr, &peer_ip))
|
|
|
|
peer_port = xconn->peer_port;
|
|
|
|
else
|
|
|
|
peer_port = -1; /* signal no peer address data available */
|
|
|
|
|
Improve error reporting from x11_verify().
Now the return value is a dynamically allocated string instead of a
static one, which means that the error message can include details
taken from the specific failing connection. In particular, if someone
requests an X11 authorisation protocol we don't support, we can print
its name as part of the message, which may help users debug the
problem.
One particularly important special case of this is that if the client
connection presents _no_ authorisation - which is surely by far the
most likely thing to happen by accident, e.g. if the auth file has
gone missing, or the hostname doesn't match for some reason - then we
now give a specific message "No authorisation provided", which I think
is considerably more helpful than just lumping that very common case
in with "Unsupported authorisation protocol". Even changing the latter
to "Unsupported authorisation protocol ''" is still not very sensible.
The problem in that case is not that the user has tried an exotic auth
protocol we've never heard of - it's that they've forgotten, or
failed, to provide one at all.
The error message for "XDM-AUTHORIZATION-1 data was wrong length" is
the other modified one: it now says what the wrong length _was_.
However, all other failures of X-A-1 are still kept deliberately
vague, because saying which part of the decrypted string didn't match
is an obvious information leak.
2023-04-01 14:53:29 +00:00
|
|
|
errmut = x11_verify(peer_ip, peer_port,
|
|
|
|
xconn->authtree, xconn->auth_protocol,
|
|
|
|
xconn->auth_data, xconn->auth_dlen, &auth_matched);
|
|
|
|
if (errmut) {
|
|
|
|
x11_send_init_error(xconn, errmut);
|
|
|
|
sfree(errmut);
|
2013-11-17 14:05:04 +00:00
|
|
|
return 0;
|
|
|
|
}
|
2013-11-17 14:05:10 +00:00
|
|
|
assert(auth_matched);
|
2001-05-06 14:35:20 +00:00
|
|
|
|
2013-11-17 14:05:41 +00:00
|
|
|
/*
|
|
|
|
* If this auth points to a connection-sharing downstream
|
|
|
|
* rather than an X display we know how to connect to
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
* directly, pass it off to the sharing module now. (This will
|
|
|
|
* have the side effect of freeing xconn.)
|
2013-11-17 14:05:41 +00:00
|
|
|
*/
|
|
|
|
if (auth_matched->share_cs) {
|
|
|
|
sshfwd_x11_sharing_handover(xconn->c, auth_matched->share_cs,
|
|
|
|
auth_matched->share_chan,
|
|
|
|
xconn->peer_addr, xconn->peer_port,
|
|
|
|
xconn->firstpkt[0],
|
|
|
|
protomajor, protominor, data, len);
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
2013-11-17 14:05:04 +00:00
|
|
|
/*
|
2013-11-17 14:05:10 +00:00
|
|
|
* Now we know we're going to accept the connection, and what
|
|
|
|
* X display to connect to. Actually connect to it.
|
2013-11-17 14:05:04 +00:00
|
|
|
*/
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
xconn->chan.initial_fixed_window_size = 0;
|
|
|
|
sshfwd_window_override_removed(xconn->c);
|
2013-11-17 14:05:10 +00:00
|
|
|
xconn->disp = auth_matched->disp;
|
2013-11-17 14:05:04 +00:00
|
|
|
xconn->s = new_connection(sk_addr_dup(xconn->disp->addr),
|
2019-09-08 19:29:00 +00:00
|
|
|
xconn->disp->realhost, xconn->disp->port,
|
Convert a lot of 'int' variables to 'bool'.
My normal habit these days, in new code, is to treat int and bool as
_almost_ completely separate types. I'm still willing to use C's
implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine,
no need to spell it out as blob.len != 0), but generally, if a
variable is going to be conceptually a boolean, I like to declare it
bool and assign to it using 'true' or 'false' rather than 0 or 1.
PuTTY is an exception, because it predates the C99 bool, and I've
stuck to its existing coding style even when adding new code to it.
But it's been annoying me more and more, so now that I've decided C99
bool is an acceptable thing to require from our toolchain in the first
place, here's a quite thorough trawl through the source doing
'boolification'. Many variables and function parameters are now typed
as bool rather than int; many assignments of 0 or 1 to those variables
are now spelled 'true' or 'false'.
I managed this thorough conversion with the help of a custom clang
plugin that I wrote to trawl the AST and apply heuristics to point out
where things might want changing. So I've even managed to do a decent
job on parts of the code I haven't looked at in years!
To make the plugin's work easier, I pushed platform front ends
generally in the direction of using standard 'bool' in preference to
platform-specific boolean types like Windows BOOL or GTK's gboolean;
I've left the platform booleans in places they _have_ to be for the
platform APIs to work right, but variables only used by my own code
have been converted wherever I found them.
In a few places there are int values that look very like booleans in
_most_ of the places they're used, but have a rarely-used third value,
or a distinction between different nonzero values that most users
don't care about. In these cases, I've _removed_ uses of 'true' and
'false' for the return values, to emphasise that there's something
more subtle going on than a simple boolean answer:
- the 'multisel' field in dialog.h's list box structure, for which
the GTK front end in particular recognises a difference between 1
and 2 but nearly everything else treats as boolean
- the 'urgent' parameter to plug_receive, where 1 vs 2 tells you
something about the specific location of the urgent pointer, but
most clients only care about 0 vs 'something nonzero'
- the return value of wc_match, where -1 indicates a syntax error in
the wildcard.
- the return values from SSH-1 RSA-key loading functions, which use
-1 for 'wrong passphrase' and 0 for all other failures (so any
caller which already knows it's not loading an _encrypted private_
key can treat them as boolean)
- term->esc_query, and the 'query' parameter in toggle_mode in
terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h,
but can also hold -1 for some other intervening character that we
don't support.
In a few places there's an integer that I haven't turned into a bool
even though it really _can_ only take values 0 or 1 (and, as above,
tried to make the call sites consistent in not calling those values
true and false), on the grounds that I thought it would make it more
confusing to imply that the 0 value was in some sense 'negative' or
bad and the 1 positive or good:
- the return value of plug_accepting uses the POSIXish convention of
0=success and nonzero=error; I think if I made it bool then I'd
also want to reverse its sense, and that's a job for a separate
piece of work.
- the 'screen' parameter to lineptr() in terminal.c, where 0 and 1
represent the default and alternate screens. There's no obvious
reason why one of those should be considered 'true' or 'positive'
or 'success' - they're just indices - so I've left it as int.
ssh_scp_recv had particularly confusing semantics for its previous int
return value: its call sites used '<= 0' to check for error, but it
never actually returned a negative number, just 0 or 1. Now the
function and its call sites agree that it's a bool.
In a couple of places I've renamed variables called 'ret', because I
don't like that name any more - it's unclear whether it means the
return value (in preparation) for the _containing_ function or the
return value received from a subroutine call, and occasionally I've
accidentally used the same variable for both and introduced a bug. So
where one of those got in my way, I've renamed it to 'toret' or 'retd'
(the latter short for 'returned') in line with my usual modern
practice, but I haven't done a thorough job of finding all of them.
Finally, one amusing side effect of doing this is that I've had to
separate quite a few chained assignments. It used to be perfectly fine
to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a
the 'true' defined by stdbool.h, that idiom provokes a warning from
gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
|
|
|
false, true, false, false, &xconn->plug,
|
2021-10-30 16:36:52 +00:00
|
|
|
sshfwd_get_conf(xconn->c), NULL);
|
2013-11-17 14:05:04 +00:00
|
|
|
if ((err = sk_socket_error(xconn->s)) != NULL) {
|
|
|
|
char *err_message = dupprintf("unable to connect to"
|
|
|
|
" forwarded X server: %s", err);
|
|
|
|
x11_send_init_error(xconn, err_message);
|
|
|
|
sfree(err_message);
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
2013-11-17 14:05:17 +00:00
|
|
|
* Write a new connection header containing our replacement
|
|
|
|
* auth data.
|
2019-09-08 19:29:00 +00:00
|
|
|
*/
|
2017-02-05 11:19:12 +00:00
|
|
|
socketdatalen = 0; /* placate compiler warning */
|
2013-11-17 14:05:17 +00:00
|
|
|
socketdata = sk_getxdmdata(xconn->s, &socketdatalen);
|
|
|
|
if (socketdata && socketdatalen==6) {
|
|
|
|
sprintf(new_peer_addr, "%d.%d.%d.%d", socketdata[0],
|
|
|
|
socketdata[1], socketdata[2], socketdata[3]);
|
|
|
|
new_peer_port = GET_16BIT_MSB_FIRST(socketdata + 4);
|
|
|
|
} else {
|
|
|
|
strcpy(new_peer_addr, "0.0.0.0");
|
|
|
|
new_peer_port = 0;
|
2003-01-10 18:33:35 +00:00
|
|
|
}
|
2013-11-17 14:05:17 +00:00
|
|
|
|
|
|
|
greeting = x11_make_greeting(xconn->firstpkt[0],
|
|
|
|
protomajor, protominor,
|
|
|
|
xconn->disp->localauthproto,
|
|
|
|
xconn->disp->localauthdata,
|
|
|
|
xconn->disp->localauthdatalen,
|
|
|
|
new_peer_addr, new_peer_port,
|
|
|
|
&greeting_len);
|
2019-09-08 19:29:00 +00:00
|
|
|
|
2013-11-17 14:05:17 +00:00
|
|
|
sk_write(xconn->s, greeting, greeting_len);
|
|
|
|
|
|
|
|
smemclr(greeting, greeting_len);
|
|
|
|
sfree(greeting);
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Now we're done.
|
|
|
|
*/
|
2019-09-08 19:29:00 +00:00
|
|
|
xconn->verified = true;
|
2001-01-22 11:34:52 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* After initialisation, just copy data simply.
|
|
|
|
*/
|
|
|
|
|
Refactor ssh.c's APIs to x11fwd.c and portfwd.c.
The most important change is that, where previously ssh.c held the
Socket pointer for each X11 and port forwarding, and the support
modules would find their internal state structure by calling
sk_get_private_ptr on that Socket, it's now the other way round. ssh.c
now directly holds the internal state structure pointer for each
forwarding, and when the support module needs the Socket it looks it
up in a field of that. This will come in handy when I decouple socket
creation from logical forwarding setup, so that X forwardings can
delay actually opening a connection to an X server until they look at
the authentication data and see which server it has to be.
However, while I'm here, I've also taken the opportunity to clean up a
few other points, notably error message handling, and also the fact
that the same kind of state structure was used for both
connection-type and listening-type port forwardings. Now there are
separate PortForwarding and PortListener structure types, which seems
far more sensible.
[originally from svn r10074]
2013-11-17 14:04:41 +00:00
|
|
|
return sk_write(xconn->s, data, len);
|
2001-01-22 11:34:52 +00:00
|
|
|
}
|
2011-09-13 11:44:03 +00:00
|
|
|
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
static void x11_send_eof(Channel *chan)
|
2011-09-13 11:44:03 +00:00
|
|
|
{
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
assert(chan->vt == &X11Connection_channelvt);
|
2018-10-05 22:49:08 +00:00
|
|
|
X11Connection *xconn = container_of(chan, X11Connection, chan);
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
|
2013-11-17 14:05:04 +00:00
|
|
|
if (xconn->s) {
|
|
|
|
sk_write_eof(xconn->s);
|
|
|
|
} else {
|
|
|
|
/*
|
|
|
|
* If EOF is received from the X client before we've got to
|
|
|
|
* the point of actually connecting to an X server, then we
|
|
|
|
* should send an EOF back to the client so that the
|
|
|
|
* forwarded channel will be terminated.
|
|
|
|
*/
|
|
|
|
if (xconn->c)
|
|
|
|
sshfwd_write_eof(xconn->c);
|
|
|
|
}
|
2011-09-13 11:44:03 +00:00
|
|
|
}
|
2013-11-17 14:05:41 +00:00
|
|
|
|
Replace enum+union of local channel types with a vtable.
There's now an interface called 'Channel', which handles the local
side of an SSH connection-layer channel, in terms of knowing where to
send incoming channel data to, whether to close the channel, etc.
Channel and the previous 'struct ssh_channel' mutually refer. The
latter contains all the SSH-specific parts, and as much of the common
logic as possible: in particular, Channel doesn't have to know
anything about SSH packet formats, or which SSH protocol version is in
use, or deal with all the fiddly stuff about window sizes - with the
exception that x11fwd.c's implementation of it does have to be able to
ask for a small fixed initial window size for the bodgy system that
distinguishes upstream from downstream X forwardings.
I've taken the opportunity to move the code implementing the detailed
behaviour of agent forwarding out of ssh.c, now that all of it is on
the far side of a uniform interface. (This also means that if I later
implement agent forwarding directly to a Unix socket as an
alternative, it'll be a matter of changing just the one call to
agentf_new() that makes the Channel to plug into a forwarding.)
2018-09-12 14:03:47 +00:00
|
|
|
static char *x11_log_close_msg(Channel *chan)
|
|
|
|
{
|
|
|
|
return dupstr("Forwarded X11 connection terminated");
|
|
|
|
}
|