HADOOP-15808. Harden Token service loader use.

Contributed by Steve Loughran.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/DtFileOperations.java

@@ -24,6 +24,8 @@
 import java.text.DateFormat;
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.Iterator;
+import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
 
 import org.apache.commons.lang3.StringUtils;
@@ -130,7 +132,7 @@ public static void printTokenFile(
    *  @param creds the Credentials object to be printed out.
    *  @param alias print only tokens matching alias (null matches all).
    *  @param out print to this stream.
-   *  @throws IOException
+   *  @throws IOException failure to unmarshall a token identifier.
    */
   public static void printCredentials(
       Credentials creds, Text alias, PrintStream out)
@@ -145,8 +147,13 @@ public static void printCredentials(
           out.println(StringUtils.repeat("-", 80));
           tokenHeader = false;
         }
-        AbstractDelegationTokenIdentifier id =
+        AbstractDelegationTokenIdentifier id;
-            (AbstractDelegationTokenIdentifier) token.decodeIdentifier();
+        try {
+          id = (AbstractDelegationTokenIdentifier) token.decodeIdentifier();
+        } catch (IllegalStateException e) {
+          LOG.debug("Failed to decode token identifier", e);
+          id = null;
+        }
         out.printf(fmt, token.getKind(), token.getService(),
                    (id != null) ? id.getRenewer() : NA_STRING,
                    (id != null) ? formatDate(id.getMaxDate()) : NA_STRING,
@@ -172,7 +179,17 @@ public static void getTokenFile(File tokenFile, String fileFormat,
     Credentials creds = tokenFile.exists() ?
         Credentials.readTokenStorageFile(tokenFile, conf) : new Credentials();
     ServiceLoader<DtFetcher> loader = ServiceLoader.load(DtFetcher.class);
-    for (DtFetcher fetcher : loader) {
+    Iterator<DtFetcher> iterator = loader.iterator();
+    while (iterator.hasNext()) {
+      DtFetcher fetcher;
+      try {
+        fetcher = iterator.next();
+      } catch (ServiceConfigurationError e) {
+        // failure to load a token implementation
+        // log at debug and continue.
+        LOG.debug("Failed to load token fetcher implementation", e);
+        continue;
+      }
       if (matchService(fetcher, service, url)) {
         if (!fetcher.isTokenRequired()) {
           String message = "DtFetcher for service '" + service +

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/Token.java

@@ -35,7 +35,9 @@
 
 import java.io.*;
 import java.util.Arrays;
+import java.util.Iterator;
 import java.util.Map;
+import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
 import java.util.UUID;
 
@@ -147,14 +149,25 @@ public byte[] getIdentifier() {
     synchronized (Token.class) {
       if (tokenKindMap == null) {
         tokenKindMap = Maps.newHashMap();
-        for (TokenIdentifier id : ServiceLoader.load(TokenIdentifier.class)) {
+        // start the service load process; it's only in the "next()" calls
-          tokenKindMap.put(id.getKind(), id.getClass());
+        // where implementations are loaded.
+        final Iterator<TokenIdentifier> tokenIdentifiers =
+            ServiceLoader.load(TokenIdentifier.class).iterator();
+        while (tokenIdentifiers.hasNext()) {
+          try {
+            TokenIdentifier id = tokenIdentifiers.next();
+            tokenKindMap.put(id.getKind(), id.getClass());
+          } catch (ServiceConfigurationError e) {
+            // failure to load a token implementation
+            // log at debug and continue.
+            LOG.debug("Failed to load token identifier implementation", e);
+          }
         }
       }
       cls = tokenKindMap.get(kind);
     }
     if (cls == null) {
-      LOG.debug("Cannot find class for token kind " + kind);
+      LOG.debug("Cannot find class for token kind {}", kind);
       return null;
     }
     return cls;
@@ -163,8 +176,9 @@ public byte[] getIdentifier() {
   /**
    * Get the token identifier object, or null if it could not be constructed
    * (because the class could not be loaded, for example).
-   * @return the token identifier, or null
+   * @return the token identifier, or null if there was no class found for it
-   * @throws IOException
+   * @throws IOException failure to unmarshall the data
+   * @throws RuntimeException if the token class could not be instantiated.
    */
   @SuppressWarnings("unchecked")
   public T decodeIdentifier() throws IOException {
@@ -263,7 +277,7 @@ static class PrivateToken<T extends TokenIdentifier> extends Token<T> {
       assert !publicToken.isPrivate();
       publicService = publicToken.service;
       if (LOG.isDebugEnabled()) {
-        LOG.debug("Cloned private token " + this + " from " + publicToken);
+        LOG.debug("Cloned private token {} from {}", this, publicToken);
       }
     }
 
@@ -465,14 +479,22 @@ private synchronized TokenRenewer getRenewer() throws IOException {
     }
     renewer = TRIVIAL_RENEWER;
     synchronized (renewers) {
-      for (TokenRenewer canidate : renewers) {
+      Iterator<TokenRenewer> it = renewers.iterator();
-        if (canidate.handleKind(this.kind)) {
+      while (it.hasNext()) {
-          renewer = canidate;
+        try {
-          return renewer;
+          TokenRenewer candidate = it.next();
+          if (candidate.handleKind(this.kind)) {
+            renewer = candidate;
+            return renewer;
+          }
+        } catch (ServiceConfigurationError e) {
+          // failure to load a token implementation
+          // log at debug and continue.
+          LOG.debug("Failed to load token renewer implementation", e);
         }
       }
     }
-    LOG.warn("No TokenRenewer defined for token kind " + this.kind);
+    LOG.warn("No TokenRenewer defined for token kind {}", kind);
     return renewer;
   }
 

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java

@@ -926,7 +926,9 @@ private static void assertAuthEquals(AuthMethod expect,
   private static void assertAuthEquals(Pattern expect, String actual) {
     // this allows us to see the regexp and the value it didn't match
     if (!expect.matcher(actual).matches()) {
-      fail(); // it failed
+      // it failed
+      fail(String.format("\"%s\" did not match pattern %s",
+          actual, expect));
     }
   }
 

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/TestToken.java

@@ -28,6 +28,7 @@
 import org.apache.hadoop.security.token.delegation.TestDelegationToken.TestDelegationTokenSecretManager;
 import org.junit.Test;
 
+import static org.apache.hadoop.test.LambdaTestUtils.intercept;
 import static org.junit.Assert.*;
 
 /** Unit tests for Token */
@@ -86,13 +87,12 @@ public void testEncodeWritable() throws Exception {
         "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLM" +
              "NOPQRSTUVWXYZ01234567890!@#$%^&*()-=_+[]{}|;':,./<>?"};
     Token<AbstractDelegationTokenIdentifier> orig;
-    Token<AbstractDelegationTokenIdentifier> copy = 
+    Token<AbstractDelegationTokenIdentifier> copy = new Token<>();
-      new Token<AbstractDelegationTokenIdentifier>();
     // ensure that for each string the input and output values match
     for(int i=0; i< values.length; ++i) {
       String val = values[i];
-      System.out.println("Input = " + val);
+      Token.LOG.info("Input = {}", val);
-      orig = new Token<AbstractDelegationTokenIdentifier>(val.getBytes(),
+      orig = new Token<>(val.getBytes(),
           val.getBytes(), new Text(val), new Text(val));
       String encode = orig.encodeToUrlString();
       copy.decodeFromUrlString(encode);
@@ -107,15 +107,9 @@ public void testEncodeWritable() throws Exception {
    */
   @Test
   public void testDecodeWritableArgSanityCheck() throws Exception {
-    Token<AbstractDelegationTokenIdentifier> token =
+    Token<AbstractDelegationTokenIdentifier> token = new Token<>();
-            new Token<AbstractDelegationTokenIdentifier>();
+    intercept(HadoopIllegalArgumentException.class,
-    try {
+        () -> token.decodeFromUrlString(null));
-      token.decodeFromUrlString(null);
-      fail("Should have thrown HadoopIllegalArgumentException");
-    }
-    catch (HadoopIllegalArgumentException e) {
-      Token.LOG.info("Test decodeWritable() sanity check success.");
-    }
   }
 
   @Test
@@ -127,7 +121,7 @@ public void testDecodeIdentifier() throws IOException {
         new Text("owner"), new Text("renewer"), new Text("realUser"));
     
     Token<TestDelegationTokenIdentifier> token =
-      new Token<TestDelegationTokenIdentifier>(id, secretManager);
+        new Token<>(id, secretManager);
     TokenIdentifier idCopy = token.decodeIdentifier();
     
     assertNotSame(id, idCopy);