diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java index c7c1779de03..1827c272dcd 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java @@ -19,7 +19,6 @@ package org.apache.hadoop.crypto.key; import com.google.common.base.Preconditions; -import org.apache.commons.io.IOUtils; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSDataInputStream; @@ -38,12 +37,10 @@ import com.google.common.annotations.VisibleForTesting; import javax.crypto.spec.SecretKeySpec; import java.io.IOException; -import java.io.InputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; import java.net.URI; -import java.net.URL; import java.security.GeneralSecurityException; import java.security.Key; import java.security.KeyStore; @@ -138,44 +135,15 @@ public class JavaKeyStoreProvider extends KeyProvider { writeLock = lock.writeLock(); } - /** - * The password is either found in the environment or in a file. This - * routine implements the logic for locating the password in these - * locations. - * @return The password as a char []; null if not found. - * @throws IOException - */ - private char[] locatePassword() throws IOException { - char[] pass = null; - // Get the password file from the conf, if not present from the user's - // environment var - if (System.getenv().containsKey(KEYSTORE_PASSWORD_ENV_VAR)) { - pass = System.getenv(KEYSTORE_PASSWORD_ENV_VAR).toCharArray(); - } - if (pass == null) { - String pwFile = getConf().get(KEYSTORE_PASSWORD_FILE_KEY); - if (pwFile != null) { - ClassLoader cl = Thread.currentThread().getContextClassLoader(); - URL pwdFile = cl.getResource(pwFile); - if (pwdFile == null) { - // Provided Password file does not exist - throw new IOException("Password file does not exists"); - } - try (InputStream is = pwdFile.openStream()) { - pass = IOUtils.toString(is).trim().toCharArray(); - } - } - } - return pass; - } - /** * Open up and initialize the keyStore. - * @throws IOException + * @throws IOException If there is a problem reading the password file + * or a problem reading the keystore. */ private void locateKeystore() throws IOException { try { - password = locatePassword(); + password = ProviderUtils.locatePassword(KEYSTORE_PASSWORD_ENV_VAR, + getConf().get(KEYSTORE_PASSWORD_FILE_KEY)); if (password == null) { password = KEYSTORE_PASSWORD_DEFAULT; } @@ -331,48 +299,31 @@ public class JavaKeyStoreProvider extends KeyProvider { } } - private Path constructNewPath(Path path) { - Path newPath = new Path(path.toString() + "_NEW"); - return newPath; + private static Path constructNewPath(Path path) { + return new Path(path.toString() + "_NEW"); } - private Path constructOldPath(Path path) { - Path oldPath = new Path(path.toString() + "_OLD"); - return oldPath; + private static Path constructOldPath(Path path) { + return new Path(path.toString() + "_OLD"); } @Override public boolean needsPassword() throws IOException { - return (null == locatePassword()); - } + return (null == ProviderUtils.locatePassword(KEYSTORE_PASSWORD_ENV_VAR, + getConf().get(KEYSTORE_PASSWORD_FILE_KEY))); - @VisibleForTesting - public static final String NO_PASSWORD_WARN = - "WARNING: You have accepted the use of the default provider password\n" + - "by not configuring a password in one of the two following locations:\n"; - public static final String NO_PASSWORD_ERROR = - "ERROR: The provider cannot find a password in the expected " + - "locations.\nPlease supply a password using one of the " + - "following two mechanisms:\n"; - @VisibleForTesting public static final String NO_PASSWORD_INSTRUCTIONS = - " o In the environment variable " + - KEYSTORE_PASSWORD_ENV_VAR + "\n" + - " o In a file referred to by the configuration entry\n" + - " " + KEYSTORE_PASSWORD_FILE_KEY + ".\n" + - "Please review the documentation regarding provider passwords at\n" + - "http://hadoop.apache.org/docs/current/hadoop-project-dist/" + - "hadoop-common/CredentialProviderAPI.html#Keystore_Passwords\n"; - @VisibleForTesting public static final String NO_PASSWORD_CONT = - "Continuing with the default provider password.\n"; + } @Override public String noPasswordWarning() { - return NO_PASSWORD_WARN + NO_PASSWORD_INSTRUCTIONS + NO_PASSWORD_CONT; + return ProviderUtils.noPasswordWarning(KEYSTORE_PASSWORD_ENV_VAR, + KEYSTORE_PASSWORD_FILE_KEY); } @Override public String noPasswordError() { - return NO_PASSWORD_ERROR + NO_PASSWORD_INSTRUCTIONS; + return ProviderUtils.noPasswordError(KEYSTORE_PASSWORD_ENV_VAR, + KEYSTORE_PASSWORD_FILE_KEY); } @Override diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java index fcc21059602..5de8aa8d2e4 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java @@ -47,7 +47,8 @@ public class KeyShell extends Configured implements Tool { " [" + DeleteCommand.USAGE + "]\n" + " [" + ListCommand.USAGE + "]\n"; private static final String LIST_METADATA = "keyShell.list.metadata"; - @VisibleForTesting public static final String NO_VALID_PROVIDERS = + @VisibleForTesting + public static final String NO_VALID_PROVIDERS = "There are no valid (non-transient) providers configured.\n" + "No action has been taken. Use the -provider option to specify\n" + "a provider. If you want to use a transient provider then you\n" + @@ -60,9 +61,11 @@ public class KeyShell extends Configured implements Tool { private boolean strict = false; /** allows stdout to be captured if necessary. */ - @VisibleForTesting public PrintStream out = System.out; + @VisibleForTesting + public PrintStream out = System.out; /** allows stderr to be captured if necessary. */ - @VisibleForTesting public PrintStream err = System.err; + @VisibleForTesting + public PrintStream err = System.err; private boolean userSuppliedProvider = false; diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ProviderUtils.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ProviderUtils.java index ae08fbae35c..84ee9125114 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ProviderUtils.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ProviderUtils.java @@ -19,9 +19,13 @@ package org.apache.hadoop.security; import java.io.IOException; +import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; +import java.net.URL; +import com.google.common.annotations.VisibleForTesting; +import org.apache.commons.io.IOUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; @@ -36,6 +40,23 @@ import org.apache.hadoop.security.alias.LocalJavaKeyStoreProvider; * */ public final class ProviderUtils { + @VisibleForTesting + public static final String NO_PASSWORD_WARN = + "WARNING: You have accepted the use of the default provider password\n" + + "by not configuring a password in one of the two following locations:\n"; + @VisibleForTesting + public static final String NO_PASSWORD_ERROR = + "ERROR: The provider cannot find a password in the expected " + + "locations.\nPlease supply a password using one of the " + + "following two mechanisms:\n"; + @VisibleForTesting + public static final String NO_PASSWORD_CONT = + "Continuing with the default provider password.\n"; + @VisibleForTesting + public static final String NO_PASSWORD_INSTRUCTIONS_DOC = + "Please review the documentation regarding provider passwords in\n" + + "the keystore passwords section of the Credential Provider API\n"; + private static final Log LOG = LogFactory.getLog(ProviderUtils.class); /** @@ -174,4 +195,58 @@ public final class ProviderUtils { } return conf; } + + /** + * The password is either found in the environment or in a file. This + * routine implements the logic for locating the password in these + * locations. + * + * @param envWithPass The name of the environment variable that might + * contain the password. Must not be null. + * @param fileWithPass The name of a file that could contain the password. + * Can be null. + * @return The password as a char []; null if not found. + * @throws IOException If fileWithPass is non-null and points to a + * nonexistent file or a file that fails to open and be read properly. + */ + public static char[] locatePassword(String envWithPass, String fileWithPass) + throws IOException { + char[] pass = null; + // Get the password file from the conf, if not present from the user's + // environment var + if (System.getenv().containsKey(envWithPass)) { + pass = System.getenv(envWithPass).toCharArray(); + } + if (pass == null) { + if (fileWithPass != null) { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + URL pwdFile = cl.getResource(fileWithPass); + if (pwdFile == null) { + // Provided Password file does not exist + throw new IOException("Password file does not exist"); + } + try (InputStream is = pwdFile.openStream()) { + pass = IOUtils.toString(is).trim().toCharArray(); + } + } + } + return pass; + } + + private static String noPasswordInstruction(String envKey, String fileKey) { + return + " * In the environment variable " + envKey + "\n" + + " * In a file referred to by the configuration entry\n" + + " " + fileKey + ".\n" + + NO_PASSWORD_INSTRUCTIONS_DOC; + } + + public static String noPasswordWarning(String envKey, String fileKey) { + return NO_PASSWORD_WARN + noPasswordInstruction(envKey, fileKey) + + NO_PASSWORD_CONT; + } + + public static String noPasswordError(String envKey, String fileKey) { + return NO_PASSWORD_ERROR + noPasswordInstruction(envKey, fileKey); + } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/AbstractJavaKeyStoreProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/AbstractJavaKeyStoreProvider.java index 895b7150671..335c198f726 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/AbstractJavaKeyStoreProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/AbstractJavaKeyStoreProvider.java @@ -18,10 +18,8 @@ package org.apache.hadoop.security.alias; -import com.google.common.annotations.VisibleForTesting; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.commons.io.IOUtils; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; @@ -34,7 +32,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.URI; -import java.net.URL; import java.security.GeneralSecurityException; import java.security.KeyStore; import java.security.KeyStoreException; @@ -64,11 +61,11 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; public abstract class AbstractJavaKeyStoreProvider extends CredentialProvider { public static final Log LOG = LogFactory.getLog( AbstractJavaKeyStoreProvider.class); - public static final String CREDENTIAL_PASSWORD_NAME = + public static final String CREDENTIAL_PASSWORD_ENV_VAR = "HADOOP_CREDSTORE_PASSWORD"; - public static final String KEYSTORE_PASSWORD_FILE_KEY = + public static final String CREDENTIAL_PASSWORD_FILE_KEY = "hadoop.security.credstore.java-keystore-provider.password-file"; - public static final String KEYSTORE_PASSWORD_DEFAULT = "none"; + public static final String CREDENTIAL_PASSWORD_DEFAULT = "none"; private Path path; private final URI uri; @@ -302,45 +299,18 @@ public abstract class AbstractJavaKeyStoreProvider extends CredentialProvider { } } - /** - * The password is either found in the environment or in a file. This - * routine implements the logic for locating the password in these - * locations. - * - * @return The password as a char []; null if not found. - * @throws IOException - */ - private char[] locatePassword() throws IOException { - char[] pass = null; - if (System.getenv().containsKey(CREDENTIAL_PASSWORD_NAME)) { - pass = System.getenv(CREDENTIAL_PASSWORD_NAME).toCharArray(); - } - // if not in ENV get check for file - if (pass == null) { - String pwFile = conf.get(KEYSTORE_PASSWORD_FILE_KEY); - if (pwFile != null) { - ClassLoader cl = Thread.currentThread().getContextClassLoader(); - URL pwdFile = cl.getResource(pwFile); - if (pwdFile != null) { - try (InputStream is = pwdFile.openStream()) { - pass = IOUtils.toString(is).trim().toCharArray(); - } - } - } - } - return pass; - } - /** * Open up and initialize the keyStore. * - * @throws IOException + * @throws IOException If there is a problem reading the password file + * or a problem reading the keystore. */ private void locateKeystore() throws IOException { try { - password = locatePassword(); + password = ProviderUtils.locatePassword(CREDENTIAL_PASSWORD_ENV_VAR, + conf.get(CREDENTIAL_PASSWORD_FILE_KEY)); if (password == null) { - password = KEYSTORE_PASSWORD_DEFAULT.toCharArray(); + password = CREDENTIAL_PASSWORD_DEFAULT.toCharArray(); } KeyStore ks; ks = KeyStore.getInstance("jceks"); @@ -364,38 +334,21 @@ public abstract class AbstractJavaKeyStoreProvider extends CredentialProvider { @Override public boolean needsPassword() throws IOException { - return (null == locatePassword()); - } + return (null == ProviderUtils.locatePassword(CREDENTIAL_PASSWORD_ENV_VAR, + conf.get(CREDENTIAL_PASSWORD_FILE_KEY))); - @VisibleForTesting - public static final String NO_PASSWORD_WARN = - "WARNING: You have accepted the use of the default provider password\n" + - "by not configuring a password in one of the two following locations:\n"; - @VisibleForTesting - public static final String NO_PASSWORD_ERROR = - "ERROR: The provider cannot find a password in the expected " + - "locations.\nPlease supply a password using one of the " + - "following two mechanisms:\n"; - @VisibleForTesting - public static final String NO_PASSWORD_INSTRUCTIONS = - " o In the environment variable " + - CREDENTIAL_PASSWORD_NAME + "\n" + - " o In a file referred to by the configuration entry\n" + - " " + KEYSTORE_PASSWORD_FILE_KEY + ".\n" + - "Please review the documentation regarding provider passwords at\n" + - "http://hadoop.apache.org/docs/current/hadoop-project-dist/" + - "hadoop-common/CredentialProviderAPI.html#Keystore_Passwords\n"; - @VisibleForTesting public static final String NO_PASSWORD_CONT = - "Continuing with the default provider password.\n"; + } @Override public String noPasswordWarning() { - return NO_PASSWORD_WARN + NO_PASSWORD_INSTRUCTIONS + NO_PASSWORD_CONT; + return ProviderUtils.noPasswordWarning(CREDENTIAL_PASSWORD_ENV_VAR, + CREDENTIAL_PASSWORD_FILE_KEY); } @Override public String noPasswordError() { - return NO_PASSWORD_ERROR + NO_PASSWORD_INSTRUCTIONS; + return ProviderUtils.noPasswordError(CREDENTIAL_PASSWORD_ENV_VAR, + CREDENTIAL_PASSWORD_FILE_KEY); } @Override diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java index 6c1e3c41673..1e09c0790e3 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java @@ -58,9 +58,11 @@ public class CredentialShell extends Configured implements Tool { private boolean strict = false; /** Allows stdout to be captured if necessary. */ - @VisibleForTesting public PrintStream out = System.out; + @VisibleForTesting + public PrintStream out = System.out; /** Allows stderr to be captured if necessary. */ - @VisibleForTesting public PrintStream err = System.err; + @VisibleForTesting + public PrintStream err = System.err; private boolean userSuppliedProvider = false; private String value = null; diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java index 4b284442d98..0b5d4e5d9fd 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java @@ -861,7 +861,7 @@ public abstract class Shell { // might be owned by a different user. For example, the NodeManager // running a User's container. builder.environment().remove( - AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_NAME); + AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_ENV_VAR); } if (dir != null) { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java index 09a4c506a42..c99a1272d0b 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java @@ -25,6 +25,7 @@ import java.util.UUID; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.security.ProviderUtils; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -115,11 +116,11 @@ public class TestKeyShell { assertTrue(outContent.toString().contains(keyName + " has been " + "successfully created")); assertTrue(outContent.toString() - .contains(JavaKeyStoreProvider.NO_PASSWORD_WARN)); + .contains(ProviderUtils.NO_PASSWORD_WARN)); assertTrue(outContent.toString() - .contains(JavaKeyStoreProvider.NO_PASSWORD_INSTRUCTIONS)); + .contains(ProviderUtils.NO_PASSWORD_INSTRUCTIONS_DOC)); assertTrue(outContent.toString() - .contains(JavaKeyStoreProvider.NO_PASSWORD_CONT)); + .contains(ProviderUtils.NO_PASSWORD_CONT)); String listOut = listKeys(ks, false); assertTrue(listOut.contains(keyName)); @@ -239,9 +240,9 @@ public class TestKeyShell { rc = ks.run(args1); assertEquals(1, rc); assertTrue(outContent.toString() - .contains(JavaKeyStoreProvider.NO_PASSWORD_ERROR)); + .contains(ProviderUtils.NO_PASSWORD_ERROR)); assertTrue(outContent.toString() - .contains(JavaKeyStoreProvider.NO_PASSWORD_INSTRUCTIONS)); + .contains(ProviderUtils.NO_PASSWORD_INSTRUCTIONS_DOC)); } @Test diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java index 2e10a0291c9..be9c5b38634 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java @@ -30,6 +30,7 @@ import java.util.List; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.security.ProviderUtils; import org.junit.Before; import org.junit.Test; @@ -64,11 +65,11 @@ public class TestCredShell { assertTrue(outContent.toString().contains("credential1 has been successfully " + "created.")); assertTrue(outContent.toString() - .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_WARN)); + .contains(ProviderUtils.NO_PASSWORD_WARN)); assertTrue(outContent.toString() - .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_INSTRUCTIONS)); + .contains(ProviderUtils.NO_PASSWORD_INSTRUCTIONS_DOC)); assertTrue(outContent.toString() - .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_CONT)); + .contains(ProviderUtils.NO_PASSWORD_CONT)); outContent.reset(); String[] args2 = {"list", "-provider", @@ -246,9 +247,9 @@ public class TestCredShell { assertFalse(outContent.toString().contains("credential1 has been " + "successfully created.")); assertTrue(outContent.toString() - .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_ERROR)); + .contains(ProviderUtils.NO_PASSWORD_ERROR)); assertTrue(outContent.toString() - .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_INSTRUCTIONS)); + .contains(ProviderUtils.NO_PASSWORD_INSTRUCTIONS_DOC)); } @Test diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java index 271b749946b..d47b9b4193b 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java @@ -162,7 +162,7 @@ public class TestShell extends Assert { private void testEnvHelper(boolean inheritParentEnv) throws Exception { Map customEnv = Collections.singletonMap( - AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_NAME, "foo"); + AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_ENV_VAR, "foo"); Shell.ShellCommandExecutor command = new ShellCommandExecutor( new String[]{"env"}, null, customEnv, 0L, inheritParentEnv); @@ -179,9 +179,9 @@ public class TestShell extends Assert { expectedEnv.putAll(customEnv); } else { assertFalse("child process environment should not have contained " - + AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_NAME, + + AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_ENV_VAR, vars.containsKey( - AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_NAME)); + AbstractJavaKeyStoreProvider.CREDENTIAL_PASSWORD_ENV_VAR)); } assertEquals(expectedEnv, vars); }