Revert "HADOOP-13157. Follow-on improvements to hadoop credential commands. Contributed by Mike Yoder."

This reverts commit e205421555.

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 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 @@ private JavaKeyStoreProvider(URI uri, Configuration conf) throws IOException {
     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
+   * @throws IOException
-   * or a problem reading the keystore.
    */
   private void locateKeystore() throws IOException {
     try {
-      password = ProviderUtils.locatePassword(KEYSTORE_PASSWORD_ENV_VAR,
+      password = locatePassword();
-          getConf().get(KEYSTORE_PASSWORD_FILE_KEY));
       if (password == null) {
         password = KEYSTORE_PASSWORD_DEFAULT;
       }
@@ -299,31 +331,48 @@ private FsPermission loadFromPath(Path p, char[] password)
     }
   }
 
-  private static Path constructNewPath(Path path) {
+  private Path constructNewPath(Path path) {
-    return new Path(path.toString() + "_NEW");
+    Path newPath = new Path(path.toString() + "_NEW");
+    return newPath;
   }
 
-  private static Path constructOldPath(Path path) {
+  private Path constructOldPath(Path path) {
-    return new Path(path.toString() + "_OLD");
+    Path oldPath = new Path(path.toString() + "_OLD");
+    return oldPath;
   }
 
   @Override
   public boolean needsPassword() throws IOException {
-    return (null == ProviderUtils.locatePassword(KEYSTORE_PASSWORD_ENV_VAR,
+    return (null == locatePassword());
-        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 ProviderUtils.noPasswordWarning(KEYSTORE_PASSWORD_ENV_VAR,
+    return NO_PASSWORD_WARN + NO_PASSWORD_INSTRUCTIONS + NO_PASSWORD_CONT;
-        KEYSTORE_PASSWORD_FILE_KEY);
   }
 
   @Override
   public String noPasswordError() {
-    return ProviderUtils.noPasswordError(KEYSTORE_PASSWORD_ENV_VAR,
+    return NO_PASSWORD_ERROR + NO_PASSWORD_INSTRUCTIONS;
-        KEYSTORE_PASSWORD_FILE_KEY);
   }
 
   @Override

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
+  @VisibleForTesting public static final String NO_VALID_PROVIDERS =
-  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
+  @VisibleForTesting public PrintStream out = System.out;
-  public PrintStream out = System.out;
   /** allows stderr to be captured if necessary. */
-  @VisibleForTesting
+  @VisibleForTesting public PrintStream err = System.err;
-  public PrintStream err = System.err;
 
   private boolean userSuppliedProvider = false;
 

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 @@
  *
  */
 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 static Configuration excludeIncompatibleCredentialProviders(
     }
     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);
-  }
 }

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.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 @@
 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 void flush() throws IOException {
     }
   }
 
+  /**
+   * 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
+   * @throws IOException
-   * or a problem reading the keystore.
    */
   private void locateKeystore() throws IOException {
     try {
-      password = ProviderUtils.locatePassword(CREDENTIAL_PASSWORD_ENV_VAR,
+      password = locatePassword();
-          conf.get(CREDENTIAL_PASSWORD_FILE_KEY));
       if (password == null) {
-        password = CREDENTIAL_PASSWORD_DEFAULT.toCharArray();
+        password = KEYSTORE_PASSWORD_DEFAULT.toCharArray();
       }
       KeyStore ks;
       ks = KeyStore.getInstance("jceks");
@@ -334,21 +364,38 @@ private void locateKeystore() throws IOException {
 
   @Override
   public boolean needsPassword() throws IOException {
-    return (null == ProviderUtils.locatePassword(CREDENTIAL_PASSWORD_ENV_VAR,
+    return (null == locatePassword());
-        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 ProviderUtils.noPasswordWarning(CREDENTIAL_PASSWORD_ENV_VAR,
+    return NO_PASSWORD_WARN + NO_PASSWORD_INSTRUCTIONS + NO_PASSWORD_CONT;
-            CREDENTIAL_PASSWORD_FILE_KEY);
   }
 
   @Override
   public String noPasswordError() {
-    return ProviderUtils.noPasswordError(CREDENTIAL_PASSWORD_ENV_VAR,
+    return NO_PASSWORD_ERROR + NO_PASSWORD_INSTRUCTIONS;
-        CREDENTIAL_PASSWORD_FILE_KEY);
   }
 
   @Override

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
+  @VisibleForTesting public PrintStream out = System.out;
-  public PrintStream out = System.out;
   /** Allows stderr to be captured if necessary. */
-  @VisibleForTesting
+  @VisibleForTesting public PrintStream err = System.err;
-  public PrintStream err = System.err;
 
   private boolean userSuppliedProvider = false;
   private String value = null;

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java

@@ -25,7 +25,6 @@
 
 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 void testKeySuccessfulKeyLifecycle() throws Exception {
     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 void testStrict() throws Exception {
     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

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java

@@ -30,7 +30,6 @@
 
 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 void testCredentialSuccessfulLifecycle() throws Exception {
     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 void testStrict() throws Exception {
     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