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