HADOOP-13433 Race in UGI.reloginFromKeytab. Contributed by Duo Zhang.

hadoop-common-project/hadoop-common/pom.xml

@@ -45,6 +45,11 @@
       <artifactId>hadoop-annotations</artifactId>
       <scope>compile</scope>
     </dependency>
+    <dependency>
+      <groupId>org.apache.hadoop</groupId>
+      <artifactId>hadoop-minikdc</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>

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

@@ -24,6 +24,8 @@ import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_TOKEN_FI
 import static org.apache.hadoop.security.UGIExceptionMessages.*;
 import static org.apache.hadoop.util.PlatformName.IBM_JAVA;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -45,6 +47,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
+import javax.security.auth.DestroyFailedException;
 import javax.security.auth.Subject;
 import javax.security.auth.callback.CallbackHandler;
 import javax.security.auth.kerberos.KerberosPrincipal;
@@ -76,8 +79,6 @@ import org.apache.hadoop.security.token.TokenIdentifier;
 import org.apache.hadoop.util.Shell;
 import org.apache.hadoop.util.StringUtils;
 import org.apache.hadoop.util.Time;
-
-import com.google.common.annotations.VisibleForTesting;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -1165,10 +1166,41 @@ public class UserGroupInformation {
     reloginFromKeytab();
   }
 
