Repository: hadoop
Updated Branches:
refs/heads/branch-2.8 a8dadd8c5 -> cb0fccad1
YARN-5836. Malicious AM can kill containers of other apps running in any node
its containers are running. Contributed by Botong Huang
(cherry picked from commit 59bfcbf3579e45ddf96db3aafccf669c8e03648f)
Conflicts:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/BaseContainerManagerTest.java
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/cb0fccad
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/cb0fccad
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/cb0fccad
Branch: refs/heads/branch-2.8
Commit: cb0fccad19071d6f179af41a3747eb32ba003c51
Parents: a8dadd8
Author: Jason Lowe <[email protected]>
Authored: Wed Nov 16 22:43:56 2016 +0000
Committer: Jason Lowe <[email protected]>
Committed: Wed Nov 16 22:43:56 2016 +0000
----------------------------------------------------------------------
.../containermanager/ContainerManagerImpl.java | 15 ++--
.../TestContainerManagerWithLCE.java | 11 +++
.../BaseContainerManagerTest.java | 15 ++++
.../containermanager/TestContainerManager.java | 84 ++++++++++++++++----
4 files changed, 103 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/cb0fccad/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.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/containermanager/ContainerManagerImpl.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java
index 0e9d86a..57272fb 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java
@@ -1250,18 +1250,21 @@ public class ContainerManagerImpl extends
CompositeService implements
if
((!nmTokenAppId.equals(containerId.getApplicationAttemptId().getApplicationId()))
|| (container != null && !nmTokenAppId.equals(container
.getContainerId().getApplicationAttemptId().getApplicationId()))) {
+ String msg;
if (stopRequest) {
- LOG.warn(identifier.getApplicationAttemptId()
+ msg = identifier.getApplicationAttemptId()
+ " attempted to stop non-application container : "
- + container.getContainerId());
+ + containerId;
NMAuditLogger.logFailure("UnknownUser", AuditConstants.STOP_CONTAINER,
- "ContainerManagerImpl", "Trying to stop unknown container!",
- nmTokenAppId, container.getContainerId());
+ "ContainerManagerImpl", "Trying to stop unknown container!",
+ nmTokenAppId, containerId);
} else {
- LOG.warn(identifier.getApplicationAttemptId()
+ msg = identifier.getApplicationAttemptId()
+ " attempted to get status for non-application container : "
- + container.getContainerId());
+ + containerId;
}
+ LOG.warn(msg);
+ throw RPCUtil.getRemoteException(msg);
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/cb0fccad/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerManagerWithLCE.java
----------------------------------------------------------------------
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerManagerWithLCE.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerManagerWithLCE.java
index 3e00885..47024c7 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerManagerWithLCE.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerManagerWithLCE.java
@@ -179,6 +179,17 @@ public class TestContainerManagerWithLCE extends
TestContainerManager {
}
@Override
+ public void testUnauthorizedRequests() throws IOException, YarnException {
+ // Don't run the test if the binary is not available.
+ if (!shouldRunTest()) {
+ LOG.info("LCE binary path is not passed. Not running the test");
+ return;
+ }
+ LOG.info("Running testUnauthorizedRequests");
+ super.testUnauthorizedRequests();
+ }
+
+ @Override
public void testStartContainerFailureWithUnknownAuxService() throws
Exception {
// Don't run the test if the binary is not available.
if (!shouldRunTest()) {
http://git-wip-us.apache.org/repos/asf/hadoop/blob/cb0fccad/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/BaseContainerManagerTest.java
----------------------------------------------------------------------
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/BaseContainerManagerTest.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/BaseContainerManagerTest.java
index 787778e..7a5e5bb 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/BaseContainerManagerTest.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/BaseContainerManagerTest.java
@@ -40,6 +40,7 @@ import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.security.token.SecretManager.InvalidToken;
import org.apache.hadoop.yarn.api.ContainerManagementProtocol;
import org.apache.hadoop.yarn.api.protocolrecords.GetContainerStatusesRequest;
+import org.apache.hadoop.yarn.api.records.ApplicationAttemptId;
import org.apache.hadoop.yarn.api.records.ApplicationId;
import org.apache.hadoop.yarn.api.records.ContainerId;
import org.apache.hadoop.yarn.api.records.ContainerState;
@@ -345,4 +346,18 @@ public abstract class BaseContainerManagerTest {
Assert.assertEquals("ContainerState is not correct (timedout)",
finalState, currentState);
}
+
+ public static ContainerId createContainerId(int id) {
+ // Use default appId = 0
+ return createContainerId(id, 0);
+ }
+
+ public static ContainerId createContainerId(int cId, int aId) {
+ ApplicationId appId = ApplicationId.newInstance(0, aId);
+ ApplicationAttemptId appAttemptId =
+ ApplicationAttemptId.newInstance(appId, 1);
+ ContainerId containerId =
+ ContainerId.newContainerId(appAttemptId, cId);
+ return containerId;
+ }
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/cb0fccad/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java
----------------------------------------------------------------------
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java
index 72a0221..68a1b00 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java
@@ -84,6 +84,7 @@ import
org.apache.hadoop.yarn.server.nodemanager.DeletionService;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.TestAuxServices.ServiceA;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.application.ApplicationState;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.container.Container;
+import
org.apache.hadoop.yarn.server.nodemanager.containermanager.container.ContainerImpl;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainerLaunch;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer;
import
org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ResourceLocalizationService;
@@ -149,17 +150,9 @@ public class TestContainerManager extends
BaseContainerManagerTest {
.getKeyId()));
return ugi;
}
-
- @Override
- protected void authorizeGetAndStopContainerRequest(ContainerId
containerId,
- Container container, boolean stopRequest, NMTokenIdentifier
identifier) throws YarnException {
- if(container == null || container.getUser().equals("Fail")){
- throw new YarnException("Reject this container");
- }
- }
};
}
-
+
@Test
public void testContainerManagerInitialization() throws IOException {
@@ -706,13 +699,12 @@ public class TestContainerManager extends
BaseContainerManagerTest {
List<ContainerId> containerIds = new ArrayList<ContainerId>();
for (int i = 0; i < 10; i++) {
- ContainerId cId = createContainerId(i);
- String user = null;
+ ContainerId cId;
if ((i & 1) == 0) {
- // container with even id fail
- user = "Fail";
+ // Containers with even id belong to an unauthorized app
+ cId = createContainerId(i, 1);
} else {
- user = "Pass";
+ cId = createContainerId(i, 0);
}
Token containerToken =
createContainerToken(cId, DUMMY_RM_IDENTIFIER, context.getNodeId(),
@@ -744,7 +736,7 @@ public class TestContainerManager extends
BaseContainerManagerTest {
// Containers with even id should fail.
Assert.assertEquals(0, entry.getKey().getContainerId() & 1);
Assert.assertTrue(entry.getValue().getMessage()
- .contains("Reject this container"));
+ .contains("attempted to get status for non-application container"));
}
// stop containers
@@ -764,11 +756,71 @@ public class TestContainerManager extends
BaseContainerManagerTest {
// Containers with even id should fail.
Assert.assertEquals(0, entry.getKey().getContainerId() & 1);
Assert.assertTrue(entry.getValue().getMessage()
- .contains("Reject this container"));
+ .contains("attempted to stop non-application container"));
}
}
@Test
+ public void testUnauthorizedRequests() throws IOException, YarnException {
+ containerManager.start();
+
+ // Create a containerId that belongs to an unauthorized appId
+ ContainerId cId = createContainerId(0, 1);
+
+ // startContainers()
+ ContainerLaunchContext containerLaunchContext =
+ recordFactory.newRecordInstance(ContainerLaunchContext.class);
+ StartContainerRequest scRequest =
+ StartContainerRequest.newInstance(containerLaunchContext,
+ createContainerToken(cId, DUMMY_RM_IDENTIFIER, context.getNodeId(),
+ user, context.getContainerTokenSecretManager()));
+ List<StartContainerRequest> list = new ArrayList<>();
+ list.add(scRequest);
+ StartContainersRequest allRequests =
+ StartContainersRequest.newInstance(list);
+ StartContainersResponse startResponse =
+ containerManager.startContainers(allRequests);
+
+ Assert.assertFalse("Should not be authorized to start container",
+ startResponse.getSuccessfullyStartedContainers().contains(cId));
+ Assert.assertTrue("Start container request should fail",
+ startResponse.getFailedRequests().containsKey(cId));
+
+ // Insert the containerId into context, make it as if it is running
+ ContainerTokenIdentifier containerTokenIdentifier =
+
BuilderUtils.newContainerTokenIdentifier(scRequest.getContainerToken());
+ Container container = new ContainerImpl(conf, null, containerLaunchContext,
+ null, metrics, containerTokenIdentifier, context);
+ context.getContainers().put(cId, container);
+
+ // stopContainers()
+ List<ContainerId> containerIds = new ArrayList<>();
+ containerIds.add(cId);
+ StopContainersRequest stopRequest =
+ StopContainersRequest.newInstance(containerIds);
+ StopContainersResponse stopResponse =
+ containerManager.stopContainers(stopRequest);
+
+ Assert.assertFalse("Should not be authorized to stop container",
+ stopResponse.getSuccessfullyStoppedContainers().contains(cId));
+ Assert.assertTrue("Stop container request should fail",
+ stopResponse.getFailedRequests().containsKey(cId));
+
+ // getContainerStatuses()
+ containerIds = new ArrayList<>();
+ containerIds.add(cId);
+ GetContainerStatusesRequest request =
+ GetContainerStatusesRequest.newInstance(containerIds);
+ GetContainerStatusesResponse response =
+ containerManager.getContainerStatuses(request);
+
+ Assert.assertEquals("Should not be authorized to get container status",
+ response.getContainerStatuses().size(), 0);
+ Assert.assertTrue("Get status request should fail",
+ response.getFailedRequests().containsKey(cId));
+ }
+
+ @Test
public void testStartContainerFailureWithUnknownAuxService() throws
Exception {
conf.setStrings(YarnConfiguration.NM_AUX_SERVICES,
new String[] { "existService" });
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
