From d4df7849a5caf749403bd89d29652f69c9c3f5a8 Mon Sep 17 00:00:00 2001
From: Andrew Wang <wang@apache.org>
Date: Fri, 25 Mar 2016 15:28:53 -0700
Subject: [PATCH] HADOOP-12962. KMS key names are incorrectly encoded when
 creating key. Contributed by Xiao Chen.

---
 .../hadoop/crypto/key/kms/server/KMS.java     | 14 +++----
 .../hadoop/crypto/key/kms/server/TestKMS.java | 42 +++++++++++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMS.java b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMS.java
index 43b07fec63..f069fcaebd 100644
--- a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMS.java
+++ b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMS.java
@@ -41,10 +41,10 @@
 import javax.ws.rs.QueryParam;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
+import javax.ws.rs.core.UriBuilder;
 
 import java.io.IOException;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
 import java.util.LinkedList;
@@ -89,9 +89,9 @@ private static KeyProvider.KeyVersion removeKeyMaterial(
         keyVersion.getVersionName(), null);
   }
 
-  private static URI getKeyURI(String name) throws URISyntaxException {
-    return new URI(KMSRESTConstants.SERVICE_VERSION + "/" +
-        KMSRESTConstants.KEY_RESOURCE + "/" + name);
+  private static URI getKeyURI(String domain, String keyName) {
+    return UriBuilder.fromPath("{a}/{b}/{c}")
+        .build(domain, KMSRESTConstants.KEY_RESOURCE, keyName);
   }
 
   @POST
@@ -151,9 +151,9 @@ public KeyVersion run() throws Exception {
     String requestURL = KMSMDCFilter.getURL();
     int idx = requestURL.lastIndexOf(KMSRESTConstants.KEYS_RESOURCE);
     requestURL = requestURL.substring(0, idx);
-    String keyURL = requestURL + KMSRESTConstants.KEY_RESOURCE + "/" + name;
-    return Response.created(getKeyURI(name)).type(MediaType.APPLICATION_JSON).
-        header("Location", keyURL).entity(json).build();
+    return Response.created(getKeyURI(KMSRESTConstants.SERVICE_VERSION, name))
+        .type(MediaType.APPLICATION_JSON)
+        .header("Location", getKeyURI(requestURL, name)).entity(json).build();
   }
 
   @DELETE
diff --git a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java
index 9b75ee11a0..8094ae2e6d 100644
--- a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java
+++ b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java
@@ -39,11 +39,15 @@
 import org.apache.hadoop.security.authorize.AuthorizationException;
 import org.apache.hadoop.security.ssl.KeyStoreTestUtil;
 import org.apache.hadoop.security.token.Token;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.apache.log4j.Level;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.security.auth.kerberos.KerberosPrincipal;
 import javax.security.auth.login.AppConfigurationEntry;
@@ -69,12 +73,14 @@
 import java.util.concurrent.Callable;
 
 public class TestKMS {
+  private static final Logger LOG = LoggerFactory.getLogger(TestKMS.class);
 
   @Before
   public void cleanUp() {
     // resetting kerberos security
     Configuration conf = new Configuration();
     UserGroupInformation.setConfiguration(conf);
+    GenericTestUtils.setLogLevel(LOG, Level.INFO);
   }
 
   public static File getTestDir() throws Exception {
@@ -380,6 +386,42 @@ public void testStartStopHttpsKerberos() throws Exception {
     testStartStop(true, true);
   }
 
+  @Test(timeout = 30000)
+  public void testSpecialKeyNames() throws Exception {
+    final String specialKey = "key %^[\n{]}|\"<>\\";
+    Configuration conf = new Configuration();
+    conf.set("hadoop.security.authentication", "kerberos");
+    UserGroupInformation.setConfiguration(conf);
+    File confDir = getTestDir();
+    conf = createBaseKMSConf(confDir);
+    conf.set(KeyAuthorizationKeyProvider.KEY_ACL + specialKey + ".ALL", "*");
+    writeConf(confDir, conf);
+
+    runServer(null, null, confDir, new KMSCallable<Void>() {
+      @Override
+      public Void call() throws Exception {
+        Configuration conf = new Configuration();
+        URI uri = createKMSUri(getKMSUrl());
+        KeyProvider kp = createProvider(uri, conf);
+        Assert.assertTrue(kp.getKeys().isEmpty());
+        Assert.assertEquals(0, kp.getKeysMetadata().length);
+
+        KeyProvider.Options options = new KeyProvider.Options(conf);
+        options.setCipher("AES/CTR/NoPadding");
+        options.setBitLength(128);
+        options.setDescription("l1");
+        LOG.info("Creating key with name '{}'", specialKey);
+
+        KeyProvider.KeyVersion kv0 = kp.createKey(specialKey, options);
+        Assert.assertNotNull(kv0);
+        Assert.assertEquals(specialKey, kv0.getName());
+        Assert.assertNotNull(kv0.getVersionName());
+        Assert.assertNotNull(kv0.getMaterial());
+        return null;
+      }
+    });
+  }
+
   @Test
   public void testKMSProvider() throws Exception {
     Configuration conf = new Configuration();