Dave Dykstra
2001-Oct-26 21:11 UTC
Patch to add "warn" value to ForwardX11 and ForwardAgent
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
Markus Friedl
2001-Oct-26 22:09 UTC
Patch to add "warn" value to ForwardX11 and ForwardAgent
On Fri, Oct 26, 2001 at 04:11:30PM -0500, Dave Dykstra wrote:> 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.why is this better then having ForwardX11=no and using -X to enable forwarding on a 'need' basis? -m
Circa 2001-Oct-30 12:03:29 +1100 dixit Damien Miller: : On Mon, 29 Oct 2001, Dave Dykstra wrote: : > With "warn", you can still use forwarding even though you know a server is : > not very well secured, and you will be notified if someone does actually : > break into the server and try to take over the forwarding on your session. : : By which stage it is too late. : : What would be nicer is some way for the client to get the user to accept : / reject each forwarding request. http://c.home.cern.ch/c/cons/www/mxconns/ -- jim knoble | jmknoble at pobox.com | http://www.pobox.com/~jmknoble/ (GnuPG fingerprint: 31C4:8AAC:F24E:A70C:4000::BBF4:289F:EAA8:1381:1491) -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 262 bytes Desc: not available Url : http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20011029/72d9f8c0/attachment.bin