Subject: X forwarding

From: der Mouse <[email-masked]>
Date: Sat, 17 Apr 2004 21:26:04 -0400 (EDT)

I'd like to dredge up something from the past (most recently, and
perhaps most significantly, last November).

I was starting to implement X forwarding, when I realized that the
x11-req defined in connect-18 was unnecessarily complex, and, as far as
I could see, pointlessly so.  Rather than drag out stuff that I see
from the archives has been hashed over before, I'll just ask two
questions that I didn't see answers to in the archives.  (It's possible
I missed them; I just grepped for "x11-req".  If they have been
answered, something like a message-ID to grep for would be much
appreciated.)

I can see no value whatever in making the client pass an X
authentication method and data blob to the server.  The client has no
idea what authentication methods may be supported by the X
implementation on the server side, so it has no way to select an
appropriate authentication method.  There is no additional security
gained from doing so, since any forwarded X connection will perforce be
coming from the same entity the client gave the data blob to, so it
serves no authentication use to return it.

However, the people who designed the protocol are not stupid, so there
must be something I'm missing.  This, then, is my first question: why
are those fields there?

[email-masked] wrote, of X forwarding, that another field -
"display id" is what I remember seeing - seems called for.  I find this
necessary too, for reasons very similar to what I mentioned recently
when writing about forwarding TCP connections.

> In more general terms, letting the server do the authentication is in
> a way making an (X11) client responsible for X11 server security,
> which is not good practice.

I don't understand the point here.  I have been unable to come up with
a threat model against which having the client provide an
authentication method and data blob provides any defense.  My other
question: can anyone give an example?

I'm implementing X forwarding as a private request which carries only a
uint32 cookie and a screen number; when the server wants to forward an
X connection, the channel open carries the cookie and the first six
bytes of the X protocol stream sent by the client (I could have sent
these as the first data on the new connection, but this way seems
marginally simpler).  The rest of the Xclient->Xserver protocol setup
data (the authorization name and data) are not sent; the client is
expected to replace that anyway.

What security horrors am I risking thereby?

/~\ The ASCII				der Mouse
\ / Ribbon Campaign
 X  Against HTML	       [email-masked]
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B

---

From: Simon Tatham <[email-masked]>
Date: Sun, 18 Apr 2004 08:05:30 +0100

der Mouse  <[email-masked]> wrote:
> Rather than drag out stuff that I see from the archives has been
> hashed over before, I'll just ask two questions that I didn't see
> answers to in the archives.  (It's possible I missed them; I just
> grepped for "x11-req".  If they have been answered, something like a
> message-ID to grep for would be much appreciated.)
> 
> authentication method and data blob to the server.
[...]
> There is no additional security gained from doing so, since any
> forwarded X connection will perforce be coming from the same entity
> the client gave the data blob to, so it serves no authentication use
> to return it.

Yes, I suggested precisely this (that forwarded X connections over
the SSH connection should be unauthenticated, allowing the client to
insert its local auth and independently allowing the server to
invent fake auth). <[email-masked]>, 23 Jan
2001.

The consensus at the time was that several people thought my idea
was better than the existing method, some other people thought it
was at least no worse, but everybody (including me :-) agreed that
even back in early 2001 it was much too late to change it for the
only marginal gain.

> uint32 cookie and a screen number;
[...]
> What security horrors am I risking thereby?

Surely the main horror you're risking is that nobody else's server
will support your private request and everybody else's clients will
expect you to support the standard one?
-- 
Simon Tatham         "loop, infinite _see_ infinite loop"
<[email-masked]>     - Index, Borland Pascal Language Guide

---

From: der Mouse <[email-masked]>
Date: Sun, 18 Apr 2004 03:24:31 -0400 (EDT)

>> There is no additional security gained from [passing X authorization
>> data from ssh client to ssh server] [...]
> The consensus at the time was that several people thought my idea was
> better than the existing method, some other people thought it was at
> least no worse, but everybody (including me :-) agreed that even back
> in early 2001 it was much too late to change it for the only marginal
> gain.

Only marginal gain?  I don't consider getting rid of assumptions on the
part of the client about what X authorization mechanisms are supported
on the server "only marginal gain". :-)

