HADOOP-14246. Authentication Tokens should use SecureRandom instead of Random and 256 bit secrets (Conttributed by Robert Konter via Daniel Templeton)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/5b2e824c Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/5b2e824c Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/5b2e824c Branch: refs/heads/HDFS-10467 Commit: 5b2e824cf71c497eafbac674d2f6922bc59d3bd9 Parents: a631172 Author: Daniel Templeton <templ...@apache.org> Authored: Wed Apr 12 11:17:31 2017 -0700 Committer: Inigo <inigo...@apache.org> Committed: Mon Apr 17 11:17:02 2017 -0700 ---------------------------------------------------------------------- .../util/RandomSignerSecretProvider.java | 9 +- .../util/ZKSignerSecretProvider.java | 10 +- .../util/TestRandomSignerSecretProvider.java | 68 ++++++-- .../util/TestZKSignerSecretProvider.java | 154 ++++++++++++++++--- 4 files changed, 205 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/5b2e824c/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/5b2e824c/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 48dfaaa..a7fc76f 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; @@ -149,7 +150,7 @@ public class ZKSignerSecretProvider extends RolloverSignerSecretProvider { public ZKSignerSecretProvider() { super(); - rand = new Random(); + rand = new SecureRandom(); } /** @@ -342,8 +343,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/5b2e824c/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/5b2e824c/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; + } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
