Re: Re: [PATCH] allow indefinite ForwardX11Timeout by setting it to 0

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

 



On Thu, 28 Jun 2018, Ian Sutton wrote:

> > Bump, ccing djm@xxxxxxxxxxx as annotate indicates they committed most of the
> > code near these changes.
> >
> > If bumping patches is discouraged please let me know--I don't mean to be
> > rude but would like to have an r+ or r- for this small changeset.
> 
> This is reasonable and I'd like to see it in the tree, if someone gets
> a chance to review soon. Thanks

Sorry for being slow - I think that patch has one problem related to
setting x11_refuse_time (it would clobber an existing one) and IMO it
would be better to not rely on "timeout 0" on the xauth commandline
disabling the timeout, as that behaviour is not documented.

diff --git a/clientloop.c b/clientloop.c
index 68cc94b..bcd98bf 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -271,7 +271,7 @@ client_x11_get_proto(struct ssh *ssh, const char *display,
     const char *xauth_path, u_int trusted, u_int timeout,
     char **_proto, char **_data)
 {
-	char cmd[1024], line[512], xdisplay[512];
+	char *cmd, line[512], xdisplay[512];
 	char xauthfile[PATH_MAX], xauthdir[PATH_MAX];
 	static char proto[512], data[512];
 	FILE *f;
@@ -335,19 +335,31 @@ client_x11_get_proto(struct ssh *ssh, const char *display,
 				return -1;
 			}
 
-			if (timeout >= UINT_MAX - X11_TIMEOUT_SLACK)
-				x11_timeout_real = UINT_MAX;
-			else
-				x11_timeout_real = timeout + X11_TIMEOUT_SLACK;
-			if ((r = snprintf(cmd, sizeof(cmd),
-			    "%s -f %s generate %s " SSH_X11_PROTO
-			    " untrusted timeout %u 2>" _PATH_DEVNULL,
-			    xauth_path, xauthfile, display,
-			    x11_timeout_real)) < 0 ||
-			    (size_t)r >= sizeof(cmd))
-				fatal("%s: cmd too long", __func__);
+			if (timeout == 0) {
+				/* auth doesn't time out */
+				xasprintf(&cmd, "%s -f %s generate %s %s "
+				    "untrusted 2>%s",
+				    xauth_path, xauthfile, display,
+				    SSH_X11_PROTO, _PATH_DEVNULL);
+			} else {
+				/* Add some slack to requested expiry */
+				if (timeout < UINT_MAX - X11_TIMEOUT_SLACK)
+					x11_timeout_real = timeout +
+					    X11_TIMEOUT_SLACK;
+				else {
+					/* Don't overflow on long timeouts */
+					x11_timeout_real = UINT_MAX;
+				}
+				
+				xasprintf(&cmd, "%s -f %s generate %s %s "
+				    "untrusted timeout %u 2>%s",
+				    xauth_path, xauthfile, display,
+				    SSH_X11_PROTO, x11_timeout_real,
+				    _PATH_DEVNULL);
+			}
 			debug2("%s: %s", __func__, cmd);
-			if (x11_refuse_time == 0) {
+
+			if (timeout != 0 && x11_refuse_time == 0) {
 				now = monotime() + 1;
 				if (UINT_MAX - timeout < now)
 					x11_refuse_time = UINT_MAX;
@@ -358,6 +370,7 @@ client_x11_get_proto(struct ssh *ssh, const char *display,
 			}
 			if (system(cmd) == 0)
 				generated = 1;
+			free(cmd);
 		}
 
 		/*
@@ -366,7 +379,7 @@ client_x11_get_proto(struct ssh *ssh, const char *display,
 		 * above.
 		 */
 		if (trusted || generated) {
-			snprintf(cmd, sizeof(cmd),
+			xasprintf(&cmd,
 			    "%s %s%s list %s 2>" _PATH_DEVNULL,
 			    xauth_path,
 			    generated ? "-f " : "" ,
@@ -379,6 +392,7 @@ client_x11_get_proto(struct ssh *ssh, const char *display,
 				got_data = 1;
 			if (f)
 				pclose(f);
+			free(cmd);
 		}
 	}
 

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev



[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux