xorg-server: Changes to 'debian-jessie'
configure.ac | 2
debian/changelog | 2
hw/xwayland/xwayland.c | 4 +
include/dix-config.h.in | 3 +
include/os.h | 22 +++++++++
os/access.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++
os/auth.c | 8 +--
os/mitauth.c | 2
os/timingsafe_memcmp.c | 45 +++++++++++++++++++
9 files changed, 190 insertions(+), 7 deletions(-)
New commits:
commit 791d9d862374cd63f302c212116085ff7951ce1c
Author: Julien Cristau <jcristau@debian.org>
Date: Fri Oct 13 14:14:46 2017 +0200
Update changelog
diff --git a/debian/changelog b/debian/changelog
index 867cbf7..5f72c45 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -21,6 +21,8 @@ xorg-server (2:1.16.4-1+deb8u2) UNRELEASED; urgency=high
* Xi: fix wrong extra length check in ProcXIChangeHierarchy (CVE-2017-12178)
* dbe: Unvalidated variable-length request in ProcDbeGetVisualInfo (CVE-2017-12177)
* Unvalidated extra length in ProcEstablishConnection (CVE-2017-12176)
+ * Use timingsafe_memcmp() to compare MIT-MAGIC-COOKIES (CVE-2017-2624)
+ * Xwayland: enable access control and default to just the local user (CVE-2015-3164)
-- Julien Cristau <jcristau@debian.org> Fri, 13 Oct 2017 11:55:04 +0200
commit 37cff4625402bf9ab40a96426f8efb9bb256ac63
Author: Ray Strode <rstrode@redhat.com>
Date: Tue May 5 16:43:44 2015 -0400
xwayland: default to local user if no xauth file given. [CVE-2015-3164 3/3]
Right now if "-auth" isn't passed on the command line, we let
any user on the system connect to the Xwayland server.
That's clearly suboptimal, given Xwayland is generally designed
to be used by one user at a time.
This commit changes the behavior, so only the user who started the
X server can connect clients to it.
Signed-off-by: Ray Strode <rstrode@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit 76636ac12f2d1dbdf7be08222f80e7505d53c451)
diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 5062ee3..9835f10 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -681,4 +681,6 @@ InitOutput(ScreenInfo * screen_info, int argc, char **argv)
if (AddScreen(xwl_screen_init, argc, argv) == -1) {
FatalError("Couldn't add screen\n");
}
+
+ LocalAccessScopeUser();
}
commit cebb2dbcddcbf76917ecea53285ca793e44a1108
Author: Ray Strode <rstrode@redhat.com>
Date: Tue May 5 16:43:43 2015 -0400
os: support new implicit local user access mode [CVE-2015-3164 2/3]
If the X server is started without a '-auth' argument, then
it gets started wide open to all local users on the system.
This isn't a great default access model, but changing it in
Xorg at this point would break backward compatibility.
Xwayland, on the other hand is new, and much more targeted
in scope. It could, in theory, be changed to allow the much
more secure default of a "user who started X server can connect
clients to that server."
This commit paves the way for that change, by adding a mechanism
for DDXs to opt-in to that behavior. They merely need to call
LocalAccessScopeUser()
in their init functions.
A subsequent commit will add that call for Xwayland.
Signed-off-by: Ray Strode <rstrode@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit 4b4b9086d02b80549981d205fb1f495edc373538)
diff --git a/include/os.h b/include/os.h
index 892a229..c67b993 100644
--- a/include/os.h
+++ b/include/os.h
@@ -451,11 +451,28 @@ extern _X_EXPORT void
ResetHosts(const char *display);
extern _X_EXPORT void
+EnableLocalAccess(void);
+
+extern _X_EXPORT void
+DisableLocalAccess(void);
+
+extern _X_EXPORT void
EnableLocalHost(void);
extern _X_EXPORT void
DisableLocalHost(void);
+#ifndef NO_LOCAL_CLIENT_CRED
+extern _X_EXPORT void
+EnableLocalUser(void);
+
+extern _X_EXPORT void
+DisableLocalUser(void);
+
+extern _X_EXPORT void
+LocalAccessScopeUser(void);
+#endif
+
extern _X_EXPORT void
AccessUsingXdmcp(void);
diff --git a/os/access.c b/os/access.c
index 61624fd..84c9c65 100644
--- a/os/access.c
+++ b/os/access.c
@@ -102,6 +102,10 @@ SOFTWARE.
#include <sys/ioctl.h>
#include <ctype.h>
+#ifndef NO_LOCAL_CLIENT_CRED
+#include <pwd.h>
+#endif
+
#if defined(TCPCONN) || defined(STREAMSCONN)
#include <netinet/in.h>
#endif /* TCPCONN || STREAMSCONN */
@@ -225,6 +229,13 @@ static int LocalHostEnabled = FALSE;
static int LocalHostRequested = FALSE;
static int UsingXdmcp = FALSE;
+static enum {
+ LOCAL_ACCESS_SCOPE_HOST = 0,
+#ifndef NO_LOCAL_CLIENT_CRED
+ LOCAL_ACCESS_SCOPE_USER,
+#endif
+} LocalAccessScope;
+
/* FamilyServerInterpreted implementation */
static Bool siAddrMatch(int family, void *addr, int len, HOST * host,
ClientPtr client);
@@ -237,6 +248,21 @@ static void siTypesInitialize(void);
*/
void
+EnableLocalAccess(void)
+{
+ switch (LocalAccessScope) {
+ case LOCAL_ACCESS_SCOPE_HOST:
+ EnableLocalHost();
+ break;
+#ifndef NO_LOCAL_CLIENT_CRED
+ case LOCAL_ACCESS_SCOPE_USER:
+ EnableLocalUser();
+ break;
+#endif
+ }
+}
+
+void
EnableLocalHost(void)
{
if (!UsingXdmcp) {
@@ -249,6 +275,21 @@ EnableLocalHost(void)
* called when authorization is enabled to keep us secure
*/
void
+DisableLocalAccess(void)
+{
+ switch (LocalAccessScope) {
+ case LOCAL_ACCESS_SCOPE_HOST:
+ DisableLocalHost();
+ break;
+#ifndef NO_LOCAL_CLIENT_CRED
+ case LOCAL_ACCESS_SCOPE_USER:
+ DisableLocalUser();
+ break;
+#endif
+ }
+}
+
+void
DisableLocalHost(void)
{
HOST *self;
@@ -262,6 +303,74 @@ DisableLocalHost(void)
}
}
+#ifndef NO_LOCAL_CLIENT_CRED
+static int GetLocalUserAddr(char **addr)
+{
+ static const char *type = "localuser";
+ static const char delimiter = '\0';
+ static const char *value;
+ struct passwd *pw;
+ int length = -1;
+
+ pw = getpwuid(getuid());
+
+ if (pw == NULL || pw->pw_name == NULL)
+ goto out;
+
+ value = pw->pw_name;
+
+ length = asprintf(addr, "%s%c%s", type, delimiter, value);
+
+ if (length == -1) {
+ goto out;
+ }
+
+ /* Trailing NUL */
+ length++;
+
+out:
+ return length;
+}
+
+void
+EnableLocalUser(void)
+{
+ char *addr = NULL;
+ int length = -1;
+
+ length = GetLocalUserAddr(&addr);
+
+ if (length == -1)
+ return;
+
+ NewHost(FamilyServerInterpreted, addr, length, TRUE);
+
+ free(addr);
+}
+
+void
+DisableLocalUser(void)
+{
+ char *addr = NULL;
+ int length = -1;
+
+ length = GetLocalUserAddr(&addr);
+
+ if (length == -1)
+ return;
+
+ RemoveHost(NULL, FamilyServerInterpreted, length, addr);
+
+ free(addr);
+}
+
+void
+LocalAccessScopeUser(void)
+{
+ LocalAccessScope = LOCAL_ACCESS_SCOPE_USER;
+}
+#endif
+
/*
* called at init time when XDMCP will be used; xdmcp always
* adds local hosts manually when needed
diff --git a/os/auth.c b/os/auth.c
index 5fcb538..7da6fc6 100644
--- a/os/auth.c
+++ b/os/auth.c
@@ -181,11 +181,11 @@ CheckAuthorization(unsigned int name_length,
/*
* If the authorization file has at least one entry for this server,
- * disable local host access. (loadauth > 0)
+ * disable local access. (loadauth > 0)
*
* If there are zero entries (either initially or when the
* authorization file is later reloaded), or if a valid
- * authorization file was never loaded, enable local host access.
+ * authorization file was never loaded, enable local access.
* (loadauth == 0 || !loaded)
*
* If the authorization file was loaded initially (with valid
@@ -194,11 +194,11 @@ CheckAuthorization(unsigned int name_length,
*/
if (loadauth > 0) {
- DisableLocalHost(); /* got at least one */
+ DisableLocalAccess(); /* got at least one */
loaded = TRUE;
}
else if (loadauth == 0 || !loaded)
- EnableLocalHost();
+ EnableLocalAccess();
}
if (name_length) {
for (i = 0; i < NUM_AUTHORIZATION; i++) {
commit ecd109407917fee775bed51737a98b4d563d53e7
Author: Ray Strode <rstrode@redhat.com>
Date: Tue May 5 16:43:42 2015 -0400
xwayland: Enable access control on open sockets [CVE-2015-3164 1/3]
Xwayland currently allows wide-open access to the X sockets
it listens on, ignoring Xauth access control.
This commit makes sure to enable access control on the sockets,
so one user can't snoop on another user's X-over-wayland
applications.
Signed-off-by: Ray Strode <rstrode@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit c4534a38b68aa07fb82318040dc8154fb48a9588)
diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 17b7bf7..5062ee3 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -463,7 +463,7 @@ listen_on_fds(struct xwl_screen *xwl_screen)
int i;
for (i = 0; i < xwl_screen->listen_fd_count; i++)
- ListenOnOpenFD(xwl_screen->listen_fds[i], TRUE);
+ ListenOnOpenFD(xwl_screen->listen_fds[i], FALSE);
}
static void
commit 873dd830ea4b1a2bbc26d616604a48950e61ca7d
Author: Matthieu Herrb <matthieu@herrb.eu>
Date: Tue Feb 28 19:18:25 2017 +0100
Use timingsafe_memcmp() to compare MIT-MAGIC-COOKIES CVE-2017-2624
Provide the function definition for systems that don't have it.
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit d7ac755f0b618eb1259d93c8a16ec6e39a18627c)
diff --git a/configure.ac b/configure.ac
index efcaa1f..63eed65 100644
--- a/configure.ac
+++ b/configure.ac
@@ -218,7 +218,7 @@ dnl Checks for library functions.
AC_CHECK_FUNCS([backtrace ffs geteuid getuid issetugid getresuid \
getdtablesize getifaddrs getpeereid getpeerucred getzoneid \
mmap seteuid shmctl64 strncasecmp vasprintf vsnprintf walkcontext])
-AC_REPLACE_FUNCS([strcasecmp strcasestr strlcat strlcpy strndup])
+AC_REPLACE_FUNCS([strcasecmp strcasestr strlcat strlcpy strndup timingsafe_memcmp])
dnl Find the math libary, then check for cbrt function in it.
AC_CHECK_LIB(m, sqrt)
diff --git a/include/dix-config.h.in b/include/dix-config.h.in
index f980a3d..1d33027 100644
--- a/include/dix-config.h.in
+++ b/include/dix-config.h.in
@@ -225,6 +225,9 @@
/* Define to 1 if you have the <sys/utsname.h> header file. */
#undef HAVE_SYS_UTSNAME_H
+/* Define to 1 if you have the `timingsafe_memcmp' function. */
+#undef HAVE_TIMINGSAFE_MEMCMP
+
/* Define to 1 if you have the <tslib.h> header file. */
#undef HAVE_TSLIB_H
diff --git a/include/os.h b/include/os.h
index d26e399..892a229 100644
--- a/include/os.h
+++ b/include/os.h
@@ -579,6 +579,11 @@ extern _X_EXPORT char *
strndup(const char *str, size_t n);
#endif
+#ifndef HAVE_TIMINGSAFE_MEMCMP
+extern _X_EXPORT int
+timingsafe_memcmp(const void *b1, const void *b2, size_t len);
+#endif
+
/* Logging. */
typedef enum _LogParameter {
XLOG_FLUSH,
diff --git a/os/mitauth.c b/os/mitauth.c
index 768a52a..efae440 100644
--- a/os/mitauth.c
+++ b/os/mitauth.c
@@ -76,7 +76,7 @@ MitCheckCookie(unsigned short data_length,
for (auth = mit_auth; auth; auth = auth->next) {
if (data_length == auth->len &&
- memcmp(data, auth->data, (int) data_length) == 0)
+ timingsafe_memcmp(data, auth->data, (int) data_length) == 0)
return auth->id;
}
*reason = "Invalid MIT-MAGIC-COOKIE-1 key";
diff --git a/os/timingsafe_memcmp.c b/os/timingsafe_memcmp.c
new file mode 100644
index 0000000..36ab362
--- /dev/null
+++ b/os/timingsafe_memcmp.c
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2014 Google Inc.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <limits.h>
+#include <string.h>
+
+int
+timingsafe_memcmp(const void *b1, const void *b2, size_t len)
+{
+ const unsigned char *p1 = b1, *p2 = b2;
+ size_t i;
+ int res = 0, done = 0;
+
+ for (i = 0; i < len; i++) {
+ /* lt is -1 if p1[i] < p2[i]; else 0. */
+ int lt = (p1[i] - p2[i]) >> CHAR_BIT;
+
+ /* gt is -1 if p1[i] > p2[i]; else 0. */
+ int gt = (p2[i] - p1[i]) >> CHAR_BIT;
+
+ /* cmp is 1 if p1[i] > p2[i]; -1 if p1[i] < p2[i]; else 0. */
+ int cmp = lt - gt;
+
+ /* set res = cmp if !done. */
+ res |= cmp & ~done;
+
+ /* set done if p1[i] != p2[i]. */
+ done |= lt | gt;
+ }
+
+ return (res);
+}
Reply to: