hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yongjun Zhang (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-14445) Delegation tokens are not shared between KMS instances
Date Tue, 30 May 2017 02:29:04 GMT

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

Yongjun Zhang commented on HADOOP-14445:
----------------------------------------

HI [~shahrs87],

Thanks much for the patch and [~jojochuang] and [~asuresh] for the discussion and review.

I did a review and here are my comments.

1. about new field "clientProvidedUriText" 
a. typo "clientProvidedUriText" should be "clientProviderUriText"
b. make it private instead of public, and add accessor methods
c. AND provide a comment on the format of its value.

2. I would like to see a description of the solution as a detailed comment somewhere in the
code, maybe right at the new field of item #1.

3. Suggest to change
{code}
 public static KeyProvider createKeyProviderFromToken(final Configuration conf,
      final String tokenServiceValue)  throws IOException {
{code}
to
{code}
 public static KeyProvider createKeyProviderFromTokenService(final Configuration conf,
      final String tokenService)  throws IOException {
{code}

3.
{code}
  /**
   * This method is created for testing of @LoadBalancingKMSClientProvider
   * since multiple kms servers in unit tests will only differ in port number
   * and multiple kms servers in real world application are designed to
   * share the same port number.
   * @param conf
   * @param tokenServiceValue
   * @return
   * @throws IOException
   */
  public static KeyProvider createKeyProviderFromToken(final Configuration conf,
      final String tokenServiceValue)  throws IOException {
    if (tokenServiceValue == null) {
      return null;
    }
    if (!tokenServiceValue.contains(",")) {
      return createKeyProviderFromUri(conf, URI.create(tokenServiceValue));
    }
    // The following code will be executed in just unit tests.
    // The syntax for kms servers will be
    // kms://localhost:port1/kms,kms://localhost:port2/kms
    String[] keyProviderUrisStr = tokenServiceValue.split(",");
    KMSClientProvider[] keyProviderArr =
        new KMSClientProvider[keyProviderUrisStr.length];
{code}

a. Comment need to be changed to say that "," only appear in tokenServiceValue of unit tests
because different ports need to be used in unit tests. 
b. And indicate what format to expect for production code, and what format to expect for unit
test.
c. We can separate the code used for test only to a separate method and call this method in
the above method, to make it more clear.

4. With this solution, if we add a new kms server, the currently running application will
not be able to use the new kms server.
And if we replace an existing kms server with a different one, currently running application
may fail. So the KMS HA may not be really HA.
This may not be a big concern, but I would like to bring it up here.

Thanks.




> 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: documentation, kms
>    Affects Versions: 2.8.0, 3.0.0-alpha1
>            Reporter: Wei-Chiu Chuang
>            Assignee: Rushabh S Shah
>         Attachments: HADOOP-14445-branch-2.8.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.3.15#6346)

---------------------------------------------------------------------
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