Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Content-Type: multipart/alternative;
 boundary="===============4148542591848310148=="
MIME-Version: 1.0
Subject: Re: Review Request 29386: ACCUMULO-2815 Client authentication via
 Kerberos
From: "Josh Elser" <josh.elser@gmail.com>
To: "Christopher Tubbs" <ctubbsii-rb@apache.org>,
 "accumulo" <dev@accumulo.apache.org>, "Josh Elser" <josh.elser@gmail.com>
Date: Wed, 07 Jan 2015 23:04:04 -0000
Message-ID: <20150107230404.26560.34926@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Josh Elser" <noreply@reviews.apache.org>
References: <20150105204628.26559.34214@reviews.apache.org>
In-Reply-To: <20150105204628.26559.34214@reviews.apache.org>
Reply-To: "Josh Elser" <josh.elser@gmail.com>

--===============4148542591848310148==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Jan. 5, 2015, 8:46 p.m., Christopher Tubbs wrote:
> > server/base/src/main/java/org/apache/accumulo/server/security/handler/KerberosAuthenticator.java, lines 68-75
> > <https://reviews.apache.org/r/29386/diff/9/?file=804864#file804864line68>
> >
> >     This makes the assumption that this method is only called on the current user. That's not true. It also supports the public API SecurityOperations.authenticateUser()
> 
> Josh Elser wrote:
>     Oh, hrm. That might be problematic? I have to look at where the token comes from. We need to differentiate between the token for the user asking, and the token for the user being asked about. The API ensures that we can at least have both of those tokens, which gives us what we need.
>     
>     I think we can proxy the second token on top of the current token to validate (the user being asked about on top of the user which is doing the asking). If that succeeds, the user being asked about should be able to access the system on their own and everything else should be transparent.
>     
>     I need to write some tests to ensure multi-user permissions operate expectedly (I'm working on MAC being able to use a KDC now).

So, the reason this struck me as odd is that I don't know how this is actually possible :)

I can't ask if another Kerberos user's credentials are valid because I shouldn't have them (and I don't *think* it's possible to have multiple active credentials passed over the wire). I think I just need to make a check that the caller's credentials are the same as the credentials we're being asked to check. Or, I'm not sure if this is impossible or I just don't know how to do it.


- Josh


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


On Jan. 7, 2015, 9:37 p.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29386/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2015, 9:37 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/MasterClient.java a9ad8a1 
>   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 
>   core/src/test/java/org/apache/accumulo/core/cli/TestClientOpts.java ff49bc0 
>   core/src/test/java/org/apache/accumulo/core/client/ClientConfigurationTest.java PRE-CREATION 
>   core/src/test/java/org/apache/accumulo/core/client/impl/ThriftTransportKeyTest.java PRE-CREATION 
>   core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java 40be70f 
>   core/src/test/java/org/apache/accumulo/core/rpc/SaslConnectionParamsTest.java PRE-CREATION 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 27d6b19 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloConfigImpl.java 26c23ed 
>   pom.xml ae188a0 
>   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/SecurityUtil.java 29e4939 
>   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/main/java/org/apache/accumulo/server/util/Admin.java ae36f1f 
>   server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java 7fdbf13 
>   server/base/src/test/java/org/apache/accumulo/server/AccumuloServerContextTest.java PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandlerTest.java PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/rpc/ThriftServerTypeTest.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 
>   shell/src/test/java/org/apache/accumulo/shell/ShellConfigTest.java 0e72c8c 
>   shell/src/test/java/org/apache/accumulo/shell/ShellOptionsJCTest.java PRE-CREATION 
>   test/pom.xml b0a926f 
>   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/harness/AccumuloClusterIT.java 8f7e1b7 
>   test/src/test/java/org/apache/accumulo/harness/MiniClusterHarness.java abdb627 
>   test/src/test/java/org/apache/accumulo/harness/SharedMiniClusterIT.java 2380f66 
>   test/src/test/java/org/apache/accumulo/harness/TestingKdc.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/harness/conf/AccumuloMiniClusterConfiguration.java 11b7530 
>   test/src/test/java/org/apache/accumulo/server/security/SystemCredentialsIT.java fb71f5f 
>   test/src/test/java/org/apache/accumulo/test/ArbitraryTablePropertiesIT.java aa5c164 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java 1fcd5a4 
>   test/src/test/java/org/apache/accumulo/test/functional/BatchScanSplitIT.java 221889b 
>   test/src/test/java/org/apache/accumulo/test/functional/KerberosIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/security/KerberosTokenTest.java PRE-CREATION 
>   test/src/test/resources/log4j.properties cb35840 
> 
> 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
> 
>


--===============4148542591848310148==--