You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Gary Helmling <gh...@gmail.com> on 2010/07/29 21:40:06 UTC

Review Request: HBASE-2742, HBASE-2016: Port of secure Hadoop RPC changes and integration with HBase RPC protocols

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/406/
-----------------------------------------------------------

Review request for hbase.


Summary
-------

This patch ports over the secure Hadoop RPC changes from the latest Yahoo 0.20 based branch (yahoo-hadoop-0.20.104).  This patch is produced against HBase trunk, but is targeted as the first step in a "security" feature branch for a full role-based access control implementation (HBASE-1697).

RPC Changes
--------------------
The primary changes are updates from the classes:
org.apache.hadoop.ipc.Client -> org.apache.hadoop.hbase.ipc.HBaseClient
org.apache.hadoop.ipc.RPC -> org.apache.hadoop.hbase.ipc.HBaseRPC
org.apache.hadoop.ipc.Server -> org.apache.hadoop.hbase.ipc.HBaseServer

The new classes were also ported:
org.apache.hadoop.hbase.security.HBaseSaslRpcClient
org.apache.hadoop.hbase.security.HBaseSaslRpcServer

Due to type dependencies on the Hadoop RPC classes, the original Hadoop SaslRpc* classes could not be used.

The RPC port provides client authentication via Kerberos, and SASL negotiation of client server connections for mutual authentication and optionally encryption, so it also provides the authentication functionality for HBASE-2016.  The ported RPC code contains dependencies on other classes in secure Hadoop/Hadoop trunk, preventing it from currently running on 0.20 branches missing the security changes.

Process Authentication
---------------------------
The HMaster and HRegionServer processes have been updated to allow configuration of the Kerberos principals used to run the processes.  The new configuration parameters are:

* hbase.master.keytab.file - Path to the keytab file containing the master principal's credentials
* hbase.master.kerberos.principal - Kerberos principal name used to login the HMaster process
* hbase.master.kerberos.https.principal - Kerberos principal name used to login the HMaster info server
* hbase.regionserver.keytab.file - Path to the keytab file containing the region server's credentials
* hbase.regionserver.kerberos.principal - Kerberos principal name used to login the HRegionServer process
* hbase.regionserver.kerberos.https.principal - Kerberos principal name used to login the HRegionServer info server

The new class org.apache.hadoop.hbase.security.HBasePolicyProvider and new file conf/hadoop-policy.xml allow restriction of the users and groups permitting to utilize each of the RPC protocol interfaces (HMasterInterface, HMasterRegionInterface, HRegionInterface).

Testing Updates
--------------------
Parts of the test code (org.apache.hadoop.hbase.HBaseTestingUtility and org.apache.hadoop.hbase.MiniHBaseCluster) were directly using the internal Hadoop UnixUserGroupInformation class to manipulate process ownership for testing.  These have been updated to use UserGroupInformation.doAs() instead.


This addresses bugs HBASE-2016 and HBASE-2742.
    http://issues.apache.org/jira/browse/HBASE-2016
    http://issues.apache.org/jira/browse/HBASE-2742


Diffs
-----

  conf/hadoop-policy.xml PRE-CREATION 
  pom.xml 2d3d75a 
  src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 2b5eeb6 
  src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 9873172 
  src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java d88c12d 
  src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
  src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java bd48a4b 
  src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java 71a0447 
  src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1157fe1 
  src/main/java/org/apache/hadoop/hbase/ipc/Status.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/master/HMaster.java e4bd30d 
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6a54736 
  src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcServer.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 280b91d 
  src/main/resources/hbase-default.xml e3a9669 
  src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 4d09fe9 
  src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 9c49e36 
  src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 0b47975 
  src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java c982662 

Diff: http://review.cloudera.org/r/406/diff


Testing
-------


Thanks,

Gary


Re: Review Request: HBASE-2742, HBASE-2016: Port of secure Hadoop RPC changes and integration with HBase RPC protocols

Posted by Gary Helmling <gh...@gmail.com>.

> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > pom.xml, line 480
> > <http://review.cloudera.org/r/406/diff/2/?file=3873#file3873line480>
> >
> >     is this build up somewhere? or you have to mvn install it locally?

Currently you have to mvn install it locally (from http://github.com/apurtell/hadoop-common), but I'll add a public mvn repo with it to ease building/testing.


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java, line 812
> > <http://review.cloudera.org/r/406/diff/2/?file=3878#file3878line812>
> >
> >     typo, but maybe also in hadoop

Copied from Hadoop, but private so seems safe to fix.


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java, line 437
> > <http://review.cloudera.org/r/406/diff/2/?file=3878#file3878line437>
> >
> >     hrm, not sure if this was just a copy from hadoop, but you added the argument e but doesn't seem to be used?

Copied from Hadoop, will clean up.


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java, line 266
> > <http://review.cloudera.org/r/406/diff/2/?file=3891#file3891line266>
> >
> >     maybe this should be part of addRegionServer?

Makes sense, I can clean up the doAs() part.  I did want to keep the test user creation separate though, since it's testing specific and LocalHBaseCluster is used in standalone mode.


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java, line 1337
> > <http://review.cloudera.org/r/406/diff/2/?file=3878#file3878line1337>
> >
> >     these are HBase todos? isn't the doAs important so that further RPCs sent by the handling of this call get done as the user, etc?

Yes, these are HBase TODO's.  Will clarify the comments.  We definitely need to maintain the caller's UGI in the execution context for servicing this RPC and would probably need to pass it through for any additional HBase RPCs that it generates.  That'll be one of the primary parts of the RBAC implementation (lumped under HBASE-1697).

But how this maps to HBase interaction as client of HDFS is still up in the air.  The phase one plan was to have HBase run as it's own server principal from the HDFS perspective, maintaining ownership of all the individual store files.  There's been some thinking about how we provide ownership (and isolation) down to the HDFS layer as a phase two, but that requires working around the HDFS lack of ACLs and has some trade-offs for the overall security picture (insulates you from defects in the HBase implementation but introduces need to pass around, and possibly store, client credentials).


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java, line 127
> > <http://review.cloudera.org/r/406/diff/2/?file=3878#file3878line127>
> >
> >     maybe not a good idea to default the max response size to 1M in hbase?

Good point.  This impacts the HBaseServer.Handler output buffer size (if exceeded it will re-instantiate the buffer with the initial size again), so trades off used heap vs. ByteArrayOutputStream instantiations.  And for large numbers of handlers of course this can consume significant heap.

Thoughts on a more appropriate size for standard HBase requests?  64k? 128k?


- Gary


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/406/#review592
-----------------------------------------------------------


On 2010-07-29 12:40:06, Gary Helmling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/406/
> -----------------------------------------------------------
> 
> (Updated 2010-07-29 12:40:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> This patch ports over the secure Hadoop RPC changes from the latest Yahoo 0.20 based branch (yahoo-hadoop-0.20.104).  This patch is produced against HBase trunk, but is targeted as the first step in a "security" feature branch for a full role-based access control implementation (HBASE-1697).
> 
> RPC Changes
> --------------------
> The primary changes are updates from the classes:
> org.apache.hadoop.ipc.Client -> org.apache.hadoop.hbase.ipc.HBaseClient
> org.apache.hadoop.ipc.RPC -> org.apache.hadoop.hbase.ipc.HBaseRPC
> org.apache.hadoop.ipc.Server -> org.apache.hadoop.hbase.ipc.HBaseServer
> 
> The new classes were also ported:
> org.apache.hadoop.hbase.security.HBaseSaslRpcClient
> org.apache.hadoop.hbase.security.HBaseSaslRpcServer
> 
> Due to type dependencies on the Hadoop RPC classes, the original Hadoop SaslRpc* classes could not be used.
> 
> The RPC port provides client authentication via Kerberos, and SASL negotiation of client server connections for mutual authentication and optionally encryption, so it also provides the authentication functionality for HBASE-2016.  The ported RPC code contains dependencies on other classes in secure Hadoop/Hadoop trunk, preventing it from currently running on 0.20 branches missing the security changes.
> 
> Process Authentication
> ---------------------------
> The HMaster and HRegionServer processes have been updated to allow configuration of the Kerberos principals used to run the processes.  The new configuration parameters are:
> 
> * hbase.master.keytab.file - Path to the keytab file containing the master principal's credentials
> * hbase.master.kerberos.principal - Kerberos principal name used to login the HMaster process
> * hbase.master.kerberos.https.principal - Kerberos principal name used to login the HMaster info server
> * hbase.regionserver.keytab.file - Path to the keytab file containing the region server's credentials
> * hbase.regionserver.kerberos.principal - Kerberos principal name used to login the HRegionServer process
> * hbase.regionserver.kerberos.https.principal - Kerberos principal name used to login the HRegionServer info server
> 
> The new class org.apache.hadoop.hbase.security.HBasePolicyProvider and new file conf/hadoop-policy.xml allow restriction of the users and groups permitting to utilize each of the RPC protocol interfaces (HMasterInterface, HMasterRegionInterface, HRegionInterface).
> 
> Testing Updates
> --------------------
> Parts of the test code (org.apache.hadoop.hbase.HBaseTestingUtility and org.apache.hadoop.hbase.MiniHBaseCluster) were directly using the internal Hadoop UnixUserGroupInformation class to manipulate process ownership for testing.  These have been updated to use UserGroupInformation.doAs() instead.
> 
> 
> This addresses bugs HBASE-2016 and HBASE-2742.
>     http://issues.apache.org/jira/browse/HBASE-2016
>     http://issues.apache.org/jira/browse/HBASE-2742
> 
> 
> Diffs
> -----
> 
>   conf/hadoop-policy.xml PRE-CREATION 
>   pom.xml 2d3d75a 
>   src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 2b5eeb6 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 9873172 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java d88c12d 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java bd48a4b 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java 71a0447 
>   src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1157fe1 
>   src/main/java/org/apache/hadoop/hbase/ipc/Status.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java e4bd30d 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6a54736 
>   src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcServer.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 280b91d 
>   src/main/resources/hbase-default.xml e3a9669 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 4d09fe9 
>   src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 9c49e36 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 0b47975 
>   src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java c982662 
> 
> Diff: http://review.cloudera.org/r/406/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gary
> 
>


Re: Review Request: HBASE-2742, HBASE-2016: Port of secure Hadoop RPC changes and integration with HBase RPC protocols

Posted by Todd Lipcon <to...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/406/#review592
-----------------------------------------------------------

Ship it!



pom.xml
<http://review.cloudera.org/r/406/#comment2258>

    is this build up somewhere? or you have to mvn install it locally?



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/406/#comment2259>

    maybe not a good idea to default the max response size to 1M in hbase?



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/406/#comment2260>

    hrm, not sure if this was just a copy from hadoop, but you added the argument e but doesn't seem to be used?



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/406/#comment2261>

    typo, but maybe also in hadoop



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/406/#comment2264>

    these are HBase todos? isn't the doAs important so that further RPCs sent by the handling of this call get done as the user, etc?



src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java
<http://review.cloudera.org/r/406/#comment2267>

    license



src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java
<http://review.cloudera.org/r/406/#comment2268>

    maybe this should be part of addRegionServer?


- Todd


On 2010-07-29 12:40:06, Gary Helmling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/406/
> -----------------------------------------------------------
> 
> (Updated 2010-07-29 12:40:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> This patch ports over the secure Hadoop RPC changes from the latest Yahoo 0.20 based branch (yahoo-hadoop-0.20.104).  This patch is produced against HBase trunk, but is targeted as the first step in a "security" feature branch for a full role-based access control implementation (HBASE-1697).
> 
> RPC Changes
> --------------------
> The primary changes are updates from the classes:
> org.apache.hadoop.ipc.Client -> org.apache.hadoop.hbase.ipc.HBaseClient
> org.apache.hadoop.ipc.RPC -> org.apache.hadoop.hbase.ipc.HBaseRPC
> org.apache.hadoop.ipc.Server -> org.apache.hadoop.hbase.ipc.HBaseServer
> 
> The new classes were also ported:
> org.apache.hadoop.hbase.security.HBaseSaslRpcClient
> org.apache.hadoop.hbase.security.HBaseSaslRpcServer
> 
> Due to type dependencies on the Hadoop RPC classes, the original Hadoop SaslRpc* classes could not be used.
> 
> The RPC port provides client authentication via Kerberos, and SASL negotiation of client server connections for mutual authentication and optionally encryption, so it also provides the authentication functionality for HBASE-2016.  The ported RPC code contains dependencies on other classes in secure Hadoop/Hadoop trunk, preventing it from currently running on 0.20 branches missing the security changes.
> 
> Process Authentication
> ---------------------------
> The HMaster and HRegionServer processes have been updated to allow configuration of the Kerberos principals used to run the processes.  The new configuration parameters are:
> 
> * hbase.master.keytab.file - Path to the keytab file containing the master principal's credentials
> * hbase.master.kerberos.principal - Kerberos principal name used to login the HMaster process
> * hbase.master.kerberos.https.principal - Kerberos principal name used to login the HMaster info server
> * hbase.regionserver.keytab.file - Path to the keytab file containing the region server's credentials
> * hbase.regionserver.kerberos.principal - Kerberos principal name used to login the HRegionServer process
> * hbase.regionserver.kerberos.https.principal - Kerberos principal name used to login the HRegionServer info server
> 
> The new class org.apache.hadoop.hbase.security.HBasePolicyProvider and new file conf/hadoop-policy.xml allow restriction of the users and groups permitting to utilize each of the RPC protocol interfaces (HMasterInterface, HMasterRegionInterface, HRegionInterface).
> 
> Testing Updates
> --------------------
> Parts of the test code (org.apache.hadoop.hbase.HBaseTestingUtility and org.apache.hadoop.hbase.MiniHBaseCluster) were directly using the internal Hadoop UnixUserGroupInformation class to manipulate process ownership for testing.  These have been updated to use UserGroupInformation.doAs() instead.
> 
> 
> This addresses bugs HBASE-2016 and HBASE-2742.
>     http://issues.apache.org/jira/browse/HBASE-2016
>     http://issues.apache.org/jira/browse/HBASE-2742
> 
> 
> Diffs
> -----
> 
>   conf/hadoop-policy.xml PRE-CREATION 
>   pom.xml 2d3d75a 
>   src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 2b5eeb6 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 9873172 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java d88c12d 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java bd48a4b 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java 71a0447 
>   src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1157fe1 
>   src/main/java/org/apache/hadoop/hbase/ipc/Status.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java e4bd30d 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6a54736 
>   src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcServer.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 280b91d 
>   src/main/resources/hbase-default.xml e3a9669 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 4d09fe9 
>   src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 9c49e36 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 0b47975 
>   src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java c982662 
> 
> Diff: http://review.cloudera.org/r/406/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gary
> 
>


Re: Review Request: HBASE-2742, HBASE-2016: Port of secure Hadoop RPC changes and integration with HBase RPC protocols

Posted by Andrew Purtell <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/406/#review586
-----------------------------------------------------------

Ship it!


It looks all quite straightforward.

Only quibble is the '-S' suffix on the HBase version. At first I thought it was typo, could possibly lead to confusion.

- Andrew


On 2010-07-29 12:40:06, Gary Helmling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/406/
> -----------------------------------------------------------
> 
> (Updated 2010-07-29 12:40:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> This patch ports over the secure Hadoop RPC changes from the latest Yahoo 0.20 based branch (yahoo-hadoop-0.20.104).  This patch is produced against HBase trunk, but is targeted as the first step in a "security" feature branch for a full role-based access control implementation (HBASE-1697).
> 
> RPC Changes
> --------------------
> The primary changes are updates from the classes:
> org.apache.hadoop.ipc.Client -> org.apache.hadoop.hbase.ipc.HBaseClient
> org.apache.hadoop.ipc.RPC -> org.apache.hadoop.hbase.ipc.HBaseRPC
> org.apache.hadoop.ipc.Server -> org.apache.hadoop.hbase.ipc.HBaseServer
> 
> The new classes were also ported:
> org.apache.hadoop.hbase.security.HBaseSaslRpcClient
> org.apache.hadoop.hbase.security.HBaseSaslRpcServer
> 
> Due to type dependencies on the Hadoop RPC classes, the original Hadoop SaslRpc* classes could not be used.
> 
> The RPC port provides client authentication via Kerberos, and SASL negotiation of client server connections for mutual authentication and optionally encryption, so it also provides the authentication functionality for HBASE-2016.  The ported RPC code contains dependencies on other classes in secure Hadoop/Hadoop trunk, preventing it from currently running on 0.20 branches missing the security changes.
> 
> Process Authentication
> ---------------------------
> The HMaster and HRegionServer processes have been updated to allow configuration of the Kerberos principals used to run the processes.  The new configuration parameters are:
> 
> * hbase.master.keytab.file - Path to the keytab file containing the master principal's credentials
> * hbase.master.kerberos.principal - Kerberos principal name used to login the HMaster process
> * hbase.master.kerberos.https.principal - Kerberos principal name used to login the HMaster info server
> * hbase.regionserver.keytab.file - Path to the keytab file containing the region server's credentials
> * hbase.regionserver.kerberos.principal - Kerberos principal name used to login the HRegionServer process
> * hbase.regionserver.kerberos.https.principal - Kerberos principal name used to login the HRegionServer info server
> 
> The new class org.apache.hadoop.hbase.security.HBasePolicyProvider and new file conf/hadoop-policy.xml allow restriction of the users and groups permitting to utilize each of the RPC protocol interfaces (HMasterInterface, HMasterRegionInterface, HRegionInterface).
> 
> Testing Updates
> --------------------
> Parts of the test code (org.apache.hadoop.hbase.HBaseTestingUtility and org.apache.hadoop.hbase.MiniHBaseCluster) were directly using the internal Hadoop UnixUserGroupInformation class to manipulate process ownership for testing.  These have been updated to use UserGroupInformation.doAs() instead.
> 
> 
> This addresses bugs HBASE-2016 and HBASE-2742.
>     http://issues.apache.org/jira/browse/HBASE-2016
>     http://issues.apache.org/jira/browse/HBASE-2742
> 
> 
> Diffs
> -----
> 
>   conf/hadoop-policy.xml PRE-CREATION 
>   pom.xml 2d3d75a 
>   src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 2b5eeb6 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 9873172 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java d88c12d 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java bd48a4b 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java 71a0447 
>   src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1157fe1 
>   src/main/java/org/apache/hadoop/hbase/ipc/Status.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java e4bd30d 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6a54736 
>   src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcServer.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 280b91d 
>   src/main/resources/hbase-default.xml e3a9669 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 4d09fe9 
>   src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 9c49e36 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 0b47975 
>   src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java c982662 
> 
> Diff: http://review.cloudera.org/r/406/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gary
> 
>


Re: Review Request: HBASE-2742, HBASE-2016: Port of secure Hadoop RPC changes and integration with HBase RPC protocols

Posted by Gary Helmling <gh...@gmail.com>.

> On 2010-08-04 22:40:35, stack wrote:
> > conf/hadoop-policy.xml, line 1
> > <http://review.cloudera.org/r/406/diff/2/?file=3872#file3872line1>
> >
> >     Should it be named hbase-policy.xml or is it a case of the name of the file being hard-coded (as it is for metrics config -- see hadoop-metrics.xml in the hbase/conf dir).

The default is down in org.apache.hadoop.security.authorize.ServiceAuthorizationManager, but looks like we can override it with "hadoop.policy.file" in config.  I'll do that and update.


> On 2010-08-04 22:40:35, stack wrote:
> > pom.xml, line 480
> > <http://review.cloudera.org/r/406/diff/2/?file=3873#file3873line480>
> >
> >     How much of this patch do you want to commit to TRUNK Gary? Would hbase 0.90.0 have a dependency on secure hadoop and if so are there implications (performance?) or are you thinking it secure hbase a separate offering, somehow?

At the moment, none of this is really eligible for trunk, due to dependency on secure hadoop.  I'm looking at enabling pluggable RPC in HBASE-2321 as a way to introduce secure RPC without the hard dependency.  Don't know how messy that will get, but I should have a good idea in the next few days.

The idea would be to make secure RPC a conditional build target and enable optional configuration of it as the RPC implementation, so that it doesn't block the ability of HBase to run on Hadoop 0.20 versions without the security features.

As long as that proves out, I would use HBASE-2321 as the framework for pluggable RPC, with a new JIRA for the secure RPC plugin.


> On 2010-08-04 22:40:35, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java, line 85
> > <http://review.cloudera.org/r/406/diff/2/?file=3888#file3888line85>
> >
> >     What changed in here?

Looks like just a whitespace change, I'll remove.  In one of the intermediate changes I had wrapped this in UserGroupInformation.doAs() to enable execution of RS's as different users in LocalHBaseCluster (since some tests require this).  But I would up moving out to MiniHBaseCluster to keep it isolated to test cases.


> On 2010-08-04 22:40:35, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java, line 116
> > <http://review.cloudera.org/r/406/diff/2/?file=3875#file3875line116>
> >
> >     Man, I hate this final stuff (I know its not you Gary.... you're just bringing it over... thats fine... but I hate it anyways).

Understood and I don't disagree. :)

Most of this RPC code is not very extensible and sometimes serves a common case, but sometimes frustrates it.  Maybe HBASE-2321 can help towards trying alternatives.  


> On 2010-08-04 22:40:35, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java, line 1
> > <http://review.cloudera.org/r/406/diff/2/?file=3874#file3874line1>
> >
> >     If we are running in insecure mode, does this thing send the only single-byte or whatever it was identifying the rpc?  Or, to ask in another way, does insecure rpc have same character with this patch as it does w/o it?

This does change the over the wire format for both secure and insecure cases in an incompatible way.

Existing format is:
"hrpc"
version (3)
UGI ticket
  "org.apache.hadoop.security.UserGroupInformation"
  "org.apache.hadoop.security.UnixUserGroupInformation"
  "STRING_UGI"
  username
  group names length
    each group name

However the UGI ticket always seems to be null so none of that is passed.


New format is:

"hrpc"
version (4)
auth method (single byte code)
short class name (HMasterInterface, HRegionInterface, HMasterRegionInterface)
username present? (true|false)
+ if true
  username
  real username present? (true|false)
  + if true
    real username


- Gary


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/406/#review680
-----------------------------------------------------------


On 2010-07-29 12:40:06, Gary Helmling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/406/
> -----------------------------------------------------------
> 
> (Updated 2010-07-29 12:40:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> This patch ports over the secure Hadoop RPC changes from the latest Yahoo 0.20 based branch (yahoo-hadoop-0.20.104).  This patch is produced against HBase trunk, but is targeted as the first step in a "security" feature branch for a full role-based access control implementation (HBASE-1697).
> 
> RPC Changes
> --------------------
> The primary changes are updates from the classes:
> org.apache.hadoop.ipc.Client -> org.apache.hadoop.hbase.ipc.HBaseClient
> org.apache.hadoop.ipc.RPC -> org.apache.hadoop.hbase.ipc.HBaseRPC
> org.apache.hadoop.ipc.Server -> org.apache.hadoop.hbase.ipc.HBaseServer
> 
> The new classes were also ported:
> org.apache.hadoop.hbase.security.HBaseSaslRpcClient
> org.apache.hadoop.hbase.security.HBaseSaslRpcServer
> 
> Due to type dependencies on the Hadoop RPC classes, the original Hadoop SaslRpc* classes could not be used.
> 
> The RPC port provides client authentication via Kerberos, and SASL negotiation of client server connections for mutual authentication and optionally encryption, so it also provides the authentication functionality for HBASE-2016.  The ported RPC code contains dependencies on other classes in secure Hadoop/Hadoop trunk, preventing it from currently running on 0.20 branches missing the security changes.
> 
> Process Authentication
> ---------------------------
> The HMaster and HRegionServer processes have been updated to allow configuration of the Kerberos principals used to run the processes.  The new configuration parameters are:
> 
> * hbase.master.keytab.file - Path to the keytab file containing the master principal's credentials
> * hbase.master.kerberos.principal - Kerberos principal name used to login the HMaster process
> * hbase.master.kerberos.https.principal - Kerberos principal name used to login the HMaster info server
> * hbase.regionserver.keytab.file - Path to the keytab file containing the region server's credentials
> * hbase.regionserver.kerberos.principal - Kerberos principal name used to login the HRegionServer process
> * hbase.regionserver.kerberos.https.principal - Kerberos principal name used to login the HRegionServer info server
> 
> The new class org.apache.hadoop.hbase.security.HBasePolicyProvider and new file conf/hadoop-policy.xml allow restriction of the users and groups permitting to utilize each of the RPC protocol interfaces (HMasterInterface, HMasterRegionInterface, HRegionInterface).
> 
> Testing Updates
> --------------------
> Parts of the test code (org.apache.hadoop.hbase.HBaseTestingUtility and org.apache.hadoop.hbase.MiniHBaseCluster) were directly using the internal Hadoop UnixUserGroupInformation class to manipulate process ownership for testing.  These have been updated to use UserGroupInformation.doAs() instead.
> 
> 
> This addresses bugs HBASE-2016 and HBASE-2742.
>     http://issues.apache.org/jira/browse/HBASE-2016
>     http://issues.apache.org/jira/browse/HBASE-2742
> 
> 
> Diffs
> -----
> 
>   conf/hadoop-policy.xml PRE-CREATION 
>   pom.xml 2d3d75a 
>   src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 2b5eeb6 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 9873172 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java d88c12d 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java bd48a4b 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java 71a0447 
>   src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1157fe1 
>   src/main/java/org/apache/hadoop/hbase/ipc/Status.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java e4bd30d 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6a54736 
>   src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcServer.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 280b91d 
>   src/main/resources/hbase-default.xml e3a9669 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 4d09fe9 
>   src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 9c49e36 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 0b47975 
>   src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java c982662 
> 
> Diff: http://review.cloudera.org/r/406/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gary
> 
>


Re: Review Request: HBASE-2742, HBASE-2016: Port of secure Hadoop RPC changes and integration with HBase RPC protocols

Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/406/#review680
-----------------------------------------------------------


Looks good to me.  See below for questions on what to do w/ this patch.


conf/hadoop-policy.xml
<http://review.cloudera.org/r/406/#comment2468>

    Should it be named hbase-policy.xml or is it a case of the name of the file being hard-coded (as it is for metrics config -- see hadoop-metrics.xml in the hbase/conf dir).



pom.xml
<http://review.cloudera.org/r/406/#comment2467>

    How much of this patch do you want to commit to TRUNK Gary? Would hbase 0.90.0 have a dependency on secure hadoop and if so are there implications (performance?) or are you thinking it secure hbase a separate offering, somehow?



src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java
<http://review.cloudera.org/r/406/#comment2469>

    If we are running in insecure mode, does this thing send the only single-byte or whatever it was identifying the rpc?  Or, to ask in another way, does insecure rpc have same character with this patch as it does w/o it?



src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java
<http://review.cloudera.org/r/406/#comment2470>

    Man, I hate this final stuff (I know its not you Gary.... you're just bringing it over... thats fine... but I hate it anyways).



src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java
<http://review.cloudera.org/r/406/#comment2471>

    What changed in here?


- stack


On 2010-07-29 12:40:06, Gary Helmling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/406/
> -----------------------------------------------------------
> 
> (Updated 2010-07-29 12:40:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> This patch ports over the secure Hadoop RPC changes from the latest Yahoo 0.20 based branch (yahoo-hadoop-0.20.104).  This patch is produced against HBase trunk, but is targeted as the first step in a "security" feature branch for a full role-based access control implementation (HBASE-1697).
> 
> RPC Changes
> --------------------
> The primary changes are updates from the classes:
> org.apache.hadoop.ipc.Client -> org.apache.hadoop.hbase.ipc.HBaseClient
> org.apache.hadoop.ipc.RPC -> org.apache.hadoop.hbase.ipc.HBaseRPC
> org.apache.hadoop.ipc.Server -> org.apache.hadoop.hbase.ipc.HBaseServer
> 
> The new classes were also ported:
> org.apache.hadoop.hbase.security.HBaseSaslRpcClient
> org.apache.hadoop.hbase.security.HBaseSaslRpcServer
> 
> Due to type dependencies on the Hadoop RPC classes, the original Hadoop SaslRpc* classes could not be used.
> 
> The RPC port provides client authentication via Kerberos, and SASL negotiation of client server connections for mutual authentication and optionally encryption, so it also provides the authentication functionality for HBASE-2016.  The ported RPC code contains dependencies on other classes in secure Hadoop/Hadoop trunk, preventing it from currently running on 0.20 branches missing the security changes.
> 
> Process Authentication
> ---------------------------
> The HMaster and HRegionServer processes have been updated to allow configuration of the Kerberos principals used to run the processes.  The new configuration parameters are:
> 
> * hbase.master.keytab.file - Path to the keytab file containing the master principal's credentials
> * hbase.master.kerberos.principal - Kerberos principal name used to login the HMaster process
> * hbase.master.kerberos.https.principal - Kerberos principal name used to login the HMaster info server
> * hbase.regionserver.keytab.file - Path to the keytab file containing the region server's credentials
> * hbase.regionserver.kerberos.principal - Kerberos principal name used to login the HRegionServer process
> * hbase.regionserver.kerberos.https.principal - Kerberos principal name used to login the HRegionServer info server
> 
> The new class org.apache.hadoop.hbase.security.HBasePolicyProvider and new file conf/hadoop-policy.xml allow restriction of the users and groups permitting to utilize each of the RPC protocol interfaces (HMasterInterface, HMasterRegionInterface, HRegionInterface).
> 
> Testing Updates
> --------------------
> Parts of the test code (org.apache.hadoop.hbase.HBaseTestingUtility and org.apache.hadoop.hbase.MiniHBaseCluster) were directly using the internal Hadoop UnixUserGroupInformation class to manipulate process ownership for testing.  These have been updated to use UserGroupInformation.doAs() instead.
> 
> 
> This addresses bugs HBASE-2016 and HBASE-2742.
>     http://issues.apache.org/jira/browse/HBASE-2016
>     http://issues.apache.org/jira/browse/HBASE-2742
> 
> 
> Diffs
> -----
> 
>   conf/hadoop-policy.xml PRE-CREATION 
>   pom.xml 2d3d75a 
>   src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 2b5eeb6 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 9873172 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java d88c12d 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java bd48a4b 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java 71a0447 
>   src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1157fe1 
>   src/main/java/org/apache/hadoop/hbase/ipc/Status.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java e4bd30d 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6a54736 
>   src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcServer.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 280b91d 
>   src/main/resources/hbase-default.xml e3a9669 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 4d09fe9 
>   src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 9c49e36 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 0b47975 
>   src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java c982662 
> 
> Diff: http://review.cloudera.org/r/406/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gary
> 
>