You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by tu...@apache.org on 2014/08/21 20:58:33 UTC

svn commit: r1619509 - in /hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common: CHANGES.txt src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderFactory.java

Author: tucu
Date: Thu Aug 21 18:58:32 2014
New Revision: 1619509

URL: http://svn.apache.org/r1619509
Log:
HADOOP-10237. JavaKeyStoreProvider needs to set keystore permissions
correctly. (Larry McCay via omalley)


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

Modified:
    hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt
    hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java
    hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderFactory.java

Modified: hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1619509&r1=1619508&r2=1619509&view=diff
==============================================================================
--- hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt (original)
+++ hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt Thu Aug 21 18:58:32 2014
@@ -188,6 +188,9 @@ Release 2.6.0 - UNRELEASED
     HADOOP-10141. Create KeyProvider API to separate encryption key storage
     from the applications. (omalley)
 
+    HADOOP-10237. JavaKeyStoreProvider needs to set keystore permissions
+    correctly. (Larry McCay via omalley)
+
 Release 2.5.0 - 2014-08-11
 
   INCOMPATIBLE CHANGES

Modified: hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java?rev=1619509&r1=1619508&r2=1619509&view=diff
==============================================================================
--- hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java (original)
+++ hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java Thu Aug 21 18:58:32 2014
@@ -21,9 +21,10 @@ package org.apache.hadoop.crypto.key;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
-
+import org.apache.hadoop.fs.permission.FsPermission;
 import javax.crypto.spec.SecretKeySpec;
 import java.io.IOException;
 import java.io.ObjectInputStream;
@@ -68,6 +69,7 @@ public class JavaKeyStoreProvider extend
   private final URI uri;
   private final Path path;
   private final FileSystem fs;
+  private final FsPermission permissions;
   private final KeyStore keyStore;
   private final char[] password;
   private boolean changed = false;
@@ -87,8 +89,14 @@ public class JavaKeyStoreProvider extend
     try {
       keyStore = KeyStore.getInstance(SCHEME_NAME);
       if (fs.exists(path)) {
+        // save off permissions in case we need to
+        // rewrite the keystore in flush()
+        FileStatus s = fs.getFileStatus(path);
+        permissions = s.getPermission();
+
         keyStore.load(fs.open(path), password);
       } else {
+        permissions = new FsPermission("700");
         // required to create an empty keystore. *sigh*
         keyStore.load(null, password);
       }
@@ -277,7 +285,7 @@ public class JavaKeyStoreProvider extend
       }
     }
     // write out the keystore
-    FSDataOutputStream out = fs.create(path, true);
+    FSDataOutputStream out = FileSystem.create(fs, path, permissions);
     try {
       keyStore.store(out, password);
     } catch (KeyStoreException e) {

Modified: hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderFactory.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderFactory.java?rev=1619509&r1=1619508&r2=1619509&view=diff
==============================================================================
--- hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderFactory.java (original)
+++ hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderFactory.java Thu Aug 21 18:58:32 2014
@@ -19,9 +19,14 @@ package org.apache.hadoop.crypto.key;
 
 import java.io.File;
 import java.io.IOException;
+import java.net.URI;
 import java.util.List;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.crypto.key.KeyProvider.KeyVersion;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.io.Text;
 import org.apache.hadoop.security.Credentials;
 import org.apache.hadoop.security.UserGroupInformation;
@@ -193,10 +198,43 @@ public class TestKeyProviderFactory {
     Configuration conf = new Configuration();
     final String ourUrl =
         JavaKeyStoreProvider.SCHEME_NAME + "://file" + tmpDir + "/test.jks";
+
     File file = new File(tmpDir, "test.jks");
     file.delete();
     conf.set(KeyProviderFactory.KEY_PROVIDER_PATH, ourUrl);
     checkSpecificProvider(conf, ourUrl);
+    Path path = KeyProvider.unnestUri(new URI(ourUrl));
+    FileSystem fs = path.getFileSystem(conf);
+    FileStatus s = fs.getFileStatus(path);
+    assertTrue(s.getPermission().toString().equals("rwx------"));
     assertTrue(file + " should exist", file.isFile());
+
+    // check permission retention after explicit change
+    fs.setPermission(path, new FsPermission("777"));
+    checkPermissionRetention(conf, ourUrl, path);
+  }
+
+  public void checkPermissionRetention(Configuration conf, String ourUrl, Path path) throws Exception {
+    KeyProvider provider = KeyProviderFactory.getProviders(conf).get(0);
+    // let's add a new key and flush and check that permissions are still set to 777
+    byte[] key = new byte[32];
+    for(int i =0; i < key.length; ++i) {
+      key[i] = (byte) i;
+    }
+    // create a new key
+    try {
+      provider.createKey("key5", key, KeyProvider.options(conf));
+    } catch (Exception e) {
+      e.printStackTrace();
+      throw e;
+    }
+    provider.flush();
+    // get a new instance of the provider to ensure it was saved correctly
+    provider = KeyProviderFactory.getProviders(conf).get(0);
+    assertArrayEquals(key, provider.getCurrentKey("key5").getMaterial());
+
+    FileSystem fs = path.getFileSystem(conf);
+    FileStatus s = fs.getFileStatus(path);
+    assertTrue("Permissions should have been retained from the preexisting keystore.", s.getPermission().toString().equals("rwxrwxrwx"));
   }
 }