+  // if the first kerberos ticket is not TGT, then remove and destroy it since
+  // the kerberos library of jdk always use the first kerberos ticket as TGT.
+  // See HADOOP-13433 for more details.
+  @VisibleForTesting
+  void fixKerberosTicketOrder() {
+    Set<Object> creds = getSubject().getPrivateCredentials();
+    synchronized (creds) {
+      for (Iterator<Object> iter = creds.iterator(); iter.hasNext();) {
+        Object cred = iter.next();
+        if (cred instanceof KerberosTicket) {
+          KerberosTicket ticket = (KerberosTicket) cred;
+          if (!ticket.getServer().getName().startsWith("krbtgt")) {
+            LOG.warn(
+                "The first kerberos ticket is not TGT"
+                    + "(the server principal is {}), remove and destroy it.",
+                ticket.getServer());
+            iter.remove();
+            try {
+              ticket.destroy();
+            } catch (DestroyFailedException e) {
+              LOG.warn("destroy ticket failed", e);
+            }
+          } else {
+            return;
+          }
+        }
+      }
+    }
+    LOG.warn("Warning, no kerberos ticket found while attempting to renew ticket");
+  }
+
   /**
    * Re-Login a user in from a keytab file. Loads a user identity from a keytab
    * file and logs them in. They become the currently logged-in user. This
-   * method assumes that {@link #loginUserFromKeytab(String, String)} had 
+   * method assumes that {@link #loginUserFromKeytab(String, String)} had
    * happened already.
    * The Subject field of this UserGroupInformation object is updated to have
    * the new credentials.
@@ -1178,11 +1210,12 @@ public class UserGroupInformation {
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
   public synchronized void reloginFromKeytab() throws IOException {
-    if (!isSecurityEnabled() ||
+    if (!isSecurityEnabled()
-         user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS ||
+        || user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS
-         !isKeytab)
+        || !isKeytab) {
       return;
-    
+    }
+
     long now = Time.now();
     if (!shouldRenewImmediatelyForTests && !hasSufficientTimeElapsed(now)) {
       return;
@@ -1194,12 +1227,12 @@ public class UserGroupInformation {
         now < getRefreshTime(tgt)) {
       return;
     }
-    
+
     LoginContext login = getLogin();
     if (login == null || keytabFile == null) {
       throw new KerberosAuthException(MUST_FIRST_LOGIN_FROM_KEYTAB);
     }
-    
+
     long start = 0;
     // register most recent relogin attempt
     user.setLastLogin(now);
@@ -1222,6 +1255,7 @@ public class UserGroupInformation {
         }
         start = Time.now();
         login.login();
+        fixKerberosTicketOrder();
         metrics.loginSuccess.add(Time.now() - start);
         setLogin(login);
       }
@@ -1233,7 +1267,7 @@ public class UserGroupInformation {
       kae.setPrincipal(keytabPrincipal);
       kae.setKeytabFile(keytabFile);
       throw kae;
-    } 
+    }
   }
 
   /**
@@ -1247,10 +1281,11 @@ public class UserGroupInformation {
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
   public synchronized void reloginFromTicketCache() throws IOException {
-    if (!isSecurityEnabled() || 
+    if (!isSecurityEnabled()
-        user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS ||
+        || user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS
-        !isKrbTkt)
+        || !isKrbTkt) {
       return;
+    }
     LoginContext login = getLogin();
     if (login == null) {
       throw new KerberosAuthException(MUST_FIRST_LOGIN);
@@ -1278,15 +1313,15 @@ public class UserGroupInformation {
         LOG.debug("Initiating re-login for " + getUserName());
       }
       login.login();
+      fixKerberosTicketOrder();
       setLogin(login);
     } catch (LoginException le) {
       KerberosAuthException kae = new KerberosAuthException(LOGIN_FAILURE, le);
       kae.setUser(getUserName());
       throw kae;
-    } 
+    }
   }
 
-
   /**
    * Log a user in from a keytab file. Loads a user identity from a keytab
    * file and login them in. This new user does not affect the currently

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

@@ -0,0 +1,158 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.security;
+
+import static org.apache.hadoop.test.LambdaTestUtils.intercept;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.security.PrivilegedExceptionAction;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.security.auth.Subject;
+import javax.security.auth.kerberos.KerberosTicket;
+import javax.security.sasl.Sasl;
+import javax.security.sasl.SaslClient;
+import javax.security.sasl.SaslException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.minikdc.KerberosSecurityTestcase;
+import org.apache.hadoop.security.SaslRpcServer.AuthMethod;
+import org.apache.hadoop.security.SaslRpcServer.QualityOfProtection;
+import org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Testcase for HADOOP-13433 that verifies the logic of fixKerberosTicketOrder.
+ */
+public class TestFixKerberosTicketOrder extends KerberosSecurityTestcase {
+
+  private String clientPrincipal = "client";
+
+  private String server1Protocol = "server1";
+
+  private String server2Protocol = "server2";
+
+  private String host = "localhost";
+
+  private String server1Principal = server1Protocol + "/" + host;
+
+  private String server2Principal = server2Protocol + "/" + host;
+
+  private File keytabFile;
+
+  private Configuration conf = new Configuration();
+
+  private Map<String, String> props;
+
+  @Before
+  public void setUp() throws Exception {
+    keytabFile = new File(getWorkDir(), "keytab");
+    getKdc().createPrincipal(keytabFile, clientPrincipal, server1Principal,
+        server2Principal);
+    SecurityUtil.setAuthenticationMethod(AuthenticationMethod.KERBEROS, conf);
+    UserGroupInformation.setConfiguration(conf);
+    UserGroupInformation.setShouldRenewImmediatelyForTests(true);
+    props = new HashMap<String, String>();
+    props.put(Sasl.QOP, QualityOfProtection.AUTHENTICATION.saslQop);
+  }
+
+  @Test
+  public void test() throws Exception {
+    UserGroupInformation ugi =
+        UserGroupInformation.loginUserFromKeytabAndReturnUGI(clientPrincipal,
+            keytabFile.getCanonicalPath());
+    ugi.doAs(new PrivilegedExceptionAction<Void>() {
+
+      @Override
+      public Void run() throws Exception {
+        SaslClient client = Sasl.createSaslClient(
+            new String[] {AuthMethod.KERBEROS.getMechanismName()},
+            clientPrincipal, server1Protocol, host, props, null);
+        client.evaluateChallenge(new byte[0]);
+        client.dispose();
+        return null;
+      }
+    });
+
+    Subject subject = ugi.getSubject();
+
+    // move tgt to the last
+    for (KerberosTicket ticket : subject
+        .getPrivateCredentials(KerberosTicket.class)) {
+      if (ticket.getServer().getName().startsWith("krbtgt")) {
+        subject.getPrivateCredentials().remove(ticket);
+        subject.getPrivateCredentials().add(ticket);
+        break;
+      }
+    }
+    // make sure the first ticket is not tgt
+    assertFalse(
+        "The first ticket is still tgt, "
+            + "the implementation in jdk may have been changed, "
+            + "please reconsider the problem in HADOOP-13433",
+        subject.getPrivateCredentials().stream()
+            .filter(c -> c instanceof KerberosTicket)
+            .map(c -> ((KerberosTicket) c).getServer().getName()).findFirst()
+            .get().startsWith("krbtgt"));
+    // should fail as we send a service ticket instead of tgt to KDC.
+    intercept(SaslException.class,
+        () -> ugi.doAs(new PrivilegedExceptionAction<Void>() {
+
+          @Override
+          public Void run() throws Exception {
+            SaslClient client = Sasl.createSaslClient(
+                new String[] {AuthMethod.KERBEROS.getMechanismName()},
+                clientPrincipal, server2Protocol, host, props, null);
+            client.evaluateChallenge(new byte[0]);
+            client.dispose();
+            return null;
+          }
+        }));
+
+    ugi.fixKerberosTicketOrder();
+
+    // check if TGT is the first ticket after the fix.
+    assertTrue("The first ticket is not tgt",
+        subject.getPrivateCredentials().stream()
+            .filter(c -> c instanceof KerberosTicket)
+            .map(c -> ((KerberosTicket) c).getServer().getName()).findFirst()
+            .get().startsWith("krbtgt"));
+
+    // make sure we can still get new service ticket after the fix.
+    ugi.doAs(new PrivilegedExceptionAction<Void>() {
+
+      @Override
+      public Void run() throws Exception {
+        SaslClient client = Sasl.createSaslClient(
+            new String[] {AuthMethod.KERBEROS.getMechanismName()},
+            clientPrincipal, server2Protocol, host, props, null);
+        client.evaluateChallenge(new byte[0]);
+        client.dispose();
+        return null;
+      }
+    });
+    assertTrue("No service ticket for " + server2Protocol + " found",
+        subject.getPrivateCredentials(KerberosTicket.class).stream()
+            .filter(t -> t.getServer().getName().startsWith(server2Protocol))
+            .findAny().isPresent());
+  }
+}

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

