On 06/01/14 22:51, Peter Hutterer wrote:
On Sun, Jan 05, 2014 at 07:34:23PM -0500, Mouse wrote:
[...the security extension...] A lot of people use X remotely with
ssh which is unsafe without something like this.
What's unsafe about that? I don't see anything offhand, but I haven't
thought about it for long enough for that to mean much.
ssh merely encrypts the connections between the two hosts, but the usual
issues with X itself still apply (every client has access to every other
client, more-or-less).
Cheers,
Peter
That's not (completely) correct.
In openssh, ssh -X gives you an untrusted client, and you need ssh -Y in
order to get a X11 client with access to the other, local, clients.
What I would like to see are mutually untrusted clients, so they can't
snoop each other and eventually you could have an environment with every
application ‘untrusted’.
The changes to Xext/security.c seem minor (see below) and for the common
case where the X11 client connects to the server using a local socket,
the pid of the process which opened the socket (usually available in
some OS-specific way) seem a good identifier (would need to take care on
pid reuse). Another option would be to assign the id from a counter.
The piece which is more delicate in my opinion is defining the
authorization protocol appropiately. The pid could be stored there,
there could be a magic value making the X server untrust the client and
fetch the id itself…
What do you think?
diff --git a/Xext/securitysrv.h b/Xext/securitysrv.h
index 8904242..d007f19 100644
--- a/Xext/securitysrv.h
+++ b/Xext/securitysrv.h
@@ -63,7 +63,7 @@ extern RESTYPE SecurityAuthorizationResType;
typedef struct {
XID id; /* resource ID */
CARD32 timeout; /* how long to live in seconds after
refcnt == 0 */
- unsigned int trustLevel; /* trusted/untrusted */
+ pid_t trustLevel; /* trusted/untrusted */
XID group; /* see embedding extension */
unsigned int refcnt; /* how many clients connected with
this auth */
unsigned int secondsRemaining; /* overflow time amount for
>49 days */
diff --git a/Xext/security.c b/Xext/security.c
index 7bf6cc4..80a2e3c 100644
--- a/Xext/security.c
+++ b/Xext/security.c
@@ -59,7 +59,7 @@ static DevPrivateKeyRec stateKeyRec;
typedef struct {
unsigned int haveState :1;
unsigned int live :1;
- unsigned int trustLevel :2;
+ pid_t trustLevel;
XID authId;
} SecurityStateRec;
@@ -122,7 +122,7 @@ SecurityDoCheck(SecurityStateRec * subj,
SecurityStateRec * obj,
return Success;
if (subj->trustLevel == XSecurityClientTrusted)
return Success;
- if (obj->trustLevel != XSecurityClientTrusted)
+ if (obj->trustLevel == subj->trustLevel)
return Success;
if ((requested | allowed) == allowed)
return Success;
@@ -401,7 +401,7 @@ ProcSecurityGenerateAuthorization(ClientPtr client)
int err; /* error to return from this function */
XID authId; /* authorization ID assigned by os
layer */
xSecurityGenerateAuthorizationReply rep; /* reply struct */
- unsigned int trustLevel; /* trust level of new auth */
+ pid_t trustLevel; /* trust level of new auth */
XID group; /* group of new auth */
CARD32 timeout; /* timeout of new auth */
CARD32 *values; /* list of supplied attributes */
@@ -438,6 +438,7 @@ ProcSecurityGenerateAuthorization(ClientPtr client)
trustLevel = XSecurityClientUntrusted;
if (stuff->valueMask & XSecurityTrustLevel) {
trustLevel = *values++;
+ /* Store the pid here */
if (trustLevel != XSecurityClientTrusted &&
trustLevel != XSecurityClientUntrusted) {
client->errorValue = trustLevel;
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
