From e9dd0d84e52f9ae4a309105033c5c02e4254b911 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Wed, 18 May 2016 17:57:46 +0900 Subject: [PATCH] Revert "HADOOP-13157. Follow-on improvements to hadoop credential commands. Contributed by Mike Yoder." This reverts commit e205421555841dbf482d6aa1464cc15788b2b1fb. --- .../crypto/key/JavaKeyStoreProvider.java | 79 +++++++++++++++---- .../apache/hadoop/crypto/key/KeyShell.java | 9 +-- .../apache/hadoop/security/ProviderUtils.java | 75 ------------------ .../alias/AbstractJavaKeyStoreProvider.java | 77 ++++++++++++++---- .../security/alias/CredentialShell.java | 6 +- .../hadoop/crypto/key/TestKeyShell.java | 11 ++- .../hadoop/security/alias/TestCredShell.java | 11 ++- 7 files changed, 141 insertions(+), 127 deletions(-) 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 1827c272dcd..c7c1779de03 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,6 +19,7 @@ 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; @@ -37,10 +38,12 @@ 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; @@ -135,15 +138,44 @@ 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 If there is a problem reading the password file - * or a problem reading the keystore. + * @throws IOException */ private void locateKeystore() throws IOException { try { - password = ProviderUtils.locatePassword(KEYSTORE_PASSWORD_ENV_VAR, - getConf().get(KEYSTORE_PASSWORD_FILE_KEY)); + password = locatePassword(); if (password == null) { password = KEYSTORE_PASSWORD_DEFAULT; } @@ -299,31 +331,48 @@ public class JavaKeyStoreProvider extends KeyProvider { } } - private static Path constructNewPath(Path path) { - return new Path(path.toString() + "_NEW"); + private Path constructNewPath(Path path) { + Path newPath = new Path(path.toString() + "_NEW"); + return newPath; } - private static Path constructOldPath(Path path) { - return new Path(path.toString() + "_OLD"); + private Path constructOldPath(Path path) { + Path oldPath = new Path(path.toString() + "_OLD"); + return oldPath; } @Override public boolean needsPassword() throws IOException { - return (null == ProviderUtils.locatePassword(KEYSTORE_PASSWORD_ENV_VAR, - getConf().get(KEYSTORE_PASSWORD_FILE_KEY))); - + return (null == locatePassword()); } + @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 ProviderUtils.noPasswordWarning(KEYSTORE_PASSWORD_ENV_VAR, - KEYSTORE_PASSWORD_FILE_KEY); + return NO_PASSWORD_WARN + NO_PASSWORD_INSTRUCTIONS + NO_PASSWORD_CONT; } @Override public String noPasswordError() { - return ProviderUtils.noPasswordError(KEYSTORE_PASSWORD_ENV_VAR, - KEYSTORE_PASSWORD_FILE_KEY); + return NO_PASSWORD_ERROR + NO_PASSWORD_INSTRUCTIONS; } @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 5de8aa8d2e4..fcc21059602 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,8 +47,7 @@ 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" + @@ -61,11 +60,9 @@ 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 84ee9125114..ae08fbae35c 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,13 +19,9 @@ 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; @@ -40,23 +36,6 @@ 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); /** @@ -195,58 +174,4 @@ 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 335c198f726..895b7150671 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,8 +18,10 @@ 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; @@ -32,6 +34,7 @@ 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; @@ -61,11 +64,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_ENV_VAR = + public static final String CREDENTIAL_PASSWORD_NAME = "HADOOP_CREDSTORE_PASSWORD"; - public static final String CREDENTIAL_PASSWORD_FILE_KEY = + public static final String KEYSTORE_PASSWORD_FILE_KEY = "hadoop.security.credstore.java-keystore-provider.password-file"; - public static final String CREDENTIAL_PASSWORD_DEFAULT = "none"; + public static final String KEYSTORE_PASSWORD_DEFAULT = "none"; private Path path; private final URI uri; @@ -299,18 +302,45 @@ 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 If there is a problem reading the password file - * or a problem reading the keystore. + * @throws IOException */ private void locateKeystore() throws IOException { try { - password = ProviderUtils.locatePassword(CREDENTIAL_PASSWORD_ENV_VAR, - conf.get(CREDENTIAL_PASSWORD_FILE_KEY)); + password = locatePassword(); if (password == null) { - password = CREDENTIAL_PASSWORD_DEFAULT.toCharArray(); + password = KEYSTORE_PASSWORD_DEFAULT.toCharArray(); } KeyStore ks; ks = KeyStore.getInstance("jceks"); @@ -334,21 +364,38 @@ public abstract class AbstractJavaKeyStoreProvider extends CredentialProvider { @Override public boolean needsPassword() throws IOException { - return (null == ProviderUtils.locatePassword(CREDENTIAL_PASSWORD_ENV_VAR, - conf.get(CREDENTIAL_PASSWORD_FILE_KEY))); - + return (null == locatePassword()); } + @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 ProviderUtils.noPasswordWarning(CREDENTIAL_PASSWORD_ENV_VAR, - CREDENTIAL_PASSWORD_FILE_KEY); + return NO_PASSWORD_WARN + NO_PASSWORD_INSTRUCTIONS + NO_PASSWORD_CONT; } @Override public String noPasswordError() { - return ProviderUtils.noPasswordError(CREDENTIAL_PASSWORD_ENV_VAR, - CREDENTIAL_PASSWORD_FILE_KEY); + return NO_PASSWORD_ERROR + NO_PASSWORD_INSTRUCTIONS; } @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 1e09c0790e3..6c1e3c41673 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,11 +58,9 @@ 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/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 6e785127760..03409595f02 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,7 +25,6 @@ import java.util.UUID; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.security.ProviderUtils; import org.apache.hadoop.test.GenericTestUtils; import org.junit.After; import org.junit.Before; @@ -117,11 +116,11 @@ public class TestKeyShell { assertTrue(outContent.toString().contains(keyName + " has been " + "successfully created")); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_WARN)); + .contains(JavaKeyStoreProvider.NO_PASSWORD_WARN)); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_INSTRUCTIONS_DOC)); + .contains(JavaKeyStoreProvider.NO_PASSWORD_INSTRUCTIONS)); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_CONT)); + .contains(JavaKeyStoreProvider.NO_PASSWORD_CONT)); String listOut = listKeys(ks, false); assertTrue(listOut.contains(keyName)); @@ -241,9 +240,9 @@ public class TestKeyShell { rc = ks.run(args1); assertEquals(1, rc); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_ERROR)); + .contains(JavaKeyStoreProvider.NO_PASSWORD_ERROR)); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_INSTRUCTIONS_DOC)); + .contains(JavaKeyStoreProvider.NO_PASSWORD_INSTRUCTIONS)); } @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 569fe738a01..5ba7cf046b4 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,7 +30,6 @@ import java.util.List; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.security.ProviderUtils; import org.apache.hadoop.test.GenericTestUtils; import org.junit.Before; import org.junit.Test; @@ -65,11 +64,11 @@ public class TestCredShell { assertTrue(outContent.toString().contains("credential1 has been successfully " + "created.")); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_WARN)); + .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_WARN)); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_INSTRUCTIONS_DOC)); + .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_INSTRUCTIONS)); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_CONT)); + .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_CONT)); outContent.reset(); String[] args2 = {"list", "-provider", @@ -247,9 +246,9 @@ public class TestCredShell { assertFalse(outContent.toString().contains("credential1 has been " + "successfully created.")); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_ERROR)); + .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_ERROR)); assertTrue(outContent.toString() - .contains(ProviderUtils.NO_PASSWORD_INSTRUCTIONS_DOC)); + .contains(AbstractJavaKeyStoreProvider.NO_PASSWORD_INSTRUCTIONS)); } @Test