@@ -0,0 +1,162 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.security;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+import java.security.PrivilegedExceptionAction;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import javax.security.auth.kerberos.KerberosTicket;
+import javax.security.sasl.Sasl;
+import javax.security.sasl.SaslClient;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.minikdc.KerberosSecurityTestcase;
+import org.apache.hadoop.security.SaslRpcServer.AuthMethod;
+import org.apache.hadoop.security.SaslRpcServer.QualityOfProtection;
+import org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Testcase for HADOOP-13433 that confirms that tgt will always be the first
+ * ticket after relogin.
+ */
+public class TestRaceWhenRelogin extends KerberosSecurityTestcase {
+
+  private int numThreads = 10;
+
+  private String clientPrincipal = "client";
+
+  private String serverProtocol = "server";
+
+  private String[] serverProtocols;
+
+  private String host = "localhost";
+
+  private String serverPrincipal = serverProtocol + "/" + host;
+
+  private String[] serverPrincipals;
+
+  private File keytabFile;
+
+  private Configuration conf = new Configuration();
+
+  private Map<String, String> props;
+
+  private UserGroupInformation ugi;
+
+  @Before
+  public void setUp() throws Exception {
+    keytabFile = new File(getWorkDir(), "keytab");
+    serverProtocols = new String[numThreads];
+    serverPrincipals = new String[numThreads];
+    for (int i = 0; i < numThreads; i++) {
+      serverProtocols[i] = serverProtocol + i;
+      serverPrincipals[i] = serverProtocols[i] + "/" + host;
+    }
+    String[] principals =
+        Arrays.copyOf(serverPrincipals, serverPrincipals.length + 2);
+    principals[numThreads] = serverPrincipal;
+    principals[numThreads + 1] = clientPrincipal;
+    getKdc().createPrincipal(keytabFile, principals);
+    SecurityUtil.setAuthenticationMethod(AuthenticationMethod.KERBEROS, conf);
+    UserGroupInformation.setConfiguration(conf);
+    UserGroupInformation.setShouldRenewImmediatelyForTests(true);
+    props = new HashMap<String, String>();
+    props.put(Sasl.QOP, QualityOfProtection.AUTHENTICATION.saslQop);
+    ugi = UserGroupInformation.loginUserFromKeytabAndReturnUGI(clientPrincipal,
+        keytabFile.getAbsolutePath());
+  }
+
+  private void relogin(AtomicBoolean pass) {
+    for (int i = 0; i < 100; i++) {
+      try {
+        ugi.reloginFromKeytab();
+      } catch (IOException e) {
+      }
+      KerberosTicket tgt = ugi.getSubject().getPrivateCredentials().stream()
+          .filter(c -> c instanceof KerberosTicket).map(c -> (KerberosTicket) c)
+          .findFirst().get();
+      if (!tgt.getServer().getName().startsWith("krbtgt")) {
+        pass.set(false);
+        return;
+      }
+      try {
+        Thread.sleep(50);
+      } catch (InterruptedException e) {
+      }
+    }
+  }
+
+  private void getServiceTicket(AtomicBoolean running, String serverProtocol) {
+    while (running.get()) {
+      try {
+        ugi.doAs(new PrivilegedExceptionAction<Void>() {
+
+          @Override
+          public Void run() throws Exception {
+            SaslClient client = Sasl.createSaslClient(
+                new String[] {AuthMethod.KERBEROS.getMechanismName()},
+                clientPrincipal, serverProtocol, host, props, null);
+            client.evaluateChallenge(new byte[0]);
+            client.dispose();
+            return null;
+          }
+        });
+      } catch (Exception e) {
+      }
+      try {
+        Thread.sleep(ThreadLocalRandom.current().nextInt(100));
+      } catch (InterruptedException e) {
+      }
+    }
+  }
+
+  @Test
+  public void test() throws InterruptedException, IOException {
+    AtomicBoolean pass = new AtomicBoolean(true);
+    Thread reloginThread = new Thread(() -> relogin(pass), "Relogin");
+
+    AtomicBoolean running = new AtomicBoolean(true);
+    Thread[] getServiceTicketThreads = new Thread[numThreads];
+    for (int i = 0; i < numThreads; i++) {
+      String serverProtocol = serverProtocols[i];
+      getServiceTicketThreads[i] =
+          new Thread(() -> getServiceTicket(running, serverProtocol),
+              "GetServiceTicket-" + i);
+    }
+    for (Thread getServiceTicketThread : getServiceTicketThreads) {
+      getServiceTicketThread.start();
+    }
+    reloginThread.start();
+    reloginThread.join();
+    running.set(false);
+    for (Thread getServiceTicketThread : getServiceTicketThreads) {
+      getServiceTicketThread.join();
+    }
+    assertTrue("tgt is not the first ticket after relogin", pass.get());
+  }
+}