hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sergey Shelukhin <ser...@hortonworks.com>
Subject Re: Review Request 46956: HIVE-13444 LLAP: add HMAC signatures to LLAP; verify them on LLAP side
Date Mon, 23 May 2016 22:11:07 GMT


> On May 20, 2016, 2:21 a.m., Siddharth Seth wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapProtocolServerImpl.java,
line 290
> > <https://reviews.apache.org/r/46956/diff/2/?file=1387298#file1387298line290>
> >
> >     What user is expected over here.
> >     1. In case of an invocation by HS2 to run a Tez query - I'm assuming this would
be the HS2 service user (which is the same as the LLAP service user). (That needs to be validated)
> >     2. In case of external services - would this be the HS2 service user or the
user associated with the external service ?
> >     
> >     If it's the HS2 user each time, is the "user"/"realuser" field in the TokenIdentifier
required ? That seems to be passed in as a null everywhere.
> >     Assuming the appId is what will be used to differentiate different external
clients ? and that in case of Tez - there's no differentiation.
> 
> Sergey Shelukhin wrote:
>     This is the calling user in case of RPC.
> 
> Siddharth Seth wrote:
>     This goes back to whether it's invoked locally or remotely.
>     Local and Remote calls by HS2 to obtain a token on behalf of an external client,
will need to pass in the user name to generate the token correctly. What's obtained on the
RPC call will almost always be the Hive user - at least for the remote call.
>     Behaviour should not change if the flag to get the token locally / remotely is changed.
> 
> Sergey Shelukhin wrote:
>     This is protobuf, it's always invoked remotely. Two paths only meet at SecretManager
level.
> 
> Siddharth Seth wrote:
>     Reference was to the way the token is obtained (locally or remotely) - not the specific
call.

fixed by only retaining the remote API for CLI and removing the setting. HS2 user should have
access to ZK paths, for registry if nothing else, so this shouldn't be a problem


> On May 20, 2016, 2:21 a.m., Siddharth Seth wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java, line
61
> > <https://reviews.apache.org/r/46956/diff/2/?file=1387304#file1387304line61>
> >
> >     I don't think we need to create a new instance of the ZKDelegationTokenSecretManager.
> >     
> >     The one created earlier to generate tokens should be passed in.
> >     
> >     The KeySigner could be an interface instead, and SecretManager (extends ZKDelegationTokenSecretManager)
can implement this. ACL checks etc are already setup there. There's no requirement to have
two independent copies of the ZKSecretManager running in the same daemon.
> 
> Sergey Shelukhin wrote:
>     This one has completely different logic and even different template parameter.
> 
> Siddharth Seth wrote:
>     Most of the logic is in the BaseSecretManager itself, correct ?
>     The same instance can be used to generate tokens, as well as sign. Is there a downside
to that ?
>     Setting up two instances would create extra threads, and confusion while debugging;
Also potentially additional load to ZK, additional logins, etc

Merged the classes. They are created separately for now, I'll add central creation in one
of the subsequent patches that adds llapcoordinator.


- Sergey


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


On May 21, 2016, 12:07 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46956/
> -----------------------------------------------------------
> 
> (Updated May 21, 2016, 12:07 a.m.)
> 
> 
> Review request for hive, Gunther Hagleitner, Jason Dere, and Siddharth Seth.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 4cfa5f1 
>   llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java
f10351b 
>   llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java PRE-CREATION

>   llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenIdentifier.java
e28eddd 
>   llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 465b204

>   llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java
2524dc2 
>   llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java de817e3

>   llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapProtocolServerImpl.java
b94fc2e 
>   llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapTokenChecker.java
03ee055 
>   llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java 8abd198

>   llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorService.java
eac0e8f 
>   llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java
74359fa 
>   llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION

>   llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java
279baf1 
>   llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TestLlapTokenChecker.java
aaaa762 
>   llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestFirstInFirstOutComparator.java
a250882 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java c9b912b 
> 
> Diff: https://reviews.apache.org/r/46956/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


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