HADOOP-15959. Revert "HADOOP-12751. While using kerberos Hadoop incorrectly assumes names with '@' to be non-simple"

This reverts commit 829a2e4d27.

(cherry picked from commit d0edd37269)
This commit is contained in:
Steve Loughran 2018-11-29 17:53:23 +00:00
parent 183ec39c4b
commit 1a448565a8
No known key found for this signature in database
GPG Key ID: D22CF846DBB162A0
7 changed files with 33 additions and 95 deletions

View File

@ -324,8 +324,8 @@ public class KerberosName {
}
}
if (result != null && nonSimplePattern.matcher(result).find()) {
LOG.info("Non-simple name {} after auth_to_local rule {}",
result, this);
throw new NoMatchingRule("Non-simple name " + result +
" after auth_to_local rule " + this);
}
if (toLowerCase && result != null) {
result = result.toLowerCase(Locale.ENGLISH);
@ -378,7 +378,7 @@ public class KerberosName {
/**
* Get the translation of the principal name into an operating system
* user name.
* @return the user name
* @return the short name
* @throws IOException throws if something is wrong with the rules
*/
public String getShortName() throws IOException {
@ -398,8 +398,7 @@ public class KerberosName {
return result;
}
}
LOG.info("No auth_to_local rules applied to {}", this);
return toString();
throw new NoMatchingRule("No rules applied to " + toString());
}
/**

View File

@ -108,7 +108,12 @@ public class TestKerberosAuthenticationHandler
kn = new KerberosName("bar@BAR");
Assert.assertEquals("bar", kn.getShortName());
kn = new KerberosName("bar@FOO");
Assert.assertEquals("bar@FOO", kn.getShortName());
try {
kn.getShortName();
Assert.fail();
}
catch (Exception ex) {
}
}
@Test(timeout=60000)

View File

@ -72,14 +72,23 @@ 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/owen@FOO.COM");
checkBadName("owen@foo/bar.com");
// no rules applied, these should pass
checkTranslation("foo@ACME.COM", "foo@ACME.COM");
checkTranslation("root/joe@FOO.COM", "root/joe@FOO.COM");
checkBadTranslation("foo@ACME.COM");
checkBadTranslation("root/joe@FOO.COM");
}
@Test

View File

@ -22,7 +22,6 @@ import org.apache.commons.io.IOUtils;
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;
@ -55,7 +54,6 @@ 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.*;
@ -131,12 +129,6 @@ 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...)}
@ -165,8 +157,6 @@ 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,
@ -210,7 +200,6 @@ 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;
@ -256,9 +245,7 @@ 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_VERIFYSHORTNAME, ARG_PRINCIPAL + " <principal>",
"Verify the short name of the specific principal does not contain '@' or '/'");
+ arg(ARG_SECURE, "", "Require the hadoop configuration to be secure");
}
private String arg(String name, String params, String meaning) {
@ -291,7 +278,6 @@ 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();
@ -391,10 +377,6 @@ public class KDiag extends Configured implements Tool, Closeable {
validateJAAS(jaas);
validateNTPConf();
if (checkShortName) {
validateShortName();
}
if (!nologin) {
title("Logging in");
if (keytab != null) {
@ -448,32 +430,6 @@ public class KDiag extends Configured implements Tool, Closeable {
aesLen, minKeyLength);
}
/**
* Verify whether auth_to_local rules transform a principal name
* <p>
* Having a local user name "bar@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>

View File

@ -470,7 +470,6 @@ 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`.
@ -566,11 +565,6 @@ 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
```

View File

@ -164,22 +164,6 @@ public class TestKDiag extends Assert {
ARG_PRINCIPAL, "foo@EXAMPLE.COM");
}
@Test
public void testKerberosName() throws Throwable {
kdiagFailure(ARG_KEYLEN, KEYLEN,
ARG_VERIFYSHORTNAME,
ARG_PRINCIPAL, "foo/foo/foo@BAR.COM");
}
@Test
public void testShortName() throws Throwable {
kdiag(ARG_KEYLEN, KEYLEN,
ARG_KEYTAB, keytab.getAbsolutePath(),
ARG_PRINCIPAL,
ARG_VERIFYSHORTNAME,
ARG_PRINCIPAL, "foo@EXAMPLE.COM");
}
@Test
public void testFileOutput() throws Throwable {
File f = new File("target/kdiag.txt");

View File

@ -332,15 +332,10 @@ public class TestUserGroupInformation {
UserGroupInformation.setConfiguration(conf);
testConstructorSuccess("user1", "user1");
testConstructorSuccess("user4@OTHER.REALM", "other-user4");
// 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("user6@example.com@OTHER.REALM");
testConstructorFailures("user7@example.com@DEFAULT.REALM");
// failure test
testConstructorFailures("user2@DEFAULT.REALM");
testConstructorFailures("user3/cron@DEFAULT.REALM");
testConstructorFailures("user5/cron@OTHER.REALM");
testConstructorFailures(null);
testConstructorFailures("");
}
@ -355,12 +350,9 @@ public class TestUserGroupInformation {
testConstructorSuccess("user1", "user1");
testConstructorSuccess("user2@DEFAULT.REALM", "user2");
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("");
}
@ -401,9 +393,8 @@ public class TestUserGroupInformation {
} catch (IllegalArgumentException e) {
String expect = (userName == null || userName.isEmpty())
? "Null user" : "Illegal principal name "+userName;
String expect2 = "Malformed Kerberos name: "+userName;
assertTrue("Did not find "+ expect + " or " + expect2 + " in " + e,
e.toString().contains(expect) || e.toString().contains(expect2));
assertTrue("Did not find "+ expect + " in " + e,
e.toString().contains(expect));
}
}