mirror of
https://github.com/honeymoose/OpenSearch.git
synced 2025-02-26 14:54:56 +00:00
[Security] Handle non-existent user in native realm (elastic/x-pack-elasticsearch#2044)
Since change elastic/x-pack-elasticsearch@f796949 authentication is not allowed to respond with null, it must be AuthenticationResult.notHandled() - Fixes 1 case where the native realm would respond null if the user was not found - Fixes some edge cases in the LDAP realm. Original commit: elastic/x-pack-elasticsearch@bc739a1d40
This commit is contained in:
Tim Vernum
GitHub
parent
1752104140
commit
b29f7a9ddb
@ -68,7 +68,7 @@ import java.util.function.Consumer;
|
||||
public class NativeUsersStore extends AbstractComponent {
|
||||
|
||||
public static final String INDEX_TYPE = "doc";
|
||||
private static final String USER_DOC_TYPE = "user";
|
||||
static final String USER_DOC_TYPE = "user";
|
||||
public static final String RESERVED_USER_TYPE = "reserved-user";
|
||||
|
||||
private final Hasher hasher = Hasher.BCRYPT;
|
||||
@ -511,7 +511,7 @@ public class NativeUsersStore extends AbstractComponent {
|
||||
void verifyPassword(String username, final SecureString password, ActionListener<AuthenticationResult> listener) {
|
||||
getUserAndPassword(username, ActionListener.wrap((userAndPassword) -> {
|
||||
if (userAndPassword == null || userAndPassword.passwordHash() == null) {
|
||||
listener.onResponse(null);
|
||||
listener.onResponse(AuthenticationResult.notHandled());
|
||||
} else if (hasher.verify(password, userAndPassword.passwordHash())) {
|
||||
listener.onResponse(AuthenticationResult.success(userAndPassword.user()));
|
||||
} else {
|
||||
|
||||
@ -5,6 +5,15 @@
|
||||
*/
|
||||
package org.elasticsearch.xpack.security.authc.ldap;
|
||||
|
||||
import java.util.HashSet;
|
||||
import java.util.Map;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.function.Consumer;
|
||||
import java.util.function.Function;
|
||||
import java.util.function.Supplier;
|
||||
|
||||
import com.unboundid.ldap.sdk.LDAPException;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
import org.apache.logging.log4j.message.ParameterizedMessage;
|
||||
@ -37,13 +46,6 @@ import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingSt
|
||||
import org.elasticsearch.xpack.security.user.User;
|
||||
import org.elasticsearch.xpack.ssl.SSLService;
|
||||
|
||||
import java.util.HashSet;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.function.Consumer;
|
||||
import java.util.function.Supplier;
|
||||
|
||||
|
||||
/**
|
||||
* Authenticates username/password tokens against ldap, locates groups and maps them to roles.
|
||||
@ -145,9 +147,11 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
|
||||
protected void doAuthenticate(UsernamePasswordToken token, ActionListener<AuthenticationResult> listener) {
|
||||
// we submit to the threadpool because authentication using LDAP will execute blocking I/O for a bind request and we don't want
|
||||
// network threads stuck waiting for a socket to connect. After the bind, then all interaction with LDAP should be async
|
||||
final CancellableLdapRunnable cancellableLdapRunnable = new CancellableLdapRunnable(listener,
|
||||
final CancellableLdapRunnable<AuthenticationResult> cancellableLdapRunnable = new CancellableLdapRunnable<>(listener,
|
||||
ex -> AuthenticationResult.unsuccessful("Authentication against realm [" + this.toString() + "] failed", ex),
|
||||
() -> sessionFactory.session(token.principal(), token.credentials(),
|
||||
contextPreservingListener(new LdapSessionActionListener("authenticate", token.principal(), listener))), logger);
|
||||
contextPreservingListener(new LdapSessionActionListener("authenticate", token.principal(), listener))), logger
|
||||
);
|
||||
threadPool.generic().execute(cancellableLdapRunnable);
|
||||
threadPool.schedule(executionTimeout, Names.SAME, cancellableLdapRunnable::maybeTimeout);
|
||||
}
|
||||
@ -159,7 +163,7 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
|
||||
// network threads stuck waiting for a socket to connect. After the bind, then all interaction with LDAP should be async
|
||||
final ActionListener<AuthenticationResult> sessionListener = ActionListener.wrap(AuthenticationResult::getUser,
|
||||
userActionListener::onFailure);
|
||||
final CancellableLdapRunnable cancellableLdapRunnable = new CancellableLdapRunnable(userActionListener,
|
||||
final CancellableLdapRunnable<User> cancellableLdapRunnable = new CancellableLdapRunnable<>(userActionListener, e -> null,
|
||||
() -> sessionFactory.unauthenticatedSession(username,
|
||||
contextPreservingListener(new LdapSessionActionListener("lookup", username, sessionListener))), logger);
|
||||
threadPool.generic().execute(cancellableLdapRunnable);
|
||||
@ -193,7 +197,7 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
|
||||
private static void buildUser(LdapSession session, String username, ActionListener<AuthenticationResult> listener,
|
||||
UserRoleMapper roleMapper) {
|
||||
if (session == null) {
|
||||
listener.onResponse(null);
|
||||
listener.onResponse(AuthenticationResult.notHandled());
|
||||
} else {
|
||||
boolean loadingGroups = false;
|
||||
try {
|
||||
@ -250,7 +254,7 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
|
||||
@Override
|
||||
public void onResponse(LdapSession session) {
|
||||
if (session == null) {
|
||||
resultListener.onResponse(null);
|
||||
resultListener.onResponse(AuthenticationResult.notHandled());
|
||||
} else {
|
||||
ldapSessionAtomicReference.set(session);
|
||||
buildUser(session, username, resultListener, roleMapper);
|
||||
@ -275,15 +279,17 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
|
||||
* be queued and not executed for a long time or ever and this causes user requests to appear
|
||||
* to hang. In these cases at least we can provide a response.
|
||||
*/
|
||||
static class CancellableLdapRunnable extends AbstractRunnable {
|
||||
static class CancellableLdapRunnable<T> extends AbstractRunnable {
|
||||
|
||||
private final Runnable in;
|
||||
private final ActionListener<?> listener;
|
||||
private final ActionListener<T> listener;
|
||||
private final Function<Exception, T> defaultValue;
|
||||
private final Logger logger;
|
||||
private final AtomicReference<LdapRunnableState> state = new AtomicReference<>(LdapRunnableState.AWAITING_EXECUTION);
|
||||
|
||||
CancellableLdapRunnable(ActionListener<?> listener, Runnable in, Logger logger) {
|
||||
CancellableLdapRunnable(ActionListener<T> listener, Function<Exception, T> defaultValue, Runnable in, Logger logger) {
|
||||
this.listener = listener;
|
||||
this.defaultValue = Objects.requireNonNull(defaultValue);
|
||||
this.in = in;
|
||||
this.logger = logger;
|
||||
}
|
||||
@ -291,9 +297,8 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
|
||||
@Override
|
||||
public void onFailure(Exception e) {
|
||||
logger.error("execution of ldap runnable failed", e);
|
||||
// this is really a exceptional state but just call the listener and maybe another realm can authenticate, otherwise
|
||||
// something as simple as a down ldap server/network error takes down auth
|
||||
listener.onResponse(null);
|
||||
final T result = defaultValue.apply(e);
|
||||
listener.onResponse(result);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@ -5,6 +5,13 @@
|
||||
*/
|
||||
package org.elasticsearch.xpack.security.authc.esnative;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.CopyOnWriteArrayList;
|
||||
|
||||
import org.elasticsearch.action.Action;
|
||||
import org.elasticsearch.action.ActionListener;
|
||||
import org.elasticsearch.action.ActionRequest;
|
||||
@ -15,26 +22,26 @@ import org.elasticsearch.action.get.GetResponse;
|
||||
import org.elasticsearch.action.support.PlainActionFuture;
|
||||
import org.elasticsearch.action.support.WriteRequest;
|
||||
import org.elasticsearch.action.update.UpdateRequest;
|
||||
import org.elasticsearch.common.bytes.BytesReference;
|
||||
import org.elasticsearch.common.collect.Tuple;
|
||||
import org.elasticsearch.common.settings.SecureString;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.index.get.GetResult;
|
||||
import org.elasticsearch.test.ESTestCase;
|
||||
import org.elasticsearch.xpack.security.InternalClient;
|
||||
import org.elasticsearch.xpack.security.SecurityLifecycleService;
|
||||
import org.elasticsearch.xpack.security.authc.AuthenticationResult;
|
||||
import org.elasticsearch.xpack.security.authc.support.Hasher;
|
||||
import org.elasticsearch.xpack.security.user.ElasticUser;
|
||||
import org.elasticsearch.xpack.security.user.KibanaUser;
|
||||
import org.elasticsearch.xpack.security.user.LogstashSystemUser;
|
||||
import org.elasticsearch.xpack.security.user.User;
|
||||
import org.junit.Before;
|
||||
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.CopyOnWriteArrayList;
|
||||
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
||||
import static org.hamcrest.CoreMatchers.containsString;
|
||||
import static org.hamcrest.CoreMatchers.equalTo;
|
||||
import static org.hamcrest.CoreMatchers.notNullValue;
|
||||
import static org.hamcrest.CoreMatchers.nullValue;
|
||||
import static org.mockito.Matchers.any;
|
||||
import static org.mockito.Mockito.doAnswer;
|
||||
@ -114,6 +121,73 @@ public class NativeUsersStoreTests extends ESTestCase {
|
||||
assertThat(userInfo.passwordHash, equalTo(ReservedRealm.EMPTY_PASSWORD_HASH));
|
||||
}
|
||||
|
||||
public void testVerifyUserWithCorrectPassword() throws Exception {
|
||||
final NativeUsersStore nativeUsersStore = startNativeUsersStore();
|
||||
final String username = randomAlphaOfLengthBetween(4, 12);
|
||||
final SecureString password = new SecureString(randomAlphaOfLengthBetween(8, 16).toCharArray());
|
||||
final String[] roles = generateRandomStringArray(4, 12, false, false);
|
||||
|
||||
final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
|
||||
nativeUsersStore.verifyPassword(username, password, future);
|
||||
|
||||
respondToGetUserRequest(username, password, roles);
|
||||
|
||||
final AuthenticationResult result = future.get();
|
||||
assertThat(result, notNullValue());
|
||||
assertThat(result.getStatus(), equalTo(AuthenticationResult.Status.SUCCESS));
|
||||
final User user = result.getUser();
|
||||
assertThat(user, notNullValue());
|
||||
assertThat(user.enabled(), equalTo(true));
|
||||
assertThat(user.principal(), equalTo(username));
|
||||
assertThat(user.roles(), equalTo(roles));
|
||||
assertThat(user.authenticatedUser(), equalTo(user));
|
||||
}
|
||||
|
||||
public void testVerifyUserWithIncorrectPassword() throws Exception {
|
||||
final NativeUsersStore nativeUsersStore = startNativeUsersStore();
|
||||
final String username = randomAlphaOfLengthBetween(4, 12);
|
||||
final SecureString correctPassword = new SecureString(randomAlphaOfLengthBetween(12, 16).toCharArray());
|
||||
final SecureString incorrectPassword = new SecureString(randomAlphaOfLengthBetween(8, 10).toCharArray());
|
||||
final String[] roles = generateRandomStringArray(4, 12, false, false);
|
||||
|
||||
final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
|
||||
nativeUsersStore.verifyPassword(username, incorrectPassword, future);
|
||||
|
||||
respondToGetUserRequest(username, correctPassword, roles);
|
||||
|
||||
final AuthenticationResult result = future.get();
|
||||
assertThat(result, notNullValue());
|
||||
assertThat(result.getStatus(), equalTo(AuthenticationResult.Status.CONTINUE));
|
||||
assertThat(result.getUser(), nullValue());
|
||||
assertThat(result.getMessage(), containsString("authentication failed"));
|
||||
}
|
||||
|
||||
public void testVerifyNonExistentUser() throws Exception {
|
||||
final NativeUsersStore nativeUsersStore = startNativeUsersStore();
|
||||
final String username = randomAlphaOfLengthBetween(4, 12);
|
||||
final SecureString password = new SecureString(randomAlphaOfLengthBetween(8, 16).toCharArray());
|
||||
|
||||
final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
|
||||
nativeUsersStore.verifyPassword(username, password, future);
|
||||
|
||||
final GetResult getResult = new GetResult(
|
||||
SecurityLifecycleService.SECURITY_INDEX_NAME,
|
||||
NativeUsersStore.INDEX_TYPE,
|
||||
NativeUsersStore.getIdForUser(NativeUsersStore.USER_DOC_TYPE, username),
|
||||
1L,
|
||||
false,
|
||||
null,
|
||||
Collections.emptyMap());
|
||||
|
||||
actionRespond(GetRequest.class, new GetResponse(getResult));
|
||||
|
||||
final AuthenticationResult result = future.get();
|
||||
assertThat(result, notNullValue());
|
||||
assertThat(result.getStatus(), equalTo(AuthenticationResult.Status.CONTINUE));
|
||||
assertThat(result.getUser(), nullValue());
|
||||
assertThat(result.getMessage(), nullValue());
|
||||
}
|
||||
|
||||
private <ARequest extends ActionRequest, AResponse extends ActionResponse> ARequest actionRespond(Class<ARequest> requestClass,
|
||||
AResponse response) {
|
||||
Tuple<ARequest, ActionListener<?>> tuple = findRequest(requestClass);
|
||||
@ -129,6 +203,26 @@ public class NativeUsersStoreTests extends ESTestCase {
|
||||
.findFirst().orElseThrow(() -> new RuntimeException("Cannot find request of type " + requestClass));
|
||||
}
|
||||
|
||||
private void respondToGetUserRequest(String username, SecureString password, String[] roles) throws IOException {
|
||||
final Map<String, Object> values = new HashMap<>();
|
||||
values.put(User.Fields.USERNAME.getPreferredName(), username);
|
||||
values.put(User.Fields.PASSWORD.getPreferredName(), String.valueOf(Hasher.BCRYPT.hash(password)));
|
||||
values.put(User.Fields.ROLES.getPreferredName(), roles);
|
||||
values.put(User.Fields.ENABLED.getPreferredName(), Boolean.TRUE);
|
||||
values.put(User.Fields.TYPE.getPreferredName(), NativeUsersStore.USER_DOC_TYPE);
|
||||
final BytesReference source = jsonBuilder().map(values).bytes();
|
||||
final GetResult getResult = new GetResult(
|
||||
SecurityLifecycleService.SECURITY_INDEX_NAME,
|
||||
NativeUsersStore.INDEX_TYPE,
|
||||
NativeUsersStore.getIdForUser(NativeUsersStore.USER_DOC_TYPE, username),
|
||||
1L,
|
||||
true,
|
||||
source,
|
||||
Collections.emptyMap());
|
||||
|
||||
|
||||
actionRespond(GetRequest.class, new GetResponse(getResult));
|
||||
}
|
||||
private NativeUsersStore startNativeUsersStore() {
|
||||
SecurityLifecycleService securityLifecycleService = mock(SecurityLifecycleService.class);
|
||||
when(securityLifecycleService.isSecurityIndexAvailable()).thenReturn(true);
|
||||
|
||||
@ -5,17 +5,18 @@
|
||||
*/
|
||||
package org.elasticsearch.xpack.security.authc.ldap;
|
||||
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
import org.elasticsearch.ElasticsearchTimeoutException;
|
||||
import org.elasticsearch.action.ActionListener;
|
||||
import org.elasticsearch.test.ESTestCase;
|
||||
import org.elasticsearch.xpack.security.authc.ldap.LdapRealm.CancellableLdapRunnable;
|
||||
import org.elasticsearch.xpack.security.user.User;
|
||||
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.hamcrest.Matchers.instanceOf;
|
||||
import static org.hamcrest.Matchers.sameInstance;
|
||||
|
||||
@ -25,10 +26,10 @@ public class CancellableLdapRunnableTests extends ESTestCase {
|
||||
AtomicReference<Exception> exceptionAtomicReference = new AtomicReference<>();
|
||||
final CancellableLdapRunnable runnable =
|
||||
new CancellableLdapRunnable(ActionListener.wrap(user -> {
|
||||
throw new AssertionError("onResponse should not be called");
|
||||
}, exceptionAtomicReference::set), () -> {
|
||||
throw new AssertionError("runnable should not be executed");
|
||||
}, logger);
|
||||
throw new AssertionError("onResponse should not be called");
|
||||
}, exceptionAtomicReference::set), e -> null, () -> {
|
||||
throw new AssertionError("runnable should not be executed");
|
||||
}, logger);
|
||||
|
||||
runnable.maybeTimeout();
|
||||
runnable.run();
|
||||
@ -45,10 +46,10 @@ public class CancellableLdapRunnableTests extends ESTestCase {
|
||||
new CancellableLdapRunnable(ActionListener.wrap(user -> {
|
||||
listenerCalled.set(true);
|
||||
throw new AssertionError("onResponse should not be called");
|
||||
}, e -> {
|
||||
}, e -> {
|
||||
listenerCalled.set(true);
|
||||
throw new AssertionError("onFailure should not be called");
|
||||
}), () -> ran.set(ran.get() == false), logger);
|
||||
}), e -> null, () -> ran.set(ran.get() == false), logger);
|
||||
|
||||
runnable.run();
|
||||
assertTrue(ran.get());
|
||||
@ -63,10 +64,10 @@ public class CancellableLdapRunnableTests extends ESTestCase {
|
||||
AtomicReference<Exception> exceptionAtomicReference = new AtomicReference<>();
|
||||
final CancellableLdapRunnable runnable =
|
||||
new CancellableLdapRunnable(ActionListener.wrap(user -> {
|
||||
throw new AssertionError("onResponse should not be called");
|
||||
}, exceptionAtomicReference::set), () -> {
|
||||
throw new AssertionError("runnable should not be executed");
|
||||
}, logger);
|
||||
throw new AssertionError("onResponse should not be called");
|
||||
}, exceptionAtomicReference::set), e -> null, () -> {
|
||||
throw new AssertionError("runnable should not be executed");
|
||||
}, logger);
|
||||
|
||||
final Exception e = new RuntimeException("foo");
|
||||
runnable.onRejection(e);
|
||||
@ -84,7 +85,7 @@ public class CancellableLdapRunnableTests extends ESTestCase {
|
||||
}, e -> {
|
||||
throw new AssertionError("onFailure should not be executed");
|
||||
});
|
||||
final CancellableLdapRunnable runnable = new CancellableLdapRunnable(listener, () -> {
|
||||
final CancellableLdapRunnable runnable = new CancellableLdapRunnable(listener, e -> null, () -> {
|
||||
runningLatch.countDown();
|
||||
try {
|
||||
timeoutCalledLatch.await();
|
||||
@ -102,4 +103,20 @@ public class CancellableLdapRunnableTests extends ESTestCase {
|
||||
listenerCalledLatch.await();
|
||||
t.join();
|
||||
}
|
||||
|
||||
public void testExceptionInRunnable() {
|
||||
AtomicReference<String> resultRef = new AtomicReference<>();
|
||||
final ActionListener<String> listener = ActionListener.wrap(resultRef::set, e -> {
|
||||
throw new AssertionError("onFailure should not be executed");
|
||||
});
|
||||
String defaultValue = randomAlphaOfLengthBetween(2, 10);
|
||||
final CancellableLdapRunnable<String> runnable = new CancellableLdapRunnable<>(listener, e -> defaultValue,
|
||||
() -> {
|
||||
throw new RuntimeException("runnable intentionally failed");
|
||||
}, logger);
|
||||
|
||||
runnable.run();
|
||||
assertThat(resultRef.get(), equalTo(defaultValue));
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user