hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xiao Chen (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-14445) Delegation tokens are not shared between KMS instances
Date Thu, 28 Dec 2017 06:36:00 GMT

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

Xiao Chen commented on HADOOP-14445:
------------------------------------

Thanks a lot [~shahrs87] for the latest patch, looks good overall.

Some comments from code review:
KMSCP:
- I think we can improve the comments with {{dtService}} to be clearer. Suggest something
along the lines of:
{quote}
/* The value is read from
   * CommonConfigurationKeysPublic.HADOOP_SECURITY_KEY_PROVIDER_PATH from
   * the conf when creating the KeyProvider, and used as the token's service.
   * With this value, later the KeyProviderFactory will be able to instantiate
   * KeyProvider from the token to renew / cancel, without the need to read
   * configs.
{quote}
- Now that we handle the port stuff nicely in unit tests, {{fallbackDefaultPortForTesting}}
and related logic can be removed.
- On {{renew}} and {{cancel}}, suggest to add a debug log when {{keyProvider == null}} for
supportability.
- Let's use {{HADOOP_SECURITY_KEY_PROVIDER_PATH}} instead of {{KeyProviderFactory.KEY_PROVIDER_PATH}}.

KMSUtil:
- When {{createKeyProviderForTests}} returns non-null value (before {{return kp}}), add a
info log, since this should only happen in tests

TestKMS:
- It's great to see extra assertions were done in {{getTokenService}}.
- {{doKMSWithZKWithDelegationToken}}, do we need to loop through the tokens and verify? After
this fix, there would only be 1 kms-dt mapping to the entire url right? IMO we should verify
there's just 1 kms-dt.
- {{doKMSWithZKWithDelegationToken}}, besides verifying renewal, we should also verify some
key operations.
- Happy to see the compat test, thanks! We should also verify some key operations too here.
- Trivial, {{createKeyProviderForTests}} should have a line break after.

HdfsKMSUtil:
- Looks like we can remove the not-used {{createKeyProvider}} method.

There is also 1 thing that I think missed in the recent compat discussions:
For the clients to 'work', besides creating the kp and the token renewer, there is also the
complication of {{DelegationTokenAuthenticatedURL}} recognizing the token. How would we handle
the case there the job submitter's {{DelegationTokenAuthenticatedURL#getDelegationTokenService}}
did not match the cluster's mapper's version, to prevent issues similar to HADOOP-14441? In
other words, if I submit a job using the new jar and get a new format token, but when it's
distributed to the cluster, the mappers are still on the old version which tries to get the
ip, it won't recognize it and will fall back to kerberos. 

IMHO, since {{token.setService}} is done in KMSCP, what matters here would be to require the
job submitter (whoever gets the token) to be on the same version of the mappers (or whatever
is actually authenticating using the token). Otherwise, handling this at code level bi-directional
would require 1) duplicating the same token with ip (for new submitter, old mapper) and 2)
falling back to check old format in DTAuthURL (for old submitter, new mapper), which sounds
tedious and ugly. Curious to learn the thoughts from you and [~daryn].

> Delegation tokens are not shared between KMS instances
> ------------------------------------------------------
>
>                 Key: HADOOP-14445
>                 URL: https://issues.apache.org/jira/browse/HADOOP-14445
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: kms
>    Affects Versions: 2.8.0, 3.0.0-alpha1
>         Environment: CDH5.7.4, Kerberized, SSL, KMS-HA, at rest encryption
>            Reporter: Wei-Chiu Chuang
>            Assignee: Rushabh S Shah
>         Attachments: HADOOP-14445-branch-2.8.002.patch, HADOOP-14445-branch-2.8.patch,
HADOOP-14445.002.patch
>
>
> As discovered in HADOOP-14441, KMS HA using LoadBalancingKMSClientProvider do not share
delegation tokens. (a client uses KMS address/port as the key for delegation token)
> {code:title=DelegationTokenAuthenticatedURL#openConnection}
> if (!creds.getAllTokens().isEmpty()) {
>         InetSocketAddress serviceAddr = new InetSocketAddress(url.getHost(),
>             url.getPort());
>         Text service = SecurityUtil.buildTokenService(serviceAddr);
>         dToken = creds.getToken(service);
> {code}
> But KMS doc states:
> {quote}
> Delegation Tokens
> Similar to HTTP authentication, KMS uses Hadoop Authentication for delegation tokens
too.
> Under HA, A KMS instance must verify the delegation token given by another KMS instance,
by checking the shared secret used to sign the delegation token. To do this, all KMS instances
must be able to retrieve the shared secret from ZooKeeper.
> {quote}
> We should either update the KMS documentation, or fix this code to share delegation tokens.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Mime
View raw message