- markus@cvs.openbsd.org 2003/10/11 08:24:08

[readconf.c readconf.h ssh.1 ssh.c ssh_config.5] remote x11 clients are now untrusted by default, uses xauth(8) to generate untrusted cookies; ForwardX11Trusted=yes restores old behaviour. ok deraadt; feedback and ok djm/fries
2003-10-15 15:54:32 +10:00 · 2003-10-15 15:54:32 +10:00 · 0a118da00e
parent a044f47679
commit 0a118da00e
6 changed files with 112 additions and 33 deletions
--- a/7
+++ b/7
@ -11,6 +11,11 @@
   - markus@cvs.openbsd.org 2003/10/08 15:21:24
     [readconf.c ssh_config.5]
     default GSS API to no in client, too; ok jakob, deraadt@
+   - markus@cvs.openbsd.org 2003/10/11 08:24:08
+     [readconf.c readconf.h ssh.1 ssh.c ssh_config.5]
+     remote x11 clients are now untrusted by default, uses xauth(8) to generate
+     untrusted cookies; ForwardX11Trusted=yes restores old behaviour.
+     ok deraadt; feedback and ok djm/fries

 20031009
 - (dtucker) [sshd_config.5] UsePAM defaults to "no".  ok djm@
@ -1328,4 +1333,4 @@
 - Fix sshd BindAddress and -b options for systems using fake-getaddrinfo.
   Report from murple@murple.net, diagnosis from dtucker@zip.com.au

-$Id: ChangeLog,v 1.3071 2003/10/15 05:52:03 dtucker Exp $
+$Id: ChangeLog,v 1.3072 2003/10/15 05:54:32 dtucker Exp $
--- a/readconf.c
+++ b/readconf.c
@ -12,7 +12,7 @@
 */

 #include "includes.h"
-RCSID("$OpenBSD: readconf.c,v 1.122 2003/10/08 15:21:24 markus Exp $");
+RCSID("$OpenBSD: readconf.c,v 1.123 2003/10/11 08:24:07 markus Exp $");

 #include "ssh.h"
 #include "xmalloc.h"
