Patch to add "warn" value to ForwardX11 and ForwardAgent

[Dave Dykstra]

Because ForwardX11 and ForwardAgent are so useful but introduce risk when
used to a not well-secured server, I added a "warn" value to the ForwardX11
and ForwardAgent options which causes the ssh client to print a big warning
whenever the forwarding is actually used.  I plan to make "ForwardX11=warn"
the default in my ssh_config distribution.

I'm not proposing that this patch go into 3.0, but hopefully it will go
into the next release.  If the patch is accepted, the team might want to
consider whether -X and -A should set their corresponding option to "warn"
rather than "yes".

This patch is for OpenSSH native.  I tested it on portable, but it applies
cleanly to native.

Damien said that native patch submissions are supposed to go into OpenBSD
GNATS but I don't find any references to GNATS on www.openbsd.com or
www.openssh.com.  How do I submit a patch to it?

- Dave Dykstra


--- readconf.c.O	Fri Oct 26 10:45:15 2001
+++ readconf.c	Fri Oct 26 11:42:22 2001
@@ -296,28 +296,44 @@
 		/* NOTREACHED */
 	case oForwardAgent:
 		intptr = &options->forward_agent;
-parse_flag:
+parse_yesnowarn:
 		arg = strdelim(&s);
 		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing yes/no argument.", filename, linenum);
+			fatal("%.200s line %d: Missing yes/no/warn argument.",
+			      filename, linenum);
 		value = 0;	/* To avoid compiler warning... */
 		if (strcmp(arg, "yes") == 0 || strcmp(arg, "true") == 0)
 			value = 1;
 		else if (strcmp(arg, "no") == 0 || strcmp(arg, "false") == 0)
 			value = 0;
+		else if (strcmp(arg, "warn") == 0)
+			value = 2;
 		else
-			fatal("%.200s line %d: Bad yes/no argument.", filename, linenum);
+			fatal("%.200s line %d: Bad yes/no/warn argument.", filename, linenum);
 		if (*activep && *intptr == -1)
 			*intptr = value;
 		break;
 
 	case oForwardX11:
 		intptr = &options->forward_x11;
-		goto parse_flag;
+		goto parse_yesnowarn;
 
 	case oGatewayPorts:
 		intptr = &options->gateway_ports;
-		goto parse_flag;
+parse_flag:
+		arg = strdelim(&s);
+		if (!arg || *arg == '\0')
+			fatal("%.200s line %d: Missing yes/no argument.", filename, linenum);
+		value = 0;	/* To avoid compiler warning... */
+		if (strcmp(arg, "yes") == 0 || strcmp(arg, "true") == 0)
+			value = 1;
+		else if (strcmp(arg, "no") == 0 || strcmp(arg, "false") == 0)
+			value = 0;
+		else
+			fatal("%.200s line %d: Bad yes/no argument.", filename, linenum);
+		if (*activep && *intptr == -1)
+			*intptr = value;
+		break;
 
 	case oUsePrivilegedPort:
 		intptr = &options->use_privileged_port;
--- clientloop.c.O	Fri Oct 26 11:47:19 2001
+++ clientloop.c	Fri Oct 26 13:32:26 2001
@@ -1234,6 +1234,40 @@
 	}
 	xfree(rtype);
 }
+static void
+client_input_agent_open(int type, int plen, void *ctxt)
+{
+	if (!options.forward_agent) {
+		deny_input_open(type, plen, ctxt);
+		return;
+	}
+	if (options.forward_agent == 2) {
+		error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
+		error("@  ssh WARNING: received agent open request from the server.         @");
+		error("@  If you did not initiate it, you are probably under attack.        @");
+		error("@  To eliminate these warnings, set the ForwardAgent option to yes,  @");
+		error("@  but note that that is risky if the server is not well-secured.    @");
+		error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
+	}
+	auth_input_open_request(type, plen, ctxt);
+}
+static void
+client_input_x11_open(int type, int plen, void *ctxt)
+{
+	if (!options.forward_x11) {
+		deny_input_open(type, plen, ctxt);
+		return;
+	}
+	if (options.forward_x11 == 2) {
+		error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
+		error("@  ssh WARNING: received X11 open request from the server.           @");
+		error("@  If you did not initiate it, you are probably under attack.        @");
+		error("@  To eliminate these warnings, set the ForwardX11 option to yes,    @");
+		error("@  but note that that is risky if the server is not well-secured.    @");
+		error("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
+	}
+	x11_input_open(type, plen, ctxt);
+}
 
 static void
 client_init_dispatch_20(void)
@@ -1265,11 +1299,8 @@
 	dispatch_set(SSH_SMSG_EXITSTATUS, &client_input_exit_status);
 	dispatch_set(SSH_SMSG_STDERR_DATA, &client_input_stderr_data);
 	dispatch_set(SSH_SMSG_STDOUT_DATA, &client_input_stdout_data);
-
-	dispatch_set(SSH_SMSG_AGENT_OPEN, options.forward_agent ?
-	    &auth_input_open_request : &deny_input_open);
-	dispatch_set(SSH_SMSG_X11_OPEN, options.forward_x11 ?
-	    &x11_input_open : &deny_input_open);
+	dispatch_set(SSH_SMSG_AGENT_OPEN, &client_input_agent_open);
+	dispatch_set(SSH_SMSG_X11_OPEN, &client_input_x11_open);
 }
 static void
 client_init_dispatch_15(void)
--- ssh.1.O	Fri Oct 26 12:56:10 2001
+++ ssh.1	Fri Oct 26 13:12:17 2001
@@ -308,6 +308,8 @@
 .Cm ForwardX11
 variable is set to
 .Dq yes
+or
+.Dq warn
 (or, see the description of the
 .Fl X
 and
@@ -846,9 +848,18 @@
 Specifies whether the connection to the authentication agent (if any)
 will be forwarded to the remote machine.
 The argument must be
-.Dq yes
+.Dq yes ,
+.Dq no ,
 or
-.Dq no .
+.Dq warn .
+If it is set to
+.Dq warn ,
+a warning is printed every time an agent authentication is requested; this
+is highly recommended if the server is not well-secured because an agent
+authentication allows an attacker to log in to any other server that has
+one of the agent's keys in an
+.Pa authorized_keys
+file.
 The default is
 .Dq no .
 .It Cm ForwardX11
@@ -857,9 +868,15 @@
 .Ev DISPLAY
 set.
 The argument must be
-.Dq yes
+.Dq yes ,
+.Dq no ,
 or
-.Dq no .
+.Dq warn .
+If it is set to
+.Dq warn ,
+a warning is printed every time an X11 connection is forwarded; this is
+highly recommended if the server is not well-secured because an X11
+connection can read and write anything on the user's X11 display.
 The default is
 .Dq no .
 .It Cm GatewayPorts