HADOOP-12751. While using kerberos Hadoop incorrectly assumes names with '@' to be non-simple. (Bolke de Bruin via stevel).
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/829a2e4d Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/829a2e4d Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/829a2e4d Branch: refs/heads/HDFS-1312 Commit: 829a2e4d271f05afb209ddc834cd4a0e85492eda Parents: f0252ad Author: Steve Loughran <ste...@apache.org> Authored: Fri May 6 19:35:59 2016 +0100 Committer: Steve Loughran <ste...@apache.org> Committed: Tue May 10 21:32:57 2016 +0100 ---------------------------------------------------------------------- .../authentication/util/KerberosName.java | 9 ++-- .../TestKerberosAuthenticationHandler.java | 7 +-- .../authentication/util/TestKerberosName.java | 17 ++------ .../java/org/apache/hadoop/security/KDiag.java | 46 +++++++++++++++++++- .../src/site/markdown/SecureMode.md | 6 +++ .../org/apache/hadoop/security/TestKDiag.java | 16 +++++++ .../security/TestUserGroupInformation.java | 27 ++++++++---- 7 files changed, 95 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/829a2e4d/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosName.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosName.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosName.java index 0bc1109..645fbc6 100644 --- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosName.java +++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosName.java @@ -323,8 +323,8 @@ public class KerberosName { } } if (result != null && nonSimplePattern.matcher(result).find()) { - throw new NoMatchingRule("Non-simple name " + result + - " after auth_to_local rule " + this); + LOG.info("Non-simple name {} after auth_to_local rule {}", + result, this); } if (toLowerCase && result != null) { result = result.toLowerCase(Locale.ENGLISH); @@ -377,7 +377,7 @@ public class KerberosName { /** * Get the translation of the principal name into an operating system * user name. - * @return the short name + * @return the user name * @throws IOException throws if something is wrong with the rules */ public String getShortName() throws IOException { @@ -397,7 +397,8 @@ public class KerberosName { return result; } } - throw new NoMatchingRule("No rules applied to " + toString()); + LOG.info("No auth_to_local rules applied to {}", this); + return toString(); } /** http://git-wip-us.apache.org/repos/asf/hadoop/blob/829a2e4d/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestKerberosAuthenticationHandler.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestKerberosAuthenticationHandler.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestKerberosAuthenticationHandler.java index 408563f..e3444ef 100644 --- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestKerberosAuthenticationHandler.java +++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestKerberosAuthenticationHandler.java @@ -109,12 +109,7 @@ public class TestKerberosAuthenticationHandler kn = new KerberosName("bar@BAR"); Assert.assertEquals("bar", kn.getShortName()); kn = new KerberosName("bar@FOO"); - try { - kn.getShortName(); - Assert.fail(); - } - catch (Exception ex) { - } + Assert.assertEquals("bar@FOO", kn.getShortName()); } @Test(timeout=60000) http://git-wip-us.apache.org/repos/asf/hadoop/blob/829a2e4d/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestKerberosName.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestKerberosName.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestKerberosName.java index 354917e..f85b3e1 100644 --- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestKerberosName.java +++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestKerberosName.java @@ -72,23 +72,14 @@ public class TestKerberosName { } } - private void checkBadTranslation(String from) { - System.out.println("Checking bad translation for " + from); - KerberosName nm = new KerberosName(from); - try { - nm.getShortName(); - Assert.fail("didn't get exception for " + from); - } catch (IOException ie) { - // PASS - } - } - @Test public void testAntiPatterns() throws Exception { checkBadName("owen/owen/o...@foo.com"); checkBadName("owen@foo/bar.com"); - checkBadTranslation("f...@acme.com"); - checkBadTranslation("root/j...@foo.com"); + + // no rules applied, these should pass + checkTranslation("f...@acme.com", "f...@acme.com"); + checkTranslation("root/j...@foo.com", "root/j...@foo.com"); } @Test http://git-wip-us.apache.org/repos/asf/hadoop/blob/829a2e4d/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/KDiag.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/KDiag.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/KDiag.java index 4c2b0c4..6cef962 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/KDiag.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/KDiag.java @@ -25,6 +25,7 @@ import org.apache.directory.shared.kerberos.components.EncryptionKey; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configured; import org.apache.hadoop.io.Text; +import org.apache.hadoop.security.authentication.util.KerberosName; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.TokenIdentifier; import org.apache.hadoop.util.ExitUtil; @@ -52,6 +53,7 @@ import java.util.Collections; import java.util.Date; import java.util.LinkedList; import java.util.List; +import java.util.regex.Pattern; import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.*; import static org.apache.hadoop.security.UserGroupInformation.*; @@ -121,6 +123,12 @@ public class KDiag extends Configured implements Tool, Closeable { private boolean nofail = false; private boolean nologin = false; private boolean jaas = false; + private boolean checkShortName = false; + + /** + * A pattern that recognizes simple/non-simple names. Per KerberosName + */ + private static final Pattern nonSimplePattern = Pattern.compile("[/@]"); /** * Flag set to true if a {@link #verify(boolean, String, String, Object...)} @@ -148,6 +156,8 @@ public class KDiag extends Configured implements Tool, Closeable { public static final String ARG_SECURE = "--secure"; + public static final String ARG_VERIFYSHORTNAME = "--verifyshortname"; + @SuppressWarnings("IOResourceOpenedButNotSafelyClosed") public KDiag(Configuration conf, PrintWriter out, @@ -191,6 +201,7 @@ public class KDiag extends Configured implements Tool, Closeable { nofail = popOption(ARG_NOFAIL, args); jaas = popOption(ARG_JAAS, args); nologin = popOption(ARG_NOLOGIN, args); + checkShortName = popOption(ARG_VERIFYSHORTNAME, args); // look for list of resources String resource; @@ -236,7 +247,9 @@ public class KDiag extends Configured implements Tool, Closeable { + arg(ARG_NOLOGIN, "", "Do not attempt to log in") + arg(ARG_OUTPUT, "<file>", "Write output to a file") + arg(ARG_RESOURCE, "<resource>", "Load an XML configuration resource") - + arg(ARG_SECURE, "", "Require the hadoop configuration to be secure"); + + arg(ARG_SECURE, "", "Require the hadoop configuration to be secure") + + arg(ARG_VERIFYSHORTNAME, ARG_PRINCIPAL + " <principal>", + "Verify the short name of the specific principal does not contain '@' or '/'"); } private String arg(String name, String params, String meaning) { @@ -269,6 +282,7 @@ public class KDiag extends Configured implements Tool, Closeable { println("%s = %d", ARG_KEYLEN, minKeyLength); println("%s = %s", ARG_KEYTAB, keytab); println("%s = %s", ARG_PRINCIPAL, principal); + println("%s = %s", ARG_VERIFYSHORTNAME, checkShortName); // Fail fast on a JVM without JCE installed. validateKeyLength(); @@ -366,6 +380,10 @@ public class KDiag extends Configured implements Tool, Closeable { validateJAAS(jaas); validateNTPConf(); + if (checkShortName) { + validateShortName(); + } + if (!nologin) { title("Logging in"); if (keytab != null) { @@ -420,6 +438,32 @@ public class KDiag extends Configured implements Tool, Closeable { } /** + * Verify whether auth_to_local rules transform a principal name + * <p> + * Having a local user name "b...@foo.com" may be harmless, so it is noted at + * info. However if what was intended is a transformation to "bar" + * it can be difficult to debug, hence this check. + */ + protected void validateShortName() { + failif(principal == null, CAT_KERBEROS, "No principal defined"); + + try { + KerberosName kn = new KerberosName(principal); + String result = kn.getShortName(); + if (nonSimplePattern.matcher(result).find()) { + warn(CAT_KERBEROS, principal + " short name: " + result + + " still contains @ or /"); + } + } catch (IOException e) { + throw new KerberosDiagsFailure(CAT_KERBEROS, e, + "Failed to get short name for " + principal, e); + } catch (IllegalArgumentException e) { + error(CAT_KERBEROS, "KerberosName(" + principal + ") failed: %s\n%s", + e, StringUtils.stringifyException(e)); + } + } + + /** * Get the default realm. * <p> * Not having a default realm may be harmless, so is noted at info. http://git-wip-us.apache.org/repos/asf/hadoop/blob/829a2e4d/hadoop-common-project/hadoop-common/src/site/markdown/SecureMode.md ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/site/markdown/SecureMode.md b/hadoop-common-project/hadoop-common/src/site/markdown/SecureMode.md index 96fcc14..f12a72a 100644 --- a/hadoop-common-project/hadoop-common/src/site/markdown/SecureMode.md +++ b/hadoop-common-project/hadoop-common/src/site/markdown/SecureMode.md @@ -476,6 +476,7 @@ KDiag: Diagnose Kerberos Problems [--out <file>] : Write output to a file. [--resource <resource>] : Load an XML configuration resource. [--secure] : Require the hadoop configuration to be secure. + [--verifyshortname <principal>]: Verify the short name of the specific principal does not contain '@' or '/' ``` #### `--jaas`: Require a JAAS file to be defined in `java.security.auth.login.config`. @@ -574,6 +575,11 @@ or implicitly set to "simple": Needless to say, an application so configured cannot talk to a secure Hadoop cluster. +#### `--verifyshortname <principal>`: validate the short name of a principal + +This verifies that the short name of a principal contains neither the `"@"` +nor `"/"` characters. + ### Example ``` http://git-wip-us.apache.org/repos/asf/hadoop/blob/829a2e4d/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestKDiag.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestKDiag.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestKDiag.java index 1ba11cc..b81c4c5 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestKDiag.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestKDiag.java @@ -165,6 +165,22 @@ public class TestKDiag extends Assert { } @Test + public void testKerberosName() throws Throwable { + kdiagFailure(ARG_KEYLEN, KEYLEN, + ARG_VERIFYSHORTNAME, + ARG_PRINCIPAL, "foo/foo/f...@bar.com"); + } + + @Test + public void testShortName() throws Throwable { + kdiag(ARG_KEYLEN, KEYLEN, + ARG_KEYTAB, keytab.getAbsolutePath(), + ARG_PRINCIPAL, + ARG_VERIFYSHORTNAME, + ARG_PRINCIPAL, "f...@example.com"); + } + + @Test public void testFileOutput() throws Throwable { File f = new File("target/kdiag.txt"); kdiag(ARG_KEYLEN, KEYLEN, http://git-wip-us.apache.org/repos/asf/hadoop/blob/829a2e4d/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 1c37b08..91f36e5 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 @@ -283,10 +283,15 @@ public class TestUserGroupInformation { UserGroupInformation.setConfiguration(conf); testConstructorSuccess("user1", "user1"); testConstructorSuccess("user4@OTHER.REALM", "other-user4"); - // failure test - testConstructorFailures("user2@DEFAULT.REALM"); - testConstructorFailures("user3/cron@DEFAULT.REALM"); - testConstructorFailures("user5/cron@OTHER.REALM"); + + // pass through test, no transformation + testConstructorSuccess("user2@DEFAULT.REALM", "user2@DEFAULT.REALM"); + testConstructorSuccess("user3/cron@DEFAULT.REALM", "user3/cron@DEFAULT.REALM"); + testConstructorSuccess("user5/cron@OTHER.REALM", "user5/cron@OTHER.REALM"); + + // failures + testConstructorFailures("us...@example.com@OTHER.REALM"); + testConstructorFailures("us...@example.com@DEFAULT.REALM"); testConstructorFailures(null); testConstructorFailures(""); } @@ -300,10 +305,13 @@ public class TestUserGroupInformation { testConstructorSuccess("user1", "user1"); testConstructorSuccess("user2@DEFAULT.REALM", "user2"); - testConstructorSuccess("user3/cron@DEFAULT.REALM", "user3"); + testConstructorSuccess("user3/cron@DEFAULT.REALM", "user3"); + + // no rules applied, local name remains the same + testConstructorSuccess("user4@OTHER.REALM", "user4@OTHER.REALM"); + testConstructorSuccess("user5/cron@OTHER.REALM", "user5/cron@OTHER.REALM"); + // failure test - testConstructorFailures("user4@OTHER.REALM"); - testConstructorFailures("user5/cron@OTHER.REALM"); testConstructorFailures(null); testConstructorFailures(""); } @@ -344,8 +352,9 @@ public class TestUserGroupInformation { } catch (IllegalArgumentException e) { String expect = (userName == null || userName.isEmpty()) ? "Null user" : "Illegal principal name "+userName; - assertTrue("Did not find "+ expect + " in " + e, - e.toString().contains(expect)); + String expect2 = "Malformed Kerberos name: "+userName; + assertTrue("Did not find "+ expect + " or " + expect2 + " in " + e, + e.toString().contains(expect) || e.toString().contains(expect2)); } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
