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

[email protected] commented on HBASE-2742:
------------------------------------------------------



bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureClient.java, 
line 128
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53577#file53577line128>
bq.  >
bq.  >     This if/else seems a little fuzzy.  Server-side, this is how it 
chooses an auth method?

Yes, this matches up with server side.  If not using kerberos auth, then does 
SIMPLE auth, otherwise if a token is present does DIGEST, otherwise requires 
kerberos ticket.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureClient.java, 
line 233
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53577#file53577line233>
bq.  >
bq.  >     Formatting.  This copied from hadoop?

Yes, from Hadoop.  Cleaning up.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureClient.java, 
line 195
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53577#file53577line195>
bq.  >
bq.  >     Should this be configurable?

Some config here is probably good, for the case of those who are very sensitive 
to latency.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureRpcEngine.java, 
line 366
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53579#file53579line366>
bq.  >
bq.  >     Why we need this?  This in all our rpc'ing?

Yes, WritableRpcEngine$Server.call() does the same.  Don't know why we'd be 
implementing RPC protocols with non-public methods, but predates me...


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureServer.java, 
line 85
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53580#file53580line85>
bq.  >
bq.  >     An insecure client cannot talk to a secure server (which makes 
'sense')

Yes, HEADER bytes are distinct and will prevent this as well.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureServer.java, 
line 597
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53580#file53580line597>
bq.  >
bq.  >     This copy/pasted from hadoop?  Else its style violation (minor nit)

Yes, fixed anyway.  (HBaseServer$Connection.processData has the same problem).


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > 
security/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java,
 line 87
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53589#file53589line87>
bq.  >
bq.  >     Should a 'secret manager' have a public constructor?

It's currently instantiated from SecureServer, so we need to access it somehow. 
 With secured ZK access, znode ACLs will prevent creating a secret manager 
instance to obtain the secret keys.  There is an opening here with coprocessors 
though, since they run in process on the RS.  We'll ultimately need to lock 
those down separately.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > 
security/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenIdentifier.java,
 line 43
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53588#file53588line43>
bq.  >
bq.  >     These are copied from hadoop?  Don't look like Gary style.

Yes, largely copied and modified.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureClient.java, 
line 147
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53577#file53577line147>
bq.  >
bq.  >     Should we set saslRpcClient to null after dispose?  Is it good to 
just swallow the exception?  Should log at least?

Good points, updating.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > 
security/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java,
 line 98
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53589#file53589line98>
bq.  >
bq.  >     ditoo

This also gets called from SecureServer, so it needs to be able to access.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > 
security/src/main/java/org/apache/hadoop/hbase/security/token/ZKLeaderManager.java,
 line 47
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53593#file53593line47>
bq.  >
bq.  >     Is this 'generic' zk facility?  Put in zk package?

Sure, will move to o.a.h.h.zookeeper.  I very briefly considered trying to 
combine with ActiveMasterManager when implementing this but that seemed too far 
out of scope.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/Status.java, line 1
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53581#file53581line1>
bq.  >
bq.  >     This has been added already.

Yes, will update the patch against that change and remove this.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureRpcEngine.java, 
line 172
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53579#file53579line172>
bq.  >
bq.  >     In secure hbase we don't do that delayed response stuff that was 
recently added to insecure rpc?

Delayed stuff should work since it's using the rpc engine interfaces, but I'll 
check if there are upstream discrepancies since this patch.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureClient.java, 
line 345
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53577#file53577line345>
bq.  >
bq.  >     What about the 'length' thing that we added to hbase? I don't see 
you processing it here.  I suppose its ok because these are different client 
and servers and they just don't do it (and things like asynchbase are not going 
to do secure hase any time soon).

Yeah, doesn't do "length" currently.  I could add it, but asynchbase would 
still have to deal with the sasl negotiation on connection setup.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureRpcEngine.java, 
line 381
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53579#file53579line381>
bq.  >
bq.  >     Where does TRACELOG come from?  Why not LOG.trace?  Is it from 
parent?  Should logging be from this class?

TRACELOG comes from HBaseServer I think.  We added it with the simple rpc call 
introspection I did.  We wanted it to be a separate logger so it could be 
easily enabled to get those stats without bringing in all the rest of the debug 
logging.


bq.  On 2011-11-07 06:59:29, Michael Stack wrote:
bq.  > security/src/main/java/org/apache/hadoop/hbase/ipc/SecureServer.java, 
line 123
bq.  > <https://reviews.apache.org/r/1991/diff/4/?file=53580#file53580line123>
bq.  >
bq.  >     If result is null, this will work?

