Mailing-List: contact common-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-issues@hadoop.apache.org
Date: Thu, 19 Nov 2015 06:22:11 +0000 (UTC)
From: "Yongjun Zhang (JIRA)" <jira@apache.org>
To: common-issues@hadoop.apache.org
Message-ID: <JIRA.12903557.1444345609000.118518.1447914131395@Atlassian.JIRA>
In-Reply-To: <JIRA.12903557.1444345609000@Atlassian.JIRA>
References: <JIRA.12903557.1444345609000@Atlassian.JIRA>
 <JIRA.12903557.1444345609999@arcas>
Subject: [jira] [Commented] (HADOOP-12468) Partial group resolution failure
 should not result in user lockout
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/HADOOP-12468?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15012956#comment-15012956 ] 

Yongjun Zhang commented on HADOOP-12468:
----------------------------------------

HI [~jojochuang],

Thanks much for the new rev and sorry for late review. It looks good except I have some cosmetic comments:

* import statements should be alphabetically ordered
{code}
import org.apache.hadoop.util.Shell.ShellCommandExecutor;
import org.apache.commons.lang.StringUtils;
{code}

* It's better not to add new class member {{user}}, instead, use parameter passing for it, like what the current implementation does.
 
* Better names for {{createShellCommandExecutor()}} / {{createGroupIDExecutor()}} are {{createGroupExecutor()}} / {{createGroupIDExecutor()}}

* Seems to me that it's better to inline {{getUnixGroupsWithCommandExecutor()}} into {{getUnixGroups}} so not to have two methods here. 

* Shorten the leading comment in resolvePartialGroupNames to (I think we don't have to 
mention TestJNIGroupsMapping here, you may add comment to this test code if prefer):
{code}
// 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.
{code}

* Better to include the "user" info in logs, for example, add user name to the following exception (some other places too):
{code}
throw new PartialGroupNameException("failed to get group id list");
{code}

* add cause when throwing exception
change
{code}
 throw new PartialGroupNameException("failed to get group id list");
{code}
to 
{code}
 throw new PartialGroupNameException("Failed to get group id list for " + user, ece);
{code}
Similarly other places.

* Suggest to move the class {{PartialGroupNameException}} to earlier in the code instead of at the end.
* In PartialGroupNameException, do something like (refer to {{KerberosName#BadFormatString}}) :
{code}
  @SuppressWarnings("serial")
  private static class PartialGroupNameException extends IOException {
    PartialGroupNameException(String msg) {
      super(msg);
    }
    PartialGroupNameException(String msg, Throwable err) {
      super(msg, err);
    }
  }
{code}

Thanks.


> Partial group resolution failure should not result in user lockout
> ------------------------------------------------------------------
>
>                 Key: HADOOP-12468
>                 URL: https://issues.apache.org/jira/browse/HADOOP-12468
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: security
>    Affects Versions: 2.6.1
>         Environment: Linux
>            Reporter: Wei-Chiu Chuang
>            Assignee: Wei-Chiu Chuang
>            Priority: Minor
>         Attachments: HADOOP-12468.001.patch, HADOOP-12468.002.patch, HADOOP-12468.003.patch, HADOOP-12468.004.patch, HADOOP-12468.005.patch, HADOOP-12468.006.patch
>
>
> If a Hadoop cluster is configured to use ShellBasedUnixGroupsMapping for user/group name mapping, occasionally some group names may become unresolvable (for example, using SSSD). 
> ShellBasedUnixGroupsMapping uses shell command "id -Gn" to retrieve the group name of a user; however, the existing logic assumes that if the exit code of the command is non-zero, the user has no group name at all. The shell command in Linux returns non-zero exit code if a group name is not resolvable. Unfortunately, it is possible that a user belongs to multiple groups, and any partial failure in group name resolution would denied the user's access.
> On the other hand, the JNI implementation (JniBasedUnixGroupsMapping) is more resilient. If any group name is unresolvable, it is simply ignored, and whatever are resolvable are returned.
> It is arguable that if the group name is not resolvable, the administrator should configure their directory/authentication service correctly, and Hadoop is in no position to handle it, but since the existing unit tests assume the output of JNI-based and shell-based implementation are the same, we should improve the shell-based group name resolution, and make it as resilient as the JNI-based one.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)