honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix role mapping DN field wildcards for users with NULL DNs (#41343) 

The `DistinguishedNamePredicate`, used for matching users to role mapping
expressions, should handle users with null DNs. But it fails to do so (and this is
a NPE bug), if the role mapping expression contains a lucene regexp or a wildcard.

The fix simplifies `DistinguishedNamePredicate` to not handle null DNs at all, and
instead use the `ExpressionModel#NULL_PREDICATE` for the DN field, just like
any other missing user field.
This commit is contained in:
Albert Zaharovits 2019-04-22 10:25:24 +03:00 committed by GitHub
parent 5c40fc9ba5
commit c3e0ae24d3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 58 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/UserRoleMapper.java
View File

@ -79,8 +79,12 @@ public interface UserRoleMapper {
public ExpressionModel asModel() {
final ExpressionModel model = new ExpressionModel();
model.defineField("username", username);
model.defineField("dn", dn, new DistinguishedNamePredicate(dn));
if (dn != null) {
// null dn fields get the default NULL_PREDICATE
model.defineField("dn", dn, new DistinguishedNamePredicate(dn));
}
model.defineField("groups", groups, groups.stream()
.filter(group -> group != null)
.<Predicate<FieldExpression.FieldValue>>map(DistinguishedNamePredicate::new)
.reduce(Predicate::or)
.orElse(fieldValue -> false)
@ -165,22 +169,19 @@ public interface UserRoleMapper {
private final DN dn;
public DistinguishedNamePredicate(String string) {
assert string != null : "DN string should not be null. Use the dedicated NULL_PREDICATE for every user null field.";
this.string = string;
this.dn = parseDn(string);
}
private static DN parseDn(String string) {
if (string == null) {
return null;
} else {
try {
return new DN(string);
} catch (LDAPException | LDAPSDKUsageException e) {
if (LOGGER.isTraceEnabled()) {
LOGGER.trace(new ParameterizedMessage("failed to parse [{}] as a DN", string), e);
}
return null;
try {
return new DN(string);
} catch (LDAPException | LDAPSDKUsageException e) {
if (LOGGER.isTraceEnabled()) {
LOGGER.trace(new ParameterizedMessage("failed to parse [{}] as a DN", string), e);
}
return null;
}
}
@ -240,7 +241,7 @@ public interface UserRoleMapper {
}
return testString.equalsIgnoreCase(dn.toNormalizedString());
}
return string == null && fieldValue.getValue() == null;
return false;
}
}
}

10
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/DistinguishedNamePredicateTests.java
View File

@ -49,27 +49,25 @@ public class DistinguishedNamePredicateTests extends ESTestCase {
}
public void testParsingMalformedInput() {
Predicate<FieldValue> predicate = new UserRoleMapper.DistinguishedNamePredicate(null);
assertPredicate(predicate, null, true);
assertPredicate(predicate, "", false);
assertPredicate(predicate, randomAlphaOfLengthBetween(1, 8), false);
predicate = new UserRoleMapper.DistinguishedNamePredicate("");
Predicate<FieldValue> predicate = new UserRoleMapper.DistinguishedNamePredicate("");
assertPredicate(predicate, null, false);
assertPredicate(predicate, "", true);
assertPredicate(predicate, randomAlphaOfLengthBetween(1, 8), false);
assertPredicate(predicate, randomAlphaOfLengthBetween(1, 8) + "*", false);
predicate = new UserRoleMapper.DistinguishedNamePredicate("foo=");
assertPredicate(predicate, null, false);
assertPredicate(predicate, "foo", false);
assertPredicate(predicate, "foo=", true);
assertPredicate(predicate, randomAlphaOfLengthBetween(5, 12), false);
assertPredicate(predicate, randomAlphaOfLengthBetween(5, 12) + "*", false);
predicate = new UserRoleMapper.DistinguishedNamePredicate("=bar");
assertPredicate(predicate, null, false);
assertPredicate(predicate, "bar", false);
assertPredicate(predicate, "=bar", true);
assertPredicate(predicate, randomAlphaOfLengthBetween(5, 12), false);
assertPredicate(predicate, randomAlphaOfLengthBetween(5, 12) + "*", false);
}
private void assertPredicate(Predicate<FieldValue> predicate, Object value, boolean expected) {

45
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/ExpressionRoleMappingTests.java
View File

@ -31,6 +31,7 @@ import org.elasticsearch.xpack.core.security.authc.RealmConfig;
import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping;
import org.elasticsearch.xpack.core.security.authc.support.mapper.TemplateRoleName;
import org.elasticsearch.xpack.core.security.authc.support.mapper.expressiondsl.AllExpression;
import org.elasticsearch.xpack.core.security.authc.support.mapper.expressiondsl.AnyExpression;
import org.elasticsearch.xpack.core.security.authc.support.mapper.expressiondsl.FieldExpression;
import org.elasticsearch.xpack.security.authc.support.UserRoleMapper;
import org.hamcrest.Matchers;
@ -59,7 +60,7 @@ public class ExpressionRoleMappingTests extends ESTestCase {
Settings.EMPTY, Mockito.mock(Environment.class), new ThreadContext(Settings.EMPTY));
}
public void testParseValidJsonWithFixedRoleNames() throws Exception {
public void testValidExpressionWithFixedRoleNames() throws Exception {
String json = "{"
+ "\"roles\": [ \"kibana_user\", \"sales\" ], "
+ "\"enabled\": true, "
@ -69,7 +70,7 @@ public class ExpressionRoleMappingTests extends ESTestCase {
+ " { \"except\": { \"field\": { \"metadata.active\" : false } } }"
+ " ]}"
+ "}";
final ExpressionRoleMapping mapping = parse(json, "ldap_sales");
ExpressionRoleMapping mapping = parse(json, "ldap_sales");
assertThat(mapping.getRoles(), Matchers.containsInAnyOrder("kibana_user", "sales"));
assertThat(mapping.getExpression(), instanceOf(AllExpression.class));
@ -96,12 +97,48 @@ public class ExpressionRoleMappingTests extends ESTestCase {
Collections.emptyList(), Collections.singletonMap("active", true), realm
);
final UserRoleMapper.UserData user4 = new UserRoleMapper.UserData(
"peter.null", null, Collections.emptyList(), Collections.singletonMap("active", true), realm
);
assertThat(mapping.getExpression().match(user1a.asModel()), equalTo(true));
assertThat(mapping.getExpression().match(user1b.asModel()), equalTo(true));
assertThat(mapping.getExpression().match(user1c.asModel()), equalTo(true));
assertThat(mapping.getExpression().match(user1d.asModel()), equalTo(true));
assertThat(mapping.getExpression().match(user2.asModel()), equalTo(false));
assertThat(mapping.getExpression().match(user3.asModel()), equalTo(false));
assertThat(mapping.getExpression().match(user2.asModel()), equalTo(false)); // metadata.active == false
assertThat(mapping.getExpression().match(user3.asModel()), equalTo(false)); // dn != ou=sales,dc=example,dc=com
assertThat(mapping.getExpression().match(user4.asModel()), equalTo(false)); // dn == null
// expression without dn
json = "{"
+ "\"roles\": [ \"superuser\", \"system_admin\", \"admin\" ], "
+ "\"enabled\": true, "
+ "\"rules\": { "
+ " \"any\": [ "
+ " { \"field\": { \"username\" : \"tony.stark\" } }, "
+ " { \"field\": { \"groups\": \"cn=admins,dc=stark-enterprises,dc=com\" } }"
+ " ]}"
+ "}";
mapping = parse(json, "stark_admin");
assertThat(mapping.getRoles(), Matchers.containsInAnyOrder("superuser", "system_admin", "admin"));
assertThat(mapping.getExpression(), instanceOf(AnyExpression.class));
final UserRoleMapper.UserData userTony = new UserRoleMapper.UserData(
"tony.stark", null, Collections.singletonList("Audi R8 owners"), Collections.singletonMap("boss", true), realm
);
final UserRoleMapper.UserData userPepper = new UserRoleMapper.UserData(
"pepper.potts", null, Arrays.asList("marvel", "cn=admins,dc=stark-enterprises,dc=com"), null, realm
);
final UserRoleMapper.UserData userMax = new UserRoleMapper.UserData(
"max.rockatansky", null, Collections.singletonList("bronze"), Collections.singletonMap("mad", true), realm
);
final UserRoleMapper.UserData userFinn = new UserRoleMapper.UserData(
"finn.hackleberry", null, Arrays.asList("hacker", null), null, realm
);
assertThat(mapping.getExpression().match(userTony.asModel()), equalTo(true));
assertThat(mapping.getExpression().match(userPepper.asModel()), equalTo(true));
assertThat(mapping.getExpression().match(userMax.asModel()), equalTo(false));
assertThat(mapping.getExpression().match(userFinn.asModel()), equalTo(false));
}
public void testParseValidJsonWithTemplatedRoleNames() throws Exception {