diff --git a/CHANGES.txt b/CHANGES.txt index e94da20df7..0f8acacacd 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -148,6 +148,10 @@ Trunk (unreleased changes) connection setup fails. This is applicable only to keytab based logins. (Devaraj Das) + HADOOP-6551. Delegation token renewing and cancelling should provide + meaningful exceptions when there are failures instead of returning + false. (omalley) + OPTIMIZATIONS BUG FIXES diff --git a/src/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java b/src/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java index 50324b0a21..fd17d4e19d 100644 --- a/src/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java +++ b/src/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java @@ -19,6 +19,8 @@ package org.apache.hadoop.security.token.delegation; import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.io.Text; + import static org.apache.hadoop.classification.InterfaceAudience.LimitedPrivate.Project.HDFS; import static org.apache.hadoop.classification.InterfaceAudience.LimitedPrivate.Project.MAPREDUCE; @@ -34,6 +36,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.SecretManager; import org.apache.hadoop.util.Daemon; @@ -185,11 +188,15 @@ public byte[] retrievePassword(TokenIdent identifier } /** - * Renew a delegation token. Canceled tokens are not renewed. Return true if - * the token is successfully renewed; false otherwise. + * Renew a delegation token. + * @param token the token to renew + * @param renewer the full principal name of the user doing the renewal + * @return the new expiration time + * @throws InvalidToken if the token is invalid + * @throws AccessControlException if the user can't renew token */ - public Boolean renewToken(Token token, - String renewer) throws InvalidToken, IOException { + public long renewToken(Token token, + String renewer) throws InvalidToken, IOException { long now = System.currentTimeMillis(); ByteArrayInputStream buf = new ByteArrayInputStream(token.getIdentifier()); DataInputStream in = new DataInputStream(buf); @@ -197,71 +204,76 @@ public Boolean renewToken(Token token, id.readFields(in); synchronized (currentTokens) { if (currentTokens.get(id) == null) { - LOG.warn("Renewal request for unknown token"); - return false; + throw new InvalidToken("Renewal request for unknown token"); } } if (id.getMaxDate() < now) { - LOG.warn("Client " + renewer + " tries to renew an expired token"); - return false; + throw new InvalidToken("User " + renewer + + " tried to renew an expired token"); } - if (id.getRenewer() == null || !id.getRenewer().toString().equals(renewer)) { - LOG.warn("Client " + renewer + " tries to renew a token with " - + "renewer specified as " + id.getRenewer()); - return false; + if (id.getRenewer() == null) { + throw new AccessControlException("User " + renewer + + " tried to renew a token without " + + "a renewer"); + } + if (!id.getRenewer().toString().equals(renewer)) { + throw new AccessControlException("Client " + renewer + + " tries to renew a token with " + + "renewer specified as " + + id.getRenewer()); } DelegationKey key = null; synchronized (this) { key = allKeys.get(id.getMasterKeyId()); } if (key == null) { - LOG.warn("Unable to find master key for keyId=" + id.getMasterKeyId() - + " from cache. Failed to renew an unexpired token with sequenceNumber=" - + id.getSequenceNumber() + ", issued by this key"); - return false; + throw new InvalidToken("Unable to find master key for keyId=" + + id.getMasterKeyId() + + " from cache. Failed to renew an unexpired token"+ + " with sequenceNumber=" + id.getSequenceNumber()); } byte[] password = createPassword(token.getIdentifier(), key.getKey()); if (!Arrays.equals(password, token.getPassword())) { - LOG.warn("Client " + renewer + " is trying to renew a token with wrong password"); - return false; + throw new AccessControlException("Client " + renewer + + " is trying to renew a token with " + + "wrong password"); } DelegationTokenInformation info = new DelegationTokenInformation( Math.min(id.getMaxDate(), now + tokenRenewInterval), password); synchronized (currentTokens) { currentTokens.put(id, info); } - return true; + return info.getRenewDate(); } /** - * Cancel a token by removing it from cache. Return true if - * token exists in cache; false otherwise. + * Cancel a token by removing it from cache. + * @throws InvalidToken for invalid token + * @throws AccessControlException if the user isn't allowed to cancel */ - public Boolean cancelToken(Token token, + public void cancelToken(Token token, String canceller) throws IOException { ByteArrayInputStream buf = new ByteArrayInputStream(token.getIdentifier()); DataInputStream in = new DataInputStream(buf); TokenIdent id = createIdentifier(); id.readFields(in); - if (id.getRenewer() == null) { - LOG.warn("Renewer is null: Invalid Identifier"); - return false; - } if (id.getUser() == null) { - LOG.warn("owner is null: Invalid Identifier"); - return false; + throw new InvalidToken("Token with no owner"); } String owner = id.getUser().getUserName(); - String renewer = id.getRenewer().toString(); - if (!canceller.equals(owner) && !canceller.equals(renewer)) { - LOG.warn(canceller + " is not authorized to cancel the token"); - return false; + Text renewer = id.getRenewer(); + if (!canceller.equals(owner) && + (renewer == null || !canceller.equals(renewer.toString()))) { + throw new AccessControlException(canceller + + " is not authorized to cancel the token"); } DelegationTokenInformation info = null; synchronized (currentTokens) { info = currentTokens.remove(id); } - return info != null; + if (info == null) { + throw new InvalidToken("Token not found"); + } } /** diff --git a/src/test/core/org/apache/hadoop/security/token/delegation/TestDelegationToken.java b/src/test/core/org/apache/hadoop/security/token/delegation/TestDelegationToken.java index 244ff5aea7..b10e77370c 100644 --- a/src/test/core/org/apache/hadoop/security/token/delegation/TestDelegationToken.java +++ b/src/test/core/org/apache/hadoop/security/token/delegation/TestDelegationToken.java @@ -23,26 +23,28 @@ import java.io.DataInputStream; import java.io.DataOutput; import java.io.IOException; +import java.security.PrivilegedExceptionAction; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; import java.util.List; import junit.framework.Assert; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.hadoop.io.DataInputBuffer; import org.apache.hadoop.io.DataOutputBuffer; import org.apache.hadoop.io.Text; import org.apache.hadoop.io.Writable; +import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.token.Token; -import org.apache.hadoop.security.token.TokenIdentifier; import org.apache.hadoop.security.token.SecretManager.InvalidToken; +import org.apache.hadoop.util.StringUtils; import org.junit.Test; -import org.mortbay.log.Log; import static org.junit.Assert.*; public class TestDelegationToken { + private static final Log LOG = LogFactory.getLog(TestDelegationToken.class); private static final Text KIND = new Text("MY KIND"); public static class TestDelegationTokenIdentifier @@ -137,25 +139,44 @@ private Token generateDelegationToken( owner), new Text(renewer), null); return new Token(dtId, dtSecretManager); } + + private void shouldThrow(PrivilegedExceptionAction action, + Class except) { + try { + action.run(); + Assert.fail("action did not throw " + except); + } catch (Throwable th) { + LOG.info("Caught an exception: " + StringUtils.stringifyException(th)); + assertEquals("action threw wrong exception", except, th.getClass()); + } + } + @Test public void testDelegationTokenSecretManager() throws Exception { - TestDelegationTokenSecretManager dtSecretManager = + final TestDelegationTokenSecretManager dtSecretManager = new TestDelegationTokenSecretManager(24*60*60*1000, 3*1000,1*1000,3600000); try { dtSecretManager.startThreads(); - Token token = generateDelegationToken( + final Token token = + generateDelegationToken( dtSecretManager, "SomeUser", "JobTracker"); // Fake renewer should not be able to renew - Assert.assertFalse(dtSecretManager.renewToken(token, "FakeRenewer")); - Assert.assertTrue(dtSecretManager.renewToken(token, "JobTracker")); + shouldThrow(new PrivilegedExceptionAction() { + public Object run() throws Exception { + dtSecretManager.renewToken(token, "FakeRenewer"); + return null; + } + }, AccessControlException.class); + long time = dtSecretManager.renewToken(token, "JobTracker"); + assertTrue("renew time is in future", time > System.currentTimeMillis()); TestDelegationTokenIdentifier identifier = new TestDelegationTokenIdentifier(); byte[] tokenId = token.getIdentifier(); identifier.readFields(new DataInputStream( new ByteArrayInputStream(tokenId))); Assert.assertTrue(null != dtSecretManager.retrievePassword(identifier)); - Log.info("Sleep to expire the token"); + LOG.info("Sleep to expire the token"); Thread.sleep(2000); //Token should be expired try { @@ -165,31 +186,49 @@ public void testDelegationTokenSecretManager() throws Exception { } catch (InvalidToken e) { //Success } - Assert.assertTrue(dtSecretManager.renewToken(token, "JobTracker")); - Log.info("Sleep beyond the max lifetime"); + dtSecretManager.renewToken(token, "JobTracker"); + LOG.info("Sleep beyond the max lifetime"); Thread.sleep(2000); - Assert.assertFalse(dtSecretManager.renewToken(token, "JobTracker")); + + shouldThrow(new PrivilegedExceptionAction() { + public Object run() throws Exception { + dtSecretManager.renewToken(token, "JobTracker"); + return null; + } + }, InvalidToken.class); } finally { dtSecretManager.stopThreads(); } } + @Test public void testCancelDelegationToken() throws Exception { - TestDelegationTokenSecretManager dtSecretManager = + final TestDelegationTokenSecretManager dtSecretManager = new TestDelegationTokenSecretManager(24*60*60*1000, 10*1000,1*1000,3600000); try { dtSecretManager.startThreads(); - Token token = generateDelegationToken( - dtSecretManager, "SomeUser", "JobTracker"); + final Token token = + generateDelegationToken(dtSecretManager, "SomeUser", "JobTracker"); //Fake renewer should not be able to renew - Assert.assertFalse(dtSecretManager.cancelToken(token, "FakeCanceller")); - Assert.assertTrue(dtSecretManager.cancelToken(token, "JobTracker")); - Assert.assertFalse(dtSecretManager.renewToken(token, "JobTracker")); + shouldThrow(new PrivilegedExceptionAction() { + public Object run() throws Exception { + dtSecretManager.renewToken(token, "FakeCanceller"); + return null; + } + }, AccessControlException.class); + dtSecretManager.cancelToken(token, "JobTracker"); + shouldThrow(new PrivilegedExceptionAction() { + public Object run() throws Exception { + dtSecretManager.renewToken(token, "JobTracker"); + return null; + } + }, InvalidToken.class); } finally { dtSecretManager.stopThreads(); } } + @Test public void testRollMasterKey() throws Exception { TestDelegationTokenSecretManager dtSecretManager = @@ -226,6 +265,7 @@ public void testRollMasterKey() throws Exception { dtSecretManager.stopThreads(); } } + @Test @SuppressWarnings("unchecked") public void testDelegationTokenSelector() throws Exception {