HADOOP-9511. Adding support for additional input streams (FSDataInputStream and RandomAccessFile) in SecureIOUtils so as to help YARN-578. Contributed by Omkar Vinit Joshi.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1479014 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -56,6 +56,10 @@ Release 2.0.5-beta - UNRELEASED
     HADOOP-9523. Provide a generic IBM java vendor flag in PlatformName.java
     to support non-Sun JREs. (Tian Hong Wang via suresh)
 
+    HADOOP-9511. Adding support for additional input streams (FSDataInputStream
+    and RandomAccessFile) in SecureIOUtils so as to help YARN-578. (Omkar Vinit
+    Joshi via vinodkv)
+
   OPTIMIZATIONS
 
     HADOOP-9150. Avoid unnecessary DNS resolution attempts for logical URIs

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java

@@ -22,8 +22,10 @@ import java.io.FileDescriptor;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.RandomAccessFile;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsPermission;
@@ -33,6 +35,8 @@ import org.apache.hadoop.io.nativeio.NativeIOException;
 import org.apache.hadoop.io.nativeio.NativeIO.Stat;
 import org.apache.hadoop.security.UserGroupInformation;
 
+import com.google.common.annotations.VisibleForTesting;
+
 /**
  * This class provides secure APIs for opening and creating files on the local
  * disk. The main issue this class tries to handle is that of symlink traversal.
@@ -88,6 +92,95 @@ public class SecureIOUtils {
   private final static boolean skipSecurity;
   private final static FileSystem rawFilesystem;
 
+  /**
+   * Open the given File for random read access, verifying the expected user/
+   * group constraints if security is enabled.
+   * 
+   * Note that this function provides no additional security checks if hadoop
+   * security is disabled, since doing the checks would be too expensive when
+   * native libraries are not available.
+   * 
+   * @param f file that we are trying to open
+   * @param mode mode in which we want to open the random access file
+   * @param expectedOwner the expected user owner for the file
+   * @param expectedGroup the expected group owner for the file
+   * @throws IOException if an IO error occurred or if the user/group does
+   * not match when security is enabled.
+   */
+  public static RandomAccessFile openForRandomRead(File f,
+      String mode, String expectedOwner, String expectedGroup)
+      throws IOException {
+    if (!UserGroupInformation.isSecurityEnabled()) {
+      return new RandomAccessFile(f, mode);
+    }
+    return forceSecureOpenForRandomRead(f, mode, expectedOwner, expectedGroup);
+  }
+
+  /**
+   * Same as openForRandomRead except that it will run even if security is off.
+   * This is used by unit tests.
+   */
+  @VisibleForTesting
+  protected static RandomAccessFile forceSecureOpenForRandomRead(File f,
+      String mode, String expectedOwner, String expectedGroup)
+      throws IOException {
+    RandomAccessFile raf = new RandomAccessFile(f, mode);
+    boolean success = false;
+    try {
+      Stat stat = NativeIO.getFstat(raf.getFD());
+      checkStat(f, stat.getOwner(), stat.getGroup(), expectedOwner,
+          expectedGroup);
+      success = true;
+      return raf;
+    } finally {
+      if (!success) {
+        raf.close();
+      }
+    }
+  }
+
+  /**
+   * Opens the {@link FSDataInputStream} on the requested file on local file
+   * system, verifying the expected user/group constraints if security is
+   * enabled.
+   * @param file absolute path of the file
+   * @param expectedOwner the expected user owner for the file
+   * @param expectedGroup the expected group owner for the file
+   * @throws IOException if an IO Error occurred or the user/group does not
+   * match if security is enabled
+   */
+  public static FSDataInputStream openFSDataInputStream(File file,
+      String expectedOwner, String expectedGroup) throws IOException {
+    if (!UserGroupInformation.isSecurityEnabled()) {
+      return rawFilesystem.open(new Path(file.getAbsolutePath()));
+    }
+    return forceSecureOpenFSDataInputStream(file, expectedOwner, expectedGroup);
+  }
+
+  /**
+   * Same as openFSDataInputStream except that it will run even if security is
+   * off. This is used by unit tests.
+   */
+  @VisibleForTesting
+  protected static FSDataInputStream forceSecureOpenFSDataInputStream(
+      File file,
+      String expectedOwner, String expectedGroup) throws IOException {
+    final FSDataInputStream in =
+        rawFilesystem.open(new Path(file.getAbsolutePath()));
+    boolean success = false;
+    try {
+      Stat stat = NativeIO.getFstat(in.getFileDescriptor());
+      checkStat(file, stat.getOwner(), stat.getGroup(), expectedOwner,
+          expectedGroup);
+      success = true;
+      return in;
+    } finally {
+      if (!success) {
+        in.close();
+      }
+    }
+  }
+
   /**
    * Open the given File for read access, verifying the expected user/group
    * constraints if security is enabled.
@@ -114,7 +207,8 @@ public class SecureIOUtils {
    * Same as openForRead() except that it will run even if security is off.
    * This is used by unit tests.
    */