For my own purposes, I need an extra cookie, at least a small integer,
which forces me to use a new request in any case.  (I don't consider it
acceptable to hijack part of the authorization cookie to carry this
information.)

The existing X forwarding is also rather underspecified; for example,
it is clear that at least one end is expected to get its fingers into
the X protocol, but it is not clear whether the data flowing through
the X channel is pure X protocol or not (it might, for example, have
the authorization information stripped - after all, the authorization
information is the whole reason X forwarding is treated any differently
from vanilla TCP forwarding).

>> I'm implementing X forwarding as [...]

That's hardly a security problem, and in text which I notice you cut I
quoted someone (our Swedish elf, I think it was) talking about
insecurities in one way as compared to the other, but without including
any specifics.

As for compatability, now that I have X forwarding working at all, I
will probably make it do something at least vaguely sensible with the X
requests from connect-18 (though of course they will be incompatible
with the connection sharing that I suspect will be one of the major
features my version will offer).

/~\ The ASCII				der Mouse
\ / Ribbon Campaign

---

From: [email-masked] (Niels Möller)
Date: 19 Apr 2004 18:31:56 +0200

> authentication method and data blob to the server.

The only value I see is that the munging of the X-protocol data
stream, and the "fake cookie"-handling, is moved to the client rather
than the server. When having the choice, I often prefer a complex
client over a complex server. Besides that, I agree with what you're
saying.

> question: can anyone give an example?

Maybe not a good example, but I think you may failure mode when
talking to a server that doesn't do any authentication at all (it just
binds a tcp socket and starts forwarding connections). If the client
expects a cookie in the ssh channel, it will reject unathorized
connections.

Including the cookie will of course not provide any protection from an
compromised server (say, it opens a tcp socket and forwards it, and in
addition stores the supplied cookie somewhere where the attacker can
read it).

But I think it may provide some protection from servers that are
misconfigured rather than compromised. I agree all this is a pretty
weak argument.

> marginally simpler).

Please write up a spec for this.

Including details such as the expectation that the server will ensure
(by creating a new random cookie, or by some *stronger* authentication
method) that only authorized processes can use the X forwarding. And
in which way the forwarding is associated to a session (for example,
if the forwarding is cancelled automatically when the session channel
is closed).

The lack of a display id is one of the reasons my "connection sharing
thing", lshg, doesn't support X11 forwarding, so I may want to use
your new request type.

Regards,
/Niels

---

From: der Mouse <[email-masked]>
Date: Mon, 19 Apr 2004 13:44:16 -0400 (EDT)

>>> In more general terms, letting the server do the authentication is
>>> in a way making an (X11) client responsible for X11 server
>>> security, which is not good practice.

>> I don't understand the point here.  I have been unable to come up
>> with a threat model against which having the client provide an
>> authentication method and data blob provides any defense.

> talking to a server that doesn't do any authentication at all [...]

True enough.  I was considering only malicious servers and servers that
cared about the security of the forwarded connection, not servers that,
while not actively compromised, just didn't care.  (I suppose my stance
is that you shouldn't be having your client attempt X forwarding when
connecting to a machine running a server whose X forwarding security
stance you don't know.)

> If the client expects a cookie in the ssh channel, it will reject
> unathorized connections.

This depends on the server doing something with the cookie like putting
it in a .Xauthority file, as opposed to just saving it and bashing it
into the authorization data fields of the X connection-open packet.
But doing that while not taking any other measures to secure the
connection is getting pretty close to the poin twhere I'd call it
actively malicious.

>> I'm implementing X forwarding as a private request which carries
>> only a uint32 cookie and a screen number; when the server wants to
>> forward an X connection, the channel open carries the cookie and the
>> first six bytes of the X protocol stream sent by the client (I could
>> have sent these as the first data on the new connection, but this
>> way seems marginally simpler).

Sure.  See below.

> [...] that only authorized processes can use the X forwarding.  And

To request X forwarding, the client sends

     byte      SSH_MSG_CHANNEL_REQUEST
     uint32    recipient channel
     string    "[email-masked]"
     boolean   want reply
     uint32    token
     uint32    x11 screen number

