HDFS-1033. In secure clusters, NN and SNN should verify that the remote principal during image and edits transfer.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/hdfs/trunk@962907 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -18,6 +18,9 @@ Trunk (unreleased changes)
     HDFS-1006. getImage/putImage http requests should be https for the case 
     of security enabled. (borya and jghoman via jghoman)
 
+    HDFS-1033. In secure clusters, NN and SNN should verify that the remote 
+    principal during image and edits transfer. (jghoman)
+
   IMPROVEMENTS
 
     HDFS-1096. fix for prev. commit. (boryas)

src/java/org/apache/hadoop/hdfs/server/namenode/GetImageServlet.java

@@ -17,6 +17,11 @@
  */
 package org.apache.hadoop.hdfs.server.namenode;
 
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_KRB_HTTPS_USER_NAME_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_USER_NAME_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_SECONDARY_NAMENODE_KRB_HTTPS_USER_NAME_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_SECONDARY_NAMENODE_USER_NAME_KEY;
+
 import java.security.PrivilegedExceptionAction;
 import java.util.*;
 import java.io.*;
@@ -26,6 +31,9 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hdfs.DFSConfigKeys;
@@ -41,6 +49,8 @@ import org.apache.hadoop.util.StringUtils;
 public class GetImageServlet extends HttpServlet {
   private static final long serialVersionUID = -7669068179452648952L;
 
+  private static final Log LOG = LogFactory.getLog(GetImageServlet.class);
+
   @SuppressWarnings("unchecked")
   public void doGet(final HttpServletRequest request,
                     final HttpServletResponse response
@@ -51,9 +61,17 @@ public class GetImageServlet extends HttpServlet {
       final FSImage nnImage = (FSImage)context.getAttribute("name.system.image");
       final TransferFsImage ff = new TransferFsImage(pmap, request, response);
       final Configuration conf = (Configuration)getServletContext().getAttribute("name.conf");
+      
+      if(UserGroupInformation.isSecurityEnabled() && 
+          !isValidRequestor(request.getRemoteUser(), conf)) {
+        response.sendError(HttpServletResponse.SC_FORBIDDEN, 
+            "Only Namenode and Secondary Namenode may access this servlet");
+        LOG.warn("Received non-NN/SNN request for image or edits from " 
+            + request.getRemoteHost());
+        return;
+      }
 
-     UserGroupInformation.getCurrentUser().doAs(new PrivilegedExceptionAction<Void>() {
-
+      UserGroupInformation.getCurrentUser().doAs(new PrivilegedExceptionAction<Void>() {
         @Override
         public Void run() throws Exception {
           if (ff.getImage()) {
@@ -103,4 +121,25 @@ public class GetImageServlet extends HttpServlet {
       response.getOutputStream().close();
     }
   }
+  
+  protected boolean isValidRequestor(String remoteUser, Configuration conf) {
+    if(remoteUser == null) { // This really shouldn't happen...
+      LOG.warn("Received null remoteUser while authorizing access to getImage servlet");
+      return false;
+    }
+
+    String [] validRequestors = {conf.get(DFS_NAMENODE_KRB_HTTPS_USER_NAME_KEY),
+                                 conf.get(DFS_NAMENODE_USER_NAME_KEY),
+                                 conf.get(DFS_SECONDARY_NAMENODE_KRB_HTTPS_USER_NAME_KEY),
+                                 conf.get(DFS_SECONDARY_NAMENODE_USER_NAME_KEY) };
+
+    for(String v : validRequestors) {
+      if(v != null && v.equals(remoteUser)) {
+        if(LOG.isDebugEnabled()) LOG.debug("isValidRequestor is allowing: " + remoteUser);
+        return true;
+      }
+    }
+    if(LOG.isDebugEnabled()) LOG.debug("isValidRequestor is rejecting: " + remoteUser);
+    return false;
+  }
 }

src/java/org/apache/hadoop/hdfs/server/namenode/SecondaryNameNode.java

@@ -115,8 +115,8 @@ public class SecondaryNameNode implements Runnable {
   public SecondaryNameNode(Configuration conf)  throws IOException {
     UserGroupInformation.setConfiguration(conf);
     DFSUtil.login(conf, 
-        DFSConfigKeys.DFS_NAMENODE_KEYTAB_FILE_KEY,
-        DFSConfigKeys.DFS_NAMENODE_USER_NAME_KEY);
+        DFSConfigKeys.DFS_SECONDARY_NAMENODE_KEYTAB_FILE_KEY,
+        DFSConfigKeys.DFS_SECONDARY_NAMENODE_USER_NAME_KEY);
 
     try {
       initialize(conf);

src/test/unit/org/apache/hadoop/hdfs/server/namenode/TestGetImageServlet.java

@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdfs.server.namenode;
+
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_KRB_HTTPS_USER_NAME_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_USER_NAME_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_SECONDARY_NAMENODE_KRB_HTTPS_USER_NAME_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_SECONDARY_NAMENODE_USER_NAME_KEY;
+import static org.junit.Assert.*;
+
+import org.apache.hadoop.conf.Configuration;
+import org.junit.Test;
+
+public class TestGetImageServlet {
+  
+  // Worker class to poke the isValidRequestor method with verifying it accepts
+  // or rejects with these standard allowed principals
+  private void verifyIsValidReqBehavior(GetImageServlet gim, 
+                                        boolean shouldSucceed, String msg) {
+    final String [] validRequestors = {DFS_NAMENODE_KRB_HTTPS_USER_NAME_KEY,
+                                       DFS_NAMENODE_USER_NAME_KEY,
+                                       DFS_SECONDARY_NAMENODE_KRB_HTTPS_USER_NAME_KEY,
+                                       DFS_SECONDARY_NAMENODE_USER_NAME_KEY };
+    
+    for(String v : validRequestors) {
+      Configuration conf = new Configuration();
+      conf.set(v, "a");
+      assertEquals(msg + v, gim.isValidRequestor(shouldSucceed ? "a" : "b", conf), 
+                   shouldSucceed);
+    }
+  }
+  
+  @Test
+  public void IsValidRequestorAcceptsCorrectly() {
+    GetImageServlet gim = new GetImageServlet();
+
+    verifyIsValidReqBehavior(gim, true, 
+        "isValidRequestor has rejected a valid requestor: ");
+  }
+  
+  @Test
+  public void IsValidRequestorRejectsCorrectly() {
+    GetImageServlet gim = new GetImageServlet();
+    
+    // Don't set any valid requestors
+    assertFalse("isValidRequestor allowed a requestor despite no values being set",
+                gim.isValidRequestor("not set", new Configuration()));
+    
+    verifyIsValidReqBehavior(gim, false, 
+        "isValidRequestor has allowed an invalid requestor: ");
+  }
+
+}