You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by cn...@apache.org on 2013/12/11 18:02:34 UTC

svn commit: r1550187 - in /hadoop/common/branches/branch-1: ./ src/core/ src/core/org/apache/hadoop/fs/ src/core/org/apache/hadoop/security/ src/core/org/apache/hadoop/util/ src/test/org/apache/hadoop/security/

Author: cnauroth
Date: Wed Dec 11 17:02:34 2013
New Revision: 1550187

URL: http://svn.apache.org/r1550187
Log:
HADOOP-10142. Avoid groups lookup for unprivileged users such as "dr.who".  Contributed by vinay, backported by Xi Fang.

Modified:
    hadoop/common/branches/branch-1/CHANGES.txt
    hadoop/common/branches/branch-1/src/core/core-default.xml
    hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/CommonConfigurationKeys.java
    hadoop/common/branches/branch-1/src/core/org/apache/hadoop/security/Groups.java
    hadoop/common/branches/branch-1/src/core/org/apache/hadoop/util/StringUtils.java
    hadoop/common/branches/branch-1/src/test/org/apache/hadoop/security/TestGroupsCaching.java

Modified: hadoop/common/branches/branch-1/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1/CHANGES.txt?rev=1550187&r1=1550186&r2=1550187&view=diff
==============================================================================
--- hadoop/common/branches/branch-1/CHANGES.txt (original)
+++ hadoop/common/branches/branch-1/CHANGES.txt Wed Dec 11 17:02:34 2013
@@ -166,6 +166,9 @@ Release 1.3.0 - unreleased
     MAPRDUCE-5457. Add a KeyOnlyTextOutputReader to enable streaming to write
     out text files without separators (Sandy Ryza)
 
+    HADOOP-10142. Avoid groups lookup for unprivileged users such as "dr.who"
+    (vinay, backported by Xi Fang via cnauroth)
+
 Release 1.2.2 - unreleased
 
   INCOMPATIBLE CHANGES

Modified: hadoop/common/branches/branch-1/src/core/core-default.xml
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1/src/core/core-default.xml?rev=1550187&r1=1550186&r2=1550187&view=diff
==============================================================================
--- hadoop/common/branches/branch-1/src/core/core-default.xml (original)
+++ hadoop/common/branches/branch-1/src/core/core-default.xml Wed Dec 11 17:02:34 2013
@@ -629,4 +629,17 @@
   </description>
 </property>
 
+<property>
+  <name>hadoop.user.group.static.mapping.overrides</name>
+  <value>dr.who=;</value>
+  <description>
+    Static mapping of user to groups. This will override the groups if
+    available in the system for the specified user. In otherwords, groups
+    look-up will not happen for these users, instead groups mapped in this
+    configuration will be used.
+    Mapping should be in this format.
+    user1=group1,group2;user2=;user3=group2;
+    Default, "dr.who=;" will consider "dr.who" as user without groups.
+  </description>
+</property>
 </configuration>

