Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-2096

Secure local filesystem IO from symlink vulnerabilities

    Details

    • Hadoop Flags:
      Reviewed
    • Release Note:
      The TaskTracker now uses the libhadoop JNI library to operate securely on local files when security is enabled. Secure clusters must ensure that libhadoop.so is available to the TaskTracker.

      Description

      This JIRA is to contribute a patch developed on the private security@ mailing list.

      The vulnerability is that MR daemons occasionally open files that are located in a path where the user has write access. A malicious user may place a symlink in place of the expected file in order to cause the daemon to instead read another file on the system – one which the attacker may not naturally be able to access. This includes delegation tokens belong to other users, log files, keytabs, etc.

      1. mapreduce-2096.2.txt
        24 kB
        Todd Lipcon
      2. mapreduce-2096.txt
        24 kB
        Todd Lipcon
      3. mapreduce-2096-index-oob.txt
        0.5 kB
        Todd Lipcon
      4. secure-files-9.txt
        1.81 MB
        Todd Lipcon
      5. secure-files-authorized-jvm-fix.txt
        2 kB
        Todd Lipcon

        Issue Links

          Activity

          Hide
          Todd Lipcon added a comment -

          Here's the 0.20-based patch that we developed on the security mailing list. We'll use this JIRA to update it to trunk (with the common half going to HADOOP-6978).

          We also just identified one more place that we need to do this behavior - jobconf_history.jsp needs to use the secure IO code.

          Show
          Todd Lipcon added a comment - Here's the 0.20-based patch that we developed on the security mailing list. We'll use this JIRA to update it to trunk (with the common half going to HADOOP-6978 ). We also just identified one more place that we need to do this behavior - jobconf_history.jsp needs to use the secure IO code.
          Hide
          Owen O'Malley added a comment -

          I think we need to move away from checking in any of the autoconf/automake generated files. Let's run autoreconf everytime and make the ignored by subversion and git.

          It should also be noted that Devaraj contributed significantly to this patch.

          Show
          Owen O'Malley added a comment - I think we need to move away from checking in any of the autoconf/automake generated files. Let's run autoreconf everytime and make the ignored by subversion and git. It should also be noted that Devaraj contributed significantly to this patch.
          Hide
          Todd Lipcon added a comment -

          In testing I came upon one spot where the new JVM authorization in this patch was incorrectly triggered:

          2010-10-03 22:30:36,252 WARN org.apache.hadoop.mapred.TaskRunner: attempt_201010032228_0001_m_000001_2 Reporting Diagnostics
          java.io.IOException: JVM with mapred is not authorized for job_201010032228_0001
          at org.apache.hadoop.mapred.TaskTracker.ensureAuthorizedJVM(TaskTracker.java:3096)
          at org.apache.hadoop.mapred.TaskTracker.reportDiagnosticInfo(TaskTracker.java:3164)
          at org.apache.hadoop.mapred.TaskRunner.run(TaskRunner.java:232)

          This patch fixes the above issue by having TaskRunner call to a version of reportDiagonsticInfo that doesn't authorize the caller.

          Show
          Todd Lipcon added a comment - In testing I came upon one spot where the new JVM authorization in this patch was incorrectly triggered: 2010-10-03 22:30:36,252 WARN org.apache.hadoop.mapred.TaskRunner: attempt_201010032228_0001_m_000001_2 Reporting Diagnostics java.io.IOException: JVM with mapred is not authorized for job_201010032228_0001 at org.apache.hadoop.mapred.TaskTracker.ensureAuthorizedJVM(TaskTracker.java:3096) at org.apache.hadoop.mapred.TaskTracker.reportDiagnosticInfo(TaskTracker.java:3164) at org.apache.hadoop.mapred.TaskRunner.run(TaskRunner.java:232) This patch fixes the above issue by having TaskRunner call to a version of reportDiagonsticInfo that doesn't authorize the caller.
          Hide
          Devaraj Das added a comment -

          This patch fixes the above issue by having TaskRunner call to a version of reportDiagonsticInfo that doesn't authorize the caller.

          The same should be done for fsError.

          Show
          Devaraj Das added a comment - This patch fixes the above issue by having TaskRunner call to a version of reportDiagonsticInfo that doesn't authorize the caller. The same should be done for fsError.
          Hide
          Todd Lipcon added a comment -

          Another small bug fix - if a task fails before it produces any logs (eg if taskcontroller is misconfigured) then this patch will throw an IndexOutOfBoundsException in the TaskLogsTruncater. I think this patch should fix it.

          Show
          Todd Lipcon added a comment - Another small bug fix - if a task fails before it produces any logs (eg if taskcontroller is misconfigured) then this patch will throw an IndexOutOfBoundsException in the TaskLogsTruncater. I think this patch should fix it.
          Hide
          Owen O'Malley added a comment -

          How is the trunk patch going for this one?

          Show
          Owen O'Malley added a comment - How is the trunk patch going for this one?
          Hide
          Todd Lipcon added a comment -

          Thanks for reminding me to upload the trunk work. I put a patch up on HADOOP-6978 which blocks this. Working on the MR trunk patch as well while that one gets reviewed/committed.

          Show
          Todd Lipcon added a comment - Thanks for reminding me to upload the trunk work. I put a patch up on HADOOP-6978 which blocks this. Working on the MR trunk patch as well while that one gets reviewed/committed.
          Hide
          Todd Lipcon added a comment -

          Does anyone have a suggestion on how to get common's native library build onto mapreduce's library path post-split?

          It seems we should be publishing a tarball of common/build/native into maven, and then retrieving it with ivy from mapreduce, perhaps? Does anyone have a better idea or should I open a JIRA to publish the native build as an artifact?

          Show
          Todd Lipcon added a comment - Does anyone have a suggestion on how to get common's native library build onto mapreduce's library path post-split? It seems we should be publishing a tarball of common/build/native into maven, and then retrieving it with ivy from mapreduce, perhaps? Does anyone have a better idea or should I open a JIRA to publish the native build as an artifact?
          Hide
          Devaraj Das added a comment -

          One thought - could we get away with opening the files in C using open(...O_RDONLY|O_NOFOLLOW), and have the JNI return a fdobject that is then used to get the fileinputstreams ? I am wondering whether this is sufficient for preventing the symlink attacks..

          Show
          Devaraj Das added a comment - One thought - could we get away with opening the files in C using open(...O_RDONLY|O_NOFOLLOW), and have the JNI return a fdobject that is then used to get the fileinputstreams ? I am wondering whether this is sufficient for preventing the symlink attacks..
          Hide
          Todd Lipcon added a comment -

          Unfortunately O_NOFOLLOW is not sufficient. The man page for open(2) says:

          If pathname is a symbolic link, then the open fails... Symbolic links in earlier components of the pathname will still be followed.

          ... meaning that a user can still exploit this by substituting a symlink for some intermediate path component, and read someone else's stderr/stdout file.

          Show
          Todd Lipcon added a comment - Unfortunately O_NOFOLLOW is not sufficient. The man page for open(2) says: If pathname is a symbolic link, then the open fails... Symbolic links in earlier components of the pathname will still be followed. ... meaning that a user can still exploit this by substituting a symlink for some intermediate path component, and read someone else's stderr/stdout file.
          Hide
          Devaraj Das added a comment -

          Does anyone have a suggestion on how to get common's native library build onto mapreduce's library path post-split?

          Do we need it immediately? I believe we don't have any MR tests with security ON. If that is the case, we could go ahead with a MR patch that is not dependent on the native libs (HADOOP-6978).

          Show
          Devaraj Das added a comment - Does anyone have a suggestion on how to get common's native library build onto mapreduce's library path post-split? Do we need it immediately? I believe we don't have any MR tests with security ON. If that is the case, we could go ahead with a MR patch that is not dependent on the native libs ( HADOOP-6978 ).
          Hide
          Todd Lipcon added a comment -

          We don't need it absolutely immediately, except that it's very tough to verify this change even manually in the current state of the project

          I've got the patch forward ported but still need to fix up one or two things and run through the non-secure unit tests. I should post a final version this weekend.

          Show
          Todd Lipcon added a comment - We don't need it absolutely immediately, except that it's very tough to verify this change even manually in the current state of the project I've got the patch forward ported but still need to fix up one or two things and run through the non-secure unit tests. I should post a final version this weekend.
          Hide
          Todd Lipcon added a comment -

          patch for trunk. This passes commit tests but haven't run the full suite.

          Show
          Todd Lipcon added a comment - patch for trunk. This passes commit tests but haven't run the full suite.
          Hide
          Devaraj Das added a comment -

          Patch looks fine. Todd, could you please get back with the results from running the full test suite + test-patch.

          Show
          Devaraj Das added a comment - Patch looks fine. Todd, could you please get back with the results from running the full test suite + test-patch.
          Hide
          Todd Lipcon added a comment -

          Findbugs found a case where we weren't checking the return value of InputStream.skip. New patch uses IOUtils.skipFully. Rerunning tests.

          Show
          Todd Lipcon added a comment - Findbugs found a case where we weren't checking the return value of InputStream.skip. New patch uses IOUtils.skipFully. Rerunning tests.
          Hide
          Todd Lipcon added a comment -

          Results on mapreduce-2096.2.txt:

          [exec] +1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to include 3 new or modified tests.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          [exec]
          [exec] +1 system test framework. The patch passed system test framework compile.

          Unit tests pass except for the known timeouts from trunk.

          Show
          Todd Lipcon added a comment - Results on mapreduce-2096.2.txt: [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system test framework. The patch passed system test framework compile. Unit tests pass except for the known timeouts from trunk.
          Hide
          Devaraj Das added a comment -

          +1

          Show
          Devaraj Das added a comment - +1
          Hide
          Todd Lipcon added a comment -

          Committed to trunk and 0.22

          Show
          Todd Lipcon added a comment - Committed to trunk and 0.22
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #572 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/572/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #572 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/572/ )
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-22-branch #33 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-22-branch/33/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-22-branch #33 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-22-branch/33/ )
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #643 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk/643/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #643 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk/643/ )

            People

            • Assignee:
              Todd Lipcon
              Reporter:
              Todd Lipcon
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development