hive-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sergey Shelukhin (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HIVE-13445) LLAP: token should encode application and cluster ids
Date Sat, 23 Apr 2016 00:49:12 GMT

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

Sergey Shelukhin edited comment on HIVE-13445 at 4/23/16 12:48 AM:
-------------------------------------------------------------------

{noformat}
Any possibility of performing some basic sanity checks inside LlapProtocolServerImpl - or
is that already in place via the RPC layer validating the presence of a LLAP token. Don't
like the fact that the security chceks are 3 calls deep - but that seems the best place for
them rightnow.
{noformat}
The RPC layer validates the presence of the token.

{noformat}
String hostName = MetricsUtils.getHostName(); - Not necessarily related to this patch, but
getting it from YARN is more consistent (when yarn is available). Have seen lots of issues
around figuring out hostnames otherwise.
{noformat}
Is the yarn option already used somewhere? We could just change the utility method to use
it too.

{noformat}
LlapDeamon: appName = UUID.randomUUID().toString();

Ths won't work on distributed clusters, right ? Tokens use this as the appSecret. Each node
will generate a different appSecret. daemonId.getAppSecret is being used as the clusterId
in LlapTokenIdentifier.
{noformat}
We assume this is only used in tests. It won't work indeed. Added a comment

{noformat}
In LlapTokenChecker - why are we iterating over tokens even after an LLAPToken has been found
? Are multiple tokens expected. This is in checkPermissions as well as getTokenInfo
{noformat}
Not really expected at this point; I wonder if external clients could be using something like
that.


{noformat}
It looks like we end up taking the first request and linking it with the query. Also subsequent
requests are validated against this. Assuming that this becomes more useful once signing comes
in - to make sure someone is not submitting with incorrect parameters.
{noformat}
Yes, if we also validate it against the signature. In general, though, we assume that whoever
can submit fragments (ie has the specific token) can also kill fragments. The key is not being
able to submit/kill/etc. fragments for an app with a different token.

{noformat}
TaskExecutorService.findQueryByFragment - think we're better off implementing this in QueryInfo
itself rather than going to the scheduler to find out this information. need to check if QueryInfo
has state information about which fragments are linked to a query.
{noformat}
It doesn't, as far as I can tell.

{noformat}
getDelegationToken(String appSecret) - even in case of Tez, should this be associated with
the sessionId. That prevents a lot of the if (token.appSecret == null) checks and will simplify
the code.
{noformat}
Don't understand. Can you elaborate?

{noformat}
Forgot to mention, we should add some tests to validate token functionality, and how the system
interacts with QueryInfo etc.
{noformat}
Separate JIRA?

{noformat}
More on this. If eventually, we're going to validate this via signatures for external access
- do we actually need to store the appSecret/appId for the Query. Instead, we could validate
future requests against the already stored applicationId for a fragment / query.
{noformat}
The app ID has to come from somewhere with each request; terminate/etc. requests themselves
are not signed, so we cannot rely on the user to give us the correct app Id to verify against
the fragment. The appId when submitting can indeed come from the signed message, not from
the token (or it could be verified to be the same from both).

But, I think we'd still need it in the token for other requests. I am actually not sure how
the token will work with signing right now, more specifically - will we be able to get away
with not having appsecret be a secret? I think we will if HS2 would generate and sign it.
However, if the client is allowed to pass it in, some other client might also pass in the
same appId (if it's not a secret), and get the same token. So I assume we'd still store it,
although it won't really be called secret, it's just something that the signer (HS2) has to
generate.

Fixing the rest.


was (Author: sershe):
{noformat}
Any possibility of performing some basic sanity checks inside LlapProtocolServerImpl - or
is that already in place via the RPC layer validating the presence of a LLAP token. Don't
like the fact that the security chceks are 3 calls deep - but that seems the best place for
them rightnow.
{noformat}
The RPC layer validates the presence of the token.

{noformat}
String hostName = MetricsUtils.getHostName(); - Not necessarily related to this patch, but
getting it from YARN is more consistent (when yarn is available). Have seen lots of issues
around figuring out hostnames otherwise.
{noformat}
Is the yarn option already used somewhere? We could just change the utility method to use
it too.

{noformat}
LlapDeamon: appName = UUID.randomUUID().toString();

Ths won't work on distributed clusters, right ? Tokens use this as the appSecret. Each node
will generate a different appSecret. daemonId.getAppSecret is being used as the clusterId
in LlapTokenIdentifier.
{noformat}
We assume this is only used in tests. It won't work indeed. Added a comment

{noformat}
In LlapTokenChecker - why are we iterating over tokens even after an LLAPToken has been found
? Are multiple tokens expected. This is in checkPermissions as well as getTokenInfo
{noformat}
Not really expected at this point; I wonder if external clients could be using something like
that.


{noformat}
It looks like we end up taking the first request and linking it with the query. Also subsequent
requests are validated against this. Assuming that this becomes more useful once signing comes
in - to make sure someone is not submitting with incorrect parameters.
{noformat}
Yes, if we also validate it against the signature. In general, though, we assume that whoever
can submit fragments (ie has the specific token) can also kill fragments. The key is not being
able to submit/kill/etc. fragments for an app with a different token.

{noformat}
TaskExecutorService.findQueryByFragment - think we're better off implementing this in QueryInfo
itself rather than going to the scheduler to find out this information. need to check if QueryInfo
has state information about which fragments are linked to a query.
{noformat}
It doesn't, as far as I can tell.

{noformat}
getDelegationToken(String appSecret) - even in case of Tez, should this be associated with
the sessionId. That prevents a lot of the if (token.appSecret == null) checks and will simplify
the code.
{noformat}
Don't understand. Can you elaborate?

{noformat}
Forgot to mention, we should add some tests to validate token functionality, and how the system
interacts with QueryInfo etc.
{noformat}
Separate JIRA?

{noformat}
More on this. If eventually, we're going to validate this via signatures for external access
- do we actually need to store the appSecret/appId for the Query. Instead, we could validate
future requests against the already stored applicationId for a fragment / query.
{noformat}
The app ID has to come from somewhere with each request; terminate/etc. requests themselves
are not signed, so we cannot rely on the user to give us the correct app Id to verify against
the fragment. The appId when submitting can indeed come from the signed message, not from
the token (or it could be verified to be the same from both).

But, I think we'd still need it in the token for other requests. I am actually not sure how
the token will work with signing right now, more specifically - will we be able to get away
with not having appsecret be a secret? I think we will if HS2 would generate and sign it.
However, if the client is allowed to pass it in, some other client might also pass in the
same appId and secret, and get the same token. So I assume we'd still store it, although it
won't really be called secret, it's just something that the signer (HS2) has to generate.

Fixing the rest.

> LLAP: token should encode application and cluster ids
> -----------------------------------------------------
>
>                 Key: HIVE-13445
>                 URL: https://issues.apache.org/jira/browse/HIVE-13445
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Sergey Shelukhin
>            Assignee: Sergey Shelukhin
>         Attachments: HIVE-13445.01.patch, HIVE-13445.02.patch, HIVE-13445.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message