File openssh-6.6p1-untrusted_X_forwarding.patch of Package openssh.10219
# HG changeset patch
# Parent 6c50ec85ae480cf2aa8ebce8c9f4ff17265dde6e
Prevent X clients from circumventing X SECURITY restrictions by falling back
to implicit authentication upon MIT-MAGIC-COOKIE authentication fail by
disallowing X11 authentication after ForwardX11Timeout.
CVE-2016-1908
bsc#962313
c.f. https://thejh.net/written-stuff/openssh-6.8-xsecurity
Backport of upstream:
commit f98a09cacff7baad8748c9aa217afd155a4d493f
Author: mmcc@openbsd.org <mmcc@openbsd.org>
Date: Tue Oct 20 03:36:35 2015 +0000
Replace a function-local allocation with stack memory.
ok djm@
Upstream-ID: c09fbbab637053a2ab9f33ca142b4e20a4c5a17e
commit ed4ce82dbfa8a3a3c8ea6fa0db113c71e234416c
Author: djm@openbsd.org <djm@openbsd.org>
Date: Wed Jan 13 23:04:47 2016 +0000
upstream commit
eliminate fallback from untrusted X11 forwarding to trusted
forwarding when the X server disables the SECURITY extension; Reported by
Thomas Hoger; ok deraadt@
Upstream-ID: f76195bd2064615a63ef9674a0e4096b0713f938
diff --git a/openssh-6.6p1/clientloop.c b/openssh-6.6p1/clientloop.c
--- a/openssh-6.6p1/clientloop.c
+++ b/openssh-6.6p1/clientloop.c
@@ -286,105 +286,121 @@ set_control_persist_exit_time(void)
}
#define SSH_X11_VALID_DISPLAY_CHARS ":/.-_"
static int
client_x11_display_valid(const char *display)
{
size_t i, dlen;
+ if (display == NULL)
+ return 0;
+
dlen = strlen(display);
for (i = 0; i < dlen; i++) {
if (!isalnum((u_char)display[i]) &&
strchr(SSH_X11_VALID_DISPLAY_CHARS, display[i]) == NULL) {
debug("Invalid character '%c' in DISPLAY", display[i]);
return 0;
}
}
return 1;
}
#define SSH_X11_PROTO "MIT-MAGIC-COOKIE-1"
#define X11_TIMEOUT_SLACK 60
-void
+int
client_x11_get_proto(const char *display, const char *xauth_path,
u_int trusted, u_int timeout, char **_proto, char **_data)
{
- char cmd[1024];
- char line[512];
- char xdisplay[512];
+ char cmd[1024], line[512], xdisplay[512];
+ char xauthfile[PATH_MAX], xauthdir[PATH_MAX];
static char proto[512], data[512];
FILE *f;
- int got_data = 0, generated = 0, do_unlink = 0, i;
- char *xauthdir, *xauthfile;
+ int got_data = 0, generated = 0, do_unlink = 0, i, r;
struct stat st;
u_int now, x11_timeout_real;
- xauthdir = xauthfile = NULL;
*_proto = proto;
*_data = data;
- proto[0] = data[0] = '\0';
+ proto[0] = data[0] = xauthfile[0] = xauthdir[0] = '\0';
- if (xauth_path == NULL ||(stat(xauth_path, &st) == -1)) {
+ if (!client_x11_display_valid(display)) {
+ logit("DISPLAY \"%s\" invalid; disabling X11 forwarding",
+ display);
+ return -1;
+ }
+ if (xauth_path != NULL && stat(xauth_path, &st) == -1) {
debug("No xauth program.");
- } else if (!client_x11_display_valid(display)) {
- logit("DISPLAY '%s' invalid, falling back to fake xauth data",
- display);
- } else {
- if (display == NULL) {
- debug("x11_get_proto: DISPLAY not set");
- return;
- }
+ xauth_path = NULL;
+ }
+
+ if (xauth_path != NULL) {
/*
* 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);
+ if ((r = snprintf(xdisplay, sizeof(xdisplay), "unix:%s",
+ display + 10)) < 0 ||
+ (size_t)r >= sizeof(xdisplay)) {
+ error("%s: display name too long", __func__);
+ return -1;
+ }
display = xdisplay;
}
if (trusted == 0) {
- xauthdir = xmalloc(MAXPATHLEN);
- xauthfile = xmalloc(MAXPATHLEN);
- mktemp_proto(xauthdir, MAXPATHLEN);
/*
+ * Generate an untrusted X11 auth cookie.
+ *
* The authentication cookie should briefly outlive
* ssh's willingness to forward X11 connections to
* avoid nasty fail-open behaviour in the X server.
*/
+ mktemp_proto(xauthdir, sizeof(xauthdir));
+ if (mkdtemp(xauthdir) == NULL) {
+ error("%s: mkdtemp: %s",
+ __func__, strerror(errno));
+ return -1;
+ }
+ do_unlink = 1;
+ if ((r = snprintf(xauthfile, sizeof(xauthfile),
+ "%s/xauthfile", xauthdir)) < 0 ||
+ (size_t)r >= sizeof(xauthfile)) {
+ error("%s: xauthfile path too long", __func__);
+ unlink(xauthfile);
+ rmdir(xauthdir);
+ return -1;
+ }
+
if (timeout >= UINT_MAX - X11_TIMEOUT_SLACK)
x11_timeout_real = UINT_MAX;
else
x11_timeout_real = timeout + X11_TIMEOUT_SLACK;
- 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 %u 2>" _PATH_DEVNULL,
- xauth_path, xauthfile, display,
- x11_timeout_real);
- debug2("x11_get_proto: %s", cmd);
- if (x11_refuse_time == 0) {
- now = monotime() + 1;
- if (UINT_MAX - timeout < now)
- x11_refuse_time = UINT_MAX;
- else
- x11_refuse_time = now + timeout;
- channel_set_x11_refuse_time(
- x11_refuse_time);
- }
- if (system(cmd) == 0)
- generated = 1;
+ 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__);
+ debug2("%s: %s", __func__, cmd);
+ if (x11_refuse_time == 0) {
+ now = monotime() + 1;
+ if (UINT_MAX - timeout < now)
+ x11_refuse_time = UINT_MAX;
+ else
+ x11_refuse_time = now + timeout;
+ channel_set_x11_refuse_time(x11_refuse_time);
}
+ if (system(cmd) == 0)
+ generated = 1;
}
/*
* When in untrusted mode, we read the cookie only if it was
* successfully generated as an untrusted one in the step
* above.
*/
if (trusted || generated) {
@@ -396,27 +412,30 @@ client_x11_get_proto(const char *display
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);
- } else
- error("Warning: untrusted X11 forwarding setup failed: "
- "xauth key data not generated");
+ }
}
if (do_unlink) {
unlink(xauthfile);
rmdir(xauthdir);
}
- free(xauthdir);
- free(xauthfile);
+
+ /* Don't fall back to fake X11 data for untrusted forwarding */
+ if (!trusted && !got_data) {
+ error("Warning: untrusted X11 forwarding setup failed: "
+ "xauth key data not generated");
+ return -1;
+ }
/*
* If we didn't get authentication data, just make up some
* data. The forwarding code will check the validity of the
* response anyway, and substitute this data. The X11
* server, however, will ignore this fake data and use
* whatever authentication mechanisms it was using otherwise
* for the local connection.
@@ -430,16 +449,18 @@ client_x11_get_proto(const char *display
for (i = 0; i < 16; i++) {
if (i % 4 == 0)
rnd = arc4random();
snprintf(data + 2 * i, sizeof data - 2 * i, "%02x",
rnd & 0xff);
rnd >>= 8;
}
}
+
+ return 0;
}
/*
* This is called when the interactive is entered. This checks if there is
* an EOF coming on stdin. We must check this explicitly, as select() does
* not appear to wake up when redirecting from /dev/null.
*/
diff --git a/openssh-6.6p1/clientloop.h b/openssh-6.6p1/clientloop.h
--- a/openssh-6.6p1/clientloop.h
+++ b/openssh-6.6p1/clientloop.h
@@ -1,9 +1,9 @@
-/* $OpenBSD: clientloop.h,v 1.31 2013/06/02 23:36:29 dtucker Exp $ */
+/* $OpenBSD: clientloop.h,v 1.32 2016/01/13 23:04:47 djm Exp $ */
/*
* Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
* All rights reserved
*
* As far as I am concerned, the code I have written for this software
* can be used freely for any purpose. Any derived versions of this
@@ -34,17 +34,17 @@
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include <termios.h>
/* Client side main loop for the interactive session. */
int client_loop(int, int, int);
-void client_x11_get_proto(const char *, const char *, u_int, u_int,
+int client_x11_get_proto(const char *, const char *, u_int, u_int,
char **, char **);
void client_global_request_reply_fwd(int, u_int32_t, void *);
void client_session2_setup(int, int, int, const char *, struct termios *,
int, Buffer *, char **);
int client_request_tun_fwd(int, int, int);
void client_stop_mux(void);
/* Escape filter for protocol 2 sessions */
diff --git a/openssh-6.6p1/mux.c b/openssh-6.6p1/mux.c
--- a/openssh-6.6p1/mux.c
+++ b/openssh-6.6p1/mux.c
@@ -1256,26 +1256,28 @@ mux_session_confirm(int id, int success,
goto done;
}
display = getenv("DISPLAY");
if (cctx->want_x_fwd && options.forward_x11 && display != NULL) {
char *proto, *data;
/* Get reasonable local authentication information. */
- client_x11_get_proto(display, options.xauth_location,
+ if (client_x11_get_proto(display, options.xauth_location,
options.forward_x11_trusted, options.forward_x11_timeout,
- &proto, &data);
- /* Request forwarding with authentication spoofing. */
- debug("Requesting X11 forwarding with authentication "
- "spoofing.");
- x11_request_forwarding_with_spoofing(id, display, proto,
- data, 1);
- client_expect_confirm(id, "X11 forwarding", CONFIRM_WARN);
- /* XXX exit_on_forward_failure */
+ &proto, &data) == 0) {
+ /* Request forwarding with authentication spoofing. */
+ debug("Requesting X11 forwarding with authentication "
+ "spoofing.");
+ x11_request_forwarding_with_spoofing(id, display, proto,
+ data, 1);
+ /* XXX exit_on_forward_failure */
+ client_expect_confirm(id, "X11 forwarding",
+ CONFIRM_WARN);
+ }
}
if (cctx->want_agent_fwd && options.forward_agent) {
debug("Requesting authentication agent forwarding.");
channel_request_start(id, "auth-agent-req@openssh.com", 0);
packet_send();
}
diff --git a/openssh-6.6p1/ssh.c b/openssh-6.6p1/ssh.c
--- a/openssh-6.6p1/ssh.c
+++ b/openssh-6.6p1/ssh.c
@@ -1413,16 +1413,17 @@ static int
ssh_session(void)
{
int type;
int interactive = 0;
int have_tty = 0;
struct winsize ws;
char *cp;
const char *display;
+ char *proto = NULL, *data = NULL;
/* Enable compression if requested. */
if (options.compression) {
debug("Requesting compression at level %d.",
options.compression_level);
if (options.compression_level < 1 ||
options.compression_level > 9)
@@ -1481,23 +1482,19 @@ ssh_session(void)
logit("Warning: Remote host failed or refused to "
"allocate a pseudo tty.");
else
packet_disconnect("Protocol error waiting for pty "
"request response.");
}
/* Request X11 forwarding if enabled and DISPLAY is set. */
display = getenv("DISPLAY");
- if (options.forward_x11 && display != NULL) {
- char *proto, *data;
- /* Get reasonable local authentication information. */
- client_x11_get_proto(display, options.xauth_location,
- options.forward_x11_trusted,
- options.forward_x11_timeout,
- &proto, &data);
+ if (options.forward_x11 && client_x11_get_proto(display,
+ options.xauth_location, options.forward_x11_trusted,
+ options.forward_x11_timeout, &proto, &data) == 0) {
/* Request forwarding with authentication spoofing. */
debug("Requesting X11 forwarding with authentication "
"spoofing.");
x11_request_forwarding_with_spoofing(0, display, proto,
data, 0);
/* Read response from the server. */
type = packet_read();
if (type == SSH_SMSG_SUCCESS) {
@@ -1577,27 +1574,25 @@ ssh_session(void)
/* request pty/x11/agent/tcpfwd/shell for channel */
static void
ssh_session2_setup(int id, int success, void *arg)
{
extern char **environ;
const char *display;
int interactive = tty_flag;
+ char *proto = NULL, *data = NULL;
if (!success)
return; /* No need for error message, channels code sens one */
display = getenv("DISPLAY");
- if (options.forward_x11 && display != NULL) {
- char *proto, *data;
- /* Get reasonable local authentication information. */
- client_x11_get_proto(display, options.xauth_location,
- options.forward_x11_trusted,
- options.forward_x11_timeout, &proto, &data);
+ if (options.forward_x11 && client_x11_get_proto(display,
+ options.xauth_location, options.forward_x11_trusted,
+ options.forward_x11_timeout, &proto, &data) == 0) {
/* Request forwarding with authentication spoofing. */
debug("Requesting X11 forwarding with authentication "
"spoofing.");
x11_request_forwarding_with_spoofing(id, display, proto,
data, 1);
client_expect_confirm(id, "X11 forwarding", CONFIRM_WARN);
/* XXX exit_on_forward_failure */
interactive = 1;