-  static FileInputStream forceSecureOpenForRead(File f, String expectedOwner,
+  @VisibleForTesting
+  protected static FileInputStream forceSecureOpenForRead(File f, String expectedOwner,
       String expectedGroup) throws IOException {
 
     FileInputStream fis = new FileInputStream(f);

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSecureIOUtils.java

@@ -17,73 +17,133 @@
  */
 package org.apache.hadoop.io;
 
+import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.io.nativeio.NativeIO;
-
+import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
-import static org.junit.Assume.*;
-import static org.junit.Assert.*;
-import java.io.IOException;
-import java.io.File;
-import java.io.FileOutputStream;
 
 public class TestSecureIOUtils {
-  private static String realOwner, realGroup; 
+
-  private static final File testFilePath =
+  private static String realOwner, realGroup;
-      new File(System.getProperty("test.build.data"), "TestSecureIOContext");
+  private static File testFilePathIs;
+  private static File testFilePathRaf;
+  private static File testFilePathFadis;
+  private static FileSystem fs;
 
   @BeforeClass
   public static void makeTestFile() throws Exception {
-    FileOutputStream fos = new FileOutputStream(testFilePath);
-    fos.write("hello".getBytes("UTF-8"));
-    fos.close();
-
     Configuration conf = new Configuration();
-    FileSystem rawFS = FileSystem.getLocal(conf).getRaw();
+    fs = FileSystem.getLocal(conf).getRaw();
-    FileStatus stat = rawFS.getFileStatus(
+    testFilePathIs =
-      new Path(testFilePath.toString()));
+        new File((new Path("target", TestSecureIOUtils.class.getSimpleName()
+            + "1")).toUri().getRawPath());
+    testFilePathRaf =
+        new File((new Path("target", TestSecureIOUtils.class.getSimpleName()
+            + "2")).toUri().getRawPath());
+    testFilePathFadis =
+        new File((new Path("target", TestSecureIOUtils.class.getSimpleName()
+            + "3")).toUri().getRawPath());
+    for (File f : new File[] { testFilePathIs, testFilePathRaf,
+        testFilePathFadis }) {
+      FileOutputStream fos = new FileOutputStream(f);
+      fos.write("hello".getBytes("UTF-8"));
+      fos.close();
+    }
+
+    FileStatus stat = fs.getFileStatus(
+        new Path(testFilePathIs.toString()));
+    // RealOwner and RealGroup would be same for all three files.
     realOwner = stat.getOwner();
     realGroup = stat.getGroup();
   }
 
-  @Test
+  @Test(timeout = 10000)
   public void testReadUnrestricted() throws IOException {
-    SecureIOUtils.openForRead(testFilePath, null, null).close();
+    SecureIOUtils.openForRead(testFilePathIs, null, null).close();
+    SecureIOUtils.openFSDataInputStream(testFilePathFadis, null, null).close();
+    SecureIOUtils.openForRandomRead(testFilePathRaf, "r", null, null).close();
   }
 
-  @Test
+  @Test(timeout = 10000)
   public void testReadCorrectlyRestrictedWithSecurity() throws IOException {
     SecureIOUtils
-      .openForRead(testFilePath, realOwner, realGroup).close();
+        .openForRead(testFilePathIs, realOwner, realGroup).close();
+    SecureIOUtils
+        .openFSDataInputStream(testFilePathFadis, realOwner, realGroup).close();
+    SecureIOUtils.openForRandomRead(testFilePathRaf, "r", realOwner, realGroup)
+        .close();
   }
 
-  @Test
+  @Test(timeout = 10000)
   public void testReadIncorrectlyRestrictedWithSecurity() throws IOException {
     // this will only run if libs are available
     assumeTrue(NativeIO.isAvailable());
 
     System.out.println("Running test with native libs...");
+    String invalidUser = "InvalidUser";
 
+    // We need to make sure that forceSecure.. call works only if
+    // the file belongs to expectedOwner.
+
+    // InputStream
     try {
       SecureIOUtils
-        .forceSecureOpenForRead(testFilePath, "invalidUser", null).close();
+          .forceSecureOpenForRead(testFilePathIs, invalidUser, realGroup)
-      fail("Didn't throw expection for wrong ownership!");
+          .close();
+      fail("Didn't throw expection for wrong user ownership!");
+
+    } catch (IOException ioe) {
+      // expected
+    }
+
+    // FSDataInputStream
+    try {
+      SecureIOUtils
+          .forceSecureOpenFSDataInputStream(testFilePathFadis, invalidUser,
+              realGroup).close();
+      fail("Didn't throw expection for wrong user ownership!");
+    } catch (IOException ioe) {
+      // expected
+    }
+
+    // RandomAccessFile
+    try {
+      SecureIOUtils
+          .forceSecureOpenForRandomRead(testFilePathRaf, "r", invalidUser,
+              realGroup).close();
+      fail("Didn't throw expection for wrong user ownership!");
     } catch (IOException ioe) {
       // expected
     }
   }
 
-  @Test
+  @Test(timeout = 10000)
   public void testCreateForWrite() throws IOException {
     try {
-      SecureIOUtils.createForWrite(testFilePath, 0777);
+      SecureIOUtils.createForWrite(testFilePathIs, 0777);
-      fail("Was able to create file at " + testFilePath);
+      fail("Was able to create file at " + testFilePathIs);
     } catch (SecureIOUtils.AlreadyExistsException aee) {
       // expected
     }
   }
+
+  @AfterClass
+  public static void removeTestFile() throws Exception {
+    // cleaning files
+    for (File f : new File[] { testFilePathIs, testFilePathRaf,
+        testFilePathFadis }) {
+      f.delete();
+    }
+  }
 }