Hi,
I've been working on an updated patch for JDK-8036559, where root does
not have the ability to attach to unprivileged users' JVMs. I originally
mentioned this problem back in 2013, and proposed a patch only for Linux
[1]. The result was that the fix had to provide support for all affected
platforms, and to include tests.
We worked around this issue in our project, but I revisited this bug
recently. I investigated the issue on Windows, which has a very
different implementation from the other platforms. I discovered that
this bug does not appear to affect Windows. Using the test programs
attached to Red Hat Bugzilla bug #1311638 [2], I verified the correct
behaviour using the following steps:
(Open cmd.exe)
runas /user:test cmd.exe
runas /user:Administrator cmd.exe
(In test's shell)
set TMP=C:\Users\Public\java_temp
cd C:\Users\Public\Documents
javac AttachTarget.java
java AttachTarget
(In Administrator's shell)
set TMP=C:\Users\Public\java_temp
cd C:\Users\Public\Documents
javac -cp .;C:\Progra~1\Java\jdk1.8.0_74\lib\tools.jar AttachClient.java
java -cp .;C:\Progra~1\Java\jdk1.8.0_74\lib\tools.jar AttachClient
(outputs 'Target ok: AttachTarget')
My updated patches target JDK 9, and includes support for Linux,
Solaris, Mac OSX, and AIX. As far as tests are concerned, I'm not sure
how to add tests for this bug, since doing so would require the test to
be run as root. I am attaching the patches to this email, since I am not
an OpenJDK committer and do not have access to cr.openjdk.java.net.
Thanks,
Elliott
[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2013-June/010077.html
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1311638
# HG changeset patch
# User Elliott Baron <eba...@redhat.com>
# Date 1458161105 14400
# Wed Mar 16 16:45:05 2016 -0400
# Node ID f7083305c52f4a1ec06f900a0b0cbe9d7a303683
# Parent b84b5f6d9ed527c51d6184d140b85ae43c4c2e1f
8036559: Attach API does not allow root to connect to process owned by others
diff --git a/src/jdk.attach/aix/native/libattach/VirtualMachineImpl.c b/src/jdk.attach/aix/native/libattach/VirtualMachineImpl.c
--- a/src/jdk.attach/aix/native/libattach/VirtualMachineImpl.c
+++ b/src/jdk.attach/aix/native/libattach/VirtualMachineImpl.c
@@ -174,7 +174,8 @@
/*
* Check that the path is owned by the effective uid/gid of this
- * process. Also check that group/other access is not allowed.
+ * process, unless the effective uid/gid of this process is root.
+ * Also check that group/other access is not allowed.
*/
uid = geteuid();
gid = getegid();
@@ -188,11 +189,12 @@
if (res == 0) {
char msg[100];
jboolean isError = JNI_FALSE;
- if (sb.st_uid != uid) {
+ jboolean isRoot = (jboolean)(uid == 0 && gid == 0);
+ if (sb.st_uid != uid && !isRoot) {
jio_snprintf(msg, sizeof(msg)-1,
"file should be owned by the current user (which is %d) but is owned by %d", uid, sb.st_uid);
isError = JNI_TRUE;
- } else if (sb.st_gid != gid) {
+ } else if (sb.st_gid != gid && !isRoot) {
jio_snprintf(msg, sizeof(msg)-1,
"file's group should be the current group (which is %d) but the group is %d", gid, sb.st_gid);
isError = JNI_TRUE;
diff --git a/src/jdk.attach/linux/native/libattach/VirtualMachineImpl.c b/src/jdk.attach/linux/native/libattach/VirtualMachineImpl.c
--- a/src/jdk.attach/linux/native/libattach/VirtualMachineImpl.c
+++ b/src/jdk.attach/linux/native/libattach/VirtualMachineImpl.c
@@ -364,7 +364,8 @@
/*
* Check that the path is owned by the effective uid/gid of this
- * process. Also check that group/other access is not allowed.
+ * process, unless the effective uid/gid of this process is root.
+ * Also check that group/other access is not allowed.
*/
uid = geteuid();
gid = getegid();
@@ -378,11 +379,12 @@
if (res == 0) {
char msg[100];
jboolean isError = JNI_FALSE;
- if (sb.st_uid != uid) {
+ jboolean isRoot = (jboolean)(uid == 0 && gid == 0);
+ if (sb.st_uid != uid && !isRoot) {
jio_snprintf(msg, sizeof(msg)-1,
"file should be owned by the current user (which is %d) but is owned by %d", uid, sb.st_uid);
isError = JNI_TRUE;
- } else if (sb.st_gid != gid) {
+ } else if (sb.st_gid != gid && !isRoot) {
jio_snprintf(msg, sizeof(msg)-1,
"file's group should be the current group (which is %d) but the group is %d", gid, sb.st_gid);
isError = JNI_TRUE;
diff --git a/src/jdk.attach/macosx/native/libattach/VirtualMachineImpl.c b/src/jdk.attach/macosx/native/libattach/VirtualMachineImpl.c
--- a/src/jdk.attach/macosx/native/libattach/VirtualMachineImpl.c
+++ b/src/jdk.attach/macosx/native/libattach/VirtualMachineImpl.c
@@ -146,7 +146,8 @@
/*
* Check that the path is owned by the effective uid/gid of this
- * process. Also check that group/other access is not allowed.
+ * process, unless the effective uid/gid of this process is root.
+ * Also check that group/other access is not allowed.
*/
uid = geteuid();
gid = getegid();
@@ -160,11 +161,12 @@
if (res == 0) {
char msg[100];
jboolean isError = JNI_FALSE;
- if (sb.st_uid != uid) {
+ jboolean isRoot = (jboolean)(uid == 0 && gid == 0);
+ if (sb.st_uid != uid && !isRoot) {
jio_snprintf(msg, sizeof(msg)-1,
"file should be owned by the current user (which is %d) but is owned by %d", uid, sb.st_uid);
isError = JNI_TRUE;
- } else if (sb.st_gid != gid) {
+ } else if (sb.st_gid != gid && !isRoot) {
jio_snprintf(msg, sizeof(msg)-1,
"file's group should be the current group (which is %d) but the group is %d", gid, sb.st_gid);
isError = JNI_TRUE;
diff --git a/src/jdk.attach/solaris/native/libattach/VirtualMachineImpl.c b/src/jdk.attach/solaris/native/libattach/VirtualMachineImpl.c
--- a/src/jdk.attach/solaris/native/libattach/VirtualMachineImpl.c
+++ b/src/jdk.attach/solaris/native/libattach/VirtualMachineImpl.c
@@ -107,7 +107,8 @@
/*
* Check that the path is owned by the effective uid/gid of this
- * process. Also check that group/other access is not allowed.
+ * process, unless the effective uid/gid of this process is root.
+ * Also check that group/other access is not allowed.
*/
uid = geteuid();
gid = getegid();
@@ -121,11 +122,12 @@
if (res == 0) {
char msg[100];
jboolean isError = JNI_FALSE;
- if (sb.st_uid != uid) {
+ jboolean isRoot = (jboolean)(uid == 0 && gid == 0);
+ if (sb.st_uid != uid && !isRoot) {
jio_snprintf(msg, sizeof(msg)-1,
"file should be owned by the current user (which is %d) but is owned by %d", uid, sb.st_uid);
isError = JNI_TRUE;
- } else if (sb.st_gid != gid) {
+ } else if (sb.st_gid != gid && !isRoot) {
jio_snprintf(msg, sizeof(msg)-1,
"file's group should be the current group (which is %d) but the group is %d", gid, sb.st_gid);
isError = JNI_TRUE;
# HG changeset patch
# User Elliott Baron <eba...@redhat.com>
# Date 1458161697 14400
# Wed Mar 16 16:54:57 2016 -0400
# Node ID 008e5df501c02a0e6543af59f5b421a8dee21a80
# Parent 0e6f2f47479c07aaac27de3b8d17043d11bb1578
8036559: Attach API does not allow root to connect to process owned by others
diff --git a/src/os/aix/vm/attachListener_aix.cpp b/src/os/aix/vm/attachListener_aix.cpp
--- a/src/os/aix/vm/attachListener_aix.cpp
+++ b/src/os/aix/vm/attachListener_aix.cpp
@@ -54,7 +54,7 @@
// 1. The well known file that the socket is bound to has permission 400
// 2. When a client connect, the SO_PEERID socket option is used to
// obtain the credentials of client. We check that the effective uid
-// of the client matches this process.
+// of the client matches this process, or is root.
// forward reference
class AixAttachOperation;
@@ -390,7 +390,9 @@
uid_t euid = geteuid();
gid_t egid = getegid();
- if (cred_info.euid != euid || cred_info.egid != egid) {
+ // root is always allowed
+ if (cred_info.euid != euid && cred_info.euid != 0 || cred_info.egid != egid &&
+ cred_info.egid != 0) {
int res;
RESTARTABLE(::close(s), res);
continue;
@@ -547,7 +549,7 @@
if (ret == 0) {
// simple check to avoid starting the attach mechanism when
// a bogus user creates the file
- if (st.st_uid == geteuid()) {
+ if (st.st_uid == geteuid() || st.st_uid == 0) {
init();
return true;
}
diff --git a/src/os/bsd/vm/attachListener_bsd.cpp b/src/os/bsd/vm/attachListener_bsd.cpp
--- a/src/os/bsd/vm/attachListener_bsd.cpp
+++ b/src/os/bsd/vm/attachListener_bsd.cpp
@@ -53,7 +53,7 @@
// 1. The well known file that the socket is bound to has permission 400
// 2. When a client connect, the SO_PEERCRED socket option is used to
// obtain the credentials of client. We check that the effective uid
-// of the client matches this process.
+// of the client matches this process, or is root.
// forward reference
class BsdAttachOperation;
@@ -351,7 +351,8 @@
uid_t euid = geteuid();
gid_t egid = getegid();
- if (puid != euid || pgid != egid) {
+ // root is always allowed
+ if (puid != euid && puid != 0 || pgid != egid && pgid != 0) {
::close(s);
continue;
}
@@ -503,7 +504,7 @@
if (ret == 0) {
// simple check to avoid starting the attach mechanism when
// a bogus user creates the file
- if (st.st_uid == geteuid()) {
+ if (st.st_uid == geteuid() || st.st_uid == 0) {
init();
return true;
}
diff --git a/src/os/linux/vm/attachListener_linux.cpp b/src/os/linux/vm/attachListener_linux.cpp
--- a/src/os/linux/vm/attachListener_linux.cpp
+++ b/src/os/linux/vm/attachListener_linux.cpp
@@ -53,7 +53,7 @@
// 1. The well known file that the socket is bound to has permission 400
// 2. When a client connect, the SO_PEERCRED socket option is used to
// obtain the credentials of client. We check that the effective uid
-// of the client matches this process.
+// of the client matches this process, or is root.
// forward reference
class LinuxAttachOperation;
@@ -348,7 +348,9 @@
uid_t euid = geteuid();
gid_t egid = getegid();
- if (cred_info.uid != euid || cred_info.gid != egid) {
+ // root is always allowed
+ if (cred_info.uid != euid && cred_info.uid != 0 || cred_info.gid != egid &&
+ cred_info.gid != 0) {
::close(s);
continue;
}
@@ -503,7 +505,7 @@
if (ret == 0) {
// simple check to avoid starting the attach mechanism when
// a bogus user creates the file
- if (st.st_uid == geteuid()) {
+ if (st.st_uid == geteuid() || st.st_uid == 0) {
init();
return true;
}
diff --git a/src/os/solaris/vm/attachListener_solaris.cpp b/src/os/solaris/vm/attachListener_solaris.cpp
--- a/src/os/solaris/vm/attachListener_solaris.cpp
+++ b/src/os/solaris/vm/attachListener_solaris.cpp
@@ -50,15 +50,15 @@
// client (tools) can locate it. To enqueue an operation to the VM the
// client calls through the door which invokes the enqueue function in
// this process. The credentials of the client are checked and if the
-// effective uid matches this process then the operation is enqueued.
-// When an operation completes the attach listener is required to send the
-// operation result and any result data to the client. In this implementation
-// the result is returned via a UNIX domain socket. A pair of connected
-// sockets (socketpair) is created in the enqueue function and the file
-// descriptor for one of the sockets is returned to the client as the
-// return from the door call. The other end is retained in this process.
-// When the operation completes the result is sent to the client and
-// the socket is closed.
+// effective uid matches this process, or is root, then the operation is
+// enqueued. When an operation completes the attach listener is required
+// to send the operation result and any result data to the client.
+// In this implementation the result is returned via a UNIX domain socket.
+// A pair of connected sockets (socketpair) is created in the enqueue
+// function and the file descriptor for one of the sockets is returned to
+// the client as the return from the door call. The other end is retained
+// in this process. When the operation completes the result is sent to the
+// client and the socket is closed.
// forward reference
class SolarisAttachOperation;
@@ -215,8 +215,9 @@
uid_t ucred_euid = ucred_geteuid(cred_info);
gid_t ucred_egid = ucred_getegid(cred_info);
- // check that the effective uid/gid matches
- if (ucred_euid == euid && ucred_egid == egid) {
+ // check that the effective uid/gid matches, or is root
+ if (ucred_euid == euid && ucred_egid == egid ||
+ ucred_euid == 0 && ucred_egid == 0) {
ret = 0; // allow
}
@@ -652,7 +653,7 @@
if (ret == 0) {
// simple check to avoid starting the attach mechanism when
// a bogus user creates the file
- if (st.st_uid == geteuid()) {
+ if (st.st_uid == geteuid() || st.st_uid == 0) {
init();
return true;
}
