From 6716f159645639edc669900308250492dfef5641 Mon Sep 17 00:00:00 2001 From: Colin Patrick Mccabe Date: Tue, 13 Oct 2015 12:18:03 -0700 Subject: [PATCH] HADOOP-12344. Improve validateSocketPathSecurity0 error message (Casey Brotherton via Colin P. McCabe) --- .../hadoop-common/CHANGES.txt | 3 + .../apache/hadoop/net/unix/DomainSocket.java | 3 + .../org/apache/hadoop/net/unix/DomainSocket.c | 63 +++++++++++++------ .../hadoop/net/unix/TestDomainSocket.java | 7 +-- 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 9ab19f9d87a..3d3de166b9f 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -882,6 +882,9 @@ Release 2.8.0 - UNRELEASED HADOOP-11104. org.apache.hadoop.metrics2.lib.MetricsRegistry needs numerical parameter checking. (Ray Chiang via aajisaka) + HADOOP-12344. Improve validateSocketPathSecurity0 error message (Casey + Brotherton via Colin P. McCabe) + OPTIMIZATIONS HADOOP-11785. Reduce the number of listStatus operation in distcp diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/unix/DomainSocket.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/unix/DomainSocket.java index 6166ba870bc..f1035e2dd64 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/unix/DomainSocket.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/unix/DomainSocket.java @@ -92,6 +92,9 @@ public class DomainSocket implements Closeable { * misconfigurations. System administrators do not commonly change * permissions on these paths while the server is running. * + * For more information on Security exceptions see this wiki page: + * https://wiki.apache.org/hadoop/SocketPathSecurity + * * @param path the path to validate * @param skipComponents the number of starting path components to skip * validation for (used only for testing) diff --git a/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/net/unix/DomainSocket.c b/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/net/unix/DomainSocket.c index e658d8f67eb..c653a273563 100644 --- a/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/net/unix/DomainSocket.c +++ b/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/net/unix/DomainSocket.c @@ -268,7 +268,7 @@ Java_org_apache_hadoop_net_unix_DomainSocket_validateSocketPathSecurity0( JNIEnv *env, jclass clazz, jobject jstr, jint skipComponents) { jint utfLength; - char path[PATH_MAX], check[PATH_MAX], *token, *rest; + char path[PATH_MAX], check[PATH_MAX], *token, *rest, *rest_free; struct stat st; int ret, mode, strlenPath; uid_t uid; @@ -280,6 +280,7 @@ JNIEnv *env, jclass clazz, jobject jstr, jint skipComponents) "no longer than %zd UTF-8 bytes.", (sizeof(path)-1)); goto done; } + (*env)->GetStringUTFRegion(env, jstr, 0, utfLength, path); path [ utfLength ] = 0; jthr = (*env)->ExceptionOccurred(env); @@ -304,7 +305,16 @@ JNIEnv *env, jclass clazz, jobject jstr, jint skipComponents) // last one. We don't validate the last component, since it's not supposed to // be a directory. (If it is a directory, we will fail to create the socket // later with EISDIR or similar.) - for (check[0] = '/', check[1] = '\0', rest = path, token = ""; + rest=strdup(path); + if ( rest == NULL ){ + ret = errno; + jthr = newIOException(env,"memory allocation failure trying to copy a path" + " with %d length. error code %d (%s). ", strlenPath, ret, terror(ret)); + goto done; + }; + rest_free=rest; + + for (check[0] = '/', check[1] = '\0', token = ""; token && rest && rest[0]; token = strtok_r(rest, "/", &rest)) { if (strcmp(check, "/") != 0) { @@ -322,36 +332,51 @@ JNIEnv *env, jclass clazz, jobject jstr, jint skipComponents) } if (stat(check, &st) < 0) { ret = errno; - jthr = newIOException(env, "failed to stat a path component: '%s'. " - "error code %d (%s)", check, ret, terror(ret)); + jthr = newIOException(env, "failed to stat a path component: " + "'%s' in '%s'. error code %d (%s). " + "Ensure that the path is configured correctly.", + check, path, ret, terror(ret)); goto done; } mode = st.st_mode & 0777; if (mode & 0002) { - jthr = newIOException(env, "the path component: '%s' is " - "world-writable. Its permissions are 0%03o. Please fix " - "this or select a different socket path.", check, mode); + jthr = newIOException(env, "The path component: '%s' in '%s' has " + "permissions 0%03o uid %ld and gid %ld. " + "It is not protected because it " + "is world-writable. This might help: 'chmod o-w %s'. " + "For more information: " + "https://wiki.apache.org/hadoop/SocketPathSecurity", + check, path, mode, (long long)st.st_uid, (long long)st.st_gid, check); goto done; } if ((mode & 0020) && (st.st_gid != 0)) { - jthr = newIOException(env, "the path component: '%s' is " - "group-writable, and the group is not root. Its permissions are " - "0%03o, and it is owned by gid %d. Please fix this or " - "select a different socket path.", check, mode, st.st_gid); + jthr = newIOException(env, "The path component: '%s' in '%s' has " + "permissions 0%03o uid %ld and gid %ld. " + "It is not protected because it " + "is group-writable and not owned by root. " + "This might help: 'chmod g-w %s' or 'chown root %s'. " + "For more information: " + "https://wiki.apache.org/hadoop/SocketPathSecurity", + check, path, mode, (long long)st.st_uid, (long long)st.st_gid, + check, check); goto done; } - if ((mode & 0200) && (st.st_uid != 0) && - (st.st_uid != uid)) { - jthr = newIOException(env, "the path component: '%s' is " - "owned by a user who is not root and not you. Your effective user " - "id is %d; the path is owned by user id %d, and its permissions are " - "0%03o. Please fix this or select a different socket path.", - check, uid, st.st_uid, mode); - goto done; + if ((mode & 0200) && (st.st_uid != 0) && (st.st_uid != uid)) { + jthr = newIOException(env, "The path component: '%s' in '%s' has " + "permissions 0%03o uid %ld and gid %ld. " + "It is not protected because it " + "is owned by a user who is not root " + "and not the effective user: '%ld'. " + "This might help: 'chown root %s' or 'chown %ld %s'. " + "For more information: " + "https://wiki.apache.org/hadoop/SocketPathSecurity", + check, path, mode, (long long)st.st_uid, (long long)st.st_gid, + (long long)uid, check, (long long)uid, check); goto done; } } done: + if ( rest_free ) free(rest_free); if (jthr) { (*env)->Throw(env, jthr); } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/unix/TestDomainSocket.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/unix/TestDomainSocket.java index 9fe8fae201a..8a5a0a42250 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/unix/TestDomainSocket.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/unix/TestDomainSocket.java @@ -712,9 +712,8 @@ public class TestDomainSocket { try { testValidateSocketPath(prefix + "/foo/bar/baz", prefix); } catch (IOException e) { - GenericTestUtils.assertExceptionContains("/foo' is world-writable. " + - "Its permissions are 0707. Please fix this or select a " + - "different socket path.", e); + GenericTestUtils.assertExceptionContains("world-writable" ,e); + GenericTestUtils.assertExceptionContains("/foo'" ,e); } try { testValidateSocketPath(prefix + "/nope", prefix); @@ -723,7 +722,7 @@ public class TestDomainSocket { "component: ", e); } // Root should be secure - DomainSocket.validateSocketPathSecurity0("/foo", 1); + DomainSocket.validateSocketPathSecurity0("/foo", 0); } finally { tmp.close(); }