From 32c85598213cd5b17bc43793ae21adc74a58ab60 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 24 Feb 2022 23:26:14 +1100 Subject: [PATCH 1/2] Restrict run-as to realm and api_key authentication types This PR removes run-as support for authentication types other than realm and API key. The change essentially makes the behaviour closer to the existing one (in released versions) except for API keys. This is not to say that the existing behaviour is the best. But we need more time to agree on the new behaviour. Relates: #79809 --- .../core/security/authc/Authentication.java | 1 + .../security/authc/AuthenticationTests.java | 32 +------ .../xpack/security/authc/RunAsIntegTests.java | 29 +------ .../token/TransportCreateTokenAction.java | 2 - .../security/authc/AuthenticatorChain.java | 21 +++-- .../authc/AuthenticatorChainTests.java | 84 +++++++++++++++++++ 6 files changed, 106 insertions(+), 63 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index 3f2fa385845ad..314184a0189e6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -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(), diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java index 5ac45a8f41ef9..02277baee927e 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java @@ -207,9 +207,6 @@ 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(); @@ -217,9 +214,6 @@ public void testRunAsAuthenticationWithDomain() { 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(); @@ -227,9 +221,6 @@ public void testRunAsAuthenticationWithDomain() { 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(); @@ -237,9 +228,6 @@ public void testRunAsAuthenticationWithDomain() { 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(); @@ -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()) { @@ -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 { diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/RunAsIntegTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/RunAsIntegTests.java index 5a6a93ffaebe0..55fe797c27090 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/RunAsIntegTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/RunAsIntegTests.java @@ -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"); @@ -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( @@ -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) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java index 4918fd7081d91..491add9f05b6b 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java @@ -74,8 +74,6 @@ protected void doExecute(Task task, CreateTokenRequest request, ActionListener listener - ) { + // Package private for test + void maybeLookupRunAsUser(Authenticator.Context context, Authentication authentication, ActionListener listener) { if (false == runAsEnabled) { finishAuthentication(context, authentication, listener); return; @@ -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) + ); + finishAuthentication(context, authentication, listener); + return; + } + // Now we have a valid runAsUsername realmsAuthenticator.lookupRunAsUser(context, authentication, ActionListener.wrap(tuple -> { final Authentication finalAuth; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java index 4376c70119f52..e1539b16b0a1c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java @@ -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; @@ -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; @@ -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> listener = (ActionListener>) invocation.getArguments()[2]; + listener.onResponse(null); + return null; + }).when(realmsAuthenticator).lookupRunAsUser(any(), any(), any()); + final PlainActionFuture 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 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)); } From 9b571b44b59c65ec2b7cb25db2c2823b2b0c5ca3 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 28 Feb 2022 11:16:50 +1100 Subject: [PATCH 2/2] address feedback --- .../elasticsearch/xpack/security/authc/AuthenticatorChain.java | 2 +- .../xpack/security/authc/AuthenticatorChainTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java index a0cae6cc7087d..44cef4c8fe5ef 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java @@ -209,7 +209,7 @@ void maybeLookupRunAsUser(Authenticator.Context context, Authentication authenti 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 [{}]", + "ignore run-as header since it is currently not supported for authentication type [{}]", authentication.getAuthenticationType().name().toLowerCase(Locale.ROOT) ); finishAuthentication(context, authentication, listener); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java index e1539b16b0a1c..2d729f509d3fd 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java @@ -353,7 +353,7 @@ public void testRunAsIsIgnoredForUnsupportedAuthenticationTypes() throws Illegal "run-as", AuthenticatorChain.class.getName(), Level.INFO, - "ignore run-as header since it is not supported for authentication type [" + "ignore run-as header since it is currently not supported for authentication type [" + authentication.getAuthenticationType().name().toLowerCase(Locale.ROOT) + "]" )