HADOOP-12468. Partial group resolution failure should not result in user lockout. (Wei-Chiu Chuang via Yongjun Zhang)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/0348e769 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/0348e769 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/0348e769 Branch: refs/heads/HDFS-1312 Commit: 0348e769abc507c69d644db7bc56d31d971c51d1 Parents: d57fd18 Author: Yongjun Zhang <yzh...@cloudera.com> Authored: Wed Nov 25 18:37:52 2015 -0800 Committer: Yongjun Zhang <yzh...@cloudera.com> Committed: Wed Nov 25 18:37:52 2015 -0800 ---------------------------------------------------------------------- hadoop-common-project/hadoop-common/CHANGES.txt | 3 + .../security/ShellBasedUnixGroupsMapping.java | 181 ++++++++++++++-- .../main/java/org/apache/hadoop/util/Shell.java | 19 +- .../TestShellBasedUnixGroupsMapping.java | 213 +++++++++++++++++++ 4 files changed, 393 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/0348e769/hadoop-common-project/hadoop-common/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 2bda09c..7cdf21b 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -1377,6 +1377,9 @@ Release 2.8.0 - UNRELEASED HADOOP-12598. Add XML namespace declarations for some hadoop/tools modules. (Xin Wang via aajisaka) + HADOOP-12468. Partial group resolution failure should not result in user + lockout. (Wei-Chiu Chuang via Yongjun Zhang) + OPTIMIZATIONS HADOOP-12051. ProtobufRpcEngine.invoke() should use Exception.toString() http://git-wip-us.apache.org/repos/asf/hadoop/blob/0348e769/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java index da6e434..9b80be9 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java @@ -21,12 +21,14 @@ import java.io.IOException; import java.util.LinkedList; import java.util.List; import java.util.StringTokenizer; +import org.apache.commons.lang.StringUtils; 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.util.Shell; import org.apache.hadoop.util.Shell.ExitCodeException; +import org.apache.hadoop.util.Shell.ShellCommandExecutor; /** * A simple shell-based implementation of {@link GroupMappingServiceProvider} @@ -40,16 +42,34 @@ public class ShellBasedUnixGroupsMapping private static final Log LOG = LogFactory.getLog(ShellBasedUnixGroupsMapping.class); - + + @SuppressWarnings("serial") + private static class PartialGroupNameException extends IOException { + public PartialGroupNameException(String message) { + super(message); + } + + public PartialGroupNameException(String message, Throwable err) { + super(message, err); + } + + @Override + public String toString() { + final StringBuilder sb = + new StringBuilder("PartialGroupNameException "); + sb.append(super.getMessage()); + return sb.toString(); + } + } /** * Returns list of groups for a user * - * @param user get groups for this user + * @param userName get groups for this user * @return list of groups for a given user */ @Override - public List<String> getGroups(String user) throws IOException { - return getUnixGroups(user); + public List<String> getGroups(String userName) throws IOException { + return getUnixGroups(userName); } /** @@ -70,30 +90,52 @@ public class ShellBasedUnixGroupsMapping // does nothing in this provider of user to groups mapping } - /** + /** + * Create a ShellCommandExecutor object using the user's name. + * + * @param userName user's name + * @return a ShellCommandExecutor object + */ + protected ShellCommandExecutor createGroupExecutor(String userName) { + return new ShellCommandExecutor( + Shell.getGroupsForUserCommand(userName), null, null, 0L); + } + + /** + * Create a ShellCommandExecutor object for fetch a user's group id list. + * + * @param userName the user's name + * @return a ShellCommandExecutor object + */ + protected ShellCommandExecutor createGroupIDExecutor(String userName) { + return new ShellCommandExecutor( + Shell.getGroupsIDForUserCommand(userName), null, null, 0L); + } + + /** * Get the current user's group list from Unix by running the command 'groups' - * NOTE. For non-existing user it will return EMPTY list - * @param user user name + * NOTE. For non-existing user it will return EMPTY list. + * + * @param user get groups for this user * @return the groups list that the <code>user</code> belongs to. The primary * group is returned first. * @throws IOException if encounter any error when running the command */ - private static List<String> getUnixGroups(final String user) throws IOException { - String result = ""; + private List<String> getUnixGroups(String user) throws IOException { + ShellCommandExecutor executor = createGroupExecutor(user); + + List<String> groups; try { - result = Shell.execCommand(Shell.getGroupsForUserCommand(user)); + executor.execute(); + groups = resolveFullGroupNames(executor.getOutput()); } catch (ExitCodeException e) { - // if we didn't get the group - just return empty list; - LOG.warn("got exception trying to get groups for user " + user + ": " - + e.getMessage()); - return new LinkedList<String>(); - } - - StringTokenizer tokenizer = - new StringTokenizer(result, Shell.TOKEN_SEPARATOR_REGEX); - List<String> groups = new LinkedList<String>(); - while (tokenizer.hasMoreTokens()) { - groups.add(tokenizer.nextToken()); + try { + groups = resolvePartialGroupNames(user, e.getMessage(), + executor.getOutput()); + } catch (PartialGroupNameException pge) { + LOG.warn("unable to return groups for user " + user, pge); + return new LinkedList<>(); + } } // remove duplicated primary group @@ -108,4 +150,101 @@ public class ShellBasedUnixGroupsMapping return groups; } + + /** + * Attempt to parse group names given that some names are not resolvable. + * Use the group id list to identify those that are not resolved. + * + * @param groupNames a string representing a list of group names + * @param groupIDs a string representing a list of group ids + * @return a linked list of group names + * @throws PartialGroupNameException + */ + private List<String> parsePartialGroupNames(String groupNames, + String groupIDs) throws PartialGroupNameException { + StringTokenizer nameTokenizer = + new StringTokenizer(groupNames, Shell.TOKEN_SEPARATOR_REGEX); + StringTokenizer idTokenizer = + new StringTokenizer(groupIDs, Shell.TOKEN_SEPARATOR_REGEX); + List<String> groups = new LinkedList<String>(); + while (nameTokenizer.hasMoreTokens()) { + // check for unresolvable group names. + if (!idTokenizer.hasMoreTokens()) { + throw new PartialGroupNameException("Number of group names and ids do" + + " not match. group name =" + groupNames + ", group id = " + groupIDs); + } + String groupName = nameTokenizer.nextToken(); + String groupID = idTokenizer.nextToken(); + if (!StringUtils.isNumeric(groupName) || + !groupName.equals(groupID)) { + // if the group name is non-numeric, it is resolved. + // if the group name is numeric, but is not the same as group id, + // regard it as a group name. + // if unfortunately, some group names are not resolvable, and + // the group name is the same as the group id, regard it as not + // resolved. + groups.add(groupName); + } + } + return groups; + } + + /** + * Attempt to partially resolve group names. + * + * @param userName the user's name + * @param errMessage error message from the shell command + * @param groupNames the incomplete list of group names + * @return a list of resolved group names + * @throws PartialGroupNameException + */ + private List<String> resolvePartialGroupNames(String userName, + String errMessage, String groupNames) throws PartialGroupNameException { + // Exception may indicate that some group names are not resolvable. + // Shell-based implementation should tolerate unresolvable groups names, + // and return resolvable ones, similar to what JNI-based implementation + // does. + if (Shell.WINDOWS) { + throw new PartialGroupNameException("Does not support partial group" + + " name resolution on Windows. " + errMessage); + } + if (groupNames.isEmpty()) { + throw new PartialGroupNameException("The user name '" + userName + + "' is not found. " + errMessage); + } else { + LOG.warn("Some group names for '" + userName + "' are not resolvable. " + + errMessage); + // attempt to partially resolve group names + try { + ShellCommandExecutor exec2 = createGroupIDExecutor(userName); + exec2.execute(); + return parsePartialGroupNames(groupNames, exec2.getOutput()); + } catch (ExitCodeException ece) { + // If exception is thrown trying to get group id list, + // something is terribly wrong, so give up. + throw new PartialGroupNameException("failed to get group id list for " + + "user '" + userName + "'", ece); + } catch (IOException ioe) { + throw new PartialGroupNameException("can't execute the shell command to" + + " get the list of group id for user '" + userName + "'", ioe); + } + } + } + + /** + * Split group names into a linked list. + * + * @param groupNames a string representing the user's group names + * @return a linked list of group names + */ + private List<String> resolveFullGroupNames(String groupNames) { + StringTokenizer tokenizer = + new StringTokenizer(groupNames, Shell.TOKEN_SEPARATOR_REGEX); + List<String> groups = new LinkedList<String>(); + while (tokenizer.hasMoreTokens()) { + groups.add(tokenizer.nextToken()); + } + + return groups; + } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/0348e769/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 a451369..df9ffa7 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 @@ -185,8 +185,23 @@ public abstract class Shell { //'groups username' command return is inconsistent across different unixes return WINDOWS ? new String[] - { getWinUtilsPath(), "groups", "-F", "\"" + user + "\"" } - : new String [] {"bash", "-c", "id -gn " + user + "&& id -Gn " + user}; + {getWinUtilsPath(), "groups", "-F", "\"" + user + "\""} + : new String[] {"bash", "-c", "id -gn " + user + "; id -Gn " + user}; + } + + /** + * A command to get a given user's group id list. + * The command will get the user's primary group + * first and finally get the groups list which includes the primary group. + * i.e. the user's primary group will be included twice. + * This command does not support Windows and will only return group names. + */ + public static String[] getGroupsIDForUserCommand(final String user) { + //'groups username' command return is inconsistent across different unixes + return WINDOWS ? + new String[] + {getWinUtilsPath(), "groups", "-F", "\"" + user + "\""} + : new String[] {"bash", "-c", "id -g " + user + "; id -G " + user}; } /** A command to get a given netgroup's user list. */ http://git-wip-us.apache.org/repos/asf/hadoop/blob/0348e769/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestShellBasedUnixGroupsMapping.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestShellBasedUnixGroupsMapping.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestShellBasedUnixGroupsMapping.java new file mode 100644 index 0000000..f28cc62 --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestShellBasedUnixGroupsMapping.java @@ -0,0 +1,213 @@ +/** + * 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.security; + +import java.io.IOException; +import java.util.List; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.util.Shell.ExitCodeException; +import org.apache.hadoop.util.Shell.ShellCommandExecutor; +import org.junit.Test; +import static org.junit.Assert.*; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class TestShellBasedUnixGroupsMapping { + private static final Log LOG = + LogFactory.getLog(TestShellBasedUnixGroupsMapping.class); + + private class TestGroupUserNotExist + extends ShellBasedUnixGroupsMapping { + /** + * Create a ShellCommandExecutor object which returns exit code 1, + * emulating the case that the user does not exist. + * + * @param userName not used + * @return a mock ShellCommandExecutor object + */ + @Override + protected ShellCommandExecutor createGroupExecutor(String userName) { + ShellCommandExecutor executor = mock(ShellCommandExecutor.class); + + try { + doThrow(new ExitCodeException(1, + "id: foobarusernotexist: No such user")). + when(executor).execute(); + + when(executor.getOutput()).thenReturn(""); + } catch (IOException e) { + LOG.warn(e.getMessage()); + } + return executor; + } + } + + @Test + public void testGetGroupsNonexistentUser() throws Exception { + TestGroupUserNotExist mapping = new TestGroupUserNotExist(); + + List<String> groups = mapping.getGroups("foobarusernotexist"); + assertTrue(groups.isEmpty()); + } + + private class TestGroupNotResolvable + extends ShellBasedUnixGroupsMapping { + /** + * Create a ShellCommandExecutor object which returns partially resolved + * group names for a user. + * + * @param userName not used + * @return a mock ShellCommandExecutor object + */ + @Override + protected ShellCommandExecutor createGroupExecutor(String userName) { + ShellCommandExecutor executor = mock(ShellCommandExecutor.class); + + try { + // There is both a group name 9999 and a group ID 9999. + // This is treated as unresolvable group. + doThrow(new ExitCodeException(1, "cannot find name for group ID 9999")). + when(executor).execute(); + + when(executor.getOutput()).thenReturn("9999\n9999 abc def"); + } catch (IOException e) { + LOG.warn(e.getMessage()); + } + return executor; + } + + @Override + protected ShellCommandExecutor createGroupIDExecutor(String userName) { + ShellCommandExecutor executor = mock(ShellCommandExecutor.class); + + when(executor.getOutput()).thenReturn("9999\n9999 1 2"); + return executor; + } + } + + @Test + public void testGetGroupsNotResolvable() throws Exception { + TestGroupNotResolvable mapping = new TestGroupNotResolvable(); + + List<String> groups = mapping.getGroups("user"); + assertTrue(groups.size() == 2); + assertTrue(groups.contains("abc")); + assertTrue(groups.contains("def")); + } + + private class TestNumericGroupResolvable + extends ShellBasedUnixGroupsMapping { + /** + * Create a ShellCommandExecutor object which returns numerical group + * names of a user. + * + * @param userName not used + * @return a mock ShellCommandExecutor object + */ + @Override + protected ShellCommandExecutor createGroupExecutor(String userName) { + ShellCommandExecutor executor = mock(ShellCommandExecutor.class); + + try { + // There is a numerical group 23, but no group name 23. + // Thus 23 is treated as a resolvable group name. + doNothing().when(executor).execute(); + when(executor.getOutput()).thenReturn("23\n23 groupname zzz"); + } catch (IOException e) { + LOG.warn(e.getMessage()); + } + return executor; + } + + @Override + protected ShellCommandExecutor createGroupIDExecutor(String userName) { + ShellCommandExecutor executor = mock(ShellCommandExecutor.class); + + try { + doNothing().when(executor).execute(); + when(executor.getOutput()).thenReturn("111\n111 112 113"); + } catch (IOException e) { + LOG.warn(e.getMessage()); + } + return executor; + } + } + + @Test + public void testGetNumericGroupsResolvable() throws Exception { + TestNumericGroupResolvable mapping = new TestNumericGroupResolvable(); + + List<String> groups = mapping.getGroups("user"); + assertTrue(groups.size() == 3); + assertTrue(groups.contains("23")); + assertTrue(groups.contains("groupname")); + assertTrue(groups.contains("zzz")); + } + + private class TestGroupResolvable + extends ShellBasedUnixGroupsMapping { + /** + * Create a ShellCommandExecutor object to return the group names of a user. + * + * @param userName not used + * @return a mock ShellCommandExecutor object + */ + @Override + protected ShellCommandExecutor createGroupExecutor(String userName) { + ShellCommandExecutor executor = mock(ShellCommandExecutor.class); + + try { + doNothing().when(executor).execute(); + when(executor.getOutput()).thenReturn("abc\ndef abc hij"); + } catch (IOException e) { + LOG.warn(e.getMessage()); + } + return executor; + } + + @Override + protected ShellCommandExecutor createGroupIDExecutor(String userName) { + ShellCommandExecutor executor = mock(ShellCommandExecutor.class); + + try { + doNothing().when(executor).execute(); + when(executor.getOutput()).thenReturn("1\n1 2 3"); + } catch (IOException e) { + LOG.warn(e.getMessage()); + } + return executor; + } + } + + @Test + public void testGetGroupsResolvable() throws Exception { + TestGroupResolvable mapping = new TestGroupResolvable(); + + List<String> groups = mapping.getGroups("user"); + assertTrue(groups.size() == 3); + assertTrue(groups.contains("abc")); + assertTrue(groups.contains("def")); + assertTrue(groups.contains("hij")); + } +} + +
