Skip to content

Commit 542b224

Browse files
steveloughransunilgovind
authored andcommitted
HADOOP-15959. Revert "HADOOP-12751. While using kerberos Hadoop incorrectly assumes names with '@' to be non-simple"
This reverts commit 829a2e4. (cherry picked from commit d0edd37)
1 parent fa48363 commit 542b224

File tree

7 files changed

+33
-95
lines changed

7 files changed

+33
-95
lines changed

hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosName.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,8 @@ String apply(String[] params) throws IOException {
324324
}
325325
}
326326
if (result != null && nonSimplePattern.matcher(result).find()) {
327-
LOG.info("Non-simple name {} after auth_to_local rule {}",
328-
result, this);
327+
throw new NoMatchingRule("Non-simple name " + result +
328+
" after auth_to_local rule " + this);
329329
}
330330
if (toLowerCase && result != null) {
331331
result = result.toLowerCase(Locale.ENGLISH);
@@ -378,7 +378,7 @@ public static class NoMatchingRule extends IOException {
378378
/**
379379
* Get the translation of the principal name into an operating system
380380
* user name.
381-
* @return the user name
381+
* @return the short name
382382
* @throws IOException throws if something is wrong with the rules
383383
*/
384384
public String getShortName() throws IOException {
@@ -398,8 +398,7 @@ public String getShortName() throws IOException {
398398
return result;
399399
}
400400
}
401-
LOG.info("No auth_to_local rules applied to {}", this);
402-
return toString();
401+
throw new NoMatchingRule("No rules applied to " + toString());
403402
}
404403

405404
/**

hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestKerberosAuthenticationHandler.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,12 @@ public void testNameRules() throws Exception {
108108
kn = new KerberosName("bar@BAR");
109109
Assert.assertEquals("bar", kn.getShortName());
110110
kn = new KerberosName("bar@FOO");
111-
Assert.assertEquals("bar@FOO", kn.getShortName());
111+
try {
112+
kn.getShortName();
113+
Assert.fail();
114+
}
115+
catch (Exception ex) {
116+
}
112117
}
113118

114119
@Test(timeout=60000)

hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestKerberosName.java

+13-4
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,23 @@ private void checkBadName(String name) {
7272
}
7373
}
7474

75+
private void checkBadTranslation(String from) {
76+
System.out.println("Checking bad translation for " + from);
77+
KerberosName nm = new KerberosName(from);
78+
try {
79+
nm.getShortName();
80+
Assert.fail("didn't get exception for " + from);
81+
} catch (IOException ie) {
82+
// PASS
83+
}
84+
}
85+
7586
@Test
7687
public void testAntiPatterns() throws Exception {
7788
checkBadName("owen/owen/[email protected]");
7889
checkBadName("owen@foo/bar.com");
79-
80-
// no rules applied, these should pass
81-
checkTranslation("[email protected]", "[email protected]");
82-
checkTranslation("root/[email protected]", "root/[email protected]");
90+
checkBadTranslation("[email protected]");
91+
checkBadTranslation("root/[email protected]");
8392
}
8493

8594
@Test

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

+1-45
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.apache.hadoop.conf.Configuration;
2323
import org.apache.hadoop.conf.Configured;
2424
import org.apache.hadoop.io.Text;
25-
import org.apache.hadoop.security.authentication.util.KerberosName;
2625
import org.apache.hadoop.security.token.Token;
2726
import org.apache.hadoop.security.token.TokenIdentifier;
2827
import org.apache.hadoop.util.ExitUtil;
@@ -55,7 +54,6 @@
5554
import java.util.Date;
5655
import java.util.LinkedList;
5756
import java.util.List;
58-
import java.util.regex.Pattern;
5957

6058
import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.*;
6159
import static org.apache.hadoop.security.UserGroupInformation.*;
@@ -131,12 +129,6 @@ public class KDiag extends Configured implements Tool, Closeable {
131129
private boolean nofail = false;
132130
private boolean nologin = false;
133131
private boolean jaas = false;
134-
private boolean checkShortName = false;
135-
136-
/**
137-
* A pattern that recognizes simple/non-simple names. Per KerberosName
138-
*/
139-
private static final Pattern nonSimplePattern = Pattern.compile("[/@]");
140132

141133
/**
142134
* Flag set to true if a {@link #verify(boolean, String, String, Object...)}
@@ -165,8 +157,6 @@ public class KDiag extends Configured implements Tool, Closeable {
165157

166158
public static final String ARG_SECURE = "--secure";
167159

168-
public static final String ARG_VERIFYSHORTNAME = "--verifyshortname";
169-
170160
@SuppressWarnings("IOResourceOpenedButNotSafelyClosed")
171161
public KDiag(Configuration conf,
172162
PrintWriter out,
@@ -210,7 +200,6 @@ public int run(String[] argv) throws Exception {
210200
nofail = popOption(ARG_NOFAIL, args);
211201
jaas = popOption(ARG_JAAS, args);
212202
nologin = popOption(ARG_NOLOGIN, args);
213-
checkShortName = popOption(ARG_VERIFYSHORTNAME, args);
214203

215204
// look for list of resources
216205
String resource;
@@ -256,9 +245,7 @@ private String usage() {
256245
+ arg(ARG_NOLOGIN, "", "Do not attempt to log in")
257246
+ arg(ARG_OUTPUT, "<file>", "Write output to a file")
258247
+ arg(ARG_RESOURCE, "<resource>", "Load an XML configuration resource")
259-
+ arg(ARG_SECURE, "", "Require the hadoop configuration to be secure")
260-
+ arg(ARG_VERIFYSHORTNAME, ARG_PRINCIPAL + " <principal>",
261-
"Verify the short name of the specific principal does not contain '@' or '/'");
248+
+ arg(ARG_SECURE, "", "Require the hadoop configuration to be secure");
262249
}
263250

264251
private String arg(String name, String params, String meaning) {
@@ -291,7 +278,6 @@ public boolean execute() throws Exception {
291278
println("%s = %d", ARG_KEYLEN, minKeyLength);
292279
println("%s = %s", ARG_KEYTAB, keytab);
293280
println("%s = %s", ARG_PRINCIPAL, principal);
294-
println("%s = %s", ARG_VERIFYSHORTNAME, checkShortName);
295281

296282
// Fail fast on a JVM without JCE installed.
297283
validateKeyLength();
@@ -391,10 +377,6 @@ public boolean execute() throws Exception {
391377
validateJAAS(jaas);
392378
validateNTPConf();
393379

394-
if (checkShortName) {
395-
validateShortName();
396-
}
397-
398380
if (!nologin) {
399381
title("Logging in");
400382
if (keytab != null) {
@@ -448,32 +430,6 @@ protected void validateKeyLength() throws NoSuchAlgorithmException {
448430
aesLen, minKeyLength);
449431
}
450432

451-
/**
452-
* Verify whether auth_to_local rules transform a principal name
453-
* <p>
454-
* Having a local user name "[email protected]" may be harmless, so it is noted at
455-
* info. However if what was intended is a transformation to "bar"
456-
* it can be difficult to debug, hence this check.
457-
*/
458-
protected void validateShortName() {
459-
failif(principal == null, CAT_KERBEROS, "No principal defined");
460-
461-
try {
462-
KerberosName kn = new KerberosName(principal);
463-
String result = kn.getShortName();
464-
if (nonSimplePattern.matcher(result).find()) {
465-
warn(CAT_KERBEROS, principal + " short name: " + result
466-
+ " still contains @ or /");
467-
}
468-
} catch (IOException e) {
469-
throw new KerberosDiagsFailure(CAT_KERBEROS, e,
470-
"Failed to get short name for " + principal, e);
471-
} catch (IllegalArgumentException e) {
472-
error(CAT_KERBEROS, "KerberosName(" + principal + ") failed: %s\n%s",
473-
e, StringUtils.stringifyException(e));
474-
}
475-
}
476-
477433
/**
478434
* Get the default realm.
479435
* <p>

hadoop-common-project/hadoop-common/src/site/markdown/SecureMode.md

-6
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,6 @@ KDiag: Diagnose Kerberos Problems
470470
[--out <file>] : Write output to a file.
471471
[--resource <resource>] : Load an XML configuration resource.
472472
[--secure] : Require the hadoop configuration to be secure.
473-
[--verifyshortname <principal>]: Verify the short name of the specific principal does not contain '@' or '/'
474473
```
475474

476475
#### `--jaas`: Require a JAAS file to be defined in `java.security.auth.login.config`.
@@ -566,11 +565,6 @@ or implicitly set to "simple":
566565

567566
Needless to say, an application so configured cannot talk to a secure Hadoop cluster.
568567

569-
#### `--verifyshortname <principal>`: validate the short name of a principal
570-
571-
This verifies that the short name of a principal contains neither the `"@"`
572-
nor `"/"` characters.
573-
574568
### Example
575569

576570
```

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

-16
Original file line numberDiff line numberDiff line change
@@ -164,22 +164,6 @@ public void testKeytabAndPrincipal() throws Throwable {
164164
ARG_PRINCIPAL, "[email protected]");
165165
}
166166

167-
@Test
168-
public void testKerberosName() throws Throwable {
169-
kdiagFailure(ARG_KEYLEN, KEYLEN,
170-
ARG_VERIFYSHORTNAME,
171-
ARG_PRINCIPAL, "foo/foo/[email protected]");
172-
}
173-
174-
@Test
175-
public void testShortName() throws Throwable {
176-
kdiag(ARG_KEYLEN, KEYLEN,
177-
ARG_KEYTAB, keytab.getAbsolutePath(),
178-
ARG_PRINCIPAL,
179-
ARG_VERIFYSHORTNAME,
180-
ARG_PRINCIPAL, "[email protected]");
181-
}
182-
183167
@Test
184168
public void testFileOutput() throws Throwable {
185169
File f = new File("target/kdiag.txt");

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

+9-18
Original file line numberDiff line numberDiff line change
@@ -332,15 +332,10 @@ public void testConstructorWithRules() throws Exception {
332332
UserGroupInformation.setConfiguration(conf);
333333
testConstructorSuccess("user1", "user1");
334334
testConstructorSuccess("[email protected]", "other-user4");
335-
336-
// pass through test, no transformation
337-
testConstructorSuccess("[email protected]", "[email protected]");
338-
testConstructorSuccess("user3/[email protected]", "user3/[email protected]");
339-
testConstructorSuccess("user5/[email protected]", "user5/[email protected]");
340-
341-
// failures
342-
testConstructorFailures("[email protected]@OTHER.REALM");
343-
testConstructorFailures("[email protected]@DEFAULT.REALM");
335+
// failure test
336+
testConstructorFailures("[email protected]");
337+
testConstructorFailures("user3/[email protected]");
338+
testConstructorFailures("user5/[email protected]");
344339
testConstructorFailures(null);
345340
testConstructorFailures("");
346341
}
@@ -354,13 +349,10 @@ public void testConstructorWithKerberos() throws Exception {
354349

355350
testConstructorSuccess("user1", "user1");
356351
testConstructorSuccess("[email protected]", "user2");
357-
testConstructorSuccess("user3/[email protected]", "user3");
358-
359-
// no rules applied, local name remains the same
360-
testConstructorSuccess("[email protected]", "[email protected]");
361-
testConstructorSuccess("user5/[email protected]", "user5/[email protected]");
362-
352+
testConstructorSuccess("user3/[email protected]", "user3");
363353
// failure test
354+
testConstructorFailures("[email protected]");
355+
testConstructorFailures("user5/[email protected]");
364356
testConstructorFailures(null);
365357
testConstructorFailures("");
366358
}
@@ -401,9 +393,8 @@ private void testConstructorFailures(String userName) {
401393
} catch (IllegalArgumentException e) {
402394
String expect = (userName == null || userName.isEmpty())
403395
? "Null user" : "Illegal principal name "+userName;
404-
String expect2 = "Malformed Kerberos name: "+userName;
405-
assertTrue("Did not find "+ expect + " or " + expect2 + " in " + e,
406-
e.toString().contains(expect) || e.toString().contains(expect2));
396+
assertTrue("Did not find "+ expect + " in " + e,
397+
e.toString().contains(expect));
407398
}
408399
}
409400

0 commit comments

Comments
 (0)