From bfc73613fc74c669efc9e4722e8f1bb48d5ced3a Mon Sep 17 00:00:00 2001 From: Vinod Kumar Vavilapalli Date: Fri, 3 May 2013 23:43:12 +0000 Subject: [PATCH] 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/trunk@1479010 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 4 + .../org/apache/hadoop/io/SecureIOUtils.java | 99 ++++++++++++++- .../apache/hadoop/io/TestSecureIOUtils.java | 114 +++++++++++++----- 3 files changed, 186 insertions(+), 31 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 5d92c600d0..e83c3db6e9 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -597,6 +597,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 diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java index 70c29b810a..68333df7b1 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java @@ -18,22 +18,23 @@ package org.apache.hadoop.io; import java.io.File; -import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; +import java.io.RandomAccessFile; import java.util.Arrays; 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; -import org.apache.hadoop.io.nativeio.Errno; import org.apache.hadoop.io.nativeio.NativeIO; -import org.apache.hadoop.io.nativeio.NativeIOException; import org.apache.hadoop.io.nativeio.NativeIO.POSIX.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. @@ -89,6 +90,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.POSIX.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.POSIX.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. @@ -115,7 +205,8 @@ public static FileInputStream openForRead(File f, String expectedOwner, * 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); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSecureIOUtils.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSecureIOUtils.java index a48fb6770b..91c0f1b442 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSecureIOUtils.java +++ b/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 = - new File(System.getProperty("test.build.data"), "TestSecureIOContext"); + + private static String realOwner, realGroup; + 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(); - FileStatus stat = rawFS.getFileStatus( - new Path(testFilePath.toString())); + fs = FileSystem.getLocal(conf).getRaw(); + testFilePathIs = + 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(); - fail("Didn't throw expection for wrong ownership!"); + .forceSecureOpenForRead(testFilePathIs, invalidUser, realGroup) + .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); - fail("Was able to create file at " + testFilePath); + SecureIOUtils.createForWrite(testFilePathIs, 0777); + 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(); + } + } }