Modified: hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/CommonConfigurationKeys.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/CommonConfigurationKeys.java?rev=1550187&r1=1550186&r2=1550187&view=diff
==============================================================================
--- hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/CommonConfigurationKeys.java (original)
+++ hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/CommonConfigurationKeys.java Wed Dec 11 17:02:34 2013
@@ -82,6 +82,14 @@ public class CommonConfigurationKeys {
       "hadoop.skip.worker.version.check";
   public static final boolean HADOOP_SKIP_VERSION_CHECK_DEFAULT = false;
 
+  /**
+   * User->groups static mapping to override the groups lookup
+   */
+  public static final String HADOOP_USER_GROUP_STATIC_OVERRIDES = 
+      "hadoop.user.group.static.mapping.overrides";
+  public static final String HADOOP_USER_GROUP_STATIC_OVERRIDES_DEFAULT =
+      "dr.who=;";
+
   /** Enable/Disable aliases serving from jetty */
   public static final String HADOOP_JETTY_LOGS_SERVE_ALIASES =
     "hadoop.jetty.logs.serve.aliases";

Modified: hadoop/common/branches/branch-1/src/core/org/apache/hadoop/security/Groups.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1/src/core/org/apache/hadoop/security/Groups.java?rev=1550187&r1=1550186&r2=1550187&view=diff
==============================================================================
--- hadoop/common/branches/branch-1/src/core/org/apache/hadoop/security/Groups.java (original)
+++ hadoop/common/branches/branch-1/src/core/org/apache/hadoop/security/Groups.java Wed Dec 11 17:02:34 2013
@@ -18,12 +18,17 @@
 package org.apache.hadoop.security;
 
 import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.CommonConfigurationKeys;
 import org.apache.hadoop.util.ReflectionUtils;
+import org.apache.hadoop.util.StringUtils;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -42,6 +47,8 @@ public class Groups {
   
   private final Map<String, CachedGroups> userToGroupsMap = 
     new ConcurrentHashMap<String, CachedGroups>();
+  private final Map<String, List<String>> staticUserToGroupsMap = 
+      new HashMap<String, List<String>>();
   private final long cacheTimeout;
 
   public Groups(Configuration conf) {
@@ -54,11 +61,42 @@ public class Groups {
     
     cacheTimeout = 
       conf.getLong("hadoop.security.groups.cache.secs", 5*60) * 1000;
-    
+    parseStaticMapping(conf);
+
     if(LOG.isDebugEnabled())
       LOG.debug("Group mapping impl=" + impl.getClass().getName() + 
         "; cacheTimeout=" + cacheTimeout);
   }
+
+  /*
+   * Parse the hadoop.user.group.static.mapping.overrides configuration to
+   * staticUserToGroupsMap
+   */
+  private void parseStaticMapping(Configuration conf) {
+    String staticMapping = conf.get(
+        CommonConfigurationKeys.HADOOP_USER_GROUP_STATIC_OVERRIDES,
+        CommonConfigurationKeys.HADOOP_USER_GROUP_STATIC_OVERRIDES_DEFAULT);
+    Collection<String> mappings = StringUtils.getStringCollection(
+        staticMapping, ";");
+    for (String users : mappings) {
+      Collection<String> userToGroups = StringUtils.getStringCollection(users,
+          "=");
+      if (userToGroups.size() < 1 || userToGroups.size() > 2) {
+        throw new IllegalArgumentException("Configuration "
+            + CommonConfigurationKeys.HADOOP_USER_GROUP_STATIC_OVERRIDES
+            + " is invalid");
+      }
+      String[] userToGroupsArray = userToGroups.toArray(new String[userToGroups
+          .size()]);
+      String user = userToGroupsArray[0];
+      List<String> groups = Collections.emptyList();
+      if (userToGroupsArray.length == 2) {
+        groups = (List<String>) StringUtils
+            .getStringCollection(userToGroupsArray[1]);
+      }
+      staticUserToGroupsMap.put(user, groups);
+    }
+  }
   
   /**
    * Get the group memberships of a given user.
@@ -67,6 +105,11 @@ public class Groups {
    * @throws IOException
    */
   public List<String> getGroups(String user) throws IOException {
+    // No need to lookup for groups of static users
+    List<String> staticMapping = staticUserToGroupsMap.get(user);
+    if (staticMapping != null) {
+      return staticMapping;
+    }
     // Return cached value if available
     CachedGroups groups = userToGroupsMap.get(user);
     long now = System.currentTimeMillis();

Modified: hadoop/common/branches/branch-1/src/core/org/apache/hadoop/util/StringUtils.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1/src/core/org/apache/hadoop/util/StringUtils.java?rev=1550187&r1=1550186&r2=1550187&view=diff
==============================================================================
--- hadoop/common/branches/branch-1/src/core/org/apache/hadoop/util/StringUtils.java (original)
+++ hadoop/common/branches/branch-1/src/core/org/apache/hadoop/util/StringUtils.java Wed Dec 11 17:02:34 2013
@@ -308,10 +308,24 @@ public class StringUtils {
    * @return an <code>ArrayList</code> of string values
    */
   public static Collection<String> getStringCollection(String str){
+    String delim = ",";
+    return getStringCollection(str, delim);
+  }
+
+  /**
+   * Returns a collection of strings.
+   * 
+   * @param str
+   *          String to parse
+   * @param delim
+   *          delimiter to separate the values
+   * @return Collection of parsed elements.
+   */
+  public static Collection<String> getStringCollection(String str, String delim) {
     List<String> values = new ArrayList<String>();
     if (str == null)
       return values;
-    StringTokenizer tokenizer = new StringTokenizer (str,",");
+    StringTokenizer tokenizer = new StringTokenizer(str, delim);
     values = new ArrayList<String>();
     while (tokenizer.hasMoreTokens()) {
       values.add(tokenizer.nextToken());

Modified: hadoop/common/branches/branch-1/src/test/org/apache/hadoop/security/TestGroupsCaching.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1/src/test/org/apache/hadoop/security/TestGroupsCaching.java?rev=1550187&r1=1550186&r2=1550187&view=diff
==============================================================================
--- hadoop/common/branches/branch-1/src/test/org/apache/hadoop/security/TestGroupsCaching.java (original)
+++ hadoop/common/branches/branch-1/src/test/org/apache/hadoop/security/TestGroupsCaching.java Wed Dec 11 17:02:34 2013
@@ -19,6 +19,7 @@ package org.apache.hadoop.security;
 
 import java.io.IOException;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.HashSet;
@@ -26,8 +27,10 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 
+import org.junit.Before;
 import org.junit.Test;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.fail;
 
 import org.apache.hadoop.classification.InterfaceAudience;
@@ -44,10 +47,12 @@ import org.apache.hadoop.security.Groups
 
 public class TestGroupsCaching {
   public static final Log LOG = LogFactory.getLog(TestGroupsCaching.class);
-  private static Configuration conf = new Configuration();
   private static String[] myGroups = {"grp1", "grp2"};
+  private Configuration conf;
 
-  static {
+  @Before
+  public void setup() {
+    conf = new Configuration();
     conf.setClass(CommonConfigurationKeys.HADOOP_SECURITY_GROUP_MAPPING,
       FakeGroupMapping.class,
       ShellBasedUnixGroupsMapping.class);
@@ -89,7 +94,7 @@ public class TestGroupsCaching {
   }
 
   @Test
-  public void TestGroupsCachingDefault() throws Exception {
+  public void testGroupsCachingDefault() throws Exception {
     Groups groups = new Groups(conf);
     groups.cacheGroupsAdd(Arrays.asList(myGroups));
     groups.refresh();
@@ -118,4 +123,45 @@ public class TestGroupsCaching {
     FakeGroupMapping.clearBlackList();
     assertTrue(groups.getGroups("user1").size() == 2);
   }
+
+  public static class FakeunPrivilegedGroupMapping extends FakeGroupMapping {
+    private static boolean invoked = false;
+    @Override
+    public List<String> getGroups(String user) throws IOException {
+      invoked = true;
+      return super.getGroups(user);
+    }
+  }
+
+  /*
+   * Group lookup should not happen for static users
+   */
+  @Test
+  public void testGroupLookupForStaticUsers() throws Exception {
+    conf.setClass(CommonConfigurationKeys.HADOOP_SECURITY_GROUP_MAPPING,
+        FakeunPrivilegedGroupMapping.class, ShellBasedUnixGroupsMapping.class);
+    conf.set(CommonConfigurationKeys.HADOOP_USER_GROUP_STATIC_OVERRIDES, "me=;user1=group1;user2=group1,group2");
+    Groups groups = new Groups(conf);
+    List<String> userGroups = groups.getGroups("me");
+    assertTrue("non-empty groups for static user", userGroups.isEmpty());
+    assertFalse("group lookup done for static user",
+        FakeunPrivilegedGroupMapping.invoked);
+    
+    List<String> expected = new ArrayList<String>();
+    expected.add("group1");
+
+    FakeunPrivilegedGroupMapping.invoked = false;
+    userGroups = groups.getGroups("user1");
+    assertTrue("groups not correct", expected.equals(userGroups));
+    assertFalse("group lookup done for unprivileged user",
+        FakeunPrivilegedGroupMapping.invoked);
+
+    expected.add("group2");
+    FakeunPrivilegedGroupMapping.invoked = false;
+    userGroups = groups.getGroups("user2");
+    assertTrue("groups not correct", expected.equals(userGroups));
+    assertFalse("group lookup done for unprivileged user",
+        FakeunPrivilegedGroupMapping.invoked);
+
+  }
 }