@ -89,7 +89,7 @@ RCSID("$OpenBSD: readconf.c,v 1.122 2003/10/08 15:21:24 markus Exp $");

 typedef enum {
 	oBadOption,
-	oForwardAgent, oForwardX11, oGatewayPorts,
+	oForwardAgent, oForwardX11, oForwardX11Trusted, oGatewayPorts,
 	oPasswordAuthentication, oRSAAuthentication,
 	oChallengeResponseAuthentication, oXAuthLocation,
 	oIdentityFile, oHostName, oPort, oCipher, oRemoteForward, oLocalForward,
@ -116,6 +116,7 @@ static struct {
 } keywords[] = {
 	{ "forwardagent", oForwardAgent },
 	{ "forwardx11", oForwardX11 },
+	{ "forwardx11trusted", oForwardX11Trusted },
 	{ "xauthlocation", oXAuthLocation },
 	{ "gatewayports", oGatewayPorts },
 	{ "useprivilegedport", oUsePrivilegedPort },
@ -342,6 +343,10 @@ parse_flag:
 		intptr = &options->forward_x11;
 		goto parse_flag;

+	case oForwardX11Trusted:
+		intptr = &options->forward_x11_trusted;
+		goto parse_flag;
+
 	case oGatewayPorts:
 		intptr = &options->gateway_ports;
 		goto parse_flag;
@ -806,6 +811,7 @@ initialize_options(Options * options)
 	memset(options, 'X', sizeof(*options));
 	options->forward_agent = -1;
 	options->forward_x11 = -1;
+	options->forward_x11_trusted = -1;
 	options->xauth_location = NULL;
 	options->gateway_ports = -1;
 	options->use_privileged_port = -1;
@ -872,6 +878,8 @@ fill_default_options(Options * options)
 		options->forward_agent = 0;
 	if (options->forward_x11 == -1)
 		options->forward_x11 = 0;
+	if (options->forward_x11_trusted == -1)
+		options->forward_x11_trusted = 0;
 	if (options->xauth_location == NULL)
 		options->xauth_location = _PATH_XAUTH;
 	if (options->gateway_ports == -1)
--- a/readconf.h
+++ b/readconf.h
@ -1,4 +1,4 @@
-/*	$OpenBSD: readconf.h,v 1.55 2003/09/01 18:15:50 markus Exp $	*/
+/*	$OpenBSD: readconf.h,v 1.56 2003/10/11 08:24:08 markus Exp $	*/

 /*
 * Author: Tatu Ylonen <ylo@cs.hut.fi>
@ -30,6 +30,7 @@ typedef struct {
 typedef struct {
 	int     forward_agent;	/* Forward authentication agent. */
 	int     forward_x11;	/* Forward X11 display. */
+	int     forward_x11_trusted;	/* Trust Forward X11 display. */
 	char   *xauth_location;	/* Location for xauth program */
 	int     gateway_ports;	/* Allow remote connects to forwarded ports. */
 	int     use_privileged_port;	/* Don't use privileged port if false. */
--- a/ssh.1
+++ b/ssh.1
@ -34,7 +34,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.\" $OpenBSD: ssh.1,v 1.177 2003/10/08 08:27:36 jmc Exp $
+.\" $OpenBSD: ssh.1,v 1.178 2003/10/11 08:24:08 markus Exp $
 .Dd September 25, 1999
 .Dt SSH 1
 .Os
@ -43,7 +43,7 @@
 .Nd OpenSSH SSH client (remote login program)
 .Sh SYNOPSIS
 .Nm ssh
-.Op Fl 1246AaCfgkNnqsTtVvXx
+.Op Fl 1246AaCfgkNnqsTtVvXxY
 .Op Fl b Ar bind_address
 .Op Fl c Ar cipher_spec
 .Op Fl D Ar port
@ -624,6 +624,7 @@ For full details of the options listed below, and their possible values, see
 .It EscapeChar
 .It ForwardAgent
 .It ForwardX11
+.It ForwardX11Trusted
 .It GatewayPorts
 .It GlobalKnownHostsFile
 .It GSSAPIAuthentication
@ -732,6 +733,8 @@ can access the local X11 display through the forwarded connection.
 An attacker may then be able to perform activities such as keystroke monitoring.
 .It Fl x
 Disables X11 forwarding.
+.It Fl Y
+Enables trusted X11 forwarding.
 .El
 .Sh CONFIGURATION FILES
 .Nm
--- a/ssh.c
+++ b/ssh.c
@ -13,7 +13,7 @@
 * called by a name other than "ssh" or "Secure Shell".
 *
 * Copyright (c) 1999 Niels Provos.  All rights reserved.
- * Copyright (c) 2000, 2001, 2002 Markus Friedl.  All rights reserved.
+ * Copyright (c) 2000, 2001, 2002, 2003 Markus Friedl.  All rights reserved.
 *
 * Modified to work with SSL by Niels Provos <provos@citi.umich.edu>
 * in Canada (German citizen).
@ -40,7 +40,7 @@
 */

 #include "includes.h"
-RCSID("$OpenBSD: ssh.c,v 1.201 2003/09/01 18:15:50 markus Exp $");
+RCSID("$OpenBSD: ssh.c,v 1.202 2003/10/11 08:24:08 markus Exp $");

 #include <openssl/evp.h>
 #include <openssl/err.h>
@ -155,6 +155,7 @@ usage(void)
 	fprintf(stderr, "  -A          Enable authentication agent forwarding.\n");
 	fprintf(stderr, "  -a          Disable authentication agent forwarding (default).\n");
 	fprintf(stderr, "  -X          Enable X11 connection forwarding.\n");
+	fprintf(stderr, "  -Y          Enable trusted X11 connection forwarding.\n");
 	fprintf(stderr, "  -x          Disable X11 connection forwarding (default).\n");
 	fprintf(stderr, "  -i file     Identity for public key authentication "
 	    "(default: ~/.ssh/identity)\n");
@ -264,7 +265,7 @@ main(int ac, char **av)

 again:
 	while ((opt = getopt(ac, av,
-	    "1246ab:c:e:fgi:kl:m:no:p:qstvxACD:F:I:L:NPR:TVX")) != -1) {
+	    "1246ab:c:e:fgi:kl:m:no:p:qstvxACD:F:I:L:NPR:TVXY")) != -1) {
 		switch (opt) {
 		case '1':
 			options.protocol = SSH_PROTO_1;
@ -291,6 +292,10 @@ again:
 		case 'X':
 			options.forward_x11 = 1;
 			break;
+		case 'Y':
+			options.forward_x11 = 1;
+			options.forward_x11_trusted = 1;
+			break;
 		case 'g':
 			options.gateway_ports = 1;
 			break;
@ -721,19 +726,25 @@ again:
 	return exit_status;
 }

+#define SSH_X11_PROTO "MIT-MAGIC-COOKIE-1"
+
 static void
 x11_get_proto(char **_proto, char **_data)
 {
+	char cmd[1024];
 	char line[512];
+	char xdisplay[512];
 	static char proto[512], data[512];
 	FILE *f;
-	int got_data = 0, i;
-	char *display;
+	int got_data = 0, generated = 0, do_unlink = 0, i;
+	char *display, *xauthdir, *xauthfile;
 	struct stat st;

+	xauthdir = xauthfile = NULL;
 	*_proto = proto;
 	*_data = data;
 	proto[0] = data[0] = '\0';
+
 	if (!options.xauth_location ||
 	    (stat(options.xauth_location, &st) == -1)) {
 		debug("No xauth program.");
@ -742,28 +753,59 @@ x11_get_proto(char **_proto, char **_data)
 			debug("x11_get_proto: DISPLAY not set");
 			return;
 		}
-		/* Try to get Xauthority information for the display. */
-		if (strncmp(display, "localhost:", 10) == 0)
-			/*
-			 * Handle FamilyLocal case where $DISPLAY does
-			 * not match an authorization entry.  For this we
-			 * just try "xauth list unix:displaynum.screennum".
-			 * XXX: "localhost" match to determine FamilyLocal
-			 *      is not perfect.
-			 */
-			snprintf(line, sizeof line, "%s list unix:%s 2>"
-			    _PATH_DEVNULL, options.xauth_location, display+10);
-		else
-			snprintf(line, sizeof line, "%s list %.200s 2>"
-			    _PATH_DEVNULL, options.xauth_location, display);
-		debug2("x11_get_proto: %s", line);
-		f = popen(line, "r");
+		/*
+		 * Handle FamilyLocal case where $DISPLAY does
+		 * not match an authorization entry.  For this we
+		 * just try "xauth list unix:displaynum.screennum".
+		 * XXX: "localhost" match to determine FamilyLocal
+		 *      is not perfect.
+		 */
+		if (strncmp(display, "localhost:", 10) == 0) {
+			snprintf(xdisplay, sizeof(xdisplay), "unix:%s",
+			    display + 10);
+			display = xdisplay;
+		}
+		if (options.forward_x11_trusted == 0) {
+			xauthdir = xmalloc(MAXPATHLEN);
+			xauthfile = xmalloc(MAXPATHLEN);
+			strlcpy(xauthdir, "/tmp/ssh-XXXXXXXXXX", MAXPATHLEN);
+			if (mkdtemp(xauthdir) != NULL) {
+				do_unlink = 1;
+				snprintf(xauthfile, MAXPATHLEN, "%s/xauthfile",
+				    xauthdir);
+				snprintf(cmd, sizeof(cmd),
+				    "%s -f %s generate %s " SSH_X11_PROTO
+				    " untrusted timeout 120 2>" _PATH_DEVNULL,
+				    options.xauth_location, xauthfile, display);
+				debug2("x11_get_proto: %s", cmd);
+				if (system(cmd) == 0)
+					generated = 1;
+			}
+		}
+		snprintf(cmd, sizeof(cmd),
+		    "%s %s%s list %s . 2>" _PATH_DEVNULL,
+		    options.xauth_location,
+		    generated ? "-f " : "" ,
+		    generated ? xauthfile : "",
+		    display);
+		debug2("x11_get_proto: %s", cmd);
+		f = popen(cmd, "r");
 		if (f && fgets(line, sizeof(line), f) &&
 		    sscanf(line, "%*s %511s %511s", proto, data) == 2)
 			got_data = 1;
 		if (f)
 			pclose(f);
 	}
+
+	if (do_unlink) {
+		unlink(xauthfile);
+		rmdir(xauthdir);
+	}
+	if (xauthdir)
+		xfree(xauthdir);
+	if (xauthfile)
+		xfree(xauthfile);
+
 	/*
 	 * If we didn't get authentication data, just make up some
 	 * data.  The forwarding code will check the validity of the
@ -775,12 +817,14 @@ x11_get_proto(char **_proto, char **_data)
 	if (!got_data) {
 		u_int32_t rand = 0;

-		logit("Warning: No xauth data; using fake authentication data for X11 forwarding.");
-		strlcpy(proto, "MIT-MAGIC-COOKIE-1", sizeof proto);
+		logit("Warning: No xauth data; "
+		    "using fake authentication data for X11 forwarding.");
+		strlcpy(proto, SSH_X11_PROTO, sizeof proto);
 		for (i = 0; i < 16; i++) {
 			if (i % 4 == 0)
 				rand = arc4random();
-			snprintf(data + 2 * i, sizeof data - 2 * i, "%02x", rand & 0xff);
+			snprintf(data + 2 * i, sizeof data - 2 * i, "%02x",
+			    rand & 0xff);
 			rand >>= 8;
 		}
 	}
--- a/ssh_config.5
+++ b/ssh_config.5
@ -34,7 +34,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.\" $OpenBSD: ssh_config.5,v 1.21 2003/10/08 15:21:24 markus Exp $
+.\" $OpenBSD: ssh_config.5,v 1.22 2003/10/11 08:24:08 markus Exp $
 .Dd September 25, 1999
 .Dt SSH_CONFIG 5
 .Os
@ -306,9 +306,27 @@ The default is
 .Pp
 X11 forwarding should be enabled with caution.
 Users with the ability to bypass file permissions on the remote host
-(for the user's X authorization database)
+(for the user's X11 authorization database)
 can access the local X11 display through the forwarded connection.
-An attacker may then be able to perform activities such as keystroke monitoring.
+An attacker may then be able to perform activities such as keystroke monitoring
+if the
+.Cm ForwardX11Trusted
+option is also enabled.
+.It Cm ForwardX11Trusted
+If the this option is set to
+.Dq yes
+then remote X11 clients will have full access to the original X11 display.
+If this option is set to
+.Dq no
+then remote X11 clients will be considered untrusted and prevented
+from stealing or tampering with data belonging to trusted X11
+clients.
+.Pp
+The default is
+.Dq no .
+.Pp
+See the X11 SECURITY extension specification for full details on
+the restrictions imposed on untrusted clients.
 .It Cm GatewayPorts
 Specifies whether remote hosts are allowed to connect to local
 forwarded ports.