Hadoop Common
  1. Hadoop Common
  2. HADOOP-10656

The password keystore file is not picked by LDAP group mapping

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.0
    • Fix Version/s: 2.5.0
    • Component/s: security
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The user configured password file(LDAP_KEYSTORE_PASSWORD_FILE_KEY) will not be picked by LdapGroupsMapping:

      In setConf():

          keystorePass =
              conf.get(LDAP_KEYSTORE_PASSWORD_KEY, LDAP_KEYSTORE_PASSWORD_DEFAULT);
          if (keystorePass.isEmpty()) {
            keystorePass = extractPassword(
              conf.get(LDAP_KEYSTORE_PASSWORD_KEY, LDAP_KEYSTORE_PASSWORD_DEFAULT)); 
          }
      
      1. HADOOP-10656.patch
        1.0 kB
        Brandon Li
      2. HADOOP-10656.002.patch
        2 kB
        Brandon Li
      3. HADOOP-10656.003.patch
        2 kB
        Brandon Li

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #1799 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1799/)
          HADOOP-10656. The password keystore file is not picked by LDAP group mapping. Contributed by Brandon Li (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1601985)

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1799 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1799/ ) HADOOP-10656 . The password keystore file is not picked by LDAP group mapping. Contributed by Brandon Li (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1601985 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk #1772 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1772/)
          HADOOP-10656. The password keystore file is not picked by LDAP group mapping. Contributed by Brandon Li (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1601985)

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #1772 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1772/ ) HADOOP-10656 . The password keystore file is not picked by LDAP group mapping. Contributed by Brandon Li (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1601985 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #581 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/581/)
          HADOOP-10656. The password keystore file is not picked by LDAP group mapping. Contributed by Brandon Li (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1601985)

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #581 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/581/ ) HADOOP-10656 . The password keystore file is not picked by LDAP group mapping. Contributed by Brandon Li (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1601985 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #5683 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5683/)
          HADOOP-10656. The password keystore file is not picked by LDAP group mapping. Contributed by Brandon Li (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1601985)

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #5683 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5683/ ) HADOOP-10656 . The password keystore file is not picked by LDAP group mapping. Contributed by Brandon Li (brandonli: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1601985 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
          Hide
          Brandon Li added a comment -

          Thank you, Chris, for the review. I've committed the patch.

          Show
          Brandon Li added a comment - Thank you, Chris, for the review. I've committed the patch.
          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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 1.3.9) 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-common.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4044//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4044//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/12649833/HADOOP-10656.003.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 1.3.9) 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-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4044//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4044//console This message is automatically generated.
          Hide
          Chris Nauroth added a comment -

          +1 for the v3 patch, pending Jenkins run. Thanks again, Brandon!

          Show
          Chris Nauroth added a comment - +1 for the v3 patch, pending Jenkins run. Thanks again, Brandon!
          Hide
          Brandon Li added a comment -

          Thank you, Chris. IOUtils#cleanup makes more sense and the change is smaller. Updated the patch.

          Show
          Brandon Li added a comment - Thank you, Chris. IOUtils#cleanup makes more sense and the change is smaller. Updated the patch.
          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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 1.3.9) 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-common.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4043//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4043//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/12649705/HADOOP-10656.002.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 1.3.9) 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-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4043//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4043//console This message is automatically generated.
          Hide
          Chris Nauroth added a comment -
              } finally {
                if (reader != null)
                  try {
                    reader.close();
                  } catch (IOException e) {
                    LOG.warn("Could not close password file: " + pwFile, e);
                  }
              }
          

          Minor nit-pick: In the above, can you please add curly braces after the if statement, just to clarify the nesting? Alternatively, you could replace that code segment with IOUtils#cleanup. +1 for the patch after that small change. Thank you again, Brandon!

          Show
          Chris Nauroth added a comment - } finally { if (reader != null ) try { reader.close(); } catch (IOException e) { LOG.warn( "Could not close password file: " + pwFile, e); } } Minor nit-pick: In the above, can you please add curly braces after the if statement, just to clarify the nesting? Alternatively, you could replace that code segment with IOUtils#cleanup . +1 for the patch after that small change. Thank you again, Brandon!
          Hide
          Brandon Li added a comment -

          Thanks for the review, Chris Nauroth. Let's use this JIRA to fix both issues. I've uploaded a new patch.

          Show
          Brandon Li added a comment - Thanks for the review, Chris Nauroth . Let's use this JIRA to fix both issues. I've uploaded a new patch.
          Hide
          Chris Nauroth added a comment -

          Hi, Brandon Li. Nice catch, and thank you for fixing it.

          This is not directly related to your patch, but I noticed that the LdapGroupsMapping#extractPassword method is susceptible to a file descriptor leak. If one of the Reader#read calls throws an IOException, then we won't close the Reader. Do you think we could fix this while we're in this class? I think we'd just need to move the Reader#close call into a finally block.

          Show
          Chris Nauroth added a comment - Hi, Brandon Li . Nice catch, and thank you for fixing it. This is not directly related to your patch, but I noticed that the LdapGroupsMapping#extractPassword method is susceptible to a file descriptor leak. If one of the Reader#read calls throws an IOException , then we won't close the Reader . Do you think we could fix this while we're in this class? I think we'd just need to move the Reader#close call into a finally block.
          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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 1.3.9) 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-common:

          org.apache.hadoop.ha.TestZKFailoverController

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4039//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4039//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/12649638/HADOOP-10656.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 1.3.9) 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-common: org.apache.hadoop.ha.TestZKFailoverController +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4039//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4039//console This message is automatically generated.

            People

            • Assignee:
              Brandon Li
              Reporter:
              Brandon Li
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development