The server is, yes, expected to take appropriate steps (some form of
authentication at least as strong as MIT-MAGIC-COOKIE-1) to ensure that
only authorized processes use the connection.

There is no single-connection boolean; that struck me as unnecessary
complexity, and if the client wants those semantics, it can cancel the
forwarding (see below) and reject any extra CHANNEL_OPENs that arrive
because of the race or because the server rejects the cancel.

The token is entirely opaque to the (ssh) server; the only things the
server ever does with it are (a) send it back when an X connection is
opened and (b) compare it when cancelling X forwarding.

Upon getting a forwarded X connetion and its passing whatever security
checks, the server generates

     byte      SSH_MSG_CHANNEL_OPEN
     string    "[email-masked]"
     uint32    sender channel
     uint32    initial window size
     uint32    maximum packet size
     uint32    token from the corresponding CHANNEL_REQUEST
     byte[6]   first six bytes of the client X setup packet (the byte
                order and version numbers)
     string    connection method ("tcp" for TCP, others not yet
                specified - I'm imagining things like "decnet" and
                "local" for DECnet sockets and /tmp/.X11-unix/* - see
                below for more on this).
     connection-method-specific data
     for method "tcp":
         string    originator address (e.g. "192.168.7.38")
         uint32    originator port

The initial client->server setup packet is not carried as channel data;
the first six bytes of it are carried in the open request with the rest
being synthesized locally by the client.  In the Xserver->Xclient
direction, the channel carries pure X data right from the beginning of
the Xserver's response, but in the Xclient->Xserver direction, the
first channel data consists of the client's first request, if any.

In general, I don't like ssh's dependence on something TCP-like in the
protocol (for example, it assumes that the protocol has something like
port numbers that can fit in a uint32, and it thus is not possible to
run ssh, as defined, over DECnet stream sockets).  Since I didn't want
to redesign the whole protocol, I've lived with this in most respects.
But since I'm designing a new request for X, I wanted to fix at least
that much of it, especially since the connection in question is
entirely out of ssh's purview.  (This also addresses the question I saw
in the archives about "what originator info do I use for an AF_LOCAL X
connection?").

I have not actually implemented cancelling X forwarding.  Here's a
first stab at it:

     byte      SSH_MSG_CHANNEL_REQUEST
     uint32    recipient channel
     string    "[email-masked]"
     boolean   want reply
     uint32    token
     uint32    x11 screen number

This cancels all previous X forwardings for that channel, token, and
screen number.  I see no need for a way to wildcard any of these; if
others disagree, two additional bits (to indicate wildcarding of the
token or screen number) could be added, either as two booleans or as a
uint32 flags bitmask....

/~\ The ASCII				der Mouse
\ / Ribbon Campaign

---

From: Nicolas Williams <[email-masked]>
Date: Tue, 20 Apr 2004 00:21:59 -0500

>      string    "[email-masked]"

If all the server has to do is repeat the token then what difference
does it make?  Does it help the server keep track of forwarded displays?
Does it work like a generation number for when local display listeners
are re-used?

> because of the race or because the server rejects the cancel.

This makes sense, yes.

> opened and (b) compare it when cancelling X forwarding.

See above.

>                 order and version numbers)

What good does this byte[6] field do?  It doesn't really help the
client, since it's going to see those first 6 bytes in the channel data
stream anyways.  It can always forcefully close an x11 channel that it
did accept, if it doesn't like it.

>          string    originator address (e.g. "192.168.7.38")

What sorts of decisions do you expect a client to make based on this
information (which, after all, the client has to trust the server to be
honest about)?

I figure most clients will want to force local vs. non-local display
opens on the server, so why not just ask for this in the request
(e.g., through a boolean field)?

What about multiply-forwarded display?  How could the server know?  Does
it matter?

I think a "local-only" boolean in the client's x11 request ought to
suffice.  Or, to more closely match the tcpip-forward request, let the
client specify a display name pattern to use on the server side.

> first channel data consists of the client's first request, if any.

See above.

BTW, the terms "client" and "server" are very confusing here.  We need
to keep X11 display client/server and SSHv2 client/server terminology
straight.  SSHv2 servers here are X11 clients and SSHv2 clients are
proxies to X11 display servers.

> in the archives about "what originator info do I use for an AF_LOCAL X

See above.  I'd rather not have any of this in the channel open request.
I think all any client will end up caring about will be the security of
the forwarded display, not where connections to it come from but how.

> token or screen number) could be added, either as two booleans or as a

Can there be more than one X11 display forwarded per-session channel?  I
don't think so... but then, maybe that's just because in the *nix world
there's only one DISPLAY environment variable per-process pointing to
one display only So don't worry about wildcarding.

See above about the token.

Nico
--

---

From: der Mouse <[email-masked]>
Date: Tue, 20 Apr 2004 06:22:00 -0400 (EDT)

> does it make?

It allows the (ssh) client to tell which forwarding request a given
forwarded X connection corresponds to (and, in particular, allows
determining which session it goes with - this is cruicial for making X
forwarding work with connection sharing).

>> Upon getting a forwarded X connetion and its passing whatever
>> security checks, the server generates

> What good does this byte[6] field do?

It's just the place I chose to put that data.  I'm not fanatical about
putting it there; if it would make you feel better you could put it at
the beginning of the channel data, perhaps even with four zero bytes
and two arbitrary bytes appended so the channel data stream is valid X
protocol in isolation (I can't really see why this would matter, but I
infer some such desire from your stating that the (ssh) client will see
them in the channel data stream despite my trying to clearly state that
that data blob is _not_ present in the channel data stream).

> It doesn't really help the client, since it's going to see those
> first 6 bytes in the channel data stream anyways.

I don't understand.  I wrote text (later) in which I tried to make it
clear that that data _isn't_ in the channel data stream; you even
quoted it, so you apparently read it.  Was I not clear enough?  Or do
you just find it so horrible a thing to contemplate that you cannot
even admit the possibility?  Or what?

> It can always forcefully close an x11 channel that it did accept, if
> it doesn't like it.

I don't include it for the client to see whether it likes it, but
rather because the (ssh) client has to get those bytes _somehow_.

>>      string    connection method ([...]).
> information (which, after all, the client has to trust the server to
> be honest about)?

None in particular.  Why do you assume that making decisions based on
that information is the only reason to pass it?  I have no trouble
imagining the (ssh) client maintaining some kind of user-interface
display of forwarded X connections, in which case displaying where the
connection is from could be good.

> What about multiply-forwarded display?  How could the server know?
> Does it matter?

I sure hope it doesn't matter, because I don't think it can know, at
least not for the TCP case.

> BTW, the terms "client" and "server" are very confusing here.

Yes.  I've been trying to indicate which is which when I think
confusion is likely, though I've doubtless missed some cases.

>> [...wildcarding...]

> Can there be more than one X11 display forwarded per-session channel?

The (ssh) client certainly can send more than one X forwarding request
for a session.  What happens if it does is a good question, especially
for OSes like most Unices that communicate X display information to X
clients via something which (like $DISPLAY) can't handle more than one
display - I suspect it would have to be left up to the implementation.
(My own implementation, as it stands now, rejects all but the first; I
may, when I implement cancels, allow a forwarding request whenever
there are no uncancelled forwarding requests on that session.)

/~\ The ASCII				der Mouse
\ / Ribbon Campaign

---

From: [email-masked] (Niels Möller)
Date: 20 Apr 2004 13:43:08 +0200

> > Please write up a spec for this.

Some comments. Unfortunately, it's several months since I hacked on
the X11-forwarding, so I don't remember all the relevant details.

>      string    "[email-masked]"

Looks ok to me. I'm not sure about the single-connection thing, at
times I wished for a similar feature on the general tcpforwarding. You
could rename "token" to "display id", as it's meant to identify an
X11 display.

>          string    originator address (e.g. "192.168.7.38")

I think this is a little too complex for my taste. I think I
understand why you want to include the part of the initial X message
that comes before the authentication data, but it does make the
request more dependent on X protocol details than I really like. I
think I'd prefer something like

     string    prefix
     string    originator address
     uint32    originator port

  The prefix is the initial part of the X11 client's initial message,
  before the authentication data. It's usually 6 bytes, and the first
  byte is the byte-order mark, 'B' for a big-endian client, 'L' for a
  little endian client. (XXX Some X implementations seem to use 'b"
  and 'l' instead. I'm not sure if that is something we need to care
  about, or what a canonical X reference says about the case of the
  byte order characters).

As for the connection information, I don't quite like having the
structure of the request to depend on the connection type. If you
*really* want to support arbitrary connection information, I think I'd
prefer something like

  string    connection type  ("tcpip" or "[email-masked]")
  string    address blob     (internal structure depends on the type).

But we already have at least threee types of addresses mapped into a
string format (dns, ipv4, ipv6), so perhaps it's simpler to use just a
pair <string, number>, where the number part need not be used for
connection types where it isn't relevant.

I think it is unfortunate to introduce a new type of address
abstraction just for x11 forwarding; if the address abstraction in the
connection document isn't general enough, it's better to fix it there
(or in the architecture spec).

> screen number.

You should also say what should happen when a channel is closed. It
would make sense to me to say that a channel close on the session, or
a cancel-x11-request, should cause the ssh server to stop accepting
new x11 connections, but let existing forwarded channels live.

Finally, one alternative to using a special token to identify displays
is to just add the id of the associated session to the "x11"
CHANNEL_OPEN message,

     string    "x11-mk2"
!    uint32    session channel (receiver's id for the associated session)

This is equivalent to the new display id token, under the assumptions that

  No new x11 channels can be opened after the session channel is
  closed.

  A session can only have a single forwarded x11 display.

Both of these make sense to me.

Regards,
/Niels

---

From: der Mouse <[email-masked]>
Date: Tue, 20 Apr 2004 14:42:56 -0400 (EDT)

> You could rename "token" to "display id", as it's meant to identify
> an X11 display.

Calling it "display id" would, I suspect, confuse people into thinking
it's meant to be the display number (eg, the 2 in "hostname:2.0").

> I think I understand why you want to include the part of the initial
> X message that comes before the authentication data, but it does make
> the request more dependent on X protocol details than I really like.

You do have a good point there.  I hadn't really thought about it much;
even though I have trouble imagining this causing any problem (I don't
really think X is going to change that in the foreseeable future), I do
agree that the two protocols shouldn't be intertwined this much.

While it seems silly from an efficiency point of view, I'm now inclined
to remove that from the CHANNEL_OPEN entirely and put it in the channel
data stream, probably with zero-length authorization data (that is, the
ssh protocol as it appears on the wire then knows nothing about the X11
protocol - only the manipulation of the data stream at each end has any
knowledge of X).

> I think I'd prefer something like
>      string    prefix
>   before the authentication data.

Who says that in the revision that changes this, the part of interest
will remain a prefix?

>   It's usually 6 bytes, and the first byte is the byte-order mark,
>   'B' for a big-endian client, 'L' for a little endian client. (XXX
>   Some X implementations seem to use 'b" and 'l' instead. I'm not
>   sure if that is something we need to care about, or what a
>   canonical X reference says about the case of the byte order
>   characters).

The R4 protocol document says

     #x42      MSB first
     #x6C      LSB first

Those are ASCII 'B' and 'l'; while this is not coincidence, 'b' and 'L'
are invalid X protocol, same as 'q' and '&'.

> *really* want to support arbitrary connection information, I think
> I'd prefer something like

>   string    address blob     (internal structure depends on the type).

I suppose I can understand that.  I don't quite see what's wrong with
the way I did it, but it's certainly not a difference I'll make a fuss
over.

> string format (dns, ipv4, ipv6),

Not quite - those name (in three different ways) the machine, but not
the per-machine demultiplexing information (ports for TCP), which the
protocol shoehorns into a uint32.  DECnet, for example, uses strings
for the demultiplexer names, not small integers.

> so perhaps it's simpler to use just a pair <string, number>, where
> the number part need not be used for connection types where it isn't
> relevant.

But there's no standard way - as far as I know - to combine a DECnet
node name and demultiplexing string (I don't know the canonical DECnet
terminology for them) into a single string.

> abstraction just for x11 forwarding; if the address abstraction in
> the connection document isn't general enough, it's better to fix it
> there (or in the architecture spec).

True.  But fixing it everywhere is not practical for me alone, as it
would completely break interoperability.  I would certainly support an
effort to fix the whole protocol (I would be inclined to do it by
converting all the <host(string),port(uint32)> pairs into
<addresstype(string),info(string)> pairs, where the address type is
something like "tcp" and the string is something like (for tcp) the
<host(string),port(uint32)> pair we use now).

We already have this issue, when dealing with AF_LOCAL forwarded X
connections: the addressing scheme in use is insufficient to the task.

> You should also say what should happen when a channel is closed.

That's true.  I'm inclined to agree with you, that closing the session
implicitly cancels, but neither close nor explicit cancel breaks
existing forwadred channels.

> Finally, one alternative to using a special token to identify
> displays is to just add the id of the associated session to the "x11"
> CHANNEL_OPEN message,

Yes.  I considered that, briefly.  The principal reason I didn't do it
was practical: the way my implementation is layered, that's somewhat
inconvenient information to get at.

> Both of these make sense to me.

The former sounds more reasonable to me than the latter.  I'm rather
uncomfortable wiring the assumption that a session has at most one X
display into the protocol.  (I already have three programs that talk to
two X displays, and one that cna talk to an effectively unlimited
number...while at present all but at most one of those displays must be
named on the command line, it seems to me it would hamper innovation to
assume this will always be the case.)

/~\ The ASCII				der Mouse
\ / Ribbon Campaign

---

From: [email-masked] (Niels Möller)
Date: 20 Apr 2004 21:43:17 +0200

> protocol - only the manipulation of the data stream at each end has any

That is cleaner, at least from a narrow ssh protocol viewpoint. I
think it was something similar I was suggesting the last time X11
forwarding was discussed.

> are invalid X protocol, same as 'q' and '&'.

Thanks for the correction. RFC 1013 (UNKNOWN status) agrees with you.
I missed that detail when I read it a long time ago.

[ about representation of connection information ]

> terminology for them) into a single string.

I don't quite buy this argument.

There ought to be some character that could work as a separator?
Something (to allow the format to be extended to other non-ip
addressing schemes as well) like "decnet!address!demultiplexor". Or if
all else fails, use explicit embedded length
fields,"decnet!7:address13:demultiplexor". Both formats should work as
extensions of the current ssh conventions for representing addresses.
I'd be a little surprised if there's no "standard" way to represent
decnet endpoint addresses as strings, but I don't know decnet at all.

It's preferable to use a standard format, like an text string (in
ascii or utf8)) + an optional port number. This makes it easy for an
implementation to display and/or log the connection information for
any connection type, including types the implementation doesn't
understand.

> inconvenient information to get at.

Both ways seem ok to me. (And I think both should be as easy to
implement: using the session channel id to identify a forwarded
display is equivalent to using a separate token, and a token creation
function that by chance happens to always generate a token that equals
the channel id of the associated session channel). Using a separate
token gives the client (which chooses the values) a little more
freedom.

> display into the protocol.

I think I can agree with that, even if I don't think it matters much
in practice.

Regards,
/Niels

---

From: Nicolas Williams <[email-masked]>
Date: Tue, 20 Apr 2004 18:42:27 -0500

> forwarding work with connection sharing).

I thought the related channel ID ought to suffice.  

Damien points out a way in which multiple displays could be accessed
from a session channel, and later so do you, so ok, I'll grant the need
for some additional request matching beyond related channel ID.

[...]

You seem to have changed your mind since writing this, so I'll leave it
at this: the protocol is cleaner if this field is removed.

> connection is from could be good.

Ok.  I agree that it could be useful to have this info, but if it's
meant to be displayed and not interpreted by software, then we don't
need to worry about structuring the different transports'
address/instance data types and instead can just get away with a
free-form string.

> least not for the TCP case.

Right.

> confusion is likely, though I've doubtless missed some cases.

You've been doing ok, but other readers probably could use that
clarification.

> there are no uncancelled forwarding requests on that session.)

Ok.

There is a caveat here that has to be mentioned in any I-D text about
this: for many SSHv2 servers the x11 fwd channel request has to arrive
prior to starting the session, so clients SHOULD strive to ensure this.

Nico
--