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 ki...@apache.org on 2018/02/23 19:11:38 UTC

hadoop git commit: HADOOP-9747. Reduce unnecessary UGI synchronization. Contributed by Daryn Sharp.

Repository: hadoop
Updated Branches:
  refs/heads/trunk 3688e491d -> 59cf75887


HADOOP-9747. Reduce unnecessary UGI synchronization. Contributed by Daryn Sharp.


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

Branch: refs/heads/trunk
Commit: 59cf7588779145ad5850ad63426743dfe03d8347
Parents: 3688e49
Author: Kihwal Lee <ki...@apache.org>
Authored: Fri Feb 23 13:10:56 2018 -0600
Committer: Kihwal Lee <ki...@apache.org>
Committed: Fri Feb 23 13:10:56 2018 -0600

----------------------------------------------------------------------
 .../hadoop/fs/CommonConfigurationKeys.java      |  11 -
 .../hadoop/security/UserGroupInformation.java   | 898 +++++++++----------
 .../src/main/resources/core-default.xml         |  13 -
 .../hadoop/security/TestUGILoginFromKeytab.java | 404 ++++++++-
 .../hadoop/security/TestUGIWithMiniKdc.java     |  54 +-
 .../security/TestUserGroupInformation.java      | 113 ++-
 6 files changed, 942 insertions(+), 551 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/59cf7588/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
index ba6e4e2..043e52a 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
@@ -355,17 +355,6 @@ public class CommonConfigurationKeys extends CommonConfigurationKeysPublic {
   public static final String HADOOP_USER_GROUP_METRICS_PERCENTILES_INTERVALS =
     "hadoop.user.group.metrics.percentiles.intervals";
 
-  /* When creating UGI with UserGroupInformation(Subject), treat the passed
-   * subject external if set to true, and assume the owner of the subject
-   * should do the credential renewal.
-   *
-   * This is a temporary config to solve the compatibility issue with
-   * HADOOP-13558 and HADOOP-13805 fix, see the jiras for discussions.
-   */
-  public static final String HADOOP_TREAT_SUBJECT_EXTERNAL_KEY =
-      "hadoop.treat.subject.external";
-  public static final boolean HADOOP_TREAT_SUBJECT_EXTERNAL_DEFAULT = false;
-
   public static final String RPC_METRICS_QUANTILE_ENABLE =
       "rpc.metrics.quantile.enable";
   public static final boolean RPC_METRICS_QUANTILE_ENABLE_DEFAULT = false;

http://git-wip-us.apache.org/repos/asf/hadoop/blob/59cf7588/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
index 726e811..003a51c 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
@@ -18,8 +18,6 @@
 package org.apache.hadoop.security;
 
 import static org.apache.hadoop.fs.CommonConfigurationKeys.HADOOP_USER_GROUP_METRICS_PERCENTILES_INTERVALS;
-import static org.apache.hadoop.fs.CommonConfigurationKeys.HADOOP_TREAT_SUBJECT_EXTERNAL_KEY;
-import static org.apache.hadoop.fs.CommonConfigurationKeys.HADOOP_TREAT_SUBJECT_EXTERNAL_DEFAULT;
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_KERBEROS_MIN_SECONDS_BEFORE_RELOGIN;
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_KERBEROS_MIN_SECONDS_BEFORE_RELOGIN_DEFAULT;
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_TOKEN_FILES;
@@ -42,12 +40,14 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.EnumMap;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 
 import javax.security.auth.DestroyFailedException;
 import javax.security.auth.Subject;
@@ -56,6 +56,7 @@ import javax.security.auth.kerberos.KerberosPrincipal;
 import javax.security.auth.kerberos.KerberosTicket;
 import javax.security.auth.login.AppConfigurationEntry;
 import javax.security.auth.login.AppConfigurationEntry.LoginModuleControlFlag;
+import javax.security.auth.login.Configuration.Parameters;
 import javax.security.auth.login.LoginContext;
 import javax.security.auth.login.LoginException;
 import javax.security.auth.spi.LoginModule;
@@ -190,10 +191,8 @@ public class UserGroupInformation {
         }
         return true;
       }
-      Principal user = null;
-      // if we are using kerberos, try it out
-      if (isAuthenticationMethodEnabled(AuthenticationMethod.KERBEROS)) {
-        user = getCanonicalUser(KerberosPrincipal.class);
+      Principal user = getCanonicalUser(KerberosPrincipal.class);
+      if (user != null) {
         if (LOG.isDebugEnabled()) {
           LOG.debug("using kerberos user:"+user);
         }
@@ -222,7 +221,11 @@ public class UserGroupInformation {
 
         User userEntry = null;
         try {
-          userEntry = new User(user.getName());
+          // LoginContext will be attached later unless it's an external
+          // subject.
+          AuthenticationMethod authMethod = (user instanceof KerberosPrincipal)
+            ? AuthenticationMethod.KERBEROS : AuthenticationMethod.SIMPLE;
+          userEntry = new User(user.getName(), authMethod, null);
         } catch (Exception e) {
           throw (LoginException)(new LoginException(e.toString()).initCause(e));
         }
@@ -277,28 +280,6 @@ public class UserGroupInformation {
   private static long kerberosMinSecondsBeforeRelogin;
   /** The configuration to use */
 
-  /*
-   * This config is a temporary one for backward compatibility.
-   * It means whether to treat the subject passed to
-   * UserGroupInformation(Subject) as external. If true,
-   * -  no renewal thread will be created to do the renew credential
-   * -  reloginFromKeytab() and reloginFromTicketCache will not renew
-   *    credential.
-   * and it assumes that the owner of the subject to renew; if false, it means
-   * to retain the old behavior prior to fixing HADOOP-13558 and HADOOP-13805.
-   * The default is false.
-   */
-  private static boolean treatSubjectExternal = false;
-
-  /*
-   * Some test need the renewal thread to be created even if it does
-   *   UserGroupInformation.loginUserFromSubject(subject);
-   * The test code may set this variable to true via
-   *   setEnableRenewThreadCreationForTest(boolean)
-   * method.
-   */
-  private static boolean enableRenewThreadCreationForTest = false;
-
   private static Configuration conf;
 
   
@@ -364,15 +345,6 @@ public class UserGroupInformation {
         metrics.getGroupsQuantiles = getGroupsQuantiles;
       }
     }
-
-    treatSubjectExternal = conf.getBoolean(HADOOP_TREAT_SUBJECT_EXTERNAL_KEY,
-        HADOOP_TREAT_SUBJECT_EXTERNAL_DEFAULT);
-    if (treatSubjectExternal) {
-      LOG.info("Config " + HADOOP_TREAT_SUBJECT_EXTERNAL_KEY + " is set to "
-          + "true, the owner of the subject passed to "
-          + " UserGroupInformation(Subject) is supposed to renew the "
-          + "credential.");
-    }
   }
 
   /**
@@ -389,18 +361,6 @@ public class UserGroupInformation {
 
   @InterfaceAudience.Private
   @VisibleForTesting
-  static void setEnableRenewThreadCreationForTest(boolean b) {
-    enableRenewThreadCreationForTest = b;
-  }
-
-  @InterfaceAudience.Private
-  @VisibleForTesting
-  static boolean getEnableRenewThreadCreationForTest() {
-    return enableRenewThreadCreationForTest;
-  }
-
-  @InterfaceAudience.Private
-  @VisibleForTesting
   public static void reset() {
     authenticationMethod = null;
     conf = null;
@@ -408,7 +368,6 @@ public class UserGroupInformation {
     kerberosMinSecondsBeforeRelogin = 0;
     setLoginUser(null);
     HadoopKerberosName.setRules(null);
-    setEnableRenewThreadCreationForTest(false);
   }
   
   /**
@@ -431,17 +390,13 @@ public class UserGroupInformation {
   /**
    * Information about the logged in user.
    */
-  private static UserGroupInformation loginUser = null;
-  private static String keytabPrincipal = null;
-  private static String keytabFile = null;
+  private static final AtomicReference<UserGroupInformation> loginUserRef =
+    new AtomicReference<>();
 
   private final Subject subject;
   // All non-static fields must be read-only caches that come from the subject.
   private final User user;
-  private final boolean isKeytab;
-  private final boolean isKrbTkt;
-  private final boolean isLoginExternal;
-  
+
   private static String OS_LOGIN_MODULE_NAME;
   private static Class<? extends Principal> OS_PRINCIPAL_CLASS;
   
@@ -540,134 +495,10 @@ public class UserGroupInformation {
       return realUser.toString();
     }
   }
-  
-  /**
-   * A JAAS configuration that defines the login modules that we want
-   * to use for login.
-   */
-  private static class HadoopConfiguration 
-      extends javax.security.auth.login.Configuration {
-    private static final String SIMPLE_CONFIG_NAME = "hadoop-simple";
-    private static final String USER_KERBEROS_CONFIG_NAME = 
-      "hadoop-user-kerberos";
-    private static final String KEYTAB_KERBEROS_CONFIG_NAME = 
-      "hadoop-keytab-kerberos";
 
-    private static final Map<String, String> BASIC_JAAS_OPTIONS =
-      new HashMap<String,String>();
-    static {
-      String jaasEnvVar = System.getenv("HADOOP_JAAS_DEBUG");
-      if (jaasEnvVar != null && "true".equalsIgnoreCase(jaasEnvVar)) {
-        BASIC_JAAS_OPTIONS.put("debug", "true");
-      }
-    }
-    
-    private static final AppConfigurationEntry OS_SPECIFIC_LOGIN =
-      new AppConfigurationEntry(OS_LOGIN_MODULE_NAME,
-                                LoginModuleControlFlag.REQUIRED,
-                                BASIC_JAAS_OPTIONS);
-    private static final AppConfigurationEntry HADOOP_LOGIN =
-      new AppConfigurationEntry(HadoopLoginModule.class.getName(),
-                                LoginModuleControlFlag.REQUIRED,
-                                BASIC_JAAS_OPTIONS);
-    private static final Map<String,String> USER_KERBEROS_OPTIONS = 
-      new HashMap<String,String>();
-    static {
-      if (IBM_JAVA) {
-        USER_KERBEROS_OPTIONS.put("useDefaultCcache", "true");
-      } else {
-        USER_KERBEROS_OPTIONS.put("doNotPrompt", "true");
-        USER_KERBEROS_OPTIONS.put("useTicketCache", "true");
-      }
-      String ticketCache = System.getenv("KRB5CCNAME");
-      if (ticketCache != null) {
-        if (IBM_JAVA) {
-          // The first value searched when "useDefaultCcache" is used.
-          System.setProperty("KRB5CCNAME", ticketCache);
-        } else {
-          USER_KERBEROS_OPTIONS.put("ticketCache", ticketCache);
-        }
-      }
-      USER_KERBEROS_OPTIONS.put("renewTGT", "true");
-      USER_KERBEROS_OPTIONS.putAll(BASIC_JAAS_OPTIONS);
-    }
-    private static final AppConfigurationEntry USER_KERBEROS_LOGIN =
-      new AppConfigurationEntry(KerberosUtil.getKrb5LoginModuleName(),
-                                LoginModuleControlFlag.OPTIONAL,
-                                USER_KERBEROS_OPTIONS);
-    private static final Map<String,String> KEYTAB_KERBEROS_OPTIONS = 
-      new HashMap<String,String>();
-    static {
-      if (IBM_JAVA) {
-        KEYTAB_KERBEROS_OPTIONS.put("credsType", "both");
-      } else {
-        KEYTAB_KERBEROS_OPTIONS.put("doNotPrompt", "true");
-        KEYTAB_KERBEROS_OPTIONS.put("useKeyTab", "true");
-        KEYTAB_KERBEROS_OPTIONS.put("storeKey", "true");
-      }
-      KEYTAB_KERBEROS_OPTIONS.put("refreshKrb5Config", "true");
-      KEYTAB_KERBEROS_OPTIONS.putAll(BASIC_JAAS_OPTIONS);      
-    }
-    private static final AppConfigurationEntry KEYTAB_KERBEROS_LOGIN =
-      new AppConfigurationEntry(KerberosUtil.getKrb5LoginModuleName(),
-                                LoginModuleControlFlag.REQUIRED,
-                                KEYTAB_KERBEROS_OPTIONS);
-    
-    private static final AppConfigurationEntry[] SIMPLE_CONF = 
-      new AppConfigurationEntry[]{OS_SPECIFIC_LOGIN, HADOOP_LOGIN};
-    
-    private static final AppConfigurationEntry[] USER_KERBEROS_CONF =
-      new AppConfigurationEntry[]{OS_SPECIFIC_LOGIN, USER_KERBEROS_LOGIN,
-                                  HADOOP_LOGIN};
-
-    private static final AppConfigurationEntry[] KEYTAB_KERBEROS_CONF =
-      new AppConfigurationEntry[]{KEYTAB_KERBEROS_LOGIN, HADOOP_LOGIN};
-
-    @Override
-    public AppConfigurationEntry[] getAppConfigurationEntry(String appName) {
-      if (SIMPLE_CONFIG_NAME.equals(appName)) {
-        return SIMPLE_CONF;
-      } else if (USER_KERBEROS_CONFIG_NAME.equals(appName)) {
-        return USER_KERBEROS_CONF;
-      } else if (KEYTAB_KERBEROS_CONFIG_NAME.equals(appName)) {
-        if (IBM_JAVA) {
-          KEYTAB_KERBEROS_OPTIONS.put("useKeytab",
-              prependFileAuthority(keytabFile));
-        } else {
-          KEYTAB_KERBEROS_OPTIONS.put("keyTab", keytabFile);
-        }
-        KEYTAB_KERBEROS_OPTIONS.put("principal", keytabPrincipal);
-        return KEYTAB_KERBEROS_CONF;
-      }
-      return null;
-    }
-  }
-
-  private static String prependFileAuthority(String keytabPath) {
-    return keytabPath.startsWith("file://") ? keytabPath
-        : "file://" + keytabPath;
-  }
-
-  /**
-   * Represents a javax.security configuration that is created at runtime.
-   */
-  private static class DynamicConfiguration
-      extends javax.security.auth.login.Configuration {
-    private AppConfigurationEntry[] ace;
-    
-    DynamicConfiguration(AppConfigurationEntry[] ace) {
-      this.ace = ace;
-    }
-    
-    @Override
-    public AppConfigurationEntry[] getAppConfigurationEntry(String appName) {
-      return ace;
-    }
-  }
-
-  private static LoginContext
+  private static HadoopLoginContext
   newLoginContext(String appName, Subject subject,
-    javax.security.auth.login.Configuration loginConf)
+                  HadoopConfiguration loginConf)
       throws LoginException {
     // Temporarily switch the thread's ContextClassLoader to match this
     // class's classloader, so that we can properly load HadoopLoginModule
@@ -676,16 +507,20 @@ public class UserGroupInformation {
     ClassLoader oldCCL = t.getContextClassLoader();
     t.setContextClassLoader(HadoopLoginModule.class.getClassLoader());
     try {
-      return new LoginContext(appName, subject, null, loginConf);
+      return new HadoopLoginContext(appName, subject, loginConf);
     } finally {
       t.setContextClassLoader(oldCCL);
     }
   }
 
-  private LoginContext getLogin() {
-    return user.getLogin();
+  // return the LoginContext only if it's managed by the ugi.  externally
+  // managed login contexts will be ignored.
+  private HadoopLoginContext getLogin() {
+    LoginContext login = user.getLogin();
+    return (login instanceof HadoopLoginContext)
+      ? (HadoopLoginContext)login : null;
   }
-  
+
   private void setLogin(LoginContext login) {
     user.setLogin(login);
   }
@@ -698,31 +533,22 @@ public class UserGroupInformation {
    * @param subject the user's subject
    */
   UserGroupInformation(Subject subject) {
-    this(subject, treatSubjectExternal);
-  }
-
-  /**
-   * Create a UGI from the given subject.
-   * @param subject the subject
-   * @param isLoginExternal if the subject's keytab is managed by other UGI.
-   *                       Setting this to true will prevent UGI from attempting
-   *                       to login the keytab, or to renew it.
-   */
-  private UserGroupInformation(Subject subject, final boolean isLoginExternal) {
     this.subject = subject;
+    // do not access ANY private credentials since they are mutable
+    // during a relogin.  no principal locking necessary since
+    // relogin/logout does not remove User principal.
     this.user = subject.getPrincipals(User.class).iterator().next();
-
-    this.isKeytab = KerberosUtil.hasKerberosKeyTab(subject);
-    this.isKrbTkt = KerberosUtil.hasKerberosTicket(subject);
-    this.isLoginExternal = isLoginExternal;
+    if (user == null || user.getName() == null) {
+      throw new IllegalStateException("Subject does not contain a valid User");
+    }
   }
-  
+
   /**
    * checks if logged in using kerberos
    * @return true if the subject logged via keytab or has a Kerberos TGT
    */
   public boolean hasKerberosCredentials() {
-    return isKeytab || isKrbTkt;
+    return user.getAuthenticationMethod() == AuthenticationMethod.KERBEROS;
   }
 
   /**
@@ -732,8 +558,7 @@ public class UserGroupInformation {
    */
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
-  public synchronized
-  static UserGroupInformation getCurrentUser() throws IOException {
+  public static UserGroupInformation getCurrentUser() throws IOException {
     AccessControlContext context = AccessController.getContext();
     Subject subject = Subject.getSubject(context);
     if (subject == null || subject.getPrincipals(User.class).isEmpty()) {
@@ -779,53 +604,10 @@ public class UserGroupInformation {
     if (!isAuthenticationMethodEnabled(AuthenticationMethod.KERBEROS)) {
       return getBestUGI(null, user);
     }
-    try {
-      Map<String,String> krbOptions = new HashMap<String,String>();
-      if (IBM_JAVA) {
-        krbOptions.put("useDefaultCcache", "true");
-        // The first value searched when "useDefaultCcache" is used.
-        System.setProperty("KRB5CCNAME", ticketCache);
-      } else {
-        krbOptions.put("doNotPrompt", "true");
-        krbOptions.put("useTicketCache", "true");
-        krbOptions.put("useKeyTab", "false");
-        krbOptions.put("ticketCache", ticketCache);
-      }
-      krbOptions.put("renewTGT", "false");
-      krbOptions.putAll(HadoopConfiguration.BASIC_JAAS_OPTIONS);
-      AppConfigurationEntry ace = new AppConfigurationEntry(
-          KerberosUtil.getKrb5LoginModuleName(),
-          LoginModuleControlFlag.REQUIRED,
-          krbOptions);
-      DynamicConfiguration dynConf =
-          new DynamicConfiguration(new AppConfigurationEntry[]{ ace });
-      LoginContext login = newLoginContext(
-          HadoopConfiguration.USER_KERBEROS_CONFIG_NAME, null, dynConf);
-      login.login();
-
-      Subject loginSubject = login.getSubject();
-      Set<Principal> loginPrincipals = loginSubject.getPrincipals();
-      if (loginPrincipals.isEmpty()) {
-        throw new RuntimeException("No login principals found!");
-      }
-      if (loginPrincipals.size() != 1) {
-        LOG.warn("found more than one principal in the ticket cache file " +
-          ticketCache);
-      }
-      User ugiUser = new User(loginPrincipals.iterator().next().getName(),
-          AuthenticationMethod.KERBEROS, login);
-      loginSubject.getPrincipals().add(ugiUser);
-      UserGroupInformation ugi = new UserGroupInformation(loginSubject, false);
-      ugi.setLogin(login);
-      ugi.setAuthenticationMethod(AuthenticationMethod.KERBEROS);
-      return ugi;
-    } catch (LoginException le) {
-      KerberosAuthException kae =
-          new KerberosAuthException(FAILURE_TO_LOGIN, le);
-      kae.setUser(user);
-      kae.setTicketCacheFile(ticketCache);
-      throw kae;
-    }
+    LoginParams params = new LoginParams();
+    params.put(LoginParam.PRINCIPAL, user);
+    params.put(LoginParam.CCACHE, ticketCache);
+    return doSubjectLogin(null, params);
   }
 
   /**
@@ -848,29 +630,40 @@ public class UserGroupInformation {
       throw new KerberosAuthException(SUBJECT_MUST_CONTAIN_PRINCIPAL);
     }
 
-    KerberosPrincipal principal =
-        subject.getPrincipals(KerberosPrincipal.class).iterator().next();
-
-    User ugiUser = new User(principal.getName(),
-        AuthenticationMethod.KERBEROS, null);
-    subject.getPrincipals().add(ugiUser);
-    UserGroupInformation ugi = new UserGroupInformation(subject);
-    ugi.setLogin(null);
-    ugi.setAuthenticationMethod(AuthenticationMethod.KERBEROS);
-    return ugi;
+    // null params indicate external subject login.  no login context will
+    // be attached.
+    return doSubjectLogin(subject, null);
   }
 
   /**
-   * Get the currently logged in user.
+   * Get the currently logged in user.  If no explicit login has occurred,
+   * the user will automatically be logged in with either kerberos credentials
+   * if available, or as the local OS user, based on security settings.
    * @return the logged in user
    * @throws IOException if login fails
    */
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
-  public synchronized 
-  static UserGroupInformation getLoginUser() throws IOException {
+  public static UserGroupInformation getLoginUser() throws IOException {
+    UserGroupInformation loginUser = loginUserRef.get();
+    // a potential race condition exists only for the initial creation of
+    // the login user.  there's no need to penalize all subsequent calls
+    // with sychronization overhead so optimistically create a login user
+    // and discard if we lose the race.
     if (loginUser == null) {
-      loginUserFromSubject(null);
+      UserGroupInformation newLoginUser = createLoginUser(null);
+      do {
+        // it's extremely unlikely that the login user will be non-null
+        // (lost CAS race), but be nulled before the subsequent get, but loop
+        // for correctness.
+        if (loginUserRef.compareAndSet(null, newLoginUser)) {
+          loginUser = newLoginUser;
+          // only spawn renewal if this login user is the winner.
+          loginUser.spawnAutoRenewalThreadForUserCreds(false);
+        } else {
+          loginUser = loginUserRef.get();
+        }
+      } while (loginUser == null);
     }
     return loginUser;
   }
@@ -902,30 +695,15 @@ public class UserGroupInformation {
    */
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
-  public synchronized 
-  static void loginUserFromSubject(Subject subject) throws IOException {
-    ensureInitialized();
-    boolean externalSubject = false;
-    try {
-      if (subject == null) {
-        subject = new Subject();
-      } else {
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Treat subject external: " + treatSubjectExternal
-              + ". When true, assuming keytab is managed extenally since "
-              + " logged in from subject");
-        }
-        externalSubject = treatSubjectExternal;
-      }
-      LoginContext login =
-          newLoginContext(authenticationMethod.getLoginAppName(), 
-                          subject, new HadoopConfiguration());
-      login.login();
+  public static void loginUserFromSubject(Subject subject) throws IOException {
+    setLoginUser(createLoginUser(subject));
+  }
 
-      UserGroupInformation realUser =
-          new UserGroupInformation(subject, externalSubject);
-      realUser.setLogin(login);
-      realUser.setAuthenticationMethod(authenticationMethod);
+  private static
+  UserGroupInformation createLoginUser(Subject subject) throws IOException {
+    UserGroupInformation realUser = doSubjectLogin(subject, null);
+    UserGroupInformation loginUser = null;
+    try {
       // If the HADOOP_PROXY_USER environment variable or property
       // is specified, create a proxy user as the logged in user.
       String proxyUser = System.getenv(HADOOP_PROXY_USER);
@@ -974,38 +752,64 @@ public class UserGroupInformation {
         LOG.debug("Loaded {} tokens", cred.numberOfTokens());
         loginUser.addCredentials(cred);
       }
-      loginUser.spawnAutoRenewalThreadForUserCreds();
-    } catch (LoginException le) {
-      LOG.debug("failure to login", le);
-      throw new KerberosAuthException(FAILURE_TO_LOGIN, le);
+    } catch (IOException ioe) {
+      LOG.debug("failure to load login credentials", ioe);
+      throw ioe;
     }
     if (LOG.isDebugEnabled()) {
       LOG.debug("UGI loginUser:"+loginUser);
-    } 
+    }
+    return loginUser;
   }
 
   @InterfaceAudience.Private
   @InterfaceStability.Unstable
   @VisibleForTesting
-  public synchronized static void setLoginUser(UserGroupInformation ugi) {
+  public static void setLoginUser(UserGroupInformation ugi) {
     // if this is to become stable, should probably logout the currently
     // logged in ugi if it's different
-    loginUser = ugi;
+    loginUserRef.set(ugi);
   }
   
+  private String getKeytab() {
+    HadoopLoginContext login = getLogin();
+    return (login != null)
+      ? login.getConfiguration().getParameters().get(LoginParam.KEYTAB)
+      : null;
+  }
+
+  /**
+   * Is the ugi managed by the UGI or an external subject?
+   * @return true if managed by UGI.
+   */
+  private boolean isHadoopLogin() {
+    // checks if the private hadoop login context is managing the ugi.
+    return getLogin() != null;
+  }
+
   /**
-   * Is this user logged in from a keytab file?
+   * Is this user logged in from a keytab file managed by the UGI?
    * @return true if the credentials are from a keytab file.
    */
   public boolean isFromKeytab() {
-    return isKeytab;
+    // can't simply check if keytab is present since a relogin failure will
+    // have removed the keytab from priv creds.  instead, check login params.
+    return hasKerberosCredentials() && isHadoopLogin() && getKeytab() != null;
   }
   
   /**
+   *  Is this user logged in from a ticket (but no keytab) managed by the UGI?
+   * @return true if the credentials are from a ticket cache.
+   */
+  private boolean isFromTicket() {
+    return hasKerberosCredentials() && isHadoopLogin() && getKeytab() == null;
+  }
+
+  /**
    * Get the Kerberos TGT
    * @return the user's TGT or null if none was found
    */
-  private synchronized KerberosTicket getTGT() {
+  private KerberosTicket getTGT() {
     Set<KerberosTicket> tickets = subject
         .getPrivateCredentials(KerberosTicket.class);
     for (KerberosTicket ticket : tickets) {
@@ -1022,23 +826,20 @@ public class UserGroupInformation {
     return start + (long) ((end - start) * TICKET_RENEW_WINDOW);
   }
 
-  /**
-   * Should relogin if security is enabled using Kerberos, and
-   * the Subject is not owned by another UGI.
-   * @return true if this UGI should relogin
-   */
   private boolean shouldRelogin() {
-    return isSecurityEnabled()
-        && user.getAuthenticationMethod() == AuthenticationMethod.KERBEROS
-        && !isLoginExternal;
+    return hasKerberosCredentials() && isHadoopLogin();
   }
 
-  /**Spawn a thread to do periodic renewals of kerberos credentials*/
-  private void spawnAutoRenewalThreadForUserCreds() {
-    if (getEnableRenewThreadCreationForTest()) {
-      LOG.warn("Spawning thread to auto renew user credential since " +
-          " enableRenewThreadCreationForTest was set to true.");
-    } else if (!shouldRelogin() || isKeytab) {
+  @InterfaceAudience.Private
+  @InterfaceStability.Unstable
+  @VisibleForTesting
+  /**
+   * Spawn a thread to do periodic renewals of kerberos credentials from
+   * a ticket cache.  NEVER directly call this method.
+   * @param force - used by tests to forcibly spawn thread
+   */
+  void spawnAutoRenewalThreadForUserCreds(boolean force) {
+    if (!force && (!shouldRelogin() || isFromKeytab())) {
       return;
     }
 
@@ -1149,38 +950,16 @@ public class UserGroupInformation {
    */
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
-  public synchronized
+  public
   static void loginUserFromKeytab(String user,
                                   String path
                                   ) throws IOException {
     if (!isSecurityEnabled())
       return;
 
-    keytabFile = path;
-    keytabPrincipal = user;
-    Subject subject = new Subject();
-    LoginContext login; 
-    long start = 0;
-    try {
-      login = newLoginContext(HadoopConfiguration.KEYTAB_KERBEROS_CONFIG_NAME,
-            subject, new HadoopConfiguration());
-      start = Time.now();
-      login.login();
-      metrics.loginSuccess.add(Time.now() - start);
-      loginUser = new UserGroupInformation(subject, false);
-      loginUser.setLogin(login);
-      loginUser.setAuthenticationMethod(AuthenticationMethod.KERBEROS);
-    } catch (LoginException le) {
-      if (start > 0) {
-        metrics.loginFailure.add(Time.now() - start);
-      }
-      KerberosAuthException kae = new KerberosAuthException(LOGIN_FAILURE, le);
-      kae.setUser(user);
-      kae.setKeytabFile(path);
-      throw kae;
-    }
-    LOG.info("Login successful for user " + keytabPrincipal
-        + " using keytab file " + keytabFile);
+    setLoginUser(loginUserFromKeytabAndReturnUGI(user, path));
+    LOG.info("Login successful for user " + user
+        + " using keytab file " + path);
   }
 
   /**
@@ -1195,11 +974,11 @@ public class UserGroupInformation {
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
   public void logoutUserFromKeytab() throws IOException {
-    if (!isSecurityEnabled() ||
-        user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS) {
+    if (!hasKerberosCredentials()) {
       return;
     }
-    LoginContext login = getLogin();
+    HadoopLoginContext login = getLogin();
+    String keytabFile = getKeytab();
     if (login == null || keytabFile == null) {
       throw new KerberosAuthException(MUST_FIRST_LOGIN_FROM_KEYTAB);
     }
@@ -1208,9 +987,8 @@ public class UserGroupInformation {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Initiating logout for " + getUserName());
       }
-      synchronized (UserGroupInformation.class) {
-        login.logout();
-      }
+      // hadoop login context internally locks credentials.
+      login.logout();
     } catch (LoginException le) {
       KerberosAuthException kae = new KerberosAuthException(LOGOUT_FAILURE, le);
       kae.setUser(user.toString());
@@ -1218,7 +996,7 @@ public class UserGroupInformation {
       throw kae;
     }
 
-    LOG.info("Logout successful for user " + keytabPrincipal
+    LOG.info("Logout successful for user " + getUserName()
         + " using keytab file " + keytabFile);
   }
   
@@ -1228,18 +1006,8 @@ public class UserGroupInformation {
    * @throws IOException
    * @throws KerberosAuthException if it's a kerberos login exception.
    */
-  public synchronized void checkTGTAndReloginFromKeytab() throws IOException {
-    if (!isSecurityEnabled()
-        || user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS
-        || !isKeytab) {
-      return;
-    }
-    KerberosTicket tgt = getTGT();
-    if (tgt != null && !shouldRenewImmediatelyForTests &&
-        Time.now() < getRefreshTime(tgt)) {
-      return;
-    }
-    reloginFromKeytab();
+  public void checkTGTAndReloginFromKeytab() throws IOException {
+    reloginFromKeytab(true);
   }
 
   // if the first kerberos ticket is not TGT, then remove and destroy it since
@@ -1288,63 +1056,26 @@ public class UserGroupInformation {
    */
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
-  public synchronized void reloginFromKeytab() throws IOException {
-    if (!shouldRelogin() || !isKeytab) {
-      return;
-    }
-
-    long now = Time.now();
-    if (!shouldRenewImmediatelyForTests && !hasSufficientTimeElapsed(now)) {
-      return;
-    }
+  public void reloginFromKeytab() throws IOException {
+    reloginFromKeytab(false);
+  }
 
-    KerberosTicket tgt = getTGT();
-    //Return if TGT is valid and is not going to expire soon.
-    if (tgt != null && !shouldRenewImmediatelyForTests &&
-        now < getRefreshTime(tgt)) {
+  private void reloginFromKeytab(boolean checkTGT) throws IOException {
+    if (!shouldRelogin() || !isFromKeytab()) {
       return;
     }
-
-    LoginContext login = getLogin();
-    if (login == null || keytabFile == null) {
+    HadoopLoginContext login = getLogin();
+    if (login == null) {
       throw new KerberosAuthException(MUST_FIRST_LOGIN_FROM_KEYTAB);
     }
-
-    long start = 0;
-    // register most recent relogin attempt
-    user.setLastLogin(now);
-    try {
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Initiating logout for " + getUserName());
-      }
-      synchronized (UserGroupInformation.class) {
-        // clear up the kerberos state. But the tokens are not cleared! As per
-        // the Java kerberos login module code, only the kerberos credentials
-        // are cleared
-        login.logout();
-        // login and also update the subject field of this instance to
-        // have the new credentials (pass it to the LoginContext constructor)
-        login = newLoginContext(
-            HadoopConfiguration.KEYTAB_KERBEROS_CONFIG_NAME, getSubject(),
-            new HadoopConfiguration());
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Initiating re-login for " + keytabPrincipal);
-        }
-        start = Time.now();
-        login.login();
-        fixKerberosTicketOrder();
-        metrics.loginSuccess.add(Time.now() - start);
-        setLogin(login);
-      }
-    } catch (LoginException le) {
-      if (start > 0) {
-        metrics.loginFailure.add(Time.now() - start);
+    if (checkTGT) {
+      KerberosTicket tgt = getTGT();
+      if (tgt != null && !shouldRenewImmediatelyForTests &&
+        Time.now() < getRefreshTime(tgt)) {
+        return;
       }
-      KerberosAuthException kae = new KerberosAuthException(LOGIN_FAILURE, le);
-      kae.setPrincipal(keytabPrincipal);
-      kae.setKeytabFile(keytabFile);
-      throw kae;
     }
+    relogin(login);
   }
 
   /**
@@ -1357,14 +1088,31 @@ public class UserGroupInformation {
    */
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
-  public synchronized void reloginFromTicketCache() throws IOException {
-    if (!shouldRelogin() || !isKrbTkt) {
+  public void reloginFromTicketCache() throws IOException {
+    if (!shouldRelogin() || !isFromTicket()) {
       return;
     }
-    LoginContext login = getLogin();
+    HadoopLoginContext login = getLogin();
     if (login == null) {
       throw new KerberosAuthException(MUST_FIRST_LOGIN);
     }
+    relogin(login);
+  }
+
+  private void relogin(HadoopLoginContext login) throws IOException {
+    // ensure the relogin is atomic to avoid leaving credentials in an
+    // inconsistent state.  prevents other ugi instances, SASL, and SPNEGO
+    // from accessing or altering credentials during the relogin.
+    synchronized(login.getSubjectLock()) {
+      // another racing thread may have beat us to the relogin.
+      if (login == getLogin()) {
+        unprotectedRelogin(login);
+      }
+    }
+  }
+
+  private void unprotectedRelogin(HadoopLoginContext login) throws IOException {
+    assert Thread.holdsLock(login.getSubjectLock());
     long now = Time.now();
     if (!hasSufficientTimeElapsed(now)) {
       return;
@@ -1381,13 +1129,14 @@ public class UserGroupInformation {
       login.logout();
       //login and also update the subject field of this instance to 
       //have the new credentials (pass it to the LoginContext constructor)
-      login = 
-        newLoginContext(HadoopConfiguration.USER_KERBEROS_CONFIG_NAME, 
-            getSubject(), new HadoopConfiguration());
+      login = newLoginContext(
+        login.getAppName(), login.getSubject(), login.getConfiguration());
       if (LOG.isDebugEnabled()) {
         LOG.debug("Initiating re-login for " + getUserName());
       }
       login.login();
+      // this should be unnecessary.  originally added due to improper locking
+      // of the subject during relogin.
       fixKerberosTicketOrder();
       setLogin(login);
     } catch (LoginException le) {
@@ -1405,52 +1154,22 @@ public class UserGroupInformation {
    * @param path the path to the keytab file
    * @throws IOException if the keytab file can't be read
    */
-  public synchronized
+  public
   static UserGroupInformation loginUserFromKeytabAndReturnUGI(String user,
                                   String path
                                   ) throws IOException {
     if (!isSecurityEnabled())
       return UserGroupInformation.getCurrentUser();
-    String oldKeytabFile = null;
-    String oldKeytabPrincipal = null;
 
-    long start = 0;
-    try {
-      oldKeytabFile = keytabFile;
-      oldKeytabPrincipal = keytabPrincipal;
-      keytabFile = path;
-      keytabPrincipal = user;
-      Subject subject = new Subject();
-      
-      LoginContext login = newLoginContext(
-          HadoopConfiguration.KEYTAB_KERBEROS_CONFIG_NAME, subject,
-          new HadoopConfiguration());
-       
-      start = Time.now();
-      login.login();
-      metrics.loginSuccess.add(Time.now() - start);
-      UserGroupInformation newLoginUser =
-          new UserGroupInformation(subject, false);
-      newLoginUser.setLogin(login);
-      newLoginUser.setAuthenticationMethod(AuthenticationMethod.KERBEROS);
-      
-      return newLoginUser;
-    } catch (LoginException le) {
-      if (start > 0) {
-        metrics.loginFailure.add(Time.now() - start);
-      }
-      KerberosAuthException kae = new KerberosAuthException(LOGIN_FAILURE, le);
-      kae.setUser(user);
-      kae.setKeytabFile(path);
-      throw kae;
-    } finally {
-      if(oldKeytabFile != null) keytabFile = oldKeytabFile;
-      if(oldKeytabPrincipal != null) keytabPrincipal = oldKeytabPrincipal;
-    }
+    LoginParams params = new LoginParams();
+    params.put(LoginParam.PRINCIPAL, user);
+    params.put(LoginParam.KEYTAB, path);
+    return doSubjectLogin(null, params);
   }
 
   private boolean hasSufficientTimeElapsed(long now) {
-    if (now - user.getLastLogin() < kerberosMinSecondsBeforeRelogin ) {
+    if (!shouldRenewImmediatelyForTests &&
+        now - user.getLastLogin() < kerberosMinSecondsBeforeRelogin ) {
       LOG.warn("Not attempting to re-login since the last re-login was " +
           "attempted less than " + (kerberosMinSecondsBeforeRelogin/1000) +
           " seconds before. Last Login=" + user.getLastLogin());
@@ -1465,8 +1184,8 @@ public class UserGroupInformation {
    */
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
-  public synchronized static boolean isLoginKeytabBased() throws IOException {
-    return getLoginUser().isKeytab;
+  public static boolean isLoginKeytabBased() throws IOException {
+    return getLoginUser().isFromKeytab();
   }
 
   /**
@@ -1474,7 +1193,7 @@ public class UserGroupInformation {
    * @return true or false
    */
   public static boolean isLoginTicketBased()  throws IOException {
-    return getLoginUser().isKrbTkt;
+    return getLoginUser().isFromTicket();
   }
 
   /**
@@ -1503,7 +1222,7 @@ public class UserGroupInformation {
     }
     Subject subject = new Subject();
     subject.getPrincipals().add(new User(user));
-    UserGroupInformation result = new UserGroupInformation(subject, false);
+    UserGroupInformation result = new UserGroupInformation(subject);
     result.setAuthenticationMethod(authMethod);
     return result;
   }
@@ -1519,7 +1238,7 @@ public class UserGroupInformation {
     SIMPLE(AuthMethod.SIMPLE,
         HadoopConfiguration.SIMPLE_CONFIG_NAME),
     KERBEROS(AuthMethod.KERBEROS,
-        HadoopConfiguration.USER_KERBEROS_CONFIG_NAME),
+        HadoopConfiguration.KERBEROS_CONFIG_NAME),
     TOKEN(AuthMethod.TOKEN),
     CERTIFICATE(null),
     KERBEROS_SSL(null),
@@ -1578,11 +1297,9 @@ public class UserGroupInformation {
     }
     Subject subject = new Subject();
     Set<Principal> principals = subject.getPrincipals();
-    principals.add(new User(user));
+    principals.add(new User(user, AuthenticationMethod.PROXY, null));
     principals.add(new RealUser(realUser));
-    UserGroupInformation result =new UserGroupInformation(subject, false);
-    result.setAuthenticationMethod(AuthenticationMethod.PROXY);
-    return result;
+    return new UserGroupInformation(subject);
   }
 
   /**
@@ -2051,6 +1768,256 @@ public class UserGroupInformation {
   }
 
   /**
+   * Login a subject with the given parameters.  If the subject is null,
+   * the login context used to create the subject will be attached.
+   * @param subject to login, null for new subject.
+   * @param params for login, null for externally managed ugi.
+   * @return UserGroupInformation for subject
+   * @throws IOException
+   */
+  private static UserGroupInformation doSubjectLogin(
+      Subject subject, LoginParams params) throws IOException {
+    ensureInitialized();
+    // initial default login.
+    if (subject == null && params == null) {
+      params = LoginParams.getDefaults();
+    }
+    HadoopConfiguration loginConf = new HadoopConfiguration(params);
+    try {
+      HadoopLoginContext login = newLoginContext(
+        authenticationMethod.getLoginAppName(), subject, loginConf);
+      login.login();
+      UserGroupInformation ugi = new UserGroupInformation(login.getSubject());
+      // attach login context for relogin unless this was a pre-existing
+      // subject.
+      if (subject == null) {
+        params.put(LoginParam.PRINCIPAL, ugi.getUserName());
+        ugi.setLogin(login);
+      }
+      return ugi;
+    } catch (LoginException le) {
+      KerberosAuthException kae =
+        new KerberosAuthException(FAILURE_TO_LOGIN, le);
+      if (params != null) {
+        kae.setPrincipal(params.get(LoginParam.PRINCIPAL));
+        kae.setKeytabFile(params.get(LoginParam.KEYTAB));
+        kae.setTicketCacheFile(params.get(LoginParam.CCACHE));
+      }
+      throw kae;
+    }
+  }
+
+  // parameters associated with kerberos logins.  may be extended to support
+  // additional authentication methods.
+  enum LoginParam {
+    PRINCIPAL,
+    KEYTAB,
+    CCACHE,
+  }
+
+  // explicitly private to prevent external tampering.
+  private static class LoginParams extends EnumMap<LoginParam,String>
+      implements Parameters {
+    LoginParams() {
+      super(LoginParam.class);
+    }
+
+    // do not add null values, nor allow existing values to be overriden.
+    @Override
+    public String put(LoginParam param, String val) {
+      boolean add = val != null && !containsKey(param);
+      return add ? super.put(param, val) : null;
+    }
+
+    static LoginParams getDefaults() {
+      LoginParams params = new LoginParams();
+      params.put(LoginParam.PRINCIPAL, System.getenv("KRB5PRINCIPAL"));
+      params.put(LoginParam.KEYTAB, System.getenv("KRB5KEYTAB"));
+      params.put(LoginParam.CCACHE, System.getenv("KRB5CCNAME"));
+      return params;
+    }
+  }
+
+  // wrapper to allow access to fields necessary to recreate the same login
+  // context for relogin.  explicitly private to prevent external tampering.
+  private static class HadoopLoginContext extends LoginContext {
+    private final String appName;
+    private final HadoopConfiguration conf;
+
+    HadoopLoginContext(String appName, Subject subject,
+                       HadoopConfiguration conf) throws LoginException {
+      super(appName, subject, null, conf);
+      this.appName = appName;
+      this.conf = conf;
+    }
+
+    String getAppName() {
+      return appName;
+    }
+
+    HadoopConfiguration getConfiguration() {
+      return conf;
+    }
+
+    // the locking model for logins cannot rely on ugi instance synchronization
+    // since a subject will be referenced by multiple ugi instances.
+    Object getSubjectLock() {
+      Subject subject = getSubject();
+      // if subject is null, the login context will create the subject
+      // so just lock on this context.
+      return (subject == null) ? this : subject.getPrivateCredentials();
+    }
+
+    @Override
+    public void login() throws LoginException {
+      synchronized(getSubjectLock()) {
+        MutableRate metric = metrics.loginFailure;
+        long start = Time.monotonicNow();
+        try {
+          super.login();
+          metric = metrics.loginSuccess;
+        } finally {
+          metric.add(Time.monotonicNow() - start);
+        }
+      }
+    }
+
+    @Override
+    public void logout() throws LoginException {
+      synchronized(getSubjectLock()) {
+        super.logout();
+      }
+    }
+  }
+
+  /**
+   * A JAAS configuration that defines the login modules that we want
+   * to use for login.
+   */
+  @InterfaceAudience.Private
+  @InterfaceStability.Unstable
+  private static class HadoopConfiguration
+  extends javax.security.auth.login.Configuration {
+    static final String KRB5_LOGIN_MODULE =
+        KerberosUtil.getKrb5LoginModuleName();
+    static final String SIMPLE_CONFIG_NAME = "hadoop-simple";
+    static final String KERBEROS_CONFIG_NAME = "hadoop-kerberos";
+
+    private static final Map<String, String> BASIC_JAAS_OPTIONS =
+        new HashMap<String,String>();
+    static {
+      if ("true".equalsIgnoreCase(System.getenv("HADOOP_JAAS_DEBUG"))) {
+        BASIC_JAAS_OPTIONS.put("debug", "true");
+      }
+    }
+
+    static final AppConfigurationEntry OS_SPECIFIC_LOGIN =
+        new AppConfigurationEntry(
+            OS_LOGIN_MODULE_NAME,
+            LoginModuleControlFlag.REQUIRED,
+            BASIC_JAAS_OPTIONS);
+
+    static final AppConfigurationEntry HADOOP_LOGIN =
+        new AppConfigurationEntry(
+            HadoopLoginModule.class.getName(),
+            LoginModuleControlFlag.REQUIRED,
+            BASIC_JAAS_OPTIONS);
+
+    private final LoginParams params;
+
+    HadoopConfiguration(LoginParams params) {
+      this.params = params;
+    }
+
+    @Override
+    public LoginParams getParameters() {
+      return params;
+    }
+
+    @Override
+    public AppConfigurationEntry[] getAppConfigurationEntry(String appName) {
+      ArrayList<AppConfigurationEntry> entries = new ArrayList<>();
+      // login of external subject passes no params.  technically only
+      // existing credentials should be used but other components expect
+      // the login to succeed with local user fallback if no principal.
+      if (params == null || appName.equals(SIMPLE_CONFIG_NAME)) {
+        entries.add(OS_SPECIFIC_LOGIN);
+      } else if (appName.equals(KERBEROS_CONFIG_NAME)) {
+        // existing semantics are the initial default login allows local user
+        // fallback. this is not allowed when a principal explicitly
+        // specified or during a relogin.
+        if (!params.containsKey(LoginParam.PRINCIPAL)) {
+          entries.add(OS_SPECIFIC_LOGIN);
+        }
+        entries.add(getKerberosEntry());
+      }
+      entries.add(HADOOP_LOGIN);
+      return entries.toArray(new AppConfigurationEntry[0]);
+    }
+
+    private AppConfigurationEntry getKerberosEntry() {
+      final Map<String,String> options = new HashMap<>(BASIC_JAAS_OPTIONS);
+      LoginModuleControlFlag controlFlag = LoginModuleControlFlag.OPTIONAL;
+      // kerberos login is mandatory if principal is specified.  principal
+      // will not be set for initial default login, but will always be set
+      // for relogins.
+      final String principal = params.get(LoginParam.PRINCIPAL);
+      if (principal != null) {
+        options.put("principal", principal);
+        controlFlag = LoginModuleControlFlag.REQUIRED;
+      }
+
+      // use keytab if given else fallback to ticket cache.
+      if (IBM_JAVA) {
+        if (params.containsKey(LoginParam.KEYTAB)) {
+          final String keytab = params.get(LoginParam.KEYTAB);
+          if (keytab != null) {
+            options.put("useKeytab", prependFileAuthority(keytab));
+          } else {
+            options.put("useDefaultKeytab", "true");
+          }
+          options.put("credsType", "both");
+        } else {
+          String ticketCache = params.get(LoginParam.CCACHE);
+          if (ticketCache != null) {
+            options.put("useCcache", prependFileAuthority(ticketCache));
+          } else {
+            options.put("useDefaultCcache", "true");
+          }
+          options.put("renewTGT", "true");
+        }
+      } else {
+        if (params.containsKey(LoginParam.KEYTAB)) {
+          options.put("useKeyTab", "true");
+          final String keytab = params.get(LoginParam.KEYTAB);
+          if (keytab != null) {
+            options.put("keyTab", keytab);
+          }
+          options.put("storeKey", "true");
+        } else {
+          options.put("useTicketCache", "true");
+          String ticketCache = params.get(LoginParam.CCACHE);
+          if (ticketCache != null) {
+            options.put("ticketCache", ticketCache);
+          }
+          options.put("renewTGT", "true");
+        }
+        options.put("doNotPrompt", "true");
+      }
+      options.put("refreshKrb5Config", "true");
+
+      return new AppConfigurationEntry(
+          KRB5_LOGIN_MODULE, controlFlag, options);
+    }
+
+    private static String prependFileAuthority(String keytabPath) {
+      return keytabPath.startsWith("file://")
+          ? keytabPath
+          : "file://" + keytabPath;
+    }
+  }
+
+  /**
    * A test method to print out the current user's UGI.
    * @param args if there are two arguments, read the user from the keytab
    * and print it out.
@@ -2062,7 +2029,7 @@ public class UserGroupInformation {
     ugi.print();
     System.out.println("UGI: " + ugi);
     System.out.println("Auth method " + ugi.user.getAuthenticationMethod());
-    System.out.println("Keytab " + ugi.isKeytab);
+    System.out.println("Keytab " + ugi.isFromKeytab());
     System.out.println("============================================================");
     
     if (args.length == 2) {
@@ -2070,8 +2037,9 @@ public class UserGroupInformation {
       loginUserFromKeytab(args[0], args[1]);
       getCurrentUser().print();
       System.out.println("Keytab: " + ugi);
-      System.out.println("Auth method " + loginUser.user.getAuthenticationMethod());
-      System.out.println("Keytab " + loginUser.isKeytab);
+      UserGroupInformation loginUgi = getLoginUser();
+      System.out.println("Auth method " + loginUgi.getAuthenticationMethod());
+      System.out.println("Keytab " + loginUgi.isFromKeytab());
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/59cf7588/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
index b5163a1..ea26d06 100644
--- a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
+++ b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
@@ -2947,19 +2947,6 @@
     </description>
   </property>
   <property>
-    <name>hadoop.treat.subject.external</name>
-    <value>false</value>
-    <description>
-      When creating UGI with UserGroupInformation(Subject), treat the passed
-      subject external if set to true, and assume the owner of the subject
-      should do the credential renewal.
-
-      When true this property will introduce an incompatible change which
-      may require changes in client code. For more details, see the jiras:
-      HADOOP-13805,HADOOP-13558.
-    </description>
-  </property>
-  <property>
     <name>hadoop.system.tags</name>
     <value>YARN,HDFS,NAMENODE,DATANODE,REQUIRED,SECURITY,KERBEROS,PERFORMANCE,CLIENT
       ,SERVER,DEBUG,DEPRICATED,COMMON,OPTIONAL</value>

http://git-wip-us.apache.org/repos/asf/hadoop/blob/59cf7588/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
index 61fbf89..826e4b2 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGILoginFromKeytab.java
@@ -21,14 +21,41 @@ package org.apache.hadoop.security;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.CommonConfigurationKeys;
 import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.File;
+import java.io.IOException;
+import java.security.Principal;
+import java.security.PrivilegedExceptionAction;
+import java.util.Iterator;
+import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import javax.security.auth.Subject;
+import javax.security.auth.kerberos.KerberosPrincipal;
+import javax.security.auth.kerberos.KerberosTicket;
+import javax.security.auth.login.LoginContext;
 
 /**
  * Verify UGI login from keytab. Check that the UGI is
@@ -39,6 +66,7 @@ public class TestUGILoginFromKeytab {
 
   private MiniKdc kdc;
   private File workDir;
+  private ExecutorService executor;
 
   @Rule
   public final TemporaryFolder folder = new TemporaryFolder();
@@ -51,9 +79,12 @@ public class TestUGILoginFromKeytab {
     conf.set(CommonConfigurationKeys.HADOOP_SECURITY_AUTHENTICATION,
         "kerberos");
     UserGroupInformation.setConfiguration(conf);
+    UserGroupInformation.setShouldRenewImmediatelyForTests(true);
     workDir = folder.getRoot();
     kdc = new MiniKdc(MiniKdc.createConf(), workDir);
     kdc.start();
+    executor = Executors.newCachedThreadPool();
+
   }
 
   @After
@@ -61,6 +92,9 @@ public class TestUGILoginFromKeytab {
     if (kdc != null) {
       kdc.stop();
     }
+    if (executor != null) {
+      executor.shutdownNow();
+    }
   }
 
   /**
@@ -69,7 +103,6 @@ public class TestUGILoginFromKeytab {
    */
   @Test
   public void testUGILoginFromKeytab() throws Exception {
-    UserGroupInformation.setShouldRenewImmediatelyForTests(true);
     String principal = "foo";
     File keytab = new File(workDir, "foo.keytab");
     kdc.createPrincipal(keytab, principal);
@@ -80,12 +113,379 @@ public class TestUGILoginFromKeytab {
         ugi.isFromKeytab());
 
     // Verify relogin from keytab.
-    User user = ugi.getSubject().getPrincipals(User.class).iterator().next();
+    User user = getUser(ugi.getSubject());
     final long firstLogin = user.getLastLogin();
+    final LoginContext login1 = user.getLogin();
+    Assert.assertNotNull(login1);
+
     ugi.reloginFromKeytab();
     final long secondLogin = user.getLastLogin();
+    final LoginContext login2 = user.getLogin();
     Assert.assertTrue("User should have been able to relogin from keytab",
         secondLogin > firstLogin);
+    Assert.assertNotNull(login2);
+    Assert.assertNotSame(login1, login2);
+  }
+
+  @Test
+  public void testGetUGIFromKnownSubject() throws Exception {
+    KerberosPrincipal principal = new KerberosPrincipal("user");
+    File keytab = new File(workDir, "user.keytab");
+    kdc.createPrincipal(keytab, principal.getName());
+
+    UserGroupInformation ugi1 =
+      UserGroupInformation.loginUserFromKeytabAndReturnUGI(
+        principal.getName(), keytab.getPath());
+    Subject subject = ugi1.getSubject();
+    User user = getUser(subject);
+    Assert.assertNotNull(user);
+    LoginContext login = user.getLogin();
+    Assert.assertNotNull(login);
+
+    // User instance and/or login context should not change.
+    UserGroupInformation ugi2 = UserGroupInformation.getUGIFromSubject(subject);
+    Assert.assertSame(user, getUser(ugi2.getSubject()));
+    Assert.assertSame(login, user.getLogin());
+  }
+
+  @Test
+  public void testGetUGIFromExternalSubject() throws Exception {
+    KerberosPrincipal principal = new KerberosPrincipal("user");
+    File keytab = new File(workDir, "user.keytab");
+    kdc.createPrincipal(keytab, principal.getName());
+
+    UserGroupInformation ugi =
+      UserGroupInformation.loginUserFromKeytabAndReturnUGI(
+        principal.getName(), keytab.getPath());
+    Subject subject = ugi.getSubject();
+    removeUser(subject);
+
+    // first call to get the ugi should add the User instance w/o a login
+    // context.
+    UserGroupInformation ugi1 = UserGroupInformation.getUGIFromSubject(subject);
+    Assert.assertSame(subject, ugi1.getSubject());
+    User user = getUser(subject);
+    Assert.assertNotNull(user);
+    Assert.assertEquals(principal.getName(), user.getName());
+    Assert.assertNull(user.getLogin());
+
+    // subsequent call should not change the existing User instance.
+    UserGroupInformation ugi2 = UserGroupInformation.getUGIFromSubject(subject);
+    Assert.assertSame(subject, ugi2.getSubject());
+    Assert.assertSame(user, getUser(ugi2.getSubject()));
+    Assert.assertNull(user.getLogin());
+  }
+
+  @Test
+  public void testGetUGIFromExternalSubjectWithLogin() throws Exception {
+    KerberosPrincipal principal = new KerberosPrincipal("user");
+    File keytab = new File(workDir, "user.keytab");
+    kdc.createPrincipal(keytab, principal.getName());
+
+    // alter the User's login context to appear to be a foreign and
+    // unmanagable context.
+    UserGroupInformation ugi =
+      UserGroupInformation.loginUserFromKeytabAndReturnUGI(
+        principal.getName(), keytab.getPath());
+    Subject subject = ugi.getSubject();
+    User user = getUser(subject);
+    final LoginContext dummyLogin = Mockito.mock(LoginContext.class);
+    user.setLogin(dummyLogin);
+
+    // nothing should change.
+    UserGroupInformation ugi2 = UserGroupInformation.getUGIFromSubject(subject);
+    Assert.assertSame(subject, ugi2.getSubject());
+    Assert.assertSame(user, getUser(ugi2.getSubject()));
+    Assert.assertSame(dummyLogin, user.getLogin());
+  }
+
+
+  private static KerberosTicket getTicket(UserGroupInformation ugi) {
+    Set<KerberosTicket> tickets =
+        ugi.getSubject().getPrivateCredentials(KerberosTicket.class);
+    return tickets.isEmpty() ? null : tickets.iterator().next();
   }
 
+  // verify ugi has expected principal, a keytab, and has a ticket for
+  // the expected principal.
+  private static KerberosTicket checkTicketAndKeytab(UserGroupInformation ugi,
+      KerberosPrincipal principal, boolean expectIsKeytab) {
+    Assert.assertEquals("wrong principal",
+      principal.getName(), ugi.getUserName());
+    Assert.assertEquals("is not keytab",
+      expectIsKeytab, ugi.isFromKeytab());
+    KerberosTicket ticket = getTicket(ugi);
+    Assert.assertNotNull("no ticket", ticket);
+    Assert.assertEquals("wrong principal", principal, ticket.getClient());
+    return ticket;
+  }
+
+  @Test
+  public void testReloginForUGIFromSubject() throws Exception {
+    KerberosPrincipal principal1 = new KerberosPrincipal("user1");
+    File keytab1 = new File(workDir, "user1.keytab");
+    kdc.createPrincipal(keytab1, principal1.getName());
+
+    KerberosPrincipal principal2 = new KerberosPrincipal("user2");
+    File keytab2 = new File(workDir, "user2.keytab");
+    kdc.createPrincipal(keytab2, principal2.getName());
+
+    // Login a user and remove the User instance so it looks like an
+    // "external" subject.
+    final Subject extSubject =
+      UserGroupInformation.loginUserFromKeytabAndReturnUGI(
+        principal2.getName(), keytab2.getPath()).getSubject();
+    removeUser(extSubject);
+
+    // Login another user.
+    UserGroupInformation.loginUserFromKeytab(
+        principal1.getName(), keytab1.getPath());
+    final UserGroupInformation loginUser = UserGroupInformation.getLoginUser();
+
+    loginUser.doAs(new PrivilegedExceptionAction<Void>() {
+      @Override
+      public Void run() throws IOException {
+        KerberosTicket loginTicket =
+            checkTicketAndKeytab(loginUser, principal1, true);
+
+        // get the ugi for the previously logged in subject.
+        UserGroupInformation extSubjectUser =
+            UserGroupInformation.getUGIFromSubject(extSubject);
+        KerberosTicket ticket =
+          checkTicketAndKeytab(extSubjectUser, principal2, false);
+
+        // verify login user got a new ticket.
+        loginUser.reloginFromKeytab();
+        KerberosTicket newLoginTicket =
+            checkTicketAndKeytab(loginUser, principal1, true);
+        Assert.assertNotEquals(loginTicket.getAuthTime(),
+            newLoginTicket.getAuthTime());
+
+        // verify an "external" subject ticket does not change.
+        extSubjectUser.reloginFromKeytab();
+        Assert.assertSame(ticket,
+            checkTicketAndKeytab(extSubjectUser, principal2, false));
+
+        // verify subject ugi relogin did not affect the login user.
+        Assert.assertSame(newLoginTicket,
+            checkTicketAndKeytab(loginUser, principal1, true));
+
+        return null;
+      }
+    });
+  }
+
+  @Test
+  public void testReloginForLoginFromSubject() throws Exception {
+    KerberosPrincipal principal1 = new KerberosPrincipal("user1");
+    File keytab1 = new File(workDir, "user1.keytab");
+    kdc.createPrincipal(keytab1, principal1.getName());
+
+    KerberosPrincipal principal2 = new KerberosPrincipal("user2");
+    File keytab2 = new File(workDir, "user2.keytab");
+    kdc.createPrincipal(keytab2, principal2.getName());
+
+    // login principal1 with a keytab.
+    UserGroupInformation.loginUserFromKeytab(
+        principal1.getName(), keytab1.getPath());
+    final UserGroupInformation originalLoginUser =
+        UserGroupInformation.getLoginUser();
+    Assert.assertNotNull(getUser(originalLoginUser.getSubject()).getLogin());
+
+    originalLoginUser.doAs(new PrivilegedExceptionAction<Void>() {
+      @Override
+      public Void run() throws IOException {
+        KerberosTicket originalLoginUserTicket =
+            checkTicketAndKeytab(originalLoginUser, principal1, true);
+
+        // login principal2 from a subject with keytab.  it's external so
+        // no login context should be attached to the user.
+        final Subject subject =
+          UserGroupInformation.loginUserFromKeytabAndReturnUGI(
+            principal2.getName(), keytab2.getPath()).getSubject();
+        removeUser(subject);
+
+        // verify the new login user is external.
+        UserGroupInformation.loginUserFromSubject(subject);
+        Assert.assertNull(getUser(subject).getLogin());
+        UserGroupInformation extLoginUser =
+          UserGroupInformation.getLoginUser();
+        KerberosTicket extLoginUserTicket =
+            checkTicketAndKeytab(extLoginUser, principal2, false);
+
+        // verify subject-based login user does not get a new ticket, and
+        // original login user not affected.
+        extLoginUser.reloginFromKeytab();
+        Assert.assertSame(extLoginUserTicket,
+          checkTicketAndKeytab(extLoginUser, principal2, false));
+        Assert.assertSame(originalLoginUserTicket,
+          checkTicketAndKeytab(originalLoginUser, principal1, true));
+
+        // verify original login user gets a new ticket, new login user
+        // not affected.
+        originalLoginUser.reloginFromKeytab();
+        Assert.assertNotSame(originalLoginUserTicket,
+            checkTicketAndKeytab(originalLoginUser, principal1, true));
+        Assert.assertSame(extLoginUserTicket,
+            checkTicketAndKeytab(extLoginUser, principal2, false));
+        return null;
+      }
+    });
+  }
+
+  @Test
+  public void testReloginAfterFailedRelogin() throws Exception {
+    KerberosPrincipal principal = new KerberosPrincipal("user1");
+    File keytab = new File(workDir, "user1.keytab");
+    File keytabBackup = new File(keytab + ".backup");
+    kdc.createPrincipal(keytab, principal.getName());
+
+    UserGroupInformation.loginUserFromKeytab(
+        principal.getName(), keytab.getPath());
+    final UserGroupInformation loginUser = UserGroupInformation.getLoginUser();
+    checkTicketAndKeytab(loginUser, principal, true);
+
+    // move the keytab to induce a relogin failure.
+    Assert.assertTrue(keytab.renameTo(keytabBackup));
+    try {
+      loginUser.reloginFromKeytab();
+      Assert.fail("relogin should fail");
+    } catch (KerberosAuthException kae) {
+      // expected.
+    }
+
+    // even though no KeyTab object, ugi should know it's keytab based.
+    Assert.assertTrue(loginUser.isFromKeytab());
+    Assert.assertNull(getTicket(loginUser));
+
+    // move keytab back to enable relogin to succeed.
+    Assert.assertTrue(keytabBackup.renameTo(keytab));
+    loginUser.reloginFromKeytab();
+    checkTicketAndKeytab(loginUser, principal, true);
+  }
+
+  // verify getting concurrent relogins blocks to avoid indeterminate
+  // credentials corruption, but getting a ugi for the subject does not block.
+  @Test(timeout=180000)
+  public void testConcurrentRelogin() throws Exception {
+    final CyclicBarrier barrier = new CyclicBarrier(2);
+    final CountDownLatch latch = new CountDownLatch(1);
+    assertTrue(UserGroupInformation.isSecurityEnabled());
+
+    KerberosPrincipal principal = new KerberosPrincipal("testUser");
+    File keytab = new File(workDir, "user1.keytab");
+    kdc.createPrincipal(keytab, principal.getName());
+
+    // create a keytab ugi.
+    final UserGroupInformation loginUgi =
+        UserGroupInformation.loginUserFromKeytabAndReturnUGI(
+          principal.getName(), keytab.getPath());
+    assertEquals(AuthenticationMethod.KERBEROS,
+        loginUgi.getAuthenticationMethod());
+    assertTrue(loginUgi.isFromKeytab());
+
+    // create a new ugi instance based on subject from the logged in user.
+    final UserGroupInformation clonedUgi =
+        UserGroupInformation.getUGIFromSubject(loginUgi.getSubject());
+    assertEquals(AuthenticationMethod.KERBEROS,
+        clonedUgi.getAuthenticationMethod());
+    assertTrue(clonedUgi.isFromKeytab());
+
+    // cause first relogin to block on a barrier in logout to verify relogins
+    // are atomic.
+    User user = getUser(loginUgi.getSubject());
+    final LoginContext spyLogin = Mockito.spy(user.getLogin());
+    user.setLogin(spyLogin);
+    Mockito.doAnswer(new Answer<Void>(){
+      @Override
+      public Void answer(InvocationOnMock invocation)
+          throws Throwable {
+        invocation.callRealMethod();
+        latch.countDown();
+        barrier.await();
+        return null;
+      }
+    }).when(spyLogin).logout();
+
+    Future<Void> relogin = executor.submit(
+        new Callable<Void>(){
+          @Override
+          public Void call() throws Exception {
+            Thread.currentThread().setName("relogin");
+            loginUgi.reloginFromKeytab();
+            return null;
+          }
+        });
+    // wait for the thread to block on the barrier in the logout of the
+    // relogin.
+    assertTrue("first relogin didn't block",
+      latch.await(2, TimeUnit.SECONDS));
+
+    // although the logout removed the keytab instance, verify the ugi
+    // knows from its login params that it is supposed to be from a keytab.
+    assertTrue(clonedUgi.isFromKeytab());
+
+    // another concurrent re-login should block.
+    Mockito.doNothing().when(spyLogin).logout();
+    Mockito.doNothing().when(spyLogin).login();
+    Future<UserGroupInformation> clonedRelogin = executor.submit(
+        new Callable<UserGroupInformation>(){
+          @Override
+          public UserGroupInformation call() throws Exception {
+            Thread.currentThread().setName("clonedRelogin");
+            clonedUgi.reloginFromKeytab();
+            return clonedUgi;
+          }
+        });
+
+    try {
+      clonedRelogin.get(2, TimeUnit.SECONDS);
+      fail("second relogin didn't block!");
+    } catch (TimeoutException te) {
+      // expected
+    }
+
+    // concurrent UGI instantiation should not block and again should
+    // know it's supposed to be from a keytab.
+    loginUgi.doAs(new PrivilegedExceptionAction<Void>(){
+      @Override
+      public Void run() throws Exception {
+        UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
+        assertEquals(principal.getName(), ugi.getUserName());
+        assertTrue(ugi.isFromKeytab());
+        return null;
+      }
+    });
+    clonedUgi.doAs(new PrivilegedExceptionAction<Void>(){
+      @Override
+      public Void run() throws Exception {
+        UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
+        assertEquals(principal.getName(), ugi.getUserName());
+        assertTrue(ugi.isFromKeytab());
+        return null;
+      }
+    });
+
+    // second relogin should still be blocked until the original relogin
+    // is blocked.
+    assertFalse(clonedRelogin.isDone());
+    barrier.await();
+    relogin.get();
+    clonedRelogin.get();
+  }
+
+  private User getUser(Subject subject) {
+    Iterator<User> iter = subject.getPrincipals(User.class).iterator();
+    return iter.hasNext() ? iter.next() : null;
+  }
+
+  private void removeUser(Subject subject) {
+    // remove User instance so it appears to not be logged in.
+    for (Iterator<Principal> iter = subject.getPrincipals().iterator();
+         iter.hasNext(); ) {
+      if (iter.next() instanceof User) {
+        iter.remove();
+      }
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/59cf7588/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGIWithMiniKdc.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGIWithMiniKdc.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGIWithMiniKdc.java
index 6c94b1d..de74d17 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGIWithMiniKdc.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUGIWithMiniKdc.java
@@ -58,9 +58,11 @@ public class TestUGIWithMiniKdc {
 
   private void setupKdc() throws Exception {
     Properties kdcConf = MiniKdc.createConf();
-    // tgt expire time = 30 seconds
-    kdcConf.setProperty(MiniKdc.MAX_TICKET_LIFETIME, "30");
-    kdcConf.setProperty(MiniKdc.MIN_TICKET_LIFETIME, "30");
+    // tgt expire time = 2 seconds.  just testing that renewal thread retries
+    // for expiring tickets, so no need to waste time waiting for expiry to
+    // arrive.
+    kdcConf.setProperty(MiniKdc.MAX_TICKET_LIFETIME, "2");
+    kdcConf.setProperty(MiniKdc.MIN_TICKET_LIFETIME, "2");
     File kdcDir = new File(System.getProperty("test.dir", "target"));
     kdc = new MiniKdc(kdcConf, kdcDir);
     kdc.start();
@@ -70,12 +72,14 @@ public class TestUGIWithMiniKdc {
   public void testAutoRenewalThreadRetryWithKdc() throws Exception {
     GenericTestUtils.setLogLevel(UserGroupInformation.LOG, Level.DEBUG);
     final Configuration conf = new Configuration();
+    // can't rely on standard kinit, else test fails when user running
+    // the test is kinit'ed because the test renews _their TGT_.
+    conf.set("hadoop.kerberos.kinit.command", "bogus-kinit-cmd");
     // Relogin every 1 second
     conf.setLong(HADOOP_KERBEROS_MIN_SECONDS_BEFORE_RELOGIN, 1);
     SecurityUtil.setAuthenticationMethod(
         UserGroupInformation.AuthenticationMethod.KERBEROS, conf);
     UserGroupInformation.setConfiguration(conf);
-    UserGroupInformation.setEnableRenewThreadCreationForTest(true);
 
     LoginContext loginContext = null;
     try {
@@ -87,44 +91,10 @@ public class TestUGIWithMiniKdc {
       setupKdc();
       kdc.createPrincipal(keytab, principal);
 
-      // client login
-      final Subject subject =
-          new Subject(false, principals, new HashSet<>(), new HashSet<>());
-
-      loginContext = new LoginContext("", subject, null,
-          new javax.security.auth.login.Configuration() {
-            @Override
-            public AppConfigurationEntry[] getAppConfigurationEntry(
-                String name) {
-              Map<String, String> options = new HashMap<>();
-              options.put("principal", principal);
-              options.put("refreshKrb5Config", "true");
-              if (PlatformName.IBM_JAVA) {
-                options.put("useKeytab", keytab.getPath());
-                options.put("credsType", "both");
-              } else {
-                options.put("keyTab", keytab.getPath());
-                options.put("useKeyTab", "true");
-                options.put("storeKey", "true");
-                options.put("doNotPrompt", "true");
-                options.put("useTicketCache", "true");
-                options.put("renewTGT", "true");
-                options.put("isInitiator", Boolean.toString(true));
-              }
-              String ticketCache = System.getenv("KRB5CCNAME");
-              if (ticketCache != null) {
-                options.put("ticketCache", ticketCache);
-              }
-              options.put("debug", "true");
-              return new AppConfigurationEntry[] {new AppConfigurationEntry(
-                  KerberosUtil.getKrb5LoginModuleName(),
-                  AppConfigurationEntry.LoginModuleControlFlag.REQUIRED,
-                  options)};
-            }
-          });
-      loginContext.login();
-      final Subject loginSubject = loginContext.getSubject();
-      UserGroupInformation.loginUserFromSubject(loginSubject);
+      UserGroupInformation.loginUserFromKeytab(principal, keytab.getPath());
+      UserGroupInformation ugi = UserGroupInformation.getLoginUser();
+      // no ticket cache, so force the thread to test for failures.
+      ugi.spawnAutoRenewalThreadForUserCreds(true);
 
       // Verify retry happens. Do not verify retry count to reduce flakiness.
       // Detailed back-off logic is tested separately in

http://git-wip-us.apache.org/repos/asf/hadoop/blob/59cf7588/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUserGroupInformation.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUserGroupInformation.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUserGroupInformation.java
index bcb2126..9477990 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUserGroupInformation.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUserGroupInformation.java
@@ -38,6 +38,9 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.event.Level;
@@ -53,15 +56,19 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.lang.reflect.Method;
+import java.security.Principal;
 import java.security.PrivilegedExceptionAction;
 import java.util.Collection;
 import java.util.ConcurrentModificationException;
 import java.util.Date;
 import java.util.LinkedHashSet;
 import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
-
-import static org.apache.hadoop.fs.CommonConfigurationKeys.HADOOP_TREAT_SUBJECT_EXTERNAL_KEY;
 import static org.apache.hadoop.fs.CommonConfigurationKeys.HADOOP_USER_GROUP_METRICS_PERCENTILES_INTERVALS;
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_KERBEROS_MIN_SECONDS_BEFORE_RELOGIN;
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTH_TO_LOCAL;
@@ -112,7 +119,14 @@ public class TestUserGroupInformation {
       throw new RuntimeException("UGI is not using its own security conf!");
     } 
   }
-  
+
+  // must be set immediately to avoid inconsistent testing issues.
+  static {
+    // fake the realm is kerberos is enabled
+    System.setProperty("java.security.krb5.kdc", "");
+    System.setProperty("java.security.krb5.realm", "DEFAULT.REALM");
+  }
+
   /** configure ugi */
   @BeforeClass
   public static void setup() {
@@ -123,9 +137,6 @@ public class TestUserGroupInformation {
     // that finds winutils.exe
     String home = System.getenv("HADOOP_HOME");
     System.setProperty("hadoop.home.dir", (home != null ? home : "."));
-    // fake the realm is kerberos is enabled
-    System.setProperty("java.security.krb5.kdc", "");
-    System.setProperty("java.security.krb5.realm", "DEFAULT.REALM");
   }
   
   @Before
@@ -1021,7 +1032,8 @@ public class TestUserGroupInformation {
     assertTrue(credsugiTokens.contains(token2));
   }
 
-  private void testCheckTGTAfterLoginFromSubjectHelper() throws Exception {
+  @Test
+  public void testCheckTGTAfterLoginFromSubject() throws Exception {
     // security on, default is remove default realm
     SecurityUtil.setAuthenticationMethod(AuthenticationMethod.KERBEROS, conf);
     UserGroupInformation.setConfiguration(conf);
@@ -1043,17 +1055,6 @@ public class TestUserGroupInformation {
     });
   }
 
-  @Test(expected = KerberosAuthException.class)
-  public void testCheckTGTAfterLoginFromSubject() throws Exception {
-    testCheckTGTAfterLoginFromSubjectHelper();
-  }
-
-  @Test
-  public void testCheckTGTAfterLoginFromSubjectFix() throws Exception {
-    conf.setBoolean(HADOOP_TREAT_SUBJECT_EXTERNAL_KEY, true);
-    testCheckTGTAfterLoginFromSubjectHelper();
-  }
-
   @Test
   public void testGetNextRetryTime() throws Exception {
     GenericTestUtils.setLogLevel(UserGroupInformation.LOG, Level.DEBUG);
@@ -1134,4 +1135,80 @@ public class TestUserGroupInformation {
     LOG.info(str);
     assertTrue(str, lower <= lastRetry && lastRetry < upper);
   }
+
+  // verify that getCurrentUser on the same and different subjects can be
+  // concurrent.  Ie. no synchronization.
+  @Test(timeout=8000)
+  public void testConcurrentGetCurrentUser() throws Exception {
+    final CyclicBarrier barrier = new CyclicBarrier(2);
+    final CountDownLatch latch = new CountDownLatch(1);
+
+    final UserGroupInformation testUgi1 =
+        UserGroupInformation.createRemoteUser("testUgi1");
+
+    final UserGroupInformation testUgi2 =
+        UserGroupInformation.createRemoteUser("testUgi2");
+
+    // swap the User with a spy to allow getCurrentUser to block when the
+    // spy is called for the user name.
+    Set<Principal> principals = testUgi1.getSubject().getPrincipals();
+    User user =
+        testUgi1.getSubject().getPrincipals(User.class).iterator().next();
+    final User spyUser = Mockito.spy(user);
+    principals.remove(user);
+    principals.add(spyUser);
+    when(spyUser.getName()).thenAnswer(new Answer<String>(){
+      @Override
+      public String answer(InvocationOnMock invocation) throws Throwable {
+        latch.countDown();
+        barrier.await();
+        return (String)invocation.callRealMethod();
+      }
+    });
+    // wait for the thread to block on the barrier in getCurrentUser.
+    Future<UserGroupInformation> blockingLookup =
+        Executors.newSingleThreadExecutor().submit(
+            new Callable<UserGroupInformation>(){
+              @Override
+              public UserGroupInformation call() throws Exception {
+                return testUgi1.doAs(
+                    new PrivilegedExceptionAction<UserGroupInformation>() {
+                      @Override
+                      public UserGroupInformation run() throws Exception {
+                        return UserGroupInformation.getCurrentUser();
+                      }
+                    });
+              }
+            });
+    latch.await();
+
+    // old versions of mockito synchronize on returning mocked answers so
+    // the blocked getCurrentUser will block all other calls to getName.
+    // workaround this by swapping out the spy with the original User.
+    principals.remove(spyUser);
+    principals.add(user);
+    // concurrent getCurrentUser on ugi1 should not be blocked.
+    UserGroupInformation ugi;
+    ugi = testUgi1.doAs(
+        new PrivilegedExceptionAction<UserGroupInformation>() {
+          @Override
+          public UserGroupInformation run() throws Exception {
+            return UserGroupInformation.getCurrentUser();
+          }
+        });
+    assertSame(testUgi1.getSubject(), ugi.getSubject());
+    // concurrent getCurrentUser on ugi2 should not be blocked.
+    ugi = testUgi2.doAs(
+        new PrivilegedExceptionAction<UserGroupInformation>() {
+          @Override
+          public UserGroupInformation run() throws Exception {
+            return UserGroupInformation.getCurrentUser();
+          }
+        });
+    assertSame(testUgi2.getSubject(), ugi.getSubject());
+
+    // unblock the original call.
+    barrier.await();
+    assertSame(testUgi1.getSubject(), blockingLookup.get().getSubject());
+  }
 }


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