Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id A066D200B91 for ; Thu, 15 Sep 2016 06:04:42 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 9EF05160AD4; Thu, 15 Sep 2016 04:04:42 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 9995C160AB4 for ; Thu, 15 Sep 2016 06:04:41 +0200 (CEST) Received: (qmail 34834 invoked by uid 500); 15 Sep 2016 04:04:40 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 34825 invoked by uid 99); 15 Sep 2016 04:04:40 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 15 Sep 2016 04:04:40 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 7C038E008F; Thu, 15 Sep 2016 04:04:40 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: sjlee@apache.org To: common-commits@hadoop.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: hadoop git commit: Remove parent's env vars from child processes Date: Thu, 15 Sep 2016 04:04:40 +0000 (UTC) archived-at: Thu, 15 Sep 2016 04:04:42 -0000 Repository: hadoop Updated Branches: refs/heads/branch-2.6 a1272d5fc -> c3b7425bf Remove parent's env vars from child processes (cherry picked from commit b0bcb4e728e91c1c1acc514deb3f2c95626147b2) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/c3b7425b Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/c3b7425b Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/c3b7425b Branch: refs/heads/branch-2.6 Commit: c3b7425bf108a600889a4d59ae947b6f7523dcca Parents: a1272d5 Author: Robert Kanter Authored: Fri May 6 09:33:10 2016 -0700 Committer: Sangjin Lee Committed: Wed Sep 14 20:59:22 2016 -0700 ---------------------------------------------------------------------- .../main/java/org/apache/hadoop/util/Shell.java | 29 ++++++++++++- .../java/org/apache/hadoop/util/TestShell.java | 44 ++++++++++++++++++++ .../nodemanager/DefaultContainerExecutor.java | 4 +- .../nodemanager/DockerContainerExecutor.java | 8 ++-- .../nodemanager/LinuxContainerExecutor.java | 3 +- 5 files changed, 81 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/c3b7425b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java index 3aac27b..3740d51 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java @@ -32,6 +32,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.security.alias.AbstractJavaKeyStoreProvider; /** * A base class for running a Unix command. @@ -279,6 +280,8 @@ abstract public class Shell { /** If or not script timed out*/ private AtomicBoolean timedOut; + /** Indicates if the parent env vars should be inherited or not*/ + protected boolean inheritParentEnv = true; /** Centralized logic to discover and validate the sanity of the Hadoop * home directory. Returns either NULL or a directory that exists and @@ -466,6 +469,20 @@ abstract public class Shell { if (environment != null) { builder.environment().putAll(this.environment); } + + // Remove all env vars from the Builder to prevent leaking of env vars from + // the parent process. + if (!inheritParentEnv) { + // branch-2: Only do this for HADOOP_CREDSTORE_PASSWORD + // Sometimes daemons are configured to use the CredentialProvider feature + // and given their jceks password via an environment variable. We need to + // make sure to remove it so it doesn't leak to child processes, which + // might be owned by a different user. For example, the NodeManager + // running a User's container. + builder.environment().remove( + AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_NAME); + } + if (dir != null) { builder.directory(this.dir); } @@ -683,6 +700,11 @@ abstract public class Shell { this(execString, dir, env , 0L); } + public ShellCommandExecutor(String[] execString, File dir, + Map env, long timeout) { + this(execString, dir, env , timeout, true); + } + /** * Create a new instance of the ShellCommandExecutor to execute a command. * @@ -695,10 +717,12 @@ abstract public class Shell { * environment is not modified. * @param timeout Specifies the time in milliseconds, after which the * command will be killed and the status marked as timedout. - * If 0, the command will not be timed out. + * If 0, the command will not be timed out. + * @param inheritParentEnv Indicates if the process should inherit the env + * vars from the parent process or not. */ public ShellCommandExecutor(String[] execString, File dir, - Map env, long timeout) { + Map env, long timeout, boolean inheritParentEnv) { command = execString.clone(); if (dir != null) { setWorkingDirectory(dir); @@ -707,6 +731,7 @@ abstract public class Shell { setEnvironment(env); } timeOutInterval = timeout; + this.inheritParentEnv = inheritParentEnv; } http://git-wip-us.apache.org/repos/asf/hadoop/blob/c3b7425b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java index d9dc9ef..6fb1ce8 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java @@ -18,6 +18,7 @@ package org.apache.hadoop.util; import junit.framework.TestCase; +import org.apache.hadoop.security.alias.AbstractJavaKeyStoreProvider; import java.io.BufferedReader; import java.io.File; @@ -27,8 +28,13 @@ import java.io.PrintWriter; import java.lang.management.ManagementFactory; import java.lang.management.ThreadInfo; import java.lang.management.ThreadMXBean; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.apache.hadoop.fs.FileUtil; +import org.junit.Assume; +import org.junit.Test; public class TestShell extends TestCase { @@ -111,6 +117,44 @@ public class TestShell extends TestCase { shellFile.delete(); assertTrue("Script didnt not timeout" , shexc.isTimedOut()); } + + @Test + public void testEnvVarsWithInheritance() throws Exception { + Assume.assumeFalse(Shell.WINDOWS); + testEnvHelper(true); + } + + @Test + public void testEnvVarsWithoutInheritance() throws Exception { + Assume.assumeFalse(Shell.WINDOWS); + testEnvHelper(false); + } + + private void testEnvHelper(boolean inheritParentEnv) throws Exception { + Map customEnv = Collections.singletonMap( + AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_NAME, "foo"); + Shell.ShellCommandExecutor command = new Shell.ShellCommandExecutor( + new String[]{"env"}, null, customEnv, 0L, + inheritParentEnv); + command.execute(); + String[] varsArr = command.getOutput().split("\n"); + Map vars = new HashMap<>(); + for (String var : varsArr) { + int eqIndex = var.indexOf('='); + vars.put(var.substring(0, eqIndex), var.substring(eqIndex + 1)); + } + Map expectedEnv = new HashMap<>(); + expectedEnv.putAll(System.getenv()); + if (inheritParentEnv) { + expectedEnv.putAll(customEnv); + } else { + assertFalse("child process environment should not have contained " + + AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_NAME, + vars.containsKey( + AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_NAME)); + } + assertEquals(expectedEnv, vars); + } private static int countTimerThreads() { ThreadMXBean threadBean = ManagementFactory.getThreadMXBean(); http://git-wip-us.apache.org/repos/asf/hadoop/blob/c3b7425b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java index cd20449..d0f4059 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java @@ -269,7 +269,9 @@ public class DefaultContainerExecutor extends ContainerExecutor { return new ShellCommandExecutor( command, wordDir, - environment); + environment, + 0L, + false); } protected LocalWrapperScriptBuilder getLocalWrapperScriptBuilder( http://git-wip-us.apache.org/repos/asf/hadoop/blob/c3b7425b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DockerContainerExecutor.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DockerContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DockerContainerExecutor.java index 7c1947b..7c588c3 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DockerContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DockerContainerExecutor.java @@ -237,9 +237,11 @@ public class DockerContainerExecutor extends ContainerExecutor { LOG.debug("launchContainer: " + commandStr + " " + Joiner.on(" ").join(command)); } shExec = new ShellCommandExecutor( - command, - new File(containerWorkDir.toUri().getPath()), - container.getLaunchContext().getEnvironment()); // sanitized env + command, + new File(containerWorkDir.toUri().getPath()), + container.getLaunchContext().getEnvironment(), // sanitized env + 0L, + false); if (isContainerActive(containerId)) { shExec.execute(); } else { http://git-wip-us.apache.org/repos/asf/hadoop/blob/c3b7425b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java index 60e4713..b12825a 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java @@ -224,7 +224,8 @@ public class LinuxContainerExecutor extends ContainerExecutor { } buildMainArgs(command, user, appId, locId, nmAddr, localDirs); String[] commandArray = command.toArray(new String[command.size()]); - ShellCommandExecutor shExec = new ShellCommandExecutor(commandArray); + ShellCommandExecutor shExec = new ShellCommandExecutor(commandArray, + null, null, 0L, true); if (LOG.isDebugEnabled()) { LOG.debug("initApplication: " + Arrays.toString(commandArray)); } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org For additional commands, e-mail: common-commits-help@hadoop.apache.org