Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-13992

KMS should load SSL configuration the same way as SSLFactory

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha2
    • Fix Version/s: 3.0.0-alpha4
    • Component/s: kms, security
    • Labels:
      None

      Description

      HADOOP-13597 loads SSL configuration in the different way than SSLFactory and other existing methods:

      • SSLFactory#readSSLConfiguration
      • DFSUtil#loadSslConfiguration
      • WebAppUtils#loadSslConfiguration

      It should conform to the existing method.

      1. HADOOP-13992.001.patch
        6 kB
        John Zhuge
      2. HADOOP-13992.002.patch
        6 kB
        John Zhuge

        Issue Links

          Activity

          Hide
          jzhuge John Zhuge added a comment -

          Patch 001

          • Make SSLFactory#readSSLConfiguration reusable
          • Add parameter sslConf to KMSWebServer constructor
          • MiniKMS and KMSWebServer$main leverage SSLFactory#readSSLConfiguration

          Testing done

          • TestSSLFactory
          • TestKMS
          • Run hadoop key list/create/roll/delete in insecure pseudo-dist cluster
          • Run hadoop key list/create/roll/delete in ssl pseudo-dist cluster
          • Run KMS_HTTP_PORT=1234 bin/hadoop kms to verify KMS is running on port 1234
          • Run KMS_SSL_KEYSTORE_PASS=abcd bin/hadoop kms to expect wrong password
          • Run KMS_SSL_KEYSTORE_FILE=/tmp/tt bin/hadoop kms to expect invalid keystore path
          Show
          jzhuge John Zhuge added a comment - Patch 001 Make SSLFactory#readSSLConfiguration reusable Add parameter sslConf to KMSWebServer constructor MiniKMS and KMSWebServer$main leverage SSLFactory#readSSLConfiguration Testing done TestSSLFactory TestKMS Run hadoop key list/create/roll/delete in insecure pseudo-dist cluster Run hadoop key list/create/roll/delete in ssl pseudo-dist cluster Run KMS_HTTP_PORT=1234 bin/hadoop kms to verify KMS is running on port 1234 Run KMS_SSL_KEYSTORE_PASS=abcd bin/hadoop kms to expect wrong password Run KMS_SSL_KEYSTORE_FILE=/tmp/tt bin/hadoop kms to expect invalid keystore path
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 33s Maven dependency ordering for branch
          +1 mvninstall 13m 18s trunk passed
          +1 compile 10m 26s trunk passed
          +1 checkstyle 0m 36s trunk passed
          +1 mvnsite 2m 3s trunk passed
          +1 mvneclipse 0m 34s trunk passed
          +1 findbugs 2m 10s trunk passed
          +1 javadoc 1m 6s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 0m 54s the patch passed
          +1 compile 10m 40s the patch passed
          +1 javac 10m 40s the patch passed
          +1 checkstyle 0m 35s hadoop-common-project: The patch generated 0 new + 10 unchanged - 1 fixed = 10 total (was 11)
          +1 mvnsite 2m 1s the patch passed
          +1 mvneclipse 0m 35s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 12s the patch passed
          +1 javadoc 1m 6s the patch passed
          +1 unit 8m 36s hadoop-common in the patch passed.
          +1 unit 2m 13s hadoop-kms in the patch passed.
          +1 asflicense 0m 35s The patch does not generate ASF License warnings.
          65m 19s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13992
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12847710/HADOOP-13992.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 29c348564355 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / cf69557
          Default Java 1.8.0_111
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11444/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms U: hadoop-common-project
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11444/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 33s Maven dependency ordering for branch +1 mvninstall 13m 18s trunk passed +1 compile 10m 26s trunk passed +1 checkstyle 0m 36s trunk passed +1 mvnsite 2m 3s trunk passed +1 mvneclipse 0m 34s trunk passed +1 findbugs 2m 10s trunk passed +1 javadoc 1m 6s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 0m 54s the patch passed +1 compile 10m 40s the patch passed +1 javac 10m 40s the patch passed +1 checkstyle 0m 35s hadoop-common-project: The patch generated 0 new + 10 unchanged - 1 fixed = 10 total (was 11) +1 mvnsite 2m 1s the patch passed +1 mvneclipse 0m 35s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 12s the patch passed +1 javadoc 1m 6s the patch passed +1 unit 8m 36s hadoop-common in the patch passed. +1 unit 2m 13s hadoop-kms in the patch passed. +1 asflicense 0m 35s The patch does not generate ASF License warnings. 65m 19s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13992 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12847710/HADOOP-13992.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 29c348564355 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / cf69557 Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11444/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms U: hadoop-common-project Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11444/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks John Zhuge for the work here! Idea seems good.

          One question is, are we seeing SSLFactory reading more stuff from Configuration in the near future? If not it maybe slightly cleaner to use a boolean for SSL_REQUIRE_CLIENT_CERT_KEY as the input parameter of readSSLConfiguration, instead of the conf.

          Patch also needs a rebase. I'm linking this to HADOOP-13597 as well.

          Show
          xiaochen Xiao Chen added a comment - Thanks John Zhuge for the work here! Idea seems good. One question is, are we seeing SSLFactory reading more stuff from Configuration in the near future? If not it maybe slightly cleaner to use a boolean for SSL_REQUIRE_CLIENT_CERT_KEY as the input parameter of readSSLConfiguration , instead of the conf. Patch also needs a rebase. I'm linking this to HADOOP-13597 as well.
          Hide
          jzhuge John Zhuge added a comment -

          readSSLConfiguration needs conf parameter to get SSL_CLIENT_CONF_KEY or SSL_SERVER_CONF_KEY, in addition to SSL_REQUIRE_CLIENT_CERT_KEY.

          If I ever implement my proposal for HADOOP-13987, I only have to modify the readSSLConfiguration to fix SSLFactory, KMS, and HttpFS all at once. The implementation will need the conf parameter.

          Show
          jzhuge John Zhuge added a comment - readSSLConfiguration needs conf parameter to get SSL_CLIENT_CONF_KEY or SSL_SERVER_CONF_KEY , in addition to SSL_REQUIRE_CLIENT_CERT_KEY . If I ever implement my proposal for HADOOP-13987 , I only have to modify the readSSLConfiguration to fix SSLFactory, KMS, and HttpFS all at once. The implementation will need the conf parameter.
          Hide
          jzhuge John Zhuge added a comment -

          Patch 002

          • Rebased
          Show
          jzhuge John Zhuge added a comment - Patch 002 Rebased
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks for the explanation John Zhuge, WFM. +1 on patch 2 pending jenkins.

          Show
          xiaochen Xiao Chen added a comment - Thanks for the explanation John Zhuge , WFM. +1 on patch 2 pending jenkins.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 38s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 32s Maven dependency ordering for branch
          +1 mvninstall 13m 9s trunk passed
          +1 compile 12m 58s trunk passed
          +1 checkstyle 0m 33s trunk passed
          +1 mvnsite 1m 55s trunk passed
          +1 mvneclipse 0m 36s trunk passed
          +1 findbugs 1m 57s trunk passed
          +1 javadoc 1m 6s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 0m 52s the patch passed
          +1 compile 11m 39s the patch passed
          +1 javac 11m 39s the patch passed
          +1 checkstyle 0m 32s hadoop-common-project: The patch generated 0 new + 10 unchanged - 1 fixed = 10 total (was 11)
          +1 mvnsite 1m 51s the patch passed
          +1 mvneclipse 0m 37s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 7s the patch passed
          +1 javadoc 1m 5s the patch passed
          -1 unit 8m 14s hadoop-common in the patch failed.
          +1 unit 2m 15s hadoop-kms in the patch passed.
          +1 asflicense 0m 33s The patch does not generate ASF License warnings.
          67m 49s



          Reason Tests
          Failed junit tests hadoop.security.TestKDiag



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13992
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12849654/HADOOP-13992.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 99119a3aaf0a 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 2034315
          Default Java 1.8.0_121
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/11523/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11523/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms U: hadoop-common-project
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11523/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 38s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 32s Maven dependency ordering for branch +1 mvninstall 13m 9s trunk passed +1 compile 12m 58s trunk passed +1 checkstyle 0m 33s trunk passed +1 mvnsite 1m 55s trunk passed +1 mvneclipse 0m 36s trunk passed +1 findbugs 1m 57s trunk passed +1 javadoc 1m 6s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 52s the patch passed +1 compile 11m 39s the patch passed +1 javac 11m 39s the patch passed +1 checkstyle 0m 32s hadoop-common-project: The patch generated 0 new + 10 unchanged - 1 fixed = 10 total (was 11) +1 mvnsite 1m 51s the patch passed +1 mvneclipse 0m 37s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 7s the patch passed +1 javadoc 1m 5s the patch passed -1 unit 8m 14s hadoop-common in the patch failed. +1 unit 2m 15s hadoop-kms in the patch passed. +1 asflicense 0m 33s The patch does not generate ASF License warnings. 67m 49s Reason Tests Failed junit tests hadoop.security.TestKDiag Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13992 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12849654/HADOOP-13992.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 99119a3aaf0a 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2034315 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/11523/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11523/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms U: hadoop-common-project Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11523/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jzhuge John Zhuge added a comment -

          TestKDiag failure is unrelated and the test passes locally. Filed HADOOP-14030 for PreCommit TestKDiag failure.

          Show
          jzhuge John Zhuge added a comment - TestKDiag failure is unrelated and the test passes locally. Filed HADOOP-14030 for PreCommit TestKDiag failure.
          Hide
          xiaochen Xiao Chen added a comment -

          +1.
          Committed to trunk, thanks for the contribution John Zhuge.

          Show
          xiaochen Xiao Chen added a comment - +1. Committed to trunk, thanks for the contribution John Zhuge .
          Hide
          jzhuge John Zhuge added a comment -

          Thanks Xiao Chen for the review and commit !

          Show
          jzhuge John Zhuge added a comment - Thanks Xiao Chen for the review and commit !
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11186 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11186/)
          HADOOP-13992. KMS should load SSL configuration the same way as (xiao: rev ebd40056a07df5807baf0652a47ea97334038f4d)

          • (edit) hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/MiniKMS.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/SSLFactory.java
          • (edit) hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11186 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11186/ ) HADOOP-13992 . KMS should load SSL configuration the same way as (xiao: rev ebd40056a07df5807baf0652a47ea97334038f4d) (edit) hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/MiniKMS.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/SSLFactory.java (edit) hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java

            People

            • Assignee:
              jzhuge John Zhuge
              Reporter:
              jzhuge John Zhuge
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development