accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 29386: ACCUMULO-2815 Client authentication via Kerberos
Date Mon, 29 Dec 2014 16:39:00 GMT

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


Some personal review to comment on the existing TODOs I left and bring up some concerns I
have.


core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java
<https://reviews.apache.org/r/29386/#comment109661>

    To set up the handshake with a server, the client needs to know the 'instance' from the
server's kerberos principal ("accumulo" in "accumulo/localhost@MY_DOMAIN"). Reusing GENERAL_KERBEROS_PRINCIPAL
(which is the whole "accumulo/host@DOMAIN" value) lets use not have to introduce a new property.
    
    However, reusing that parameter does force the client to "know" more than it really needs
to (it doesn't have to know "host@DOMAIN", explicitly). I'm not sure if that's an issue or
not.



core/src/main/java/org/apache/accumulo/core/rpc/ThriftUtil.java
<https://reviews.apache.org/r/29386/#comment109662>

    Should this be cached? It's a DNS lookup, right?



core/src/main/java/org/apache/accumulo/core/rpc/ThriftUtil.java
<https://reviews.apache.org/r/29386/#comment109663>

    Without a "proper" DNS setup, this becomes problematic. The normal way this happens is
that a server's FQDN is used as the Kerberos instance (e.g. "host.domain.com" in "accumulo/host.domain.com@DOMAIN").
    
    This expects that the name returned for the address text matches what is specified. I
think this is "expected" enough that we can rely upon. Plus, I'm not sure what else we could
do here as it's common to use the "`_HOST`" expansion string in Hadoop configuration to keep
equivalent configuration files on all hosts.



core/src/main/java/org/apache/accumulo/core/rpc/UGIAssumingTransport.java
<https://reviews.apache.org/r/29386/#comment109664>

    We could use an AtomicReference here to work around the throw/catch/unwrap/rethrow we
have here.



server/base/src/main/java/org/apache/accumulo/server/AccumuloServerContext.java
<https://reviews.apache.org/r/29386/#comment109665>

    Touched on this earlier, the client and server configs are equivalent at this point which
pushes some additional information onto the client. The information is general though, so
I think this is ok to work with for the simplicity of a single config. We can easily change
the implementation out later if there's more we find that's missing.



server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java
<https://reviews.apache.org/r/29386/#comment109666>

    This is a big functional change over what we previously had. Before, there was *always*
a "root" user. Using Kerberos, if we create a "root" user with all of the 'admin' permissions,
we'll have created an unmanageable instance.
    
    We need to provide the name of a user that is the "root" user so the system can be used.



server/base/src/main/java/org/apache/accumulo/server/security/SystemCredentials.java
<https://reviews.apache.org/r/29386/#comment109674>

    Need to give some more thought here to prevent rogue TServers from registering themselves
with the instance.
    
    The kerberos credentials lets us know that the user with that ticket is who they say they
are, we just need to be sure that the user is allowed to join the instance. Maybe we check
the configuration value for the kerberos principal in the server-side authentication to make
sure the "primary" component of the principal matches what was given?


- Josh Elser


On Dec. 29, 2014, 4:13 p.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29386/
> -----------------------------------------------------------
> 
> (Updated Dec. 29, 2014, 4:13 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2815
>     https://issues.apache.org/jira/browse/ACCUMULO-2815
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> ACCUMULO-2815 Initial support for Kerberos client authentication.
> 
> Leverage SASL transport provided by Thrift which can speak GSSAPI, which Kerberos implements.
Introduced...
> 
> * An Accumulo KerberosToken which is an AuthenticationToken to validate users.
> * Custom thrift processor and invocation handler to ensure server RPCs have a valid KRB
identity and Accumulo authentication.
> * A KerberosAuthenticator which extends ZKAuthenticator to support Kerberos identities
seamlessly.
> * New ClientConf variables to use SASL transport and pass Kerberos server principal
> * Updated ClientOpts and Shell opts to transparently use a KerberosToken when SASL is
enabled (no extra client work).
> 
> I believe this is the "bare minimum" for Kerberos support. They are also grossly lacking
in unit and integration tests. I believe that I might have somehow broken the client address
string in the server (I saw log messages with client: null, but I'm not sure if it's due to
these changes or not). A necessary limitation in the Thrift server used is that, like the
SSL transport, the SASL transport cannot presently be used with the TFramedTransport, which
means none of the [half]async thrift servers will function with this -- we're stuck with the
TThreadPoolServer.
> 
> Performed some contrived benchmarks on my laptop (while still using it myself) to get
at big-picture view of the performance impact against "normal" operation and Kerberos alone.
Each "run" was the duration to ingest 100M records using continuous-ingest, timed with `time`,
using 'real'.
> 
> THsHaServer (our default), 6 runs:
> 
> Avg: 10m7.273s (607.273s)
> Min: 9m43.395s
> Max: 10m52.715s
> 
> TThreadPoolServer (no SASL), 5 runs:
> 
> Avg: 11m16.254s (676.254s)
> Min: 10m30.987s
> Max: 12m24.192s
> 
> TThreadPoolServer+SASL/GSSAPI (these changes), 6 runs:
> 
> Avg: 13m17.187s (797.187s)
> Min: 10m52.997s
> Max: 16m0.975s
> 
> The general takeway is that there's about 15% performance degredation in its initial
state which is in the realm of what I expected (~10%).
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java f6ea934 
>   core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java 6fe61a5

>   core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java e75bec6

>   core/src/main/java/org/apache/accumulo/core/client/impl/ConnectorImpl.java f481cc3

>   core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportKey.java 6dc846f

>   core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java 5da803b

>   core/src/main/java/org/apache/accumulo/core/client/security/tokens/KerberosToken.java
PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java e054a5f 
>   core/src/main/java/org/apache/accumulo/core/rpc/FilterTransport.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/rpc/SaslConnectionParams.java PRE-CREATION

>   core/src/main/java/org/apache/accumulo/core/rpc/TTimeoutTransport.java 6eace77 
>   core/src/main/java/org/apache/accumulo/core/rpc/ThriftUtil.java 09bd6c4 
>   core/src/main/java/org/apache/accumulo/core/rpc/UGIAssumingTransport.java PRE-CREATION

>   core/src/main/java/org/apache/accumulo/core/rpc/UGIAssumingTransportFactory.java PRE-CREATION

>   core/src/main/java/org/apache/accumulo/core/security/Credentials.java 525a958 
>   proxy/src/main/java/org/apache/accumulo/proxy/Proxy.java 4b048eb 
>   server/base/src/main/java/org/apache/accumulo/server/AccumuloServerContext.java 09ae4f4

>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java 046cfb5 
>   server/base/src/main/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandler.java
PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingWrapper.java
PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/rpc/TServerUtils.java 641c0bf

>   server/base/src/main/java/org/apache/accumulo/server/rpc/ThriftServerType.java PRE-CREATION

>   server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
5e81018 
>   server/base/src/main/java/org/apache/accumulo/server/security/SystemCredentials.java
a59d57c 
>   server/base/src/main/java/org/apache/accumulo/server/security/handler/KerberosAuthenticator.java
PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/thrift/UGIAssumingProcessor.java
PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/security/SystemCredentialsTest.java
4202a7e 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 93a9a49

>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java
f98721f 
>   server/gc/src/test/java/org/apache/accumulo/gc/SimpleGarbageCollectorTest.java 99558b8

>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java
cad1e01 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 12195fa 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 7e33300 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java d5c1d2f

>   shell/src/main/java/org/apache/accumulo/shell/Shell.java 58308ff 
>   shell/src/main/java/org/apache/accumulo/shell/ShellOptionsJC.java 8167ef8 
>   test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java eb84533 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 2ebc2e3

>   test/src/test/java/org/apache/accumulo/server/security/SystemCredentialsIT.java fb71f5f

> 
> Diff: https://reviews.apache.org/r/29386/diff/
> 
> 
> Testing
> -------
> 
> Ensure existing unit tests still function. Accumulo is functional and ran continuous
ingest multiple times using a client with only a Kerberos identity (no user/password provided).
Used MIT Kerberos with Apache Hadoop 2.6.0 and Apache ZooKeeper 3.4.5.
> 
> 
> Thanks,
> 
> Josh Elser
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message