Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict run-as to realm and api_key authentication types #84336

Merged
merged 3 commits into from
Feb 28, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ public Authentication runAs(User runAs, @Nullable RealmRef lookupRealmRef) {
Objects.requireNonNull(runAs);
assert false == runAs.isRunAs();
assert false == getUser().isRunAs();
assert AuthenticationType.REALM == getAuthenticationType() || AuthenticationType.API_KEY == getAuthenticationType();
return new Authentication(
new User(runAs, getUser()),
getAuthenticatedBy(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,39 +207,27 @@ public void testRunAsAuthenticationWithDomain() {
RealmRef lookupRealmRef = randomRealmRef(true);
// realm/token run-as
Authentication test = Authentication.newRealmAuthentication(randomUser(), authnRealmRef);
if (randomBoolean()) {
test = test.token();
}
test = test.runAs(randomUser(), lookupRealmRef);
if (randomBoolean()) {
test = test.token();
}
assertThat(test.isAssignedToDomain(), is(true));
assertThat(test.getDomain(), is(lookupRealmRef.getDomain()));
test = Authentication.newRealmAuthentication(randomUser(), randomRealmRef(false));
if (randomBoolean()) {
test = test.token();
}
test = test.runAs(randomUser(), lookupRealmRef);
if (randomBoolean()) {
test = test.token();
}
assertThat(test.isAssignedToDomain(), is(true));
assertThat(test.getDomain(), is(lookupRealmRef.getDomain()));
test = Authentication.newRealmAuthentication(randomUser(), authnRealmRef);
if (randomBoolean()) {
test = test.token();
}
test = test.runAs(randomUser(), randomRealmRef(false));
if (randomBoolean()) {
test = test.token();
}
assertThat(test.isAssignedToDomain(), is(false));
assertThat(test.getDomain(), nullValue());
test = Authentication.newRealmAuthentication(randomUser(), randomRealmRef(false));
if (randomBoolean()) {
test = test.token();
}
test = test.runAs(randomUser(), lookupRealmRef);
if (randomBoolean()) {
test = test.token();
Expand All @@ -248,9 +236,6 @@ public void testRunAsAuthenticationWithDomain() {
assertThat(test.getDomain(), is(lookupRealmRef.getDomain()));
// api key run-as
test = randomApiKeyAuthentication(randomUser(), randomAlphaOfLengthBetween(10, 20), Version.CURRENT);
if (randomBoolean()) {
test = test.token();
}
assertThat(test.isAssignedToDomain(), is(false));
assertThat(test.getDomain(), nullValue());
if (randomBoolean()) {
Expand All @@ -268,25 +253,10 @@ public void testRunAsAuthenticationWithDomain() {
assertThat(test.isAssignedToDomain(), is(false));
assertThat(test.getDomain(), nullValue());
}
// service account run-as
// service account cannot run-as
test = randomServiceAccountAuthentication();
assertThat(test.isAssignedToDomain(), is(false));
assertThat(test.getDomain(), nullValue());
if (randomBoolean()) {
test = test.runAs(randomUser(), lookupRealmRef);
if (randomBoolean()) {
test = test.token();
}
assertThat(test.isAssignedToDomain(), is(true));
assertThat(test.getDomain(), is(lookupRealmRef.getDomain()));
} else {
test = test.runAs(randomUser(), randomRealmRef(false));
if (randomBoolean()) {
test = test.token();
}
assertThat(test.isAssignedToDomain(), is(false));
assertThat(test.getDomain(), nullValue());
}
}

public void testDomainSerialize() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ public void testRunAsUsingApiKey() throws IOException {
);
assertThat(authenticateJsonView.get("username"), equalTo(runAsTestUser ? SecuritySettingsSource.TEST_USER_NAME : NO_ROLE_USER));
assertThat(authenticateJsonView.get("authentication_realm.type"), equalTo("_es_api_key"));
assertThat(authenticateJsonView.get("lookup_realm.type"), equalTo("file"));
assertThat(authenticateJsonView.get("authentication_type"), equalTo("api_key"));

final Request getUserRequest = new Request("GET", "/_security/user");
Expand All @@ -195,7 +196,7 @@ public void testRunAsUsingApiKey() throws IOException {
}
}

public void testRunAsUsingOAuthToken() throws IOException {
public void testRunAsIgnoredForOAuthToken() throws IOException {
final Request createTokenRequest = new Request("POST", "/_security/oauth2/token");
createTokenRequest.setJsonEntity("{\"grant_type\":\"client_credentials\"}");
createTokenRequest.setOptions(
Expand All @@ -208,41 +209,19 @@ public void testRunAsUsingOAuthToken() throws IOException {
createTokenResponse.getEntity().getContent()
);

final boolean runAsTestUser = randomBoolean();

final Request authenticateRequest = new Request("GET", "/_security/_authenticate");
authenticateRequest.setOptions(
authenticateRequest.getOptions()
.toBuilder()
.addHeader("Authorization", "Bearer " + tokenMapView.get("access_token"))
.addHeader(
AuthenticationServiceField.RUN_AS_USER_HEADER,
runAsTestUser ? SecuritySettingsSource.TEST_USER_NAME : NO_ROLE_USER
)
.addHeader(AuthenticationServiceField.RUN_AS_USER_HEADER, SecuritySettingsSource.TEST_USER_NAME)
);
final Response authenticateResponse = getRestClient().performRequest(authenticateRequest);
final XContentTestUtils.JsonMapView authenticateJsonView = XContentTestUtils.createJsonMapView(
authenticateResponse.getEntity().getContent()
);
assertThat(authenticateJsonView.get("username"), equalTo(runAsTestUser ? SecuritySettingsSource.TEST_USER_NAME : NO_ROLE_USER));
assertThat(authenticateJsonView.get("username"), equalTo(RUN_AS_USER));
assertThat(authenticateJsonView.get("authentication_type"), equalTo("token"));

final Request getUserRequest = new Request("GET", "/_security/user");
getUserRequest.setOptions(
getUserRequest.getOptions()
.toBuilder()
.addHeader("Authorization", "Bearer " + tokenMapView.get("access_token"))
.addHeader(
AuthenticationServiceField.RUN_AS_USER_HEADER,
runAsTestUser ? SecuritySettingsSource.TEST_USER_NAME : NO_ROLE_USER
)
);
if (runAsTestUser) {
assertThat(getRestClient().performRequest(getUserRequest).getStatusLine().getStatusCode(), equalTo(200));
} else {
final ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(getUserRequest));
assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(403));
}
}

private static Request requestForUserRunAsUser(String user) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ protected void doExecute(Task task, CreateTokenRequest request, ActionListener<C
Authentication authentication = securityContext.getAuthentication();
if (authentication.isServiceAccount()) {
// Service account itself cannot create OAuth2 tokens.
// But it is possible to create an oauth2 token if the service account run-as a different user.
// In this case, the token will be created for the run-as user (not the service account).
listener.onFailure(new ElasticsearchException("OAuth2 token creation is not supported for service accounts"));
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.elasticsearch.xpack.security.operator.OperatorPrivileges.OperatorPrivilegesService;

import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
Expand Down Expand Up @@ -189,11 +190,8 @@ private BiConsumer<Authenticator, ActionListener<AuthenticationResult<Authentica
};
}

private void maybeLookupRunAsUser(
Authenticator.Context context,
Authentication authentication,
ActionListener<Authentication> listener
) {
// Package private for test
void maybeLookupRunAsUser(Authenticator.Context context, Authentication authentication, ActionListener<Authentication> listener) {
if (false == runAsEnabled) {
finishAuthentication(context, authentication, listener);
return;
Expand All @@ -205,6 +203,19 @@ private void maybeLookupRunAsUser(
return;
}

// Run-as is supported for authentication with realm or api_key. Run-as for other authentication types is ignored.
// Both realm user and api_key can create tokens. They can also run-as another user and create tokens.
// In both cases, the created token will have a TOKEN authentication type and hence does not support run-as.
if (Authentication.AuthenticationType.REALM != authentication.getAuthenticationType()
&& Authentication.AuthenticationType.API_KEY != authentication.getAuthenticationType()) {
logger.info(
"ignore run-as header since it is not supported for authentication type [{}]",
authentication.getAuthenticationType().name().toLowerCase(Locale.ROOT)
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging at INFO level instead of WARN to avoid causing unnecessary worries. Also didn't add warning to response header because I am not sure whether it would be another unpleasant experience in Kibana (warning message spamming in dashboard etc).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a safe choice, but when we decide what to do in 8.2 we'll need to take into account that we've ignored the headers up until this point (without warning).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's a tricky situation. Even supporting API key run-as in this release has this same issue. It's in a gray area of "breaking change" vs "bug fix" vs "feature". It's probably a stretch to call it a "bug fix" since we intentionally not supporting things like token. But I also really don't think it should be breaking. I'll tweak the logging message a little bit to say:

"... it is currently not supported ..." to indicate the support might be coming.

This subtlety might not be that useful. It is definitely not officical because it is not a proper deprecation message or anything. But we don't have such tooling right now. So some subtlety in the wording might be our best bet right now.

finishAuthentication(context, authentication, listener);
return;
}

// Now we have a valid runAsUsername
realmsAuthenticator.lookupRunAsUser(context, authentication, ActionListener.wrap(tuple -> {
final Authentication finalAuth;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,24 @@

package org.elasticsearch.xpack.security.authc;

import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.node.Node;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.MockLogAppender;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
import org.elasticsearch.xpack.core.security.authc.AuthenticationServiceField;
import org.elasticsearch.xpack.core.security.authc.AuthenticationTests;
import org.elasticsearch.xpack.core.security.authc.AuthenticationToken;
import org.elasticsearch.xpack.core.security.authc.Realm;
import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer;
Expand All @@ -31,10 +38,13 @@

import java.io.IOException;
import java.util.List;
import java.util.Locale;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
Expand Down Expand Up @@ -285,6 +295,80 @@ public void testUnsuccessfulOAuth2TokenOrApiKeyWillNotFallToAnonymousOrReportMis
);
}

public void testMaybeLookupRunAsUser() {
final Authentication authentication = randomFrom(
AuthenticationTests.randomApiKeyAuthentication(AuthenticationTests.randomUser(), randomAlphaOfLength(20)),
AuthenticationTests.randomRealmAuthentication(false)
);
final String runAsUsername = "your-run-as-username";
threadContext.putHeader(AuthenticationServiceField.RUN_AS_USER_HEADER, runAsUsername);
assertThat(authentication.getUser().principal(), not(equalTo(runAsUsername)));

final AuthenticationService.AuditableRequest auditableRequest = mock(AuthenticationService.AuditableRequest.class);
final Authenticator.Context context = new Authenticator.Context(threadContext, auditableRequest, null, true, realms);

doAnswer(invocation -> {
@SuppressWarnings("unchecked")
final ActionListener<Tuple<User, Realm>> listener = (ActionListener<Tuple<User, Realm>>) invocation.getArguments()[2];
listener.onResponse(null);
return null;
}).when(realmsAuthenticator).lookupRunAsUser(any(), any(), any());
final PlainActionFuture<Authentication> future = new PlainActionFuture<>();
authenticatorChain.maybeLookupRunAsUser(context, authentication, future);
future.actionGet();
verify(realmsAuthenticator).lookupRunAsUser(eq(context), eq(authentication), any());
}

public void testRunAsIsIgnoredForUnsupportedAuthenticationTypes() throws IllegalAccessException {
final Authentication authentication = randomFrom(
AuthenticationTests.randomApiKeyAuthentication(AuthenticationTests.randomUser(), randomAlphaOfLength(20)).token(),
AuthenticationTests.randomRealmAuthentication(false).token(),
AuthenticationTests.randomServiceAccountAuthentication(),
AuthenticationTests.randomAnonymousAuthentication(),
AuthenticationTests.randomInternalAuthentication()
);
threadContext.putHeader(AuthenticationServiceField.RUN_AS_USER_HEADER, "you-shall-not-pass");
assertThat(
authentication.getUser().principal(),
not(equalTo(threadContext.getHeader(AuthenticationServiceField.RUN_AS_USER_HEADER)))
);

final AuthenticationService.AuditableRequest auditableRequest = mock(AuthenticationService.AuditableRequest.class);
final Authenticator.Context context = new Authenticator.Context(threadContext, auditableRequest, null, true, realms);

doAnswer(invocation -> {
fail("should not reach here");
return null;
}).when(realmsAuthenticator).lookupRunAsUser(any(), any(), any());

final Logger logger = LogManager.getLogger(AuthenticatorChain.class);
Loggers.setLevel(logger, Level.INFO);
final MockLogAppender appender = new MockLogAppender();
Loggers.addAppender(logger, appender);
appender.start();

try {
appender.addExpectation(
new MockLogAppender.SeenEventExpectation(
"run-as",
AuthenticatorChain.class.getName(),
Level.INFO,
"ignore run-as header since it is not supported for authentication type ["
+ authentication.getAuthenticationType().name().toLowerCase(Locale.ROOT)
+ "]"
)
);
final PlainActionFuture<Authentication> future = new PlainActionFuture<>();
authenticatorChain.maybeLookupRunAsUser(context, authentication, future);
assertThat(future.actionGet(), equalTo(authentication));
appender.assertAllExpectationsMatched();
} finally {
appender.stop();
Loggers.setLevel(logger, Level.INFO);
Loggers.removeAppender(logger, appender);
}
}

private Authenticator.Context createAuthenticatorContext() {
return createAuthenticatorContext(mock(AuthenticationService.AuditableRequest.class));
}
Expand Down