Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      This jira will add support for a superuser authenticating on behalf of a proxy user.

      1. HADOOP-6510.10.patch
        29 kB
        Jitendra Nath Pandey
      2. HADOOP-6510.11.patch
        29 kB
        Jitendra Nath Pandey
      3. HADOOP-6510.12.patch
        29 kB
        Jitendra Nath Pandey
      4. HADOOP-6510.14.patch
        29 kB
        Jitendra Nath Pandey
      5. HADOOP-6510.15.patch
        28 kB
        Jitendra Nath Pandey
      6. HADOOP-6510.16.patch
        28 kB
        Jitendra Nath Pandey
      7. HADOOP-6510.17.patch
        28 kB
        Jitendra Nath Pandey
      8. HADOOP-6510.18.patch
        29 kB
        Jitendra Nath Pandey
      9. HADOOP-6510.19.patch
        30 kB
        Jitendra Nath Pandey
      10. HADOOP-6510.20.patch
        30 kB
        Jitendra Nath Pandey
      11. HADOOP-6510.21.patch
        32 kB
        Jitendra Nath Pandey
      12. HADOOP-6510.23.patch
        35 kB
        Jitendra Nath Pandey
      13. HADOOP-6510.24.patch
        36 kB
        Jitendra Nath Pandey
      14. HADOOP-6510.25.patch
        39 kB
        Jitendra Nath Pandey
      15. HADOOP-6510.8.patch
        28 kB
        Jitendra Nath Pandey
      16. HADOOP-6510-0_20.4.patch
        51 kB
        Jitendra Nath Pandey

        Issue Links

          Activity

          Hide
          Jitendra Nath Pandey added a comment -

          This patch is on top of Kan's latest Sasl RPC patch (HADOOP-6419). It is not commitable until HADOOP-6419 is committed.

          Show
          Jitendra Nath Pandey added a comment - This patch is on top of Kan's latest Sasl RPC patch ( HADOOP-6419 ). It is not commitable until HADOOP-6419 is committed.
          Hide
          Owen O'Malley added a comment -

          The authorization for proxy users needs to be factored out into a new class hadoop.security.authorize.ProxyUsers that looks like:

          @InterfaceAudience.Private
          public class ProxyUsers {
            public ProxyUsers(Configuration conf) { ... }
            public void authorize(UserGroupInformation user, String remoteAddress) throws AccessControlException { ... }
          }
          

          The configuration must be done on a per-user basis:

          hadoop.proxyuser.bob.users = bobusers
          hadoop.proxyuser.bob,ip-addresses = host1, host2

          hadoop.proxyuser.owen.users = doug,sue,mygroup
          hadoop.proxyuser.owen.ip-addresses = host3

          so bob could impersonate and user in bobusers and owen could impersonate doug, sue, or users in mygroup.

          The ip-addresses can be hostnames.

          TokenIdentifier shouldn't have getRealUserName only DelegationTokenIdentifier.

          Your lines go much too long, please limit them to 80 chars.

          You need to factor out the code to create a UGI from the connection.

          Show
          Owen O'Malley added a comment - The authorization for proxy users needs to be factored out into a new class hadoop.security.authorize.ProxyUsers that looks like: @InterfaceAudience.Private public class ProxyUsers { public ProxyUsers(Configuration conf) { ... } public void authorize(UserGroupInformation user, String remoteAddress) throws AccessControlException { ... } } The configuration must be done on a per-user basis: hadoop.proxyuser.bob.users = bobusers hadoop.proxyuser.bob,ip-addresses = host1, host2 hadoop.proxyuser.owen.users = doug,sue,mygroup hadoop.proxyuser.owen.ip-addresses = host3 so bob could impersonate and user in bobusers and owen could impersonate doug, sue, or users in mygroup. The ip-addresses can be hostnames. TokenIdentifier shouldn't have getRealUserName only DelegationTokenIdentifier. Your lines go much too long, please limit them to 80 chars. You need to factor out the code to create a UGI from the connection.
          Hide
          Owen O'Malley added a comment -

          After off line discussion, I'd propose changing TokenIdentifier from:

          public abstract Text getUsername();
          

          to

          public abstract UserGroupInformation getUser();
          

          Tokens that don't have users can return null, while if it is a delegation token it can faithfully create the correct UGI.

          I'll also change my mind about mixing groups and users in the target list and say we should only allow group names.

          Show
          Owen O'Malley added a comment - After off line discussion, I'd propose changing TokenIdentifier from: public abstract Text getUsername(); to public abstract UserGroupInformation getUser(); Tokens that don't have users can return null, while if it is a delegation token it can faithfully create the correct UGI. I'll also change my mind about mixing groups and users in the target list and say we should only allow group names.
          Hide
          Jitendra Nath Pandey added a comment -

          New patch incorporating owen's suggestions.

          Show
          Jitendra Nath Pandey added a comment - New patch incorporating owen's suggestions.
          Hide
          Jitendra Nath Pandey added a comment -

          New patch on top of c6419-70.patch in HADOOP-6419

          Show
          Jitendra Nath Pandey added a comment - New patch on top of c6419-70.patch in HADOOP-6419
          Hide
          Jitendra Nath Pandey added a comment -

          HADOOP-6510.15.patch is submitted for hudson tests.

          Show
          Jitendra Nath Pandey added a comment - HADOOP-6510 .15.patch is submitted for hudson tests.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12434628/HADOOP-6510.15.patch
          against trunk revision 905860.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/319/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/319/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/319/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/319/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12434628/HADOOP-6510.15.patch against trunk revision 905860. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/319/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/319/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/319/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/319/console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12434632/HADOOP-6510.16.patch
          against trunk revision 905860.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/320/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/320/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/320/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/320/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12434632/HADOOP-6510.16.patch against trunk revision 905860. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/320/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/320/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/320/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/320/console This message is automatically generated.
          Hide
          Jitendra Nath Pandey added a comment -

          HADOOP-6510.17.patch submitted for hudson tests.

          Show
          Jitendra Nath Pandey added a comment - HADOOP-6510 .17.patch submitted for hudson tests.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12434743/HADOOP-6510.17.patch
          against trunk revision 906177.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/326/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/326/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/326/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/326/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12434743/HADOOP-6510.17.patch against trunk revision 906177. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/326/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/326/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/326/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/326/console This message is automatically generated.
          Hide
          Kan Zhang added a comment -

          Patch looks good. One question. It seems when a connection is set up and the ugi associated with the connection is of the form (B, A), where B is the effective user and A is the real user, it could mean 2 things in terms of what happened. One is that client A authenticated via Kerberos and wants to act as B. The other that is the client authenticated as B using a token that obtained by A for B. Can you confirm currently we don't have a need to differentiate these 2 cases? Also, can you please add a comment in the code saying we only allow doAs() for Kerberos authenticated clients and that's why in your patch you skip authorization based on real user if the authentication was done via DIGEST/token?

          Show
          Kan Zhang added a comment - Patch looks good. One question. It seems when a connection is set up and the ugi associated with the connection is of the form (B, A), where B is the effective user and A is the real user, it could mean 2 things in terms of what happened. One is that client A authenticated via Kerberos and wants to act as B. The other that is the client authenticated as B using a token that obtained by A for B. Can you confirm currently we don't have a need to differentiate these 2 cases? Also, can you please add a comment in the code saying we only allow doAs() for Kerberos authenticated clients and that's why in your patch you skip authorization based on real user if the authentication was done via DIGEST/token?
          Hide
          Jitendra Nath Pandey added a comment -

          >Can you confirm currently we don't have a need to differentiate these 2 cases.
          I think it is correct to have UGI (B,A) in the second case too where B authenticates with token, because it enables to keep track of user A who originally authenticated to get the token for B and is thus providing a capability to B.

          >add a comment...
          Comments added in the new patch.

          Show
          Jitendra Nath Pandey added a comment - >Can you confirm currently we don't have a need to differentiate these 2 cases. I think it is correct to have UGI (B,A) in the second case too where B authenticates with token, because it enables to keep track of user A who originally authenticated to get the token for B and is thus providing a capability to B. >add a comment... Comments added in the new patch.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12434937/HADOOP-6510.18.patch
          against trunk revision 906388.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/332/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/332/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/332/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/332/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12434937/HADOOP-6510.18.patch against trunk revision 906388. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/332/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/332/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/332/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/332/console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12435026/HADOOP-6510.19.patch
          against trunk revision 906388.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/334/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/334/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/334/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/334/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435026/HADOOP-6510.19.patch against trunk revision 906388. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/334/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/334/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/334/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/334/console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12435032/HADOOP-6510.20.patch
          against trunk revision 906388.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/335/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/335/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/335/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/335/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435032/HADOOP-6510.20.patch against trunk revision 906388. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/335/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/335/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/335/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/335/console This message is automatically generated.
          Hide
          Kan Zhang added a comment -

          patch looks good. Some nits.

          JavaDoc for some public classes and public methods are missing, e.g., ProxyUsers. createProxyUsers()'s JavaDoc doesn't match its signature. I wonder why test-patch didn't catch it.

          In ConnectionHeader.write(), you shouldn't try to search the return value of ugi.getTokens(), but use the token object associate with the connection.

          Show
          Kan Zhang added a comment - patch looks good. Some nits. JavaDoc for some public classes and public methods are missing, e.g., ProxyUsers. createProxyUsers()'s JavaDoc doesn't match its signature. I wonder why test-patch didn't catch it. In ConnectionHeader.write(), you shouldn't try to search the return value of ugi.getTokens(), but use the token object associate with the connection.
          Hide
          Kan Zhang added a comment -

          +1, pending hudson.

          Show
          Kan Zhang added a comment - +1, pending hudson.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12435044/HADOOP-6510.21.patch
          against trunk revision 906388.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/337/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/337/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/337/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/337/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435044/HADOOP-6510.21.patch against trunk revision 906388. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/337/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/337/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/337/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/337/console This message is automatically generated.
          Hide
          Devaraj Das added a comment -

          Few comments:
          1) There is a misleading comment in the testcase "//Unauthorized IP address" in TestDoAs.. should be "Authorized IP address". The testcases needs more comments.
          2) The RealUser class in UserGroupInformation should define equals/hashcode/toString and delegate them to the internal UserGroupInformation field.

          Show
          Devaraj Das added a comment - Few comments: 1) There is a misleading comment in the testcase "//Unauthorized IP address" in TestDoAs.. should be "Authorized IP address". The testcases needs more comments. 2) The RealUser class in UserGroupInformation should define equals/hashcode/toString and delegate them to the internal UserGroupInformation field.
          Hide
          Jitendra Nath Pandey added a comment -

          New patch added addressing the comments.

          Show
          Jitendra Nath Pandey added a comment - New patch added addressing the comments.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12435090/HADOOP-6510.23.patch
          against trunk revision 906388.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/339/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/339/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/339/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/339/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435090/HADOOP-6510.23.patch against trunk revision 906388. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/339/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/339/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/339/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/339/console This message is automatically generated.
          Hide
          Devaraj Das added a comment -

          +1

          Show
          Devaraj Das added a comment - +1
          Hide
          Devaraj Das added a comment -

          Ouch i think i have overlooked the way in which you handle the proxy users configuration (ProxyUsers.java). The authorization should be based on the real user. You currently have it based on the effective user. The method getProxySuperuserGroupConfKey should take the realUser. The authorize method should check if the effective user belongs to a group that the real user is authorized to act on behalf of, and fail the authorization if not. Also, the authorize method should fail the authorization if the configured value for the hadoop.proxyuser.<realuser>.users, for a given <realuser> is empty.

          Show
          Devaraj Das added a comment - Ouch i think i have overlooked the way in which you handle the proxy users configuration (ProxyUsers.java). The authorization should be based on the real user. You currently have it based on the effective user. The method getProxySuperuserGroupConfKey should take the realUser. The authorize method should check if the effective user belongs to a group that the real user is authorized to act on behalf of, and fail the authorization if not. Also, the authorize method should fail the authorization if the configured value for the hadoop.proxyuser.<realuser>.users, for a given <realuser> is empty.
          Hide
          Jitendra Nath Pandey added a comment -

          New patch added fixing the proxy user authorization.

          Show
          Jitendra Nath Pandey added a comment - New patch added fixing the proxy user authorization.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12435146/HADOOP-6510.24.patch
          against trunk revision 906388.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/340/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/340/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/340/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/340/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435146/HADOOP-6510.24.patch against trunk revision 906388. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/340/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/340/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/340/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/340/console This message is automatically generated.
          Hide
          Devaraj Das added a comment -

          Looks good.. A few fixes is still needed though:
          1) The ProxyUsers.java could have InterfaceAudience.Private for the entire class instead of doing it at the granularity of method definitions.
          2) The authorization for the IP address for superuser could be stricter. If the configured value is empty, then the authorization must fail.

          Show
          Devaraj Das added a comment - Looks good.. A few fixes is still needed though: 1) The ProxyUsers.java could have InterfaceAudience.Private for the entire class instead of doing it at the granularity of method definitions. 2) The authorization for the IP address for superuser could be stricter. If the configured value is empty, then the authorization must fail.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12435149/HADOOP-6510.25.patch
          against trunk revision 906388.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/341/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/341/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/341/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/341/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435149/HADOOP-6510.25.patch against trunk revision 906388. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/341/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/341/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/341/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/341/console This message is automatically generated.
          Hide
          Devaraj Das added a comment -

          I just committed this. Thanks, Jitendra!

          Show
          Devaraj Das added a comment - I just committed this. Thanks, Jitendra!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #158 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/158/)
          . Adds a way for superusers to impersonate other users in a secure environment. Contributed by Jitendra Nath Pandey.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #158 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/158/ ) . Adds a way for superusers to impersonate other users in a secure environment. Contributed by Jitendra Nath Pandey.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #229 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/229/)
          MAPREDUCE-1464. Makes a compatible change in JobTokenIdentifier to account for . Contributed by Jitendra Nath Pandey.

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #229 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/229/ ) MAPREDUCE-1464 . Makes a compatible change in JobTokenIdentifier to account for . Contributed by Jitendra Nath Pandey.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #243 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/243/)
          . Adds a way for superusers to impersonate other users in a secure environment. Contributed by Jitendra Nath Pandey.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #243 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/243/ ) . Adds a way for superusers to impersonate other users in a secure environment. Contributed by Jitendra Nath Pandey.
          Hide
          Jitendra Nath Pandey added a comment -

          Patch for hadoop-20 is added. It is a combined patch for HADOOP-6510, HDFS-935 & MAPREDUCE-1464.

          Show
          Jitendra Nath Pandey added a comment - Patch for hadoop-20 is added. It is a combined patch for HADOOP-6510 , HDFS-935 & MAPREDUCE-1464 .

            People

            • Assignee:
              Jitendra Nath Pandey
              Reporter:
              Jitendra Nath Pandey
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development