hadoop-common-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zjs...@apache.org
Subject [11/50] hadoop git commit: YARN-644: Basic null check is not performed on passed in arguments before using them in ContainerManagerImpl.startContainer
Date Sat, 09 May 2015 00:42:05 GMT
YARN-644: Basic null check is not performed on passed in arguments before using them in ContainerManagerImpl.startContainer


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/5e3981a2
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/5e3981a2
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/5e3981a2

Branch: refs/heads/YARN-2928
Commit: 5e3981a212fc32a73aa53028b74fd751fa22e7a4
Parents: 30cbf95
Author: Robert (Bobby) Evans <evans@yahoo-inc.com>
Authored: Fri May 8 11:09:29 2015 -0500
Committer: Zhijie Shen <zjshen@apache.org>
Committed: Fri May 8 17:32:47 2015 -0700

----------------------------------------------------------------------
 hadoop-yarn-project/CHANGES.txt                 |  3 +
 .../containermanager/ContainerManagerImpl.java  | 27 +++++-
 .../BaseContainerManagerTest.java               |  6 ++
 .../containermanager/TestContainerManager.java  | 87 ++++++++++++++++++++
 4 files changed, 122 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/5e3981a2/hadoop-yarn-project/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt
index ab9fb2c..31f4581 100644
--- a/hadoop-yarn-project/CHANGES.txt
+++ b/hadoop-yarn-project/CHANGES.txt
@@ -183,6 +183,9 @@ Release 2.8.0 - UNRELEASED
 
   IMPROVEMENTS
 
+    YARN-644. Basic null check is not performed on passed in arguments before
+    using them in ContainerManagerImpl.startContainer (Varun Saxena via bobby)
+
     YARN-1880. Cleanup TestApplicationClientProtocolOnHA
     (ozawa via harsh)
 

http://git-wip-us.apache.org/repos/asf/hadoop/blob/5e3981a2/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 e3470e2..bf05565 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
@@ -152,6 +152,10 @@ public class ContainerManagerImpl extends CompositeService implements
 
   private static final Log LOG = LogFactory.getLog(ContainerManagerImpl.class);
 
+  static final String INVALID_NMTOKEN_MSG = "Invalid NMToken";
+  static final String INVALID_CONTAINERTOKEN_MSG =
+      "Invalid ContainerToken";
+
   final Context context;
   private final ContainersMonitor containersMonitor;
   private Server server;
@@ -643,6 +647,9 @@ public class ContainerManagerImpl extends CompositeService implements
 
   protected void authorizeUser(UserGroupInformation remoteUgi,
       NMTokenIdentifier nmTokenIdentifier) throws YarnException {
+    if (nmTokenIdentifier == null) {
+      throw RPCUtil.getRemoteException(INVALID_NMTOKEN_MSG);
+    }
     if (!remoteUgi.getUserName().equals(
       nmTokenIdentifier.getApplicationAttemptId().toString())) {
       throw RPCUtil.getRemoteException("Expected applicationAttemptId: "
@@ -660,7 +667,12 @@ public class ContainerManagerImpl extends CompositeService implements
   @VisibleForTesting
   protected void authorizeStartRequest(NMTokenIdentifier nmTokenIdentifier,
       ContainerTokenIdentifier containerTokenIdentifier) throws YarnException {
-
+    if (nmTokenIdentifier == null) {
+      throw RPCUtil.getRemoteException(INVALID_NMTOKEN_MSG);
+    }
+    if (containerTokenIdentifier == null) {
+      throw RPCUtil.getRemoteException(INVALID_CONTAINERTOKEN_MSG);
+    }
     ContainerId containerId = containerTokenIdentifier.getContainerID();
     String containerIDStr = containerId.toString();
     boolean unauthorized = false;
@@ -719,6 +731,10 @@ public class ContainerManagerImpl extends CompositeService implements
     for (StartContainerRequest request : requests.getStartContainerRequests()) {
       ContainerId containerId = null;
       try {
+        if (request.getContainerToken() == null ||
+            request.getContainerToken().getIdentifier() == null) {
+          throw new IOException(INVALID_CONTAINERTOKEN_MSG);
+        }
         ContainerTokenIdentifier containerTokenIdentifier =
             BuilderUtils.newContainerTokenIdentifier(request.getContainerToken());
         verifyAndGetContainerTokenIdentifier(request.getContainerToken(),
@@ -958,6 +974,9 @@ public class ContainerManagerImpl extends CompositeService implements
         new HashMap<ContainerId, SerializedException>();
     UserGroupInformation remoteUgi = getRemoteUgi();
     NMTokenIdentifier identifier = selectNMTokenIdentifier(remoteUgi);
+    if (identifier == null) {
+      throw RPCUtil.getRemoteException(INVALID_NMTOKEN_MSG);
+    }
     for (ContainerId id : requests.getContainerIds()) {
       try {
         stopContainerInternal(identifier, id);
@@ -1013,6 +1032,9 @@ public class ContainerManagerImpl extends CompositeService implements
         new HashMap<ContainerId, SerializedException>();
     UserGroupInformation remoteUgi = getRemoteUgi();
     NMTokenIdentifier identifier = selectNMTokenIdentifier(remoteUgi);
+    if (identifier == null) {
+      throw RPCUtil.getRemoteException(INVALID_NMTOKEN_MSG);
+    }
     for (ContainerId id : request.getContainerIds()) {
       try {
         ContainerStatus status = getContainerStatusInternal(id, identifier);
@@ -1053,6 +1075,9 @@ public class ContainerManagerImpl extends CompositeService implements
   protected void authorizeGetAndStopContainerRequest(ContainerId containerId,
       Container container, boolean stopRequest, NMTokenIdentifier identifier)
       throws YarnException {
+    if (identifier == null) {
+      throw RPCUtil.getRemoteException(INVALID_NMTOKEN_MSG);
+    }
     /*
      * For get/stop container status; we need to verify that 1) User (NMToken)
      * application attempt only has started container. 2) Requested containerId

http://git-wip-us.apache.org/repos/asf/hadoop/blob/5e3981a2/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 3fd4922..43c2193 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
@@ -230,6 +230,12 @@ public abstract class BaseContainerManagerTest {
             ByteBuffer.wrap("AuxServiceMetaData2".getBytes()));
         return serviceData;
       }
+
+      @Override
+      protected NMTokenIdentifier selectNMTokenIdentifier(
+          UserGroupInformation remoteUgi) {
+        return new NMTokenIdentifier();
+      }
     };
   }
 

http://git-wip-us.apache.org/repos/asf/hadoop/blob/5e3981a2/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 34495a2..7bdfdfb 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
@@ -45,6 +45,9 @@ import org.apache.hadoop.yarn.api.protocolrecords.StartContainersRequest;
 import org.apache.hadoop.yarn.api.protocolrecords.StartContainersResponse;
 import org.apache.hadoop.yarn.api.protocolrecords.StopContainersRequest;
 import org.apache.hadoop.yarn.api.protocolrecords.StopContainersResponse;
+import org.apache.hadoop.yarn.api.protocolrecords.impl.pb.GetContainerStatusesRequestPBImpl;
+import org.apache.hadoop.yarn.api.protocolrecords.impl.pb.StartContainersRequestPBImpl;
+import org.apache.hadoop.yarn.api.protocolrecords.impl.pb.StopContainersRequestPBImpl;
 import org.apache.hadoop.yarn.api.records.ApplicationAttemptId;
 import org.apache.hadoop.yarn.api.records.ApplicationId;
 import org.apache.hadoop.yarn.api.records.ContainerExitStatus;
@@ -83,6 +86,7 @@ import org.apache.hadoop.yarn.util.ConverterUtils;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 public class TestContainerManager extends BaseContainerManagerTest {
 
@@ -792,6 +796,89 @@ public class TestContainerManager extends BaseContainerManagerTest {
         .contains("The auxService:" + serviceName + " does not exist"));
   }
 
+  /* Test added to verify fix in YARN-644 */
+  @Test
+  public void testNullTokens() throws Exception {
+    ContainerManagerImpl cMgrImpl =
+        new ContainerManagerImpl(context, exec, delSrvc, nodeStatusUpdater,
+        metrics, new ApplicationACLsManager(conf), dirsHandler);
+    String strExceptionMsg = "";
+    try {
+      cMgrImpl.authorizeStartRequest(null, new ContainerTokenIdentifier());
+    } catch(YarnException ye) {
+      strExceptionMsg = ye.getMessage();
+    }
+    Assert.assertEquals(strExceptionMsg,
+        ContainerManagerImpl.INVALID_NMTOKEN_MSG);
+
+    strExceptionMsg = "";
+    try {
+      cMgrImpl.authorizeStartRequest(new NMTokenIdentifier(), null);
+    } catch(YarnException ye) {
+      strExceptionMsg = ye.getMessage();
+    }
+    Assert.assertEquals(strExceptionMsg,
+        ContainerManagerImpl.INVALID_CONTAINERTOKEN_MSG);
+
+    strExceptionMsg = "";
+    try {
+      cMgrImpl.authorizeGetAndStopContainerRequest(null, null, true, null);
+    } catch(YarnException ye) {
+      strExceptionMsg = ye.getMessage();
+    }
+    Assert.assertEquals(strExceptionMsg,
+        ContainerManagerImpl.INVALID_NMTOKEN_MSG);
+
+    strExceptionMsg = "";
+    try {
+      cMgrImpl.authorizeUser(null, null);
+    } catch(YarnException ye) {
+      strExceptionMsg = ye.getMessage();
+    }
+    Assert.assertEquals(strExceptionMsg,
+        ContainerManagerImpl.INVALID_NMTOKEN_MSG);
+
+    ContainerManagerImpl spyContainerMgr = Mockito.spy(cMgrImpl);
+    UserGroupInformation ugInfo = UserGroupInformation.createRemoteUser("a");
+    Mockito.when(spyContainerMgr.getRemoteUgi()).thenReturn(ugInfo);
+    Mockito.when(spyContainerMgr.
+        selectNMTokenIdentifier(ugInfo)).thenReturn(null);
+
+    strExceptionMsg = "";
+    try {
+      spyContainerMgr.stopContainers(new StopContainersRequestPBImpl());
+    } catch(YarnException ye) {
+      strExceptionMsg = ye.getMessage();
+    }
+    Assert.assertEquals(strExceptionMsg,
+        ContainerManagerImpl.INVALID_NMTOKEN_MSG);
+
+    strExceptionMsg = "";
+    try {
+      spyContainerMgr.getContainerStatuses(
+          new GetContainerStatusesRequestPBImpl());
+    } catch(YarnException ye) {
+      strExceptionMsg = ye.getMessage();
+    }
+    Assert.assertEquals(strExceptionMsg,
+        ContainerManagerImpl.INVALID_NMTOKEN_MSG);
+
+    Mockito.doNothing().when(spyContainerMgr).authorizeUser(ugInfo, null);
+    List<StartContainerRequest> reqList
+        = new ArrayList<StartContainerRequest>();
+    reqList.add(StartContainerRequest.newInstance(null, null));
+    StartContainersRequest reqs = new StartContainersRequestPBImpl();
+    reqs.setStartContainerRequests(reqList);
+    strExceptionMsg = "";
+    try {
+      spyContainerMgr.startContainers(reqs);
+    } catch(YarnException ye) {
+      strExceptionMsg = ye.getCause().getMessage();
+    }
+    Assert.assertEquals(strExceptionMsg,
+        ContainerManagerImpl.INVALID_CONTAINERTOKEN_MSG);
+  }
+
   public static Token createContainerToken(ContainerId cId, long rmIdentifier,
       NodeId nodeId, String user,
       NMContainerTokenSecretManager containerTokenSecretManager)


Mime
View raw message