You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by rk...@apache.org on 2018/01/04 23:51:31 UTC

[1/3] hadoop git commit: HADOOP-12181. Fix intermittent test failure of TestZKSignerSecretProvider. Contributed by Masatake Iwasaki.

Repository: hadoop
Updated Branches:
  refs/heads/branch-2.7 1c798e700 -> 88d951e30


HADOOP-12181. Fix intermittent test failure of TestZKSignerSecretProvider. Contributed by Masatake Iwasaki.

(cherry picked from commit def7490b29dddca39674b5ec31a6067deed98396)


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/e03cee73
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/e03cee73
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/e03cee73

Branch: refs/heads/branch-2.7
Commit: e03cee7300b62fc8ff06e7d10e21927a3f40b717
Parents: 1c798e7
Author: Haohui Mai <wh...@apache.org>
Authored: Sun Nov 22 16:56:15 2015 -0800
Committer: Robert Kanter <rk...@cloudera.com>
Committed: Thu Jan 4 15:37:10 2018 -0800

----------------------------------------------------------------------
 .../util/TestZKSignerSecretProvider.java        | 56 ++++++++++++--------
 hadoop-common-project/hadoop-common/CHANGES.txt |  3 ++
 2 files changed, 38 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/e03cee73/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
index d7b6e17..4f8b5ae 100644
--- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
@@ -22,12 +22,21 @@ import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.Mockito;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.timeout;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class TestZKSignerSecretProvider {
 
   private TestingServer zkServer;
 
+  // rollover every 2 sec
+  private final int timeout = 4000;
+  private final long rolloverFrequency = Long.valueOf(timeout / 2);
+
   @Before
   public void setup() throws Exception {
     zkServer = new TestingServer();
@@ -45,14 +54,14 @@ public class TestZKSignerSecretProvider {
   // Test just one ZKSignerSecretProvider to verify that it works in the
   // simplest case
   public void testOne() throws Exception {
-    long rolloverFrequency = 15 * 1000; // rollover every 15 sec
     // use the same seed so we can predict the RNG
     long seed = System.currentTimeMillis();
     Random rand = new Random(seed);
     byte[] secret2 = Long.toString(rand.nextLong()).getBytes();
     byte[] secret1 = Long.toString(rand.nextLong()).getBytes();
     byte[] secret3 = Long.toString(rand.nextLong()).getBytes();
-    ZKSignerSecretProvider secretProvider = new ZKSignerSecretProvider(seed);
+    ZKSignerSecretProvider secretProvider =
+        spy(new ZKSignerSecretProvider(seed));
     Properties config = new Properties();
     config.setProperty(
         ZKSignerSecretProvider.ZOOKEEPER_CONNECTION_STRING,
@@ -68,7 +77,7 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecrets.length);
       Assert.assertArrayEquals(secret1, allSecrets[0]);
       Assert.assertNull(allSecrets[1]);
-      Thread.sleep((rolloverFrequency + 2000));
+      verify(secretProvider, timeout(timeout).times(1)).rollSecret();
 
       currentSecret = secretProvider.getCurrentSecret();
       allSecrets = secretProvider.getAllSecrets();
@@ -76,7 +85,7 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecrets.length);
       Assert.assertArrayEquals(secret2, allSecrets[0]);
       Assert.assertArrayEquals(secret1, allSecrets[1]);
-      Thread.sleep((rolloverFrequency + 2000));
+      verify(secretProvider, timeout(timeout).times(2)).rollSecret();
 
       currentSecret = secretProvider.getCurrentSecret();
       allSecrets = secretProvider.getAllSecrets();
@@ -84,7 +93,7 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecrets.length);
       Assert.assertArrayEquals(secret3, allSecrets[0]);
       Assert.assertArrayEquals(secret2, allSecrets[1]);
-      Thread.sleep((rolloverFrequency + 2000));
+      verify(secretProvider, timeout(timeout).times(3)).rollSecret();
     } finally {
       secretProvider.destroy();
     }
@@ -92,7 +101,6 @@ public class TestZKSignerSecretProvider {
 
   @Test
   public void testMultipleInit() throws Exception {
-    long rolloverFrequency = 15 * 1000; // rollover every 15 sec
     // use the same seed so we can predict the RNG
     long seedA = System.currentTimeMillis();
     Random rand = new Random(seedA);
@@ -108,9 +116,12 @@ public class TestZKSignerSecretProvider {
     rand = new Random(seedC);
     byte[] secretC2 = Long.toString(rand.nextLong()).getBytes();
     byte[] secretC1 = Long.toString(rand.nextLong()).getBytes();
-    ZKSignerSecretProvider secretProviderA = new ZKSignerSecretProvider(seedA);
-    ZKSignerSecretProvider secretProviderB = new ZKSignerSecretProvider(seedB);
-    ZKSignerSecretProvider secretProviderC = new ZKSignerSecretProvider(seedC);
+    ZKSignerSecretProvider secretProviderA =
+        spy(new ZKSignerSecretProvider(seedA));
+    ZKSignerSecretProvider secretProviderB =
+        spy(new ZKSignerSecretProvider(seedB));
+    ZKSignerSecretProvider secretProviderC =
+        spy(new ZKSignerSecretProvider(seedC));
     Properties config = new Properties();
     config.setProperty(
         ZKSignerSecretProvider.ZOOKEEPER_CONNECTION_STRING,
@@ -152,7 +163,9 @@ public class TestZKSignerSecretProvider {
         Assert.fail("It appears that they all agreed on the same secret, but "
                 + "not one of the secrets they were supposed to");
       }
-      Thread.sleep((rolloverFrequency + 2000));
+      verify(secretProviderA, timeout(timeout).times(1)).rollSecret();
+      verify(secretProviderB, timeout(timeout).times(1)).rollSecret();
+      verify(secretProviderC, timeout(timeout).times(1)).rollSecret();
 
       currentSecretA = secretProviderA.getCurrentSecret();
       allSecretsA = secretProviderA.getAllSecrets();
@@ -187,8 +200,6 @@ public class TestZKSignerSecretProvider {
 
   @Test
   public void testMultipleUnsychnronized() throws Exception {
-    long rolloverFrequency = 15 * 1000; // rollover every 15 sec
-    // use the same seed so we can predict the RNG
     long seedA = System.currentTimeMillis();
     Random rand = new Random(seedA);
     byte[] secretA2 = Long.toString(rand.nextLong()).getBytes();
@@ -200,8 +211,10 @@ public class TestZKSignerSecretProvider {
     byte[] secretB2 = Long.toString(rand.nextLong()).getBytes();
     byte[] secretB1 = Long.toString(rand.nextLong()).getBytes();
     byte[] secretB3 = Long.toString(rand.nextLong()).getBytes();
-    ZKSignerSecretProvider secretProviderA = new ZKSignerSecretProvider(seedA);
-    ZKSignerSecretProvider secretProviderB = new ZKSignerSecretProvider(seedB);
+    ZKSignerSecretProvider secretProviderA =
+        spy(new ZKSignerSecretProvider(seedA));
+    ZKSignerSecretProvider secretProviderB =
+        spy(new ZKSignerSecretProvider(seedB));
     Properties config = new Properties();
     config.setProperty(
         ZKSignerSecretProvider.ZOOKEEPER_CONNECTION_STRING,
@@ -217,7 +230,7 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecretsA.length);
       Assert.assertArrayEquals(secretA1, allSecretsA[0]);
       Assert.assertNull(allSecretsA[1]);
-      Thread.sleep((rolloverFrequency + 2000));
+      verify(secretProviderA, timeout(timeout).times(1)).rollSecret();
 
       currentSecretA = secretProviderA.getCurrentSecret();
       allSecretsA = secretProviderA.getAllSecrets();
@@ -235,7 +248,8 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecretsA.length);
       Assert.assertArrayEquals(secretA2, allSecretsB[0]);
       Assert.assertArrayEquals(secretA1, allSecretsB[1]);
-      Thread.sleep((rolloverFrequency));
+      verify(secretProviderA, timeout(timeout).times(2)).rollSecret();
+      verify(secretProviderB, timeout(timeout).times(1)).rollSecret();
 
       currentSecretA = secretProviderA.getCurrentSecret();
       allSecretsA = secretProviderA.getAllSecrets();
@@ -261,10 +275,10 @@ public class TestZKSignerSecretProvider {
   }
 
   private ServletContext getDummyServletContext() {
-    ServletContext servletContext = Mockito.mock(ServletContext.class);
-    Mockito.when(servletContext.getAttribute(ZKSignerSecretProvider
-            .ZOOKEEPER_SIGNER_SECRET_PROVIDER_CURATOR_CLIENT_ATTRIBUTE))
-            .thenReturn(null);
+    ServletContext servletContext = mock(ServletContext.class);
+    when(servletContext.getAttribute(ZKSignerSecretProvider
+        .ZOOKEEPER_SIGNER_SECRET_PROVIDER_CURATOR_CLIENT_ATTRIBUTE))
+        .thenReturn(null);
     return servletContext;
   }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/e03cee73/hadoop-common-project/hadoop-common/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index a816a87..48c9357 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -12,6 +12,9 @@ Release 2.7.6 - UNRELEASED
 
   BUG FIXES
 
+    HADOOP-12181. Fix intermittent test failure of TestZKSignerSecretProvider.
+    (Masatake Iwasaki via wheat9)
+
 Release 2.7.5 - 2017-12-14
 
   INCOMPATIBLE CHANGES


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org


[2/3] hadoop git commit: HADOOP-12611. TestZKSignerSecretProvider#testMultipleInit occasionally fail (ebadger via rkanter)

Posted by rk...@apache.org.
HADOOP-12611. TestZKSignerSecretProvider#testMultipleInit occasionally fail (ebadger via rkanter)

(cherry picked from commit c183b9de8d072a35dcde96a20b1550981f886e86)
(cherry picked from commit 7bfa595679a037c6956117ec266c7b2e62b48863)


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/9e5fffa6
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/9e5fffa6
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/9e5fffa6

Branch: refs/heads/branch-2.7
Commit: 9e5fffa65f591986eb6a305faa5109ea4668d55d
Parents: e03cee7
Author: Robert Kanter <rk...@apache.org>
Authored: Fri Oct 7 09:33:24 2016 -0700
Committer: Robert Kanter <rk...@cloudera.com>
Committed: Thu Jan 4 15:42:05 2018 -0800

----------------------------------------------------------------------
 .../util/RolloverSignerSecretProvider.java      |   2 +-
 .../util/TestZKSignerSecretProvider.java        | 223 +++++++++----------
 hadoop-common-project/hadoop-common/CHANGES.txt |   3 +
 3 files changed, 104 insertions(+), 124 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e5fffa6/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java
index bdca3e4..8ce4b23 100644
--- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java
@@ -38,7 +38,7 @@ import org.slf4j.LoggerFactory;
 public abstract class RolloverSignerSecretProvider
     extends SignerSecretProvider {
 
-  private static Logger LOG = LoggerFactory.getLogger(
+  static Logger LOG = LoggerFactory.getLogger(
     RolloverSignerSecretProvider.class);
   /**
    * Stores the currently valid secrets.  The current secret is the 0th element

http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e5fffa6/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
index 4f8b5ae..5e640bb 100644
--- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
@@ -17,7 +17,12 @@ import java.util.Arrays;
 import java.util.Properties;
 import java.util.Random;
 import javax.servlet.ServletContext;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.curator.test.TestingServer;
+import org.apache.log4j.Level;
+import org.apache.log4j.LogManager;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -25,7 +30,6 @@ import org.junit.Test;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.timeout;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -34,8 +38,13 @@ public class TestZKSignerSecretProvider {
   private TestingServer zkServer;
 
   // rollover every 2 sec
-  private final int timeout = 4000;
-  private final long rolloverFrequency = Long.valueOf(timeout / 2);
+  private final int timeout = 100;
+  private final long rolloverFrequency = timeout / 2;
+
+  static final Log LOG = LogFactory.getLog(TestZKSignerSecretProvider.class);
+  {
+    LogManager.getLogger( RolloverSignerSecretProvider.LOG.getName() ).setLevel(Level.DEBUG);
+  }
 
   @Before
   public void setup() throws Exception {
@@ -60,8 +69,8 @@ public class TestZKSignerSecretProvider {
     byte[] secret2 = Long.toString(rand.nextLong()).getBytes();
     byte[] secret1 = Long.toString(rand.nextLong()).getBytes();
     byte[] secret3 = Long.toString(rand.nextLong()).getBytes();
-    ZKSignerSecretProvider secretProvider =
-        spy(new ZKSignerSecretProvider(seed));
+    MockZKSignerSecretProvider secretProvider =
+        spy(new MockZKSignerSecretProvider(seed));
     Properties config = new Properties();
     config.setProperty(
         ZKSignerSecretProvider.ZOOKEEPER_CONNECTION_STRING,
@@ -77,7 +86,8 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecrets.length);
       Assert.assertArrayEquals(secret1, allSecrets[0]);
       Assert.assertNull(allSecrets[1]);
-      verify(secretProvider, timeout(timeout).times(1)).rollSecret();
+      verify(secretProvider, timeout(timeout).atLeastOnce()).rollSecret();
+      secretProvider.realRollSecret();
 
       currentSecret = secretProvider.getCurrentSecret();
       allSecrets = secretProvider.getAllSecrets();
@@ -85,7 +95,8 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecrets.length);
       Assert.assertArrayEquals(secret2, allSecrets[0]);
       Assert.assertArrayEquals(secret1, allSecrets[1]);
-      verify(secretProvider, timeout(timeout).times(2)).rollSecret();
+      verify(secretProvider, timeout(timeout).atLeast(2)).rollSecret();
+      secretProvider.realRollSecret();
 
       currentSecret = secretProvider.getCurrentSecret();
       allSecrets = secretProvider.getAllSecrets();
@@ -93,128 +104,70 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecrets.length);
       Assert.assertArrayEquals(secret3, allSecrets[0]);
       Assert.assertArrayEquals(secret2, allSecrets[1]);
-      verify(secretProvider, timeout(timeout).times(3)).rollSecret();
+      verify(secretProvider, timeout(timeout).atLeast(3)).rollSecret();
+      secretProvider.realRollSecret();
     } finally {
       secretProvider.destroy();
     }
   }
 
-  @Test
-  public void testMultipleInit() throws Exception {
-    // use the same seed so we can predict the RNG
-    long seedA = System.currentTimeMillis();
-    Random rand = new Random(seedA);
-    byte[] secretA2 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secretA1 = Long.toString(rand.nextLong()).getBytes();
-    // use the same seed so we can predict the RNG
-    long seedB = System.currentTimeMillis() + rand.nextLong();
-    rand = new Random(seedB);
-    byte[] secretB2 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secretB1 = Long.toString(rand.nextLong()).getBytes();
-    // use the same seed so we can predict the RNG
-    long seedC = System.currentTimeMillis() + rand.nextLong();
-    rand = new Random(seedC);
-    byte[] secretC2 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secretC1 = Long.toString(rand.nextLong()).getBytes();
-    ZKSignerSecretProvider secretProviderA =
-        spy(new ZKSignerSecretProvider(seedA));
-    ZKSignerSecretProvider secretProviderB =
-        spy(new ZKSignerSecretProvider(seedB));
-    ZKSignerSecretProvider secretProviderC =
-        spy(new ZKSignerSecretProvider(seedC));
-    Properties config = new Properties();
-    config.setProperty(
-        ZKSignerSecretProvider.ZOOKEEPER_CONNECTION_STRING,
-        zkServer.getConnectString());
-    config.setProperty(ZKSignerSecretProvider.ZOOKEEPER_PATH,
-        "/secret");
-    try {
-      secretProviderA.init(config, getDummyServletContext(), rolloverFrequency);
-      secretProviderB.init(config, getDummyServletContext(), rolloverFrequency);
-      secretProviderC.init(config, getDummyServletContext(), rolloverFrequency);
-
-      byte[] currentSecretA = secretProviderA.getCurrentSecret();
-      byte[][] allSecretsA = secretProviderA.getAllSecrets();
-      byte[] currentSecretB = secretProviderB.getCurrentSecret();
-      byte[][] allSecretsB = secretProviderB.getAllSecrets();
-      byte[] currentSecretC = secretProviderC.getCurrentSecret();
-      byte[][] allSecretsC = secretProviderC.getAllSecrets();
-      Assert.assertArrayEquals(currentSecretA, currentSecretB);
-      Assert.assertArrayEquals(currentSecretB, currentSecretC);
-      Assert.assertEquals(2, allSecretsA.length);
-      Assert.assertEquals(2, allSecretsB.length);
-      Assert.assertEquals(2, allSecretsC.length);
-      Assert.assertArrayEquals(allSecretsA[0], allSecretsB[0]);
-      Assert.assertArrayEquals(allSecretsB[0], allSecretsC[0]);
-      Assert.assertNull(allSecretsA[1]);
-      Assert.assertNull(allSecretsB[1]);
-      Assert.assertNull(allSecretsC[1]);
-      char secretChosen = 'z';
-      if (Arrays.equals(secretA1, currentSecretA)) {
-        Assert.assertArrayEquals(secretA1, allSecretsA[0]);
-        secretChosen = 'A';
-      } else if (Arrays.equals(secretB1, currentSecretB)) {
-        Assert.assertArrayEquals(secretB1, allSecretsA[0]);
-        secretChosen = 'B';
-      }else if (Arrays.equals(secretC1, currentSecretC)) {
-        Assert.assertArrayEquals(secretC1, allSecretsA[0]);
-        secretChosen = 'C';
-      } else {
-        Assert.fail("It appears that they all agreed on the same secret, but "
-                + "not one of the secrets they were supposed to");
-      }
-      verify(secretProviderA, timeout(timeout).times(1)).rollSecret();
-      verify(secretProviderB, timeout(timeout).times(1)).rollSecret();
-      verify(secretProviderC, timeout(timeout).times(1)).rollSecret();
+  /**
+   * A hack to test ZKSignerSecretProvider.
+   * We want to test that ZKSignerSecretProvider.rollSecret() is periodically
+   * called at the expected frequency, but we want to exclude the
+   * race-condition.
+   */
+  private class MockZKSignerSecretProvider extends ZKSignerSecretProvider {
+    MockZKSignerSecretProvider(long seed) {
+      super(seed);
+    }
+    @Override
+    protected synchronized void rollSecret() {
+      // this is a no-op: simply used for Mockito to verify that rollSecret()
+      // is periodically called at the expected frequency
+    }
 
-      currentSecretA = secretProviderA.getCurrentSecret();
-      allSecretsA = secretProviderA.getAllSecrets();
-      currentSecretB = secretProviderB.getCurrentSecret();
-      allSecretsB = secretProviderB.getAllSecrets();
-      currentSecretC = secretProviderC.getCurrentSecret();
-      allSecretsC = secretProviderC.getAllSecrets();
-      Assert.assertArrayEquals(currentSecretA, currentSecretB);
-      Assert.assertArrayEquals(currentSecretB, currentSecretC);
-      Assert.assertEquals(2, allSecretsA.length);
-      Assert.assertEquals(2, allSecretsB.length);
-      Assert.assertEquals(2, allSecretsC.length);
-      Assert.assertArrayEquals(allSecretsA[0], allSecretsB[0]);
-      Assert.assertArrayEquals(allSecretsB[0], allSecretsC[0]);
-      Assert.assertArrayEquals(allSecretsA[1], allSecretsB[1]);
-      Assert.assertArrayEquals(allSecretsB[1], allSecretsC[1]);
-      // The second secret used is prechosen by whoever won the init; so it
-      // should match with whichever we saw before
-      if (secretChosen == 'A') {
-        Assert.assertArrayEquals(secretA2, currentSecretA);
-      } else if (secretChosen == 'B') {
-        Assert.assertArrayEquals(secretB2, currentSecretA);
-      } else if (secretChosen == 'C') {
-        Assert.assertArrayEquals(secretC2, currentSecretA);
-      }
-    } finally {
-      secretProviderC.destroy();
-      secretProviderB.destroy();
-      secretProviderA.destroy();
+    public void realRollSecret() {
+      // the test code manually calls ZKSignerSecretProvider.rollSecret()
+      // to update the state
+      super.rollSecret();
     }
   }
 
   @Test
-  public void testMultipleUnsychnronized() throws Exception {
+  public void testMultiple1() throws Exception {
+    testMultiple(1);
+  }
+
+  @Test
+  public void testMultiple2() throws Exception {
+    testMultiple(2);
+  }
+
+  /**
+   * @param order:
+   *            1: secretProviderA wins both realRollSecret races
+   *            2: secretProviderA wins 1st race, B wins 2nd
+   * @throws Exception
+   */
+  public void testMultiple(int order) throws Exception {
     long seedA = System.currentTimeMillis();
     Random rand = new Random(seedA);
     byte[] secretA2 = Long.toString(rand.nextLong()).getBytes();
     byte[] secretA1 = Long.toString(rand.nextLong()).getBytes();
     byte[] secretA3 = Long.toString(rand.nextLong()).getBytes();
+    byte[] secretA4 = Long.toString(rand.nextLong()).getBytes();
     // use the same seed so we can predict the RNG
     long seedB = System.currentTimeMillis() + rand.nextLong();
     rand = new Random(seedB);
     byte[] secretB2 = Long.toString(rand.nextLong()).getBytes();
     byte[] secretB1 = Long.toString(rand.nextLong()).getBytes();
     byte[] secretB3 = Long.toString(rand.nextLong()).getBytes();
-    ZKSignerSecretProvider secretProviderA =
-        spy(new ZKSignerSecretProvider(seedA));
-    ZKSignerSecretProvider secretProviderB =
-        spy(new ZKSignerSecretProvider(seedB));
+    byte[] secretB4 = Long.toString(rand.nextLong()).getBytes();
+    MockZKSignerSecretProvider secretProviderA =
+        spy(new MockZKSignerSecretProvider(seedA));
+    MockZKSignerSecretProvider secretProviderB =
+        spy(new MockZKSignerSecretProvider(seedB));
     Properties config = new Properties();
     config.setProperty(
         ZKSignerSecretProvider.ZOOKEEPER_CONNECTION_STRING,
@@ -223,14 +176,24 @@ public class TestZKSignerSecretProvider {
         "/secret");
     try {
       secretProviderA.init(config, getDummyServletContext(), rolloverFrequency);
+      secretProviderB.init(config, getDummyServletContext(), rolloverFrequency);
 
       byte[] currentSecretA = secretProviderA.getCurrentSecret();
       byte[][] allSecretsA = secretProviderA.getAllSecrets();
+      byte[] currentSecretB = secretProviderB.getCurrentSecret();
+      byte[][] allSecretsB = secretProviderB.getAllSecrets();
       Assert.assertArrayEquals(secretA1, currentSecretA);
+      Assert.assertArrayEquals(secretA1, currentSecretB);
       Assert.assertEquals(2, allSecretsA.length);
+      Assert.assertEquals(2, allSecretsB.length);
       Assert.assertArrayEquals(secretA1, allSecretsA[0]);
+      Assert.assertArrayEquals(secretA1, allSecretsB[0]);
       Assert.assertNull(allSecretsA[1]);
-      verify(secretProviderA, timeout(timeout).times(1)).rollSecret();
+      Assert.assertNull(allSecretsB[1]);
+      verify(secretProviderA, timeout(timeout).atLeastOnce()).rollSecret();
+      verify(secretProviderB, timeout(timeout).atLeastOnce()).rollSecret();
+      secretProviderA.realRollSecret();
+      secretProviderB.realRollSecret();
 
       currentSecretA = secretProviderA.getCurrentSecret();
       allSecretsA = secretProviderA.getAllSecrets();
@@ -238,18 +201,32 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecretsA.length);
       Assert.assertArrayEquals(secretA2, allSecretsA[0]);
       Assert.assertArrayEquals(secretA1, allSecretsA[1]);
-      Thread.sleep((rolloverFrequency / 5));
 
-      secretProviderB.init(config, getDummyServletContext(), rolloverFrequency);
-
-      byte[] currentSecretB = secretProviderB.getCurrentSecret();
-      byte[][] allSecretsB = secretProviderB.getAllSecrets();
+      currentSecretB = secretProviderB.getCurrentSecret();
+      allSecretsB = secretProviderB.getAllSecrets();
       Assert.assertArrayEquals(secretA2, currentSecretB);
       Assert.assertEquals(2, allSecretsA.length);
       Assert.assertArrayEquals(secretA2, allSecretsB[0]);
       Assert.assertArrayEquals(secretA1, allSecretsB[1]);
-      verify(secretProviderA, timeout(timeout).times(2)).rollSecret();
-      verify(secretProviderB, timeout(timeout).times(1)).rollSecret();
+      verify(secretProviderA, timeout(timeout).atLeast(2)).rollSecret();
+      verify(secretProviderB, timeout(timeout).atLeastOnce()).rollSecret();
+
+      switch (order) {
+        case 1:
+          secretProviderA.realRollSecret();
+          secretProviderB.realRollSecret();
+          secretProviderA.realRollSecret();
+          secretProviderB.realRollSecret();
+          break;
+        case 2:
+          secretProviderB.realRollSecret();
+          secretProviderA.realRollSecret();
+          secretProviderB.realRollSecret();
+          secretProviderA.realRollSecret();
+          break;
+        default:
+          throw new Exception("Invalid order selected");
+      }
 
       currentSecretA = secretProviderA.getCurrentSecret();
       allSecretsA = secretProviderA.getAllSecrets();
@@ -260,13 +237,13 @@ public class TestZKSignerSecretProvider {
       Assert.assertEquals(2, allSecretsB.length);
       Assert.assertArrayEquals(allSecretsA[0], allSecretsB[0]);
       Assert.assertArrayEquals(allSecretsA[1], allSecretsB[1]);
-      if (Arrays.equals(secretA3, currentSecretA)) {
-        Assert.assertArrayEquals(secretA3, allSecretsA[0]);
-      } else if (Arrays.equals(secretB3, currentSecretB)) {
-        Assert.assertArrayEquals(secretB3, allSecretsA[0]);
-      } else {
-        Assert.fail("It appears that they all agreed on the same secret, but "
-                + "not one of the secrets they were supposed to");
+      switch (order) {
+        case 1:
+          Assert.assertArrayEquals(secretA4, allSecretsA[0]);
+          break;
+        case 2:
+          Assert.assertArrayEquals(secretB4, allSecretsA[0]);
+          break;
       }
     } finally {
       secretProviderB.destroy();

http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e5fffa6/hadoop-common-project/hadoop-common/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index 48c9357..49a2036 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -15,6 +15,9 @@ Release 2.7.6 - UNRELEASED
     HADOOP-12181. Fix intermittent test failure of TestZKSignerSecretProvider.
     (Masatake Iwasaki via wheat9)
 
+    HADOOP-12611. TestZKSignerSecretProvider#testMultipleInit occasionally fail.
+    (ebadger via rkanter)
+
 Release 2.7.5 - 2017-12-14
 
   INCOMPATIBLE CHANGES


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org


[3/3] hadoop git commit: HADOOP-14246. Authentication Tokens should use SecureRandom instead of Random and 256 bit secrets (Conttributed by Robert Kanter via Daniel Templeton)

Posted by rk...@apache.org.
HADOOP-14246. Authentication Tokens should use SecureRandom instead of Random and 256 bit secrets
(Conttributed by Robert Kanter via Daniel Templeton)

(cherry picked from commit 4dd6206547de8f694532579e37ba8103bafaeb12)
(cherry picked from commit f20aa38a1de73dd4a0b3a5b30636e8af246cd36a)


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/88d951e3
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/88d951e3
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/88d951e3

Branch: refs/heads/branch-2.7
Commit: 88d951e30bb34d9a6e1e2a181419a7fcc88ebfd7
Parents: 9e5fffa
Author: Daniel Templeton <te...@apache.org>
Authored: Wed Apr 12 11:17:31 2017 -0700
Committer: Robert Kanter <rk...@cloudera.com>
Committed: Thu Jan 4 15:44:56 2018 -0800

----------------------------------------------------------------------
 .../util/RandomSignerSecretProvider.java        |   9 +-
 .../util/ZKSignerSecretProvider.java            |  10 +-
 .../util/TestRandomSignerSecretProvider.java    |  68 ++++++--
 .../util/TestZKSignerSecretProvider.java        | 154 ++++++++++++++++---
 hadoop-common-project/hadoop-common/CHANGES.txt |   4 +
 5 files changed, 209 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/88d951e3/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java
index 41059a7..9245887 100644
--- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RandomSignerSecretProvider.java
@@ -15,8 +15,9 @@ package org.apache.hadoop.security.authentication.util;
 
 import com.google.common.annotations.VisibleForTesting;
 
-import java.nio.charset.Charset;
+import java.security.SecureRandom;
 import java.util.Random;
+
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 
@@ -32,7 +33,7 @@ public class RandomSignerSecretProvider extends RolloverSignerSecretProvider {
 
   public RandomSignerSecretProvider() {
     super();
-    rand = new Random();
+    rand = new SecureRandom();
   }
 
   /**
@@ -48,6 +49,8 @@ public class RandomSignerSecretProvider extends RolloverSignerSecretProvider {
 
   @Override
   protected byte[] generateNewSecret() {
-    return Long.toString(rand.nextLong()).getBytes(Charset.forName("UTF-8"));
+    byte[] secret = new byte[32]; // 32 bytes = 256 bits
+    rand.nextBytes(secret);
+    return secret;
   }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/88d951e3/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
index 11bfccd..91a2efd 100644
--- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
@@ -16,6 +16,7 @@ package org.apache.hadoop.security.authentication.util;
 import com.google.common.annotations.VisibleForTesting;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
+import java.security.SecureRandom;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -176,7 +177,7 @@ public class ZKSignerSecretProvider extends RolloverSignerSecretProvider {
 
   public ZKSignerSecretProvider() {
     super();
-    rand = new Random();
+    rand = new SecureRandom();
   }
 
   /**
@@ -369,8 +370,11 @@ public class ZKSignerSecretProvider extends RolloverSignerSecretProvider {
     }
   }
 
-  private byte[] generateRandomSecret() {
-    return Long.toString(rand.nextLong()).getBytes(Charset.forName("UTF-8"));
+  @VisibleForTesting
+  protected byte[] generateRandomSecret() {
+    byte[] secret = new byte[32]; // 32 bytes = 256 bits
+    rand.nextBytes(secret);
+    return secret;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hadoop/blob/88d951e3/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java
index 41d4967..45398c2 100644
--- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestRandomSignerSecretProvider.java
@@ -14,22 +14,37 @@
 package org.apache.hadoop.security.authentication.util;
 
 import java.util.Random;
+
+import org.apache.log4j.Level;
+import org.apache.log4j.LogManager;
 import org.junit.Assert;
 import org.junit.Test;
 
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.timeout;
+import static org.mockito.Mockito.verify;
+
 public class TestRandomSignerSecretProvider {
 
+  // rollover every 50 msec
+  private final int timeout = 100;
+  private final long rolloverFrequency = timeout / 2;
+
+  {
+    LogManager.getLogger(
+        RolloverSignerSecretProvider.LOG.getName()).setLevel(Level.DEBUG);
+  }
+
   @Test
   public void testGetAndRollSecrets() throws Exception {
-    long rolloverFrequency = 15 * 1000; // rollover every 15 sec
-    // use the same seed so we can predict the RNG
+    // Use the same seed and a "plain" Random so we can predict the RNG
     long seed = System.currentTimeMillis();
     Random rand = new Random(seed);
-    byte[] secret1 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secret2 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secret3 = Long.toString(rand.nextLong()).getBytes();
-    RandomSignerSecretProvider secretProvider =
-        new RandomSignerSecretProvider(seed);
+    byte[] secret1 = generateNewSecret(rand);
+    byte[] secret2 = generateNewSecret(rand);
+    byte[] secret3 = generateNewSecret(rand);
+    MockRandomSignerSecretProvider secretProvider =
+        spy(new MockRandomSignerSecretProvider(seed));
     try {
       secretProvider.init(null, null, rolloverFrequency);
 
@@ -39,7 +54,8 @@ public class TestRandomSignerSecretProvider {
       Assert.assertEquals(2, allSecrets.length);
       Assert.assertArrayEquals(secret1, allSecrets[0]);
       Assert.assertNull(allSecrets[1]);
-      Thread.sleep(rolloverFrequency + 2000);
+      verify(secretProvider, timeout(timeout).atLeastOnce()).rollSecret();
+      secretProvider.realRollSecret();
 
       currentSecret = secretProvider.getCurrentSecret();
       allSecrets = secretProvider.getAllSecrets();
@@ -47,7 +63,8 @@ public class TestRandomSignerSecretProvider {
       Assert.assertEquals(2, allSecrets.length);
       Assert.assertArrayEquals(secret2, allSecrets[0]);
       Assert.assertArrayEquals(secret1, allSecrets[1]);
-      Thread.sleep(rolloverFrequency + 2000);
+      verify(secretProvider, timeout(timeout).atLeast(2)).rollSecret();
+      secretProvider.realRollSecret();
 
       currentSecret = secretProvider.getCurrentSecret();
       allSecrets = secretProvider.getAllSecrets();
@@ -55,9 +72,40 @@ public class TestRandomSignerSecretProvider {
       Assert.assertEquals(2, allSecrets.length);
       Assert.assertArrayEquals(secret3, allSecrets[0]);
       Assert.assertArrayEquals(secret2, allSecrets[1]);
-      Thread.sleep(rolloverFrequency + 2000);
+      verify(secretProvider, timeout(timeout).atLeast(3)).rollSecret();
+      secretProvider.realRollSecret();
     } finally {
       secretProvider.destroy();
     }
   }
+
+  /**
+   * A hack to test RandomSignerSecretProvider.
+   * We want to test that RandomSignerSecretProvider.rollSecret() is
+   * periodically called at the expected frequency, but we want to exclude the
+   * race-condition and not take a long time to run the test.
+   */
+  private class MockRandomSignerSecretProvider
+      extends RandomSignerSecretProvider {
+    MockRandomSignerSecretProvider(long seed) {
+      super(seed);
+    }
+    @Override
+    protected synchronized void rollSecret() {
+      // this is a no-op: simply used for Mockito to verify that rollSecret()
+      // is periodically called at the expected frequency
+    }
+
+    public void realRollSecret() {
+      // the test code manually calls RandomSignerSecretProvider.rollSecret()
+      // to update the state
+      super.rollSecret();
+    }
+  }
+
+  private byte[] generateNewSecret(Random rand) {
+    byte[] secret = new byte[32];
+    rand.nextBytes(secret);
+    return secret;
+  }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/88d951e3/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
index 5e640bb..628342e 100644
--- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestZKSignerSecretProvider.java
@@ -13,13 +13,11 @@
  */
 package org.apache.hadoop.security.authentication.util;
 
-import java.util.Arrays;
+import java.nio.charset.Charset;
 import java.util.Properties;
 import java.util.Random;
 import javax.servlet.ServletContext;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.curator.test.TestingServer;
 import org.apache.log4j.Level;
 import org.apache.log4j.LogManager;
@@ -37,13 +35,13 @@ public class TestZKSignerSecretProvider {
 
   private TestingServer zkServer;
 
-  // rollover every 2 sec
+  // rollover every 50 msec
   private final int timeout = 100;
   private final long rolloverFrequency = timeout / 2;
 
-  static final Log LOG = LogFactory.getLog(TestZKSignerSecretProvider.class);
   {
-    LogManager.getLogger( RolloverSignerSecretProvider.LOG.getName() ).setLevel(Level.DEBUG);
+    LogManager.getLogger(
+        RolloverSignerSecretProvider.LOG.getName()).setLevel(Level.DEBUG);
   }
 
   @Before
@@ -63,12 +61,12 @@ public class TestZKSignerSecretProvider {
   // Test just one ZKSignerSecretProvider to verify that it works in the
   // simplest case
   public void testOne() throws Exception {
-    // use the same seed so we can predict the RNG
+    // Use the same seed and a "plain" Random so we can predict the RNG
     long seed = System.currentTimeMillis();
     Random rand = new Random(seed);
-    byte[] secret2 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secret1 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secret3 = Long.toString(rand.nextLong()).getBytes();
+    byte[] secret2 = generateNewSecret(rand);
+    byte[] secret1 = generateNewSecret(rand);
+    byte[] secret3 = generateNewSecret(rand);
     MockZKSignerSecretProvider secretProvider =
         spy(new MockZKSignerSecretProvider(seed));
     Properties config = new Properties();
@@ -115,7 +113,7 @@ public class TestZKSignerSecretProvider {
    * A hack to test ZKSignerSecretProvider.
    * We want to test that ZKSignerSecretProvider.rollSecret() is periodically
    * called at the expected frequency, but we want to exclude the
-   * race-condition.
+   * race-condition and not take a long time to run the test.
    */
   private class MockZKSignerSecretProvider extends ZKSignerSecretProvider {
     MockZKSignerSecretProvider(long seed) {
@@ -135,6 +133,116 @@ public class TestZKSignerSecretProvider {
   }
 
   @Test
+  // HADOOP-14246 increased the length of the secret from 160 bits to 256 bits.
+  // This test verifies that the upgrade goes smoothly.
+  public void testUpgradeChangeSecretLength() throws Exception {
+    // Use the same seed and a "plain" Random so we can predict the RNG
+    long seed = System.currentTimeMillis();
+    Random rand = new Random(seed);
+    byte[] secret2 = Long.toString(rand.nextLong())
+        .getBytes(Charset.forName("UTF-8"));
+    byte[] secret1 = Long.toString(rand.nextLong())
+        .getBytes(Charset.forName("UTF-8"));
+    byte[] secret3 = Long.toString(rand.nextLong())
+        .getBytes(Charset.forName("UTF-8"));
+    rand = new Random(seed);
+    // Secrets 4 and 5 get thrown away by ZK when the new secret provider tries
+    // to init
+    byte[] secret4 = generateNewSecret(rand);
+    byte[] secret5 = generateNewSecret(rand);
+    byte[] secret6 = generateNewSecret(rand);
+    byte[] secret7 = generateNewSecret(rand);
+    // Initialize the znode data with the old secret length
+    MockZKSignerSecretProvider oldSecretProvider =
+        spy(new OldMockZKSignerSecretProvider(seed));
+    Properties config = new Properties();
+    config.setProperty(
+        ZKSignerSecretProvider.ZOOKEEPER_CONNECTION_STRING,
+        zkServer.getConnectString());
+    config.setProperty(ZKSignerSecretProvider.ZOOKEEPER_PATH,
+        "/secret");
+    try {
+      oldSecretProvider.init(config, getDummyServletContext(),
+          rolloverFrequency);
+
+      byte[] currentSecret = oldSecretProvider.getCurrentSecret();
+      byte[][] allSecrets = oldSecretProvider.getAllSecrets();
+      Assert.assertArrayEquals(secret1, currentSecret);
+      Assert.assertEquals(2, allSecrets.length);
+      Assert.assertArrayEquals(secret1, allSecrets[0]);
+      Assert.assertNull(allSecrets[1]);
+      oldSecretProvider.realRollSecret();
+
+      currentSecret = oldSecretProvider.getCurrentSecret();
+      allSecrets = oldSecretProvider.getAllSecrets();
+      Assert.assertArrayEquals(secret2, currentSecret);
+      Assert.assertEquals(2, allSecrets.length);
+      Assert.assertArrayEquals(secret2, allSecrets[0]);
+      Assert.assertArrayEquals(secret1, allSecrets[1]);
+    } finally {
+      oldSecretProvider.destroy();
+    }
+    // Now use a ZKSignerSecretProvider with the newer length
+    MockZKSignerSecretProvider newSecretProvider =
+        spy(new MockZKSignerSecretProvider(seed));
+    try {
+      newSecretProvider.init(config, getDummyServletContext(),
+          rolloverFrequency);
+
+      byte[] currentSecret = newSecretProvider.getCurrentSecret();
+      byte[][] allSecrets = newSecretProvider.getAllSecrets();
+      Assert.assertArrayEquals(secret2, currentSecret);
+      Assert.assertEquals(2, allSecrets.length);
+      Assert.assertArrayEquals(secret2, allSecrets[0]);
+      Assert.assertArrayEquals(secret1, allSecrets[1]);
+      newSecretProvider.realRollSecret();
+
+      currentSecret = newSecretProvider.getCurrentSecret();
+      allSecrets = newSecretProvider.getAllSecrets();
+      Assert.assertArrayEquals(secret3, currentSecret);
+      Assert.assertEquals(2, allSecrets.length);
+      Assert.assertArrayEquals(secret3, allSecrets[0]);
+      Assert.assertArrayEquals(secret2, allSecrets[1]);
+      newSecretProvider.realRollSecret();
+
+      currentSecret = newSecretProvider.getCurrentSecret();
+      allSecrets = newSecretProvider.getAllSecrets();
+      Assert.assertArrayEquals(secret6, currentSecret);
+      Assert.assertEquals(2, allSecrets.length);
+      Assert.assertArrayEquals(secret6, allSecrets[0]);
+      Assert.assertArrayEquals(secret3, allSecrets[1]);
+      newSecretProvider.realRollSecret();
+
+      currentSecret = newSecretProvider.getCurrentSecret();
+      allSecrets = newSecretProvider.getAllSecrets();
+      Assert.assertArrayEquals(secret7, currentSecret);
+      Assert.assertEquals(2, allSecrets.length);
+      Assert.assertArrayEquals(secret7, allSecrets[0]);
+      Assert.assertArrayEquals(secret6, allSecrets[1]);
+    } finally {
+      newSecretProvider.destroy();
+    }
+  }
+
+  /**
+   * A version of {@link MockZKSignerSecretProvider} that uses the old way of
+   * generating secrets (160 bit long).
+   */
+  private class OldMockZKSignerSecretProvider
+      extends MockZKSignerSecretProvider {
+    private Random rand;
+    OldMockZKSignerSecretProvider(long seed) {
+      super(seed);
+      rand = new Random(seed);
+    }
+
+    @Override
+    protected byte[] generateRandomSecret() {
+      return Long.toString(rand.nextLong()).getBytes(Charset.forName("UTF-8"));
+    }
+  }
+
+  @Test
   public void testMultiple1() throws Exception {
     testMultiple(1);
   }
@@ -151,19 +259,19 @@ public class TestZKSignerSecretProvider {
    * @throws Exception
    */
   public void testMultiple(int order) throws Exception {
+    // Use the same seed and a "plain" Random so we can predict the RNG
     long seedA = System.currentTimeMillis();
     Random rand = new Random(seedA);
-    byte[] secretA2 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secretA1 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secretA3 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secretA4 = Long.toString(rand.nextLong()).getBytes();
-    // use the same seed so we can predict the RNG
+    byte[] secretA2 = generateNewSecret(rand);
+    byte[] secretA1 = generateNewSecret(rand);
+    byte[] secretA3 = generateNewSecret(rand);
+    byte[] secretA4 = generateNewSecret(rand);
     long seedB = System.currentTimeMillis() + rand.nextLong();
     rand = new Random(seedB);
-    byte[] secretB2 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secretB1 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secretB3 = Long.toString(rand.nextLong()).getBytes();
-    byte[] secretB4 = Long.toString(rand.nextLong()).getBytes();
+    byte[] secretB2 = generateNewSecret(rand);
+    byte[] secretB1 = generateNewSecret(rand);
+    byte[] secretB3 = generateNewSecret(rand);
+    byte[] secretB4 = generateNewSecret(rand);
     MockZKSignerSecretProvider secretProviderA =
         spy(new MockZKSignerSecretProvider(seedA));
     MockZKSignerSecretProvider secretProviderB =
@@ -258,4 +366,10 @@ public class TestZKSignerSecretProvider {
         .thenReturn(null);
     return servletContext;
   }
+
+  private byte[] generateNewSecret(Random rand) {
+    byte[] secret = new byte[32];
+    rand.nextBytes(secret);
+    return secret;
+  }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/88d951e3/hadoop-common-project/hadoop-common/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index 49a2036..1e5c164 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -8,6 +8,10 @@ Release 2.7.6 - UNRELEASED
 
   IMPROVEMENTS
 
+    HADOOP-14246. Authentication Tokens should use SecureRandom instead of
+    Random and 256 bit secrets (Conttributed by Robert Kanter via
+    Daniel Templeton)
+
   OPTIMIZATIONS
 
   BUG FIXES


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org