Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.4.1
    • Fix Version/s: 2.6.0
    • Component/s: security
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      Create a secret provider (see HADOOP-10791) that is backed by ZooKeeper and can synchronize amongst different servers.

      1. HADOOP-10868_addendum.patch
        2 kB
        Robert Kanter
      2. HADOOP-10868.patch
        86 kB
        Robert Kanter
      3. HADOOP-10868_branch-2.patch
        86 kB
        Robert Kanter
      4. HADOOP-10868.patch
        88 kB
        Robert Kanter
      5. HADOOP-10868_branch-2.patch
        87 kB
        Robert Kanter
      6. HADOOP-10868.patch
        90 kB
        Robert Kanter
      7. HADOOP-10868_branch-2.patch
        90 kB
        Robert Kanter
      8. HADOOP-10868.patch
        68 kB
        Robert Kanter
      9. HADOOP-10868_branch-2.patch
        68 kB
        Robert Kanter
      10. HADOOP-10868.patch
        68 kB
        Robert Kanter
      11. HADOOP-10868_branch-2.patch
        68 kB
        Robert Kanter
      12. HADOOP-10868.patch
        68 kB
        Robert Kanter
      13. HADOOP-10868_branch-2.patch
        68 kB
        Robert Kanter

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #1898 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1898/)
          HADOOP-10868. AuthenticationFilter should support externalizing the secret for signing and provide rotation support. (rkanter via tucu) (tucu: rev 932ae036acb96634c5dd435d57ba02ce4d5e8918)

          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRolloverSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestJaasConfiguration.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java
          • hadoop-common-project/hadoop-auth/src/site/apt/index.apt.vm
          • hadoop-common-project/hadoop-auth/pom.xml
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestStringSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestAuthenticationFilter.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestSigner.java
          • hadoop-project/pom.xml
          • hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/site/apt/Configuration.apt.vm
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
            HADOOP-10868. Addendum (tucu: rev 7e08c0f23f58aa143f0997f2472e8051175142e9)
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1898 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1898/ ) HADOOP-10868 . AuthenticationFilter should support externalizing the secret for signing and provide rotation support. (rkanter via tucu) (tucu: rev 932ae036acb96634c5dd435d57ba02ce4d5e8918) hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRolloverSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestJaasConfiguration.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java hadoop-common-project/hadoop-auth/src/site/apt/index.apt.vm hadoop-common-project/hadoop-auth/pom.xml hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestStringSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestAuthenticationFilter.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestSigner.java hadoop-project/pom.xml hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/site/apt/Configuration.apt.vm hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java hadoop-common-project/hadoop-common/CHANGES.txt HADOOP-10868 . Addendum (tucu: rev 7e08c0f23f58aa143f0997f2472e8051175142e9) hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
          Hide
          Robert Kanter added a comment -

          On a related note, have you considered having a testcase using minikdc to exercise the ZK SASL client?

          I had created a test case in Oozie for this, but it's very brittle and a little hacky (it's actually broken right now after upgrading ZooKeeper/Curator: OOZIE-1959); it also requires it's on JVM so it doesn't mess with other tests. The problem is basically that we're setting system properties and logging in at that level, and there doesn't appear to be a way to logout. I would normally have made a test like this, but for these problems it's not worth it IMO.

          Show
          Robert Kanter added a comment - On a related note, have you considered having a testcase using minikdc to exercise the ZK SASL client? I had created a test case in Oozie for this, but it's very brittle and a little hacky (it's actually broken right now after upgrading ZooKeeper/Curator: OOZIE-1959 ); it also requires it's on JVM so it doesn't mess with other tests. The problem is basically that we're setting system properties and logging in at that level, and there doesn't appear to be a way to logout. I would normally have made a test like this, but for these problems it's not worth it IMO.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk #1873 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1873/)
          HADOOP-10868. AuthenticationFilter should support externalizing the secret for signing and provide rotation support. (rkanter via tucu) (tucu: rev 932ae036acb96634c5dd435d57ba02ce4d5e8918)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-common-project/hadoop-auth/src/site/apt/Configuration.apt.vm
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestJaasConfiguration.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestSigner.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestStringSignerSecretProvider.java
          • hadoop-project/pom.xml
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestAuthenticationFilter.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRolloverSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/pom.xml
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java
          • hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java
          • hadoop-common-project/hadoop-auth/src/site/apt/index.apt.vm
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
            HADOOP-10868. Addendum (tucu: rev 7e08c0f23f58aa143f0997f2472e8051175142e9)
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #1873 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1873/ ) HADOOP-10868 . AuthenticationFilter should support externalizing the secret for signing and provide rotation support. (rkanter via tucu) (tucu: rev 932ae036acb96634c5dd435d57ba02ce4d5e8918) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-auth/src/site/apt/Configuration.apt.vm hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestJaasConfiguration.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestSigner.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestStringSignerSecretProvider.java hadoop-project/pom.xml hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestAuthenticationFilter.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRolloverSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java hadoop-common-project/hadoop-auth/pom.xml hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java hadoop-common-project/hadoop-auth/src/site/apt/index.apt.vm hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java HADOOP-10868 . Addendum (tucu: rev 7e08c0f23f58aa143f0997f2472e8051175142e9) hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #682 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/682/)
          HADOOP-10868. AuthenticationFilter should support externalizing the secret for signing and provide rotation support. (rkanter via tucu) (tucu: rev 932ae036acb96634c5dd435d57ba02ce4d5e8918)

          • hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRolloverSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestJaasConfiguration.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestSigner.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java
          • hadoop-project/pom.xml
          • hadoop-common-project/hadoop-auth/src/site/apt/index.apt.vm
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestStringSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestAuthenticationFilter.java
          • hadoop-common-project/hadoop-auth/pom.xml
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/site/apt/Configuration.apt.vm
          • hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java
            HADOOP-10868. Addendum (tucu: rev 7e08c0f23f58aa143f0997f2472e8051175142e9)
          • hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #682 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/682/ ) HADOOP-10868 . AuthenticationFilter should support externalizing the secret for signing and provide rotation support. (rkanter via tucu) (tucu: rev 932ae036acb96634c5dd435d57ba02ce4d5e8918) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRolloverSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestJaasConfiguration.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestSigner.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java hadoop-project/pom.xml hadoop-common-project/hadoop-auth/src/site/apt/index.apt.vm hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestStringSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestAuthenticationFilter.java hadoop-common-project/hadoop-auth/pom.xml hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/site/apt/Configuration.apt.vm hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java HADOOP-10868 . Addendum (tucu: rev 7e08c0f23f58aa143f0997f2472e8051175142e9) hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
          Hide
          Alejandro Abdelnur added a comment -

          Thanks for catching this Robert, I've just committed the addendum.

          On a related note, have you considered having a testcase using minikdc to exercise the ZK SASL client?

          Show
          Alejandro Abdelnur added a comment - Thanks for catching this Robert, I've just committed the addendum. On a related note, have you considered having a testcase using minikdc to exercise the ZK SASL client?
          Hide
          Robert Kanter added a comment -

          I forgot to change both places where "Client" was being set in the last patch. I've attached an addendum patch that fixes this (I also created a private static final for this). Alejandro Abdelnur, can you take a look at this?

          Show
          Robert Kanter added a comment - I forgot to change both places where "Client" was being set in the last patch. I've attached an addendum patch that fixes this (I also created a private static final for this). Alejandro Abdelnur , can you take a look at this?
          Hide
          Alejandro Abdelnur added a comment -

          Thanks Robert. Committed to trunk and branch-2

          Show
          Alejandro Abdelnur added a comment - Thanks Robert. Committed to trunk and branch-2
          Hide
          Alejandro Abdelnur added a comment -

          +1

          Show
          Alejandro Abdelnur added a comment - +1
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12668879/HADOOP-10868.patch
          against trunk revision 8008f0e.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 8 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-auth hadoop-hdfs-project/hadoop-hdfs-httpfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4730//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4730//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12668879/HADOOP-10868.patch against trunk revision 8008f0e. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 8 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-auth hadoop-hdfs-project/hadoop-hdfs-httpfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4730//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4730//console This message is automatically generated.
          Hide
          Robert Kanter added a comment -

          The new patch addresses your latest comments:

          • I moved the JaasConfiguration to be a nested class of ZKSignerSecretProvider. If it's only going to be used for that, we may as well just do this.
          • No need to unset the sysprops in @After for the tests because they are never set by the code during the tests. They only get set if "sasl" auth is used, which the tests don't do.
          Show
          Robert Kanter added a comment - The new patch addresses your latest comments: I moved the JaasConfiguration to be a nested class of ZKSignerSecretProvider. If it's only going to be used for that, we may as well just do this. No need to unset the sysprops in @After for the tests because they are never set by the code during the tests. They only get set if "sasl" auth is used, which the tests don't do.
          Hide
          Alejandro Abdelnur added a comment -

          JaasConfiguration.java:

          • Rename this class to something more specific, ie ZKSignerJaasConfiguration
          • no need for a Map of entries, a JaasConfiguration has a single entry, we can keep the name and the AppConfigurationEntry[] and return if it matches, also it does need to be static, ie:
            ...
            private AppConfigurationEntry[] entry;
            private String entryName;
          
            public JaasConfiguration(String entryName, ...) {
              this.entryName = entryName;
              ...
              AppConfigurationEntry appEntry = ...
              entry = new AppConfiguratioEntry[] { appEntr }; 
            }
           
            public AppConfigurationEntry[] getAppConfigurationEntry(String name) {
              return (entryName.equals(name)) ? entry : null;
            }
            ...
          

          SignerSecretProvider.java:

          • the init() method does not need the configPrefix, the passed configs should be already deprefixed.

          ZKSignerSecretProvider.java:

          • make a CONFIG_PREFIX constant for signer.secret.provider.zookeeper. and use it in all constants.
          • the value set under the sysprop LOGIN_CONTEXT_NAME_KEY, instead 'Client' make it ’SecretSignerClient', this uniqueness will help troubleshooting in case there is something else in the system playing with zookeeper. (BTW, I’m sure this will bite us eventually, I would open a ZK JIRA to allow Jaas configs per ZK client instance).

          Testcases

          • make sure you unset the sysprop of the ZK client with @After

          +1 after this is taken care of.

          Show
          Alejandro Abdelnur added a comment - JaasConfiguration.java : Rename this class to something more specific, ie ZKSignerJaasConfiguration no need for a Map of entries, a JaasConfiguration has a single entry, we can keep the name and the AppConfigurationEntry[] and return if it matches, also it does need to be static, ie: ... private AppConfigurationEntry[] entry; private String entryName; public JaasConfiguration( String entryName, ...) { this .entryName = entryName; ... AppConfigurationEntry appEntry = ... entry = new AppConfiguratioEntry[] { appEntr }; } public AppConfigurationEntry[] getAppConfigurationEntry( String name) { return (entryName.equals(name)) ? entry : null ; } ... SignerSecretProvider.java : the init() method does not need the configPrefix, the passed configs should be already deprefixed. ZKSignerSecretProvider.java : make a CONFIG_PREFIX constant for signer.secret.provider.zookeeper. and use it in all constants. the value set under the sysprop LOGIN_CONTEXT_NAME_KEY , instead 'Client' make it ’SecretSignerClient', this uniqueness will help troubleshooting in case there is something else in the system playing with zookeeper. (BTW, I’m sure this will bite us eventually, I would open a ZK JIRA to allow Jaas configs per ZK client instance). Testcases make sure you unset the sysprop of the ZK client with @After +1 after this is taken care of.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12668589/HADOOP-10868.patch
          against trunk revision 98588cf.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4713//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12668589/HADOOP-10868.patch against trunk revision 98588cf. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4713//console This message is automatically generated.
          Hide
          Robert Kanter added a comment -

          Sorry about that; I've just uploaded a new trunk patch (the branch-2 patch was fine)

          Show
          Robert Kanter added a comment - Sorry about that; I've just uploaded a new trunk patch (the branch-2 patch was fine)
          Hide
          Alejandro Abdelnur added a comment -

          Robert Kanter, your last trunk patch seems to have a bunch of Yarn stuff, it seems you had something else in your branch. Mind updating??

          Show
          Alejandro Abdelnur added a comment - Robert Kanter , your last trunk patch seems to have a bunch of Yarn stuff, it seems you had something else in your branch. Mind updating??
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12668432/HADOOP-10868.patch
          against trunk revision 3122daa.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 40 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-httpfs hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:

          org.apache.hadoop.ha.TestZKFailoverController
          org.apache.hadoop.ha.TestZKFailoverControllerStress
          org.apache.hadoop.hdfs.web.TestWebHdfsFileSystemContract
          org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover

          The following test timeouts occurred in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-httpfs hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:

          org.apache.hadoop.yarn.client.TestResourceTrackerOnHA

          The test build failed in hadoop-hdfs-project/hadoop-hdfs-httpfs

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4705//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4705//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12668432/HADOOP-10868.patch against trunk revision 3122daa. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 40 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-httpfs hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: org.apache.hadoop.ha.TestZKFailoverController org.apache.hadoop.ha.TestZKFailoverControllerStress org.apache.hadoop.hdfs.web.TestWebHdfsFileSystemContract org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover The following test timeouts occurred in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-httpfs hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: org.apache.hadoop.yarn.client.TestResourceTrackerOnHA The test build failed in hadoop-hdfs-project/hadoop-hdfs-httpfs +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4705//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4705//console This message is automatically generated.
          Hide
          Robert Kanter added a comment -

          New patch addresses all of Alejandro's comments.

          Show
          Robert Kanter added a comment - New patch addresses all of Alejandro's comments.
          Hide
          Alejandro Abdelnur added a comment -

          AuthenticationFilter.java:

          • getProviderClass(), in the if ("random".equals() block, shouldn’t se be setting randomSecret to true.
          • secretProvider = (SignerSecretProvider) providerClass.newInstance();, no need for the casting.

          SignerSecretProvider.java:

          • Why passing a FilterConfig, we need a ServletContext to retrieve context attributes? the config already comes in the config properties.

          ZKSignerSecretProvider.java:

          • We should have 'ZOOKEEPER_AUTH_TYPE' to indicate if ZK authentication is required or not. Supported values would be: none, userpassword, digest & sasl. Depending on the value the ZK client auth conf should be done. Looks like in the patch you’ve done none and kerberos, we can push userpassword and digest to a follow up JIRA, but the code should be refactored in order to easily add a switch/case or if/else block.

          JaasConfiguration.java: Please look at Hbase ZKUtil.JaasConfiguration, the following comments follow what is done there.

          • options should include put("refreshKrb5Config", "true")
          • options should include put("debug", #a system property to trigger debugging")
          • why do we have the set/remove/clear/get, I would pass them in the constructor, after that the config is immutable.
          • the JaasConfiguration instance should have a name and the getAppConfigurationEntry method should only return config if the requested name matches.
          Show
          Alejandro Abdelnur added a comment - AuthenticationFilter.java: getProviderClass(), in the if ("random".equals() block, shouldn’t se be setting randomSecret to true . secretProvider = (SignerSecretProvider) providerClass.newInstance(); , no need for the casting. SignerSecretProvider.java : Why passing a FilterConfig, we need a ServletContext to retrieve context attributes? the config already comes in the config properties. ZKSignerSecretProvider.java : We should have 'ZOOKEEPER_AUTH_TYPE' to indicate if ZK authentication is required or not. Supported values would be: none, userpassword, digest & sasl. Depending on the value the ZK client auth conf should be done. Looks like in the patch you’ve done none and kerberos, we can push userpassword and digest to a follow up JIRA, but the code should be refactored in order to easily add a switch/case or if/else block. JaasConfiguration.java : Please look at Hbase ZKUtil.JaasConfiguration, the following comments follow what is done there. options should include put("refreshKrb5Config", "true") options should include put("debug", #a system property to trigger debugging") why do we have the set/remove/clear/get, I would pass them in the constructor, after that the config is immutable. the JaasConfiguration instance should have a name and the getAppConfigurationEntry method should only return config if the requested name matches.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12667901/HADOOP-10868.patch
          against trunk revision 5ec7fcd.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 8 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-auth hadoop-hdfs-project/hadoop-hdfs-httpfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4695//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4695//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12667901/HADOOP-10868.patch against trunk revision 5ec7fcd. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 8 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-auth hadoop-hdfs-project/hadoop-hdfs-httpfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4695//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4695//console This message is automatically generated.
          Hide
          Robert Kanter added a comment -

          The new patch does all of the changes except for ReflectionUtils because it's in hadoop-common.

          Show
          Robert Kanter added a comment - The new patch does all of the changes except for ReflectionUtils because it's in hadoop-common.
          Hide
          Alejandro Abdelnur added a comment -

          AuthenticationFilter.java:

          The following code seems it could be (most of it) push down to each provider impl:

                  if (signerSecretProviderName.equals("string")) {
                    String signatureSecret =
                        config.getProperty(configPrefix + SIGNATURE_SECRET, null);
                    secretProvider = new StringSignerSecretProvider(signatureSecret);
                  } else if (signerSecretProviderName.equals("random")) {
                    secretProvider = new RandomSignerSecretProvider();
                    randomSecret = true;
                  } else if (signerSecretProviderName.equals("zookeeper")) {
                    Object curatorClientObj = filterConfig.getServletContext()
                            .getAttribute(
                            ZOOKEEPER_SIGNER_SECRET_PROVIDER_CURATOR_CLIENT_ATTRIBUTE);
                    if (curatorClientObj != null
                            && curatorClientObj instanceof CuratorFramework) {
                      secretProvider =
                          new ZKSignerSecretProvider((CuratorFramework) curatorClientObj);
                    } else {
                      secretProvider = new ZKSignerSecretProvider();
                    }
          

          I would have a a function that gives me the Class of the provider, i.e.:

            private Class<? extends SignerSecretProvider> getProviderClass(String name) {
              if ("random".equals(name)) {
                name = RandomSignerSecretProvider.class.getName();
              } else if ("string".equals(name)) {
                name = StringSignerSecretProvider.class.getName();
              } else if ("zookeeper".equals(name)) {
                name = ZookeeperSignerSecretProvider.class.getName();
              }
                name = RandomSignerSecretProvider.class.getName();
              }
              try {
                return (Class<SignerSecretProvider>) Thread.currentThread().
                  getContextClassLoader().loadClass(signerSecretProviderName);
              } catch (Exception ex) {
                throw new ServletException(ex);
              }
            }
          

          Then I would use ReflectionUtils.newInstance(providerClass, conf) to instantiate it.
          I would add ServletContext as param to the init() method which in the case of ZK impl would be used to retrieve the curator from the context or create a new one (all within the ZK impl). For the string impl, the string secret would obtained from the config properties of the init().

          hadoop-auth/pom.xml:

          • curator-test should be with scope 'test'

          ZKSignerSecretProvider.java:

          • line 334, log message, type 'occured'
          • the ser/deser of the data we store in zookeeper, please add a version number at the beginning. This will allow us to change things in the future and support rolling upgrades.
          • once you have a curator client, it will handle reconnections correctly? else the createCuratorClient should be you have to be redo after a failed attempt.
          • I don’t like that ZK uses system properties for the auth info, but I guess that is has to be . I’ll dig into this a bit while you take care of the other feedback.
          Show
          Alejandro Abdelnur added a comment - AuthenticationFilter.java : The following code seems it could be (most of it) push down to each provider impl: if (signerSecretProviderName.equals( "string" )) { String signatureSecret = config.getProperty(configPrefix + SIGNATURE_SECRET, null ); secretProvider = new StringSignerSecretProvider(signatureSecret); } else if (signerSecretProviderName.equals( "random" )) { secretProvider = new RandomSignerSecretProvider(); randomSecret = true ; } else if (signerSecretProviderName.equals( "zookeeper" )) { Object curatorClientObj = filterConfig.getServletContext() .getAttribute( ZOOKEEPER_SIGNER_SECRET_PROVIDER_CURATOR_CLIENT_ATTRIBUTE); if (curatorClientObj != null && curatorClientObj instanceof CuratorFramework) { secretProvider = new ZKSignerSecretProvider((CuratorFramework) curatorClientObj); } else { secretProvider = new ZKSignerSecretProvider(); } I would have a a function that gives me the Class of the provider, i.e.: private Class <? extends SignerSecretProvider> getProviderClass( String name) { if ( "random" .equals(name)) { name = RandomSignerSecretProvider.class.getName(); } else if ( "string" .equals(name)) { name = StringSignerSecretProvider.class.getName(); } else if ( "zookeeper" .equals(name)) { name = ZookeeperSignerSecretProvider.class.getName(); } name = RandomSignerSecretProvider.class.getName(); } try { return ( Class <SignerSecretProvider>) Thread .currentThread(). getContextClassLoader().loadClass(signerSecretProviderName); } catch (Exception ex) { throw new ServletException(ex); } } Then I would use ReflectionUtils.newInstance(providerClass, conf) to instantiate it. I would add ServletContext as param to the init() method which in the case of ZK impl would be used to retrieve the curator from the context or create a new one (all within the ZK impl). For the string impl, the string secret would obtained from the config properties of the init() . hadoop-auth/pom.xml : curator-test should be with scope 'test' ZKSignerSecretProvider.java : line 334, log message, type 'occured' the ser/deser of the data we store in zookeeper, please add a version number at the beginning. This will allow us to change things in the future and support rolling upgrades. once you have a curator client, it will handle reconnections correctly? else the createCuratorClient should be you have to be redo after a failed attempt. I don’t like that ZK uses system properties for the auth info, but I guess that is has to be . I’ll dig into this a bit while you take care of the other feedback.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12666388/HADOOP-10868.patch
          against trunk revision 3a0142b.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-auth.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4644//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4644//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12666388/HADOOP-10868.patch against trunk revision 3a0142b. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-auth. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4644//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4644//console This message is automatically generated.
          Hide
          Robert Kanter added a comment -

          The new patch includes the minor changes that you mentioned.

          On the issue of system properties and the JAAS stuff, as far as I can tell, this is how it must be done. I looked into this a lot when doing the same thing for Oozie and the best thing I could find was that you don't have to actually write a jaas.conf file (see HBASE-4791), which is what the JaasConfiguration class does (and is heavily based on what HBase code). I'm not a big fan of all this either, but it appears to be the only way unless ZooKeeper changes or if there's some clever trick I missed (in which case, we should update Oozie and HBase too).

          Show
          Robert Kanter added a comment - The new patch includes the minor changes that you mentioned. On the issue of system properties and the JAAS stuff, as far as I can tell, this is how it must be done. I looked into this a lot when doing the same thing for Oozie and the best thing I could find was that you don't have to actually write a jaas.conf file (see HBASE-4791 ), which is what the JaasConfiguration class does (and is heavily based on what HBase code). I'm not a big fan of all this either, but it appears to be the only way unless ZooKeeper changes or if there's some clever trick I missed (in which case, we should update Oozie and HBase too).
          Hide
          Alejandro Abdelnur added a comment -

          AuthenticationFilter.java:

          • typo in javadoc 'imeplementation'
          • ZOOKEEPER_SIGNER_SECRET_PROVIDER_ATTRIBUTE constant should have CURATOR in its name, else it is not clear.

          JaasConfiguration.java:

          • class should be annotated as @InterfaceAudience.Private

          ZKSignerSecretProvider.java:

          • createCuratorClient(), I don’t like the usage of System properties to set up context, this is not entirely safe. Is there a way to pass all config to the builder via methods?
          • The JaasConfiguration and things. If you are logged in using a UGI.loginFromKeytab() and do a ugi.doAs(), can you get rid of all this?
          Show
          Alejandro Abdelnur added a comment - AuthenticationFilter.java : typo in javadoc 'imeplementation' ZOOKEEPER_SIGNER_SECRET_PROVIDER_ATTRIBUTE constant should have CURATOR in its name, else it is not clear. JaasConfiguration.java : class should be annotated as @InterfaceAudience.Private ZKSignerSecretProvider.java : createCuratorClient() , I don’t like the usage of System properties to set up context, this is not entirely safe. Is there a way to pass all config to the builder via methods? The JaasConfiguration and things. If you are logged in using a UGI.loginFromKeytab() and do a ugi.doAs(), can you get rid of all this?
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12663560/HADOOP-10868.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-auth.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4538//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4538//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12663560/HADOOP-10868.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-auth. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4538//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4538//console This message is automatically generated.
          Hide
          Robert Kanter added a comment -

          The new patch fixes the findbugs warnings. The release audit warnings were not for the patch (looks to be some HDFS encryption thing...).

          Show
          Robert Kanter added a comment - The new patch fixes the findbugs warnings. The release audit warnings were not for the patch (looks to be some HDFS encryption thing...).
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12663449/HADOOP-10868.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings.

          -1 release audit. The applied patch generated 3 release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-auth.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4526//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/4526//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/4526//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4526//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12663449/HADOOP-10868.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings. -1 release audit . The applied patch generated 3 release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-auth. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4526//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/4526//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/4526//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4526//console This message is automatically generated.
          Hide
          Robert Kanter added a comment -

          The patch adds the ZKSignerSecretProvider and also updates a bunch of documentation/javadoc. It uses Curator to talk to ZooKeeper (this handles a lot of connection issues, etc for us). The security-related code for using Kerberos and Kerberos-backed ACLs with ZooKeeper is largely copied from Oozie.

          Here’s an overview of how ZKSignerSecretProvider works:

          • The previous, current, and next secrets, plus the next rollover date get stored in a znode
          • On startup, it will try to read from this znode to use these secrets and synchronize it’s rollover
          • The next secret get decided right after the secret gets rolled over so that rolling to it can be immediate without a network call to ZooKeeper
          • The next secret gets decided like this:
            1. All servers generate a new next secret
            2. They then all try to write to the znode at roughly the same time
            3. The znode has a version number, and only one of the servers will succeed (doesn’t matter which)
            4. The servers then load the next secret from the znode so they all have the same one
          • I did the coordination this way so that we wouldn’t need to do any leader elections and don’t need any sort of locking
          • There’s a bunch of configuration properties (mostly for Kerberos stuff)
            • A subclass of AuthenticationFilter can also provide ZKSignerSecretProvider with it’s own Curator client, preconfigured however they want. For example, I’m planning on doing this with Oozie so (a) we can reuse the same Curator client that Oozie already manages and (b) the ZK kerberos configs don’t need to be set by the user twice.
          Show
          Robert Kanter added a comment - The patch adds the ZKSignerSecretProvider and also updates a bunch of documentation/javadoc. It uses Curator to talk to ZooKeeper (this handles a lot of connection issues, etc for us). The security-related code for using Kerberos and Kerberos-backed ACLs with ZooKeeper is largely copied from Oozie. Here’s an overview of how ZKSignerSecretProvider works: The previous, current, and next secrets, plus the next rollover date get stored in a znode On startup, it will try to read from this znode to use these secrets and synchronize it’s rollover The next secret get decided right after the secret gets rolled over so that rolling to it can be immediate without a network call to ZooKeeper The next secret gets decided like this: All servers generate a new next secret They then all try to write to the znode at roughly the same time The znode has a version number, and only one of the servers will succeed (doesn’t matter which) The servers then load the next secret from the znode so they all have the same one I did the coordination this way so that we wouldn’t need to do any leader elections and don’t need any sort of locking There’s a bunch of configuration properties (mostly for Kerberos stuff) A subclass of AuthenticationFilter can also provide ZKSignerSecretProvider with it’s own Curator client, preconfigured however they want. For example, I’m planning on doing this with Oozie so (a) we can reuse the same Curator client that Oozie already manages and (b) the ZK kerberos configs don’t need to be set by the user twice.
          Hide
          Larry McCay added a comment -

          Yep, sounds good. Thanks!

          On Mon, Jul 21, 2014 at 9:29 PM, Robert Kanter (JIRA) <jira@apache.org>

          Show
          Larry McCay added a comment - Yep, sounds good. Thanks! On Mon, Jul 21, 2014 at 9:29 PM, Robert Kanter (JIRA) <jira@apache.org>
          Hide
          Robert Kanter added a comment -

          Currently, the secret used for signing tokens is static and if set to random, there's no way to synchronize it across multiple servers (e.g. for an HA environment). The idea of these two JIRAs is to allow more flexibility for how the secret is obtained by making it pluggable. Then we can do things like have a rolling secret, synchronize across multiple servers, etc. So one of the implementations could be to obtain a secret from a store via the Credential Provider API. In any case, I think it will be more clear once I put up the patch for HADOOP-10791, which I'll try to do soon.

          Show
          Robert Kanter added a comment - Currently, the secret used for signing tokens is static and if set to random, there's no way to synchronize it across multiple servers (e.g. for an HA environment). The idea of these two JIRAs is to allow more flexibility for how the secret is obtained by making it pluggable. Then we can do things like have a rolling secret, synchronize across multiple servers, etc. So one of the implementations could be to obtain a secret from a store via the Credential Provider API. In any case, I think it will be more clear once I put up the patch for HADOOP-10791 , which I'll try to do soon.
          Hide
          Larry McCay added a comment -

          hmmm, I thought that was exactly what you were doing. I must be missing something in what this jira is supposed to accomplish. If it makes sense as a credential provider later then we can probably always decorate it with a thin credential provider wrapper.

          Show
          Larry McCay added a comment - hmmm, I thought that was exactly what you were doing. I must be missing something in what this jira is supposed to accomplish. If it makes sense as a credential provider later then we can probably always decorate it with a thin credential provider wrapper.
          Hide
          Robert Kanter added a comment -

          I took a quick look at HADOOP-10607. If my understanding is correct, it basically allows you to have a store (with pluggable backings) where you put stuff like passwords and then in your site files, you can specify some kind of path or alias that refers to a password in the store instead of having to put the password in the site file. Is that right?

          If that's correct, then I'm not sure how the zk secret provider fits in with the credential provider? The only thing that comes to mind is that the String secret provider (i.e. setting the signature.secret property to something) could be specified in the store and get loaded from there. A zookeeper based credential provider sounds like a good idea, with Kerberos ACLs of course; but that would be a new JIRA.

          Can you elaborate on what you're suggesting?

          Show
          Robert Kanter added a comment - I took a quick look at HADOOP-10607 . If my understanding is correct, it basically allows you to have a store (with pluggable backings) where you put stuff like passwords and then in your site files, you can specify some kind of path or alias that refers to a password in the store instead of having to put the password in the site file. Is that right? If that's correct, then I'm not sure how the zk secret provider fits in with the credential provider? The only thing that comes to mind is that the String secret provider (i.e. setting the signature.secret property to something) could be specified in the store and get loaded from there. A zookeeper based credential provider sounds like a good idea, with Kerberos ACLs of course; but that would be a new JIRA. Can you elaborate on what you're suggesting?
          Hide
          Larry McCay added a comment -

          Hi Robert Kanter - now that it is broken out separately, it strikes me that this could actually be implemented as a credential provider based on HADOOP-10607. As long as the pluggability provided in HADOOP-10791 can accommodate the Credential Provider API, I don't really care whether it does but it would enable that provider to be used wherever cred provider API is uptaken. I think that a zk based provider would probably be great addition.

          What do you think?

          Show
          Larry McCay added a comment - Hi Robert Kanter - now that it is broken out separately, it strikes me that this could actually be implemented as a credential provider based on HADOOP-10607 . As long as the pluggability provided in HADOOP-10791 can accommodate the Credential Provider API, I don't really care whether it does but it would enable that provider to be used wherever cred provider API is uptaken. I think that a zk based provider would probably be great addition. What do you think?

            People

            • Assignee:
              Robert Kanter
              Reporter:
              Robert Kanter
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development