Hmm, this is just doing same as HBaseServer, isn't it?


- Gary


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1991/#review3072
-----------------------------------------------------------


On 2011-10-26 20:23:19, Gary Helmling wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1991/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-10-26 20:23:19)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  This patch creates a new secure RPC engine for HBase, which provides 
Kerberos based authentication of clients, and a token-based authentication 
mechanism for mapreduce jobs.  Primary components of the patch are:
bq.  
bq.  - a new maven profile for secure Hadoop/HBase: hadoop-0.20S
bq.    - Secure Hadoop dependent classes are separated under a pseudo-module in 
the security/ directory.  These source and test directories are only including 
if building the secure Hadoop profile
bq.    - Currently the security classes get packaged with the regular HBase 
build artifacts.  We need a way to at least override project.version, so we can 
append something like a "-security" suffix indicating the additional security 
components.
bq.    - The pseudo-module here is really a half-step forward.  It enables the 
security code to be optionally included in the build for now, and sets up the 
structure for a security module.  But we still will want to pursue full 
modularization (see HBASE-4336), which will allow packing the security code in 
a separate build artifact.
bq.  
bq.  - a new RPC engine providing kerberos and token-based authentication: 
org.apache.hadoop.hbase.ipc.SecureRpcEngine
bq.    - implementation under 
security/src/main/java/org/apache/hadoop/hbase/ipc/
bq.    - The implementation classes extend the existing HBaseClient and 
HBaseServer to share as much of the RPC code as possible.  The main override is 
of the connection classes to allow control over the SASL negotiation of secure 
connections
bq.  
bq.  - existing RPC changes
bq.    - The existing HBaseClient and HBaseServer have been modified to make 
subclassing possible
bq.    - All references to Hadoop UserGroupInformation have been replaced with 
org.apache.hadoop.hbase.security.User to insulate from future dependencies on 
specific Hadoop versions
bq.  
bq.  - a coprocessor endpoint for obtaining new authentication tokens: 
TokenProvider, and supporting classes for token generation and synchronization 
(incorporating HBASE-3615)
bq.    - implementation is under 
security/src/main/java/org/apache/hadoop/hbase/security/token/
bq.    - Secret keys for token generation and verification are synchronized 
throughout the cluster in zookeeper, under /hbase/tokenauth/keys
bq.  
bq.  
bq.  To enable secure RPC, add the following configuration to hbase-site.xml:
bq.  
bq.    <property>
bq.     <name>hadoop.security.authorization</name>
bq.     <value>true</value>
bq.    </property>
bq.    <property>
bq.     <name>hadoop.security.authentication</name>
bq.     <value>kerberos</value>
bq.    </property>
bq.    <property>
bq.     <name>hbase.rpc.engine</name>
bq.     <value>org.apache.hadoop.hbase.ipc.SecureRpcEngine</value>
bq.    </property>
bq.    <property>
bq.     <name>hbase.coprocessor.region.classes</name>
bq.     <value>org.apache.hadoop.hbase.security.token.TokenProvider</value>
bq.    </property>
bq.  
bq.  In addition, the master and regionserver processes must be configured for 
kerberos authentication using the properties:
bq.  
bq.   * hbase.(master|regionserver).keytab.file
bq.   * hbase.(master|regionserver).kerberos.principal
bq.   * hbase.(master|regionserver).kerberos.https.principal
bq.  
bq.  
bq.  This addresses bug HBASE-2742.
bq.      https://issues.apache.org/jira/browse/HBASE-2742
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    conf/hbase-policy.xml PRE-CREATION 
bq.    pom.xml 9d42e2b 
bq.    security/src/main/java/org/apache/hadoop/hbase/ipc/SecureClient.java 
PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/ipc/SecureConnectionHeader.java 
PRE-CREATION 
bq.    security/src/main/java/org/apache/hadoop/hbase/ipc/SecureRpcEngine.java 
PRE-CREATION 
bq.    security/src/main/java/org/apache/hadoop/hbase/ipc/SecureServer.java 
PRE-CREATION 
bq.    security/src/main/java/org/apache/hadoop/hbase/ipc/Status.java 
PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/AccessDeniedException.java
 PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java
 PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java 
PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcServer.java 
PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationKey.java
 PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationProtocol.java
 PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenIdentifier.java
 PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java
 PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSelector.java
 PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/token/TokenProvider.java
 PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java 
PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/token/ZKLeaderManager.java
 PRE-CREATION 
bq.    
security/src/main/java/org/apache/hadoop/hbase/security/token/ZKSecretWatcher.java
 PRE-CREATION 
bq.    
security/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenAuthentication.java
 PRE-CREATION 
bq.    
security/src/test/java/org/apache/hadoop/hbase/security/token/TestZKSecretWatcher.java
 PRE-CREATION 
bq.    security/src/test/resources/hbase-site.xml PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/HServerAddress.java f28240c 
bq.    src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 
06a2312 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java 904c107 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 1365411 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 18310d6 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 337da78 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 4a8918a 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java 000f99c 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java 
965102c 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1f8b629 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/RequestContext.java 
PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/RpcEngine.java 1b5629a 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 60a9248 
bq.    src/main/java/org/apache/hadoop/hbase/mapred/TableMapReduceUtil.java 
db07ed1 
bq.    src/main/java/org/apache/hadoop/hbase/mapreduce/TableMapReduceUtil.java 
ad88b76 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java 3f8f929 
bq.    src/main/java/org/apache/hadoop/hbase/security/KerberosInfo.java 
PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/security/TokenInfo.java 
PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/security/User.java 00bd06d 
bq.    src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java bb67e53 
bq.    src/main/resources/hbase-default.xml 3785533 
bq.    src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 7194c02 
bq.    src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java 3982eff 
bq.  
bq.  Diff: https://reviews.apache.org/r/1991/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  A full test suite run showed failures in TestMasterFailover (timeout) and 
TestAvroServer.  I'll look into those.
bq.  
bq.  Cluster testing (1 master + 3 slaves in ec2) with Kerberos authentication 
and map reduce token authentication, using YCSB, RowCounter, 
PerformanceEvaluation.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Gary
bq.  
bq.


                
> Provide strong authentication with a secure RPC engine
> ------------------------------------------------------
>
>                 Key: HBASE-2742
>                 URL: https://issues.apache.org/jira/browse/HBASE-2742
>             Project: HBase
>          Issue Type: Improvement
>          Components: ipc
>            Reporter: Gary Helmling
>            Priority: Critical
>             Fix For: 0.92.0
>
>
> The HBase RPC code (org.apache.hadoop.hbase.ipc.*) was originally forked off 
> of Hadoop RPC classes, with some performance tweaks added.  Those 
> optimizations have come at a cost in keeping up with Hadoop RPC changes 
> however, both bug fixes and improvements/new features.  
> In particular, this impacts how we implement security features in HBase (see 
> HBASE-1697 and HBASE-2016).  The secure Hadoop implementation (HADOOP-4487) 
> relies heavily on RPC changes to support client authentication via kerberos 
> and securing and mutual authentication of client/server connections via SASL. 
>  Making use of the built-in Hadoop RPC classes will gain us these pieces for 
> free in a secure HBase.
> So, I'm proposing that we drop the HBase forked version of RPC and convert to 
> direct use of Hadoop RPC, while working to contribute important fixes back 
> upstream to Hadoop core.  Based on a review of the HBase RPC changes, the key 
> divergences seem to be:
> HBaseClient:
>  - added use of TCP keepalive (HBASE-1754)
>  - made connection retries and sleep configurable (HBASE-1815)
>  - prevent NPE if socket == null due to creation failure (HBASE-2443)
> HBaseRPC:
>  - mapping of method names <-> codes (removed in HBASE-2219)
> HBaseServer:
>  - use of TCP keep alives (HBASE-1754)
>  - OOME in server does not trigger abort (HBASE-1198)
> HbaseObjectWritable:
>  - allows List<> serialization
>  - includes it's own class <-> code mapping (HBASE-328)
> Proposed process is:
> 1. open issues with patches on Hadoop core for important fixes/adjustments 
> from HBase RPC (HBASE-1198, HBASE-1815, HBASE-1754, HBASE-2443, plus a 
> pluggable ObjectWritable implementation in RPC.Invocation to allow use of 
> HbaseObjectWritable).
> 2. ship a Hadoop version with RPC patches applied -- ideally we should avoid 
> another copy-n-paste code fork, subject to ability to isolate changes from 
> impacting Hadoop internal RPC wire formats
> 3. if all Hadoop core patches are applied we can drop back to a plain vanilla 
> Hadoop version
> I realize there are many different opinions on how to proceed with HBase RPC, 
> so I'm hoping this issue will kick off a discussion on what the best approach 
> might be.  My own motivation is maximizing re-use of the authentication and 
> connection security work that's already gone into Hadoop core.  I'll put 
> together a set of patches around #1 and #2, but obviously we need some 
> consensus around this to move forward.  If I'm missing other differences 
> between HBase and Hadoop RPC, please list as well.  Discuss!

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to