HADOOP-10736. Add key attributes to the key shell. Contributed by Mike Yoder.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1609869 13f79535-47bb-0310-9956-ffa450edef68

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

@@ -180,6 +180,8 @@ Trunk (Unreleased)
     HADOOP-10812. Delegate KeyProviderExtension#toString to underlying
     KeyProvider. (wang)
 
+    HADOOP-10736. Add key attributes to the key shell. (Mike Yoder via wang)
+
   BUG FIXES
 
     HADOOP-9451. Fault single-layer config if node group topology is enabled.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProvider.java

@@ -23,9 +23,7 @@
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.OutputStreamWriter;
-import java.net.URI;
 import java.security.NoSuchAlgorithmException;
-import java.text.MessageFormat;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
@@ -37,7 +35,6 @@
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.Path;
 
 import javax.crypto.KeyGenerator;
 
@@ -137,9 +134,26 @@ protected Metadata(String cipher, int bitLength, String description,
     }
 
     public String toString() {
-      return MessageFormat.format(
+      final StringBuilder metaSB = new StringBuilder();
-          "cipher: {0}, length: {1} description: {2} created: {3} version: {4}",
+      metaSB.append("cipher: ").append(cipher).append(", ");
-          cipher, bitLength, description, created, versions);
+      metaSB.append("length: ").append(bitLength).append(", ");
+      metaSB.append("description: ").append(description).append(", ");
+      metaSB.append("created: ").append(created).append(", ");
+      metaSB.append("version: ").append(versions).append(", ");
+      metaSB.append("attributes: ");
+      if ((attributes != null) && !attributes.isEmpty()) {
+        for (Map.Entry<String, String> attribute : attributes.entrySet()) {
+          metaSB.append("[");
+          metaSB.append(attribute.getKey());
+          metaSB.append("=");
+          metaSB.append(attribute.getValue());
+          metaSB.append("], ");
+        }
+        metaSB.deleteCharAt(metaSB.length() - 2);  // remove last ', '
+      } else {
+        metaSB.append("null");
+      }
+      return metaSB.toString();
     }
 
     public String getDescription() {

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java

@@ -22,7 +22,9 @@
 import java.io.PrintStream;
 import java.security.InvalidParameterException;
 import java.security.NoSuchAlgorithmException;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.conf.Configured;
@@ -90,6 +92,7 @@ public int run(String[] args) throws Exception {
    */
   private int init(String[] args) throws IOException {
     final Options options = KeyProvider.options(getConf());
+    final Map<String, String> attributes = new HashMap<String, String>();
 
     for (int i = 0; i < args.length; i++) { // parse command line
       boolean moreTokens = (i < args.length - 1);
@@ -134,6 +137,23 @@ private int init(String[] args) throws IOException {
         options.setCipher(args[++i]);
       } else if ("--description".equals(args[i]) && moreTokens) {
         options.setDescription(args[++i]);
+      } else if ("--attr".equals(args[i]) && moreTokens) {
+        final String attrval[] = args[++i].split("=", 2);
+        final String attr = attrval[0].trim();
+        final String val = attrval[1].trim();
+        if (attr.isEmpty() || val.isEmpty()) {
+          out.println("\nAttributes must be in attribute=value form, " +
+                  "or quoted\nlike \"attribute = value\"\n");
+          printKeyShellUsage();
+          return -1;
+        }
+        if (attributes.containsKey(attr)) {
+          out.println("\nEach attribute must correspond to only one value:\n" +
+                  "atttribute \"" + attr + "\" was repeated\n" );
+          printKeyShellUsage();
+          return -1;
+        }
+        attributes.put(attr, val);
       } else if ("--provider".equals(args[i]) && moreTokens) {
         userSuppliedProvider = true;
         getConf().set(KeyProviderFactory.KEY_PROVIDER_PATH, args[++i]);
@@ -156,6 +176,10 @@ private int init(String[] args) throws IOException {
       return -1;
     }
 
+    if (!attributes.isEmpty()) {
+      options.setAttributes(attributes);
+    }
+
     return 0;
   }
 
@@ -404,6 +428,7 @@ private class CreateCommand extends Command {
     public static final String USAGE =
       "create <keyname> [--cipher <cipher>] [--size <size>]\n" +
       "                     [--description <description>]\n" +
+      "                     [--attr <attribute=value>]\n" +
       "                     [--provider <provider>] [--help]";
     public static final String DESC =
       "The create subcommand creates a new key for the name specified\n" +
@@ -411,7 +436,9 @@ private class CreateCommand extends Command {
       "--provider argument. You may specify a cipher with the --cipher\n" +
       "argument. The default cipher is currently \"AES/CTR/NoPadding\".\n" +
       "The default keysize is 256. You may specify the requested key\n" +
-      "length using the --size argument.\n";
+      "length using the --size argument. Arbitrary attribute=value\n" +
+      "style attributes may be specified using the --attr argument.\n" +
+      "--attr may be specified multiple times, once per attribute.\n";
 
     final String keyName;
     final Options options;

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

@@ -17,35 +17,41 @@
  */
 package org.apache.hadoop.crypto.key;
 
-import static org.junit.Assert.*;
-
 import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.IOException;
 import java.io.PrintStream;
 import java.util.UUID;
 
 import org.apache.hadoop.conf.Configuration;
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
 public class TestKeyShell {
   private final ByteArrayOutputStream outContent = new ByteArrayOutputStream();
   private final ByteArrayOutputStream errContent = new ByteArrayOutputStream();
 
-  private static File tmpDir;
-
   private PrintStream initialStdOut;
   private PrintStream initialStdErr;
 
+  /* The default JCEKS provider - for testing purposes */
+  private String jceksProvider;
+
   @Before
   public void setup() throws Exception {
     outContent.reset();
     errContent.reset();
-    tmpDir = new File(System.getProperty("test.build.data", "target"),
+    final File tmpDir = new File(System.getProperty("test.build.data", "target"),
         UUID.randomUUID().toString());
-    tmpDir.mkdirs();
+    if (!tmpDir.mkdirs()) {
+      throw new IOException("Unable to create " + tmpDir);
+    }
+    jceksProvider = "jceks://file" + tmpDir + "/keystore.jceks";
     initialStdOut = System.out;
     initialStdErr = System.err;
     System.setOut(new PrintStream(outContent));
@@ -58,65 +64,80 @@ public void cleanUp() throws Exception {
     System.setErr(initialStdErr);
   }
 
+  /**
+   * Delete a key from the default jceksProvider
+   * @param ks The KeyShell instance
+   * @param keyName The key to delete
+   * @throws Exception
+   */
+  private void deleteKey(KeyShell ks, String keyName) throws Exception {
+    int rc;
+    outContent.reset();
+    final String[] delArgs = {"delete", keyName, "--provider", jceksProvider};
+    rc = ks.run(delArgs);
+    assertEquals(0, rc);
+    assertTrue(outContent.toString().contains(keyName + " has been " +
+            "successfully deleted."));
+  }
+
+  /**
+   * Lists the keys in the jceksProvider
+   * @param ks The KeyShell instance
+   * @param wantMetadata True if you want metadata returned with the keys
+   * @return The output from the "list" call
+   * @throws Exception
+   */
+  private String listKeys(KeyShell ks, boolean wantMetadata) throws Exception {
+    int rc;
+    outContent.reset();
+    final String[] listArgs = {"list", "--provider", jceksProvider };
+    final String[] listArgsM = {"list", "--metadata", "--provider", jceksProvider };
+    rc = ks.run(wantMetadata ? listArgsM : listArgs);
+    assertEquals(0, rc);
+    return outContent.toString();
+  }
+
   @Test
   public void testKeySuccessfulKeyLifecycle() throws Exception {
-    outContent.reset();
-    String[] args1 = {"create", "key1", "--provider", 
-        "jceks://file" + tmpDir + "/keystore.jceks"};
     int rc = 0;
+    String keyName = "key1";
+
     KeyShell ks = new KeyShell();
     ks.setConf(new Configuration());
+
+    outContent.reset();
+    final String[] args1 = {"create", keyName, "--provider", jceksProvider};
     rc = ks.run(args1);
     assertEquals(0, rc);
-    assertTrue(outContent.toString().contains("key1 has been successfully " +
+    assertTrue(outContent.toString().contains(keyName + " has been " +
-    		"created."));
+            "successfully created."));
+
+    String listOut = listKeys(ks, false);
+    assertTrue(listOut.contains(keyName));
+
+    listOut = listKeys(ks, true);
+    assertTrue(listOut.contains(keyName));
+    assertTrue(listOut.contains("description"));
+    assertTrue(listOut.contains("created"));
 
     outContent.reset();
-    String[] args2 = {"list", "--provider",
+    final String[] args2 = {"roll", keyName, "--provider", jceksProvider};
-        "jceks://file" + tmpDir + "/keystore.jceks"};
     rc = ks.run(args2);
     assertEquals(0, rc);
-    assertTrue(outContent.toString().contains("key1"));
-
-    outContent.reset();
-    String[] args2a = {"list", "--metadata", "--provider",
-                      "jceks://file" + tmpDir + "/keystore.jceks"};
-    rc = ks.run(args2a);
-    assertEquals(0, rc);
-    assertTrue(outContent.toString().contains("key1"));
-    assertTrue(outContent.toString().contains("description"));
-    assertTrue(outContent.toString().contains("created"));
-
-    outContent.reset();
-    String[] args3 = {"roll", "key1", "--provider", 
-        "jceks://file" + tmpDir + "/keystore.jceks"};
-    rc = ks.run(args3);
-    assertEquals(0, rc);
     assertTrue(outContent.toString().contains("key1 has been successfully " +
     		"rolled."));
 
-    outContent.reset();
+    deleteKey(ks, keyName);
-    String[] args4 = {"delete", "key1", "--provider", 
-        "jceks://file" + tmpDir + "/keystore.jceks"};
-    rc = ks.run(args4);
-    assertEquals(0, rc);
-    assertTrue(outContent.toString().contains("key1 has been successfully " +
-    		"deleted."));
 
-    outContent.reset();
+    listOut = listKeys(ks, false);
-    String[] args5 = {"list", "--provider", 
+    assertFalse(listOut, listOut.contains(keyName));
-        "jceks://file" + tmpDir + "/keystore.jceks"};
-    rc = ks.run(args5);
-    assertEquals(0, rc);
-    assertFalse(outContent.toString(), outContent.toString().contains("key1"));
   }
   
   /* HADOOP-10586 KeyShell didn't allow -description. */
   @Test
   public void testKeySuccessfulCreationWithDescription() throws Exception {
     outContent.reset();
-    String[] args1 = {"create", "key1", "--provider",
+    final String[] args1 = {"create", "key1", "--provider", jceksProvider,
-                      "jceks://file" + tmpDir + "/keystore.jceks",
                       "--description", "someDescription"};
     int rc = 0;
     KeyShell ks = new KeyShell();
@@ -126,20 +147,16 @@ public void testKeySuccessfulCreationWithDescription() throws Exception {
     assertTrue(outContent.toString().contains("key1 has been successfully " +
         "created."));
 
-    outContent.reset();
+    String listOut = listKeys(ks, true);
-    String[] args2a = {"list", "--metadata", "--provider",
+    assertTrue(listOut.contains("description"));
-                      "jceks://file" + tmpDir + "/keystore.jceks"};
+    assertTrue(listOut.contains("someDescription"));
-    rc = ks.run(args2a);
-    assertEquals(0, rc);
-    assertTrue(outContent.toString().contains("description"));
-    assertTrue(outContent.toString().contains("someDescription"));
   }
 
   @Test
   public void testInvalidKeySize() throws Exception {
-    String[] args1 = {"create", "key1", "--size", "56", "--provider", 
+    final String[] args1 = {"create", "key1", "--size", "56", "--provider",
-        "jceks://file" + tmpDir + "/keystore.jceks"};
+            jceksProvider};
-    
+
     int rc = 0;
     KeyShell ks = new KeyShell();
     ks.setConf(new Configuration());
@@ -150,9 +167,9 @@ public void testInvalidKeySize() throws Exception {
 
   @Test
   public void testInvalidCipher() throws Exception {
-    String[] args1 = {"create", "key1", "--cipher", "LJM", "--provider", 
+    final String[] args1 = {"create", "key1", "--cipher", "LJM", "--provider",
-        "jceks://file" + tmpDir + "/keystore.jceks"};
+            jceksProvider};
-    
+
     int rc = 0;
     KeyShell ks = new KeyShell();
     ks.setConf(new Configuration());
@@ -163,7 +180,7 @@ public void testInvalidCipher() throws Exception {
 
   @Test
   public void testInvalidProvider() throws Exception {
-    String[] args1 = {"create", "key1", "--cipher", "AES", "--provider", 
+    final String[] args1 = {"create", "key1", "--cipher", "AES", "--provider",
       "sdff://file/tmp/keystore.jceks"};
     
     int rc = 0;
@@ -177,7 +194,7 @@ public void testInvalidProvider() throws Exception {
 
   @Test
   public void testTransientProviderWarning() throws Exception {
-    String[] args1 = {"create", "key1", "--cipher", "AES", "--provider", 
+    final String[] args1 = {"create", "key1", "--cipher", "AES", "--provider",
       "user:///"};
     
     int rc = 0;
@@ -191,7 +208,7 @@ public void testTransientProviderWarning() throws Exception {
   
   @Test
   public void testTransientProviderOnlyConfig() throws Exception {
-    String[] args1 = {"create", "key1"};
+    final String[] args1 = {"create", "key1"};
     
     int rc = 0;
     KeyShell ks = new KeyShell();
@@ -206,23 +223,96 @@ public void testTransientProviderOnlyConfig() throws Exception {
 
   @Test
   public void testFullCipher() throws Exception {
-    String[] args1 = {"create", "key1", "--cipher", "AES/CBC/pkcs5Padding", 
+    final String keyName = "key1";
-        "--provider", "jceks://file" + tmpDir + "/keystore.jceks"};
+    final String[] args1 = {"create", keyName, "--cipher", "AES/CBC/pkcs5Padding",
+        "--provider", jceksProvider};
     
     int rc = 0;
     KeyShell ks = new KeyShell();
     ks.setConf(new Configuration());
     rc = ks.run(args1);
     assertEquals(0, rc);
-    assertTrue(outContent.toString().contains("key1 has been successfully " +
+    assertTrue(outContent.toString().contains(keyName + " has been " +
-    		"created."));
+            "successfully " +	"created."));
 
+    deleteKey(ks, keyName);
+  }
+
+  @Test
+  public void testAttributes() throws Exception {
+    int rc;
+    KeyShell ks = new KeyShell();
+    ks.setConf(new Configuration());
+
+    /* Simple creation test */
+    final String[] args1 = {"create", "keyattr1", "--provider", jceksProvider,
+            "--attr", "foo=bar"};
+    rc = ks.run(args1);
+    assertEquals(0, rc);
+    assertTrue(outContent.toString().contains("keyattr1 has been " +
+            "successfully " + "created."));
+
+    /* ...and list to see that we have the attr */
+    String listOut = listKeys(ks, true);
+    assertTrue(listOut.contains("keyattr1"));
+    assertTrue(listOut.contains("attributes: [foo=bar]"));
+
+    /* Negative tests: no attribute */
     outContent.reset();
-    String[] args2 = {"delete", "key1", "--provider", 
+    final String[] args2 = {"create", "keyattr2", "--provider", jceksProvider,
-        "jceks://file" + tmpDir + "/keystore.jceks"};
+            "--attr", "=bar"};
+    rc = ks.run(args2);
+    assertEquals(-1, rc);
+
+    /* Not in attribute = value form */
+    outContent.reset();
+    args2[5] = "foo";
+    rc = ks.run(args2);
+    assertEquals(-1, rc);
+
+    /* No attribute or value */
+    outContent.reset();
+    args2[5] = "=";
+    rc = ks.run(args2);
+    assertEquals(-1, rc);
+
+    /* Legal: attribute is a, value is b=c */
+    outContent.reset();
+    args2[5] = "a=b=c";
     rc = ks.run(args2);
     assertEquals(0, rc);
-    assertTrue(outContent.toString().contains("key1 has been successfully " +
+
-    		"deleted."));
+    listOut = listKeys(ks, true);
+    assertTrue(listOut.contains("keyattr2"));
+    assertTrue(listOut.contains("attributes: [a=b=c]"));
+
+    /* Test several attrs together... */
+    outContent.reset();
+    final String[] args3 = {"create", "keyattr3", "--provider", jceksProvider,
+            "--attr", "foo = bar",
+            "--attr", " glarch =baz  ",
+            "--attr", "abc=def"};
+    rc = ks.run(args3);
+    assertEquals(0, rc);
+
+    /* ...and list to ensure they're there. */
+    listOut = listKeys(ks, true);
+    assertTrue(listOut.contains("keyattr3"));
+    assertTrue(listOut.contains("[foo=bar]"));
+    assertTrue(listOut.contains("[glarch=baz]"));
+    assertTrue(listOut.contains("[abc=def]"));
+
+    /* Negative test - repeated attributes should fail */
+    outContent.reset();
+    final String[] args4 = {"create", "keyattr4", "--provider", jceksProvider,
+            "--attr", "foo=bar",
+            "--attr", "foo=glarch"};
+    rc = ks.run(args4);
+    assertEquals(-1, rc);
+
+    /* Clean up to be a good citizen */
+    deleteKey(ks, "keyattr1");
+    deleteKey(ks, "keyattr2");
+    deleteKey(ks, "keyattr3");
   }
 }