HADOOP-13433 Race in UGI.reloginFromKeytab. Contributed by Duo Zhang.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/7fc3e68a Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/7fc3e68a Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/7fc3e68a Branch: refs/heads/HADOOP-13345 Commit: 7fc3e68a876132563aa2321519fc6941e37b2cae Parents: abedb8a Author: Steve Loughran <ste...@apache.org> Authored: Wed Jan 25 21:29:27 2017 +0000 Committer: Steve Loughran <ste...@apache.org> Committed: Wed Jan 25 21:29:27 2017 +0000 ---------------------------------------------------------------------- hadoop-common-project/hadoop-common/pom.xml | 5 + .../hadoop/security/UserGroupInformation.java | 65 ++++++-- .../security/TestFixKerberosTicketOrder.java | 158 ++++++++++++++++++ .../hadoop/security/TestRaceWhenRelogin.java | 162 +++++++++++++++++++ 4 files changed, 375 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/7fc3e68a/hadoop-common-project/hadoop-common/pom.xml ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/pom.xml b/hadoop-common-project/hadoop-common/pom.xml index b69de55..909cd78 100644 --- a/hadoop-common-project/hadoop-common/pom.xml +++ b/hadoop-common-project/hadoop-common/pom.xml @@ -46,6 +46,11 @@ <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> <scope>compile</scope> http://git-wip-us.apache.org/repos/asf/hadoop/blob/7fc3e68a/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 cf240ff..6574e55 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 @@ -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() || - user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS || - !isKeytab) + if (!isSecurityEnabled() + || user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS + || !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() || - user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS || - !isKrbTkt) + if (!isSecurityEnabled() + || user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS + || !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 http://git-wip-us.apache.org/repos/asf/hadoop/blob/7fc3e68a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestFixKerberosTicketOrder.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestFixKerberosTicketOrder.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestFixKerberosTicketOrder.java new file mode 100644 index 0000000..4b75a36 --- /dev/null +++ b/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()); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/hadoop/blob/7fc3e68a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestRaceWhenRelogin.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestRaceWhenRelogin.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestRaceWhenRelogin.java new file mode 100644 index 0000000..4f9946c --- /dev/null +++ b/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()); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
