diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java index 8252c7f9a9dce..98d935d637c80 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java @@ -154,7 +154,15 @@ it to the action without an associated user (not via REST or transport - this is */ final String securityAction = actionMapper.action(action, request); authcService.authenticate(securityAction, request, SystemUser.INSTANCE, - ActionListener.wrap((authc) -> authorizeRequest(authc, securityAction, request, listener), listener::onFailure)); + ActionListener.wrap((authc) -> { + if (authc != null) { + authorizeRequest(authc, securityAction, request, listener); + } else if (licenseState.isAuthAllowed() == false) { + listener.onResponse(null); + } else { + listener.onFailure(new IllegalStateException("no authentication present but auth is allowed")); + } + }, listener::onFailure)); } private void authorizeRequest(Authentication authentication, String securityAction, Request request, diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlAuthenticateAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlAuthenticateAction.java index 616f6184c3884..2cd55ea150cc8 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlAuthenticateAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlAuthenticateAction.java @@ -60,6 +60,7 @@ protected void doExecute(SamlAuthenticateRequest request, listener.onFailure(new IllegalStateException("Cannot find AuthenticationResult on thread context")); return; } + assert authentication != null : "authentication should never be null at this point"; final Map tokenMeta = (Map) result.getMetadata().get(SamlRealm.CONTEXT_TOKEN_DATA); tokenService.createUserToken(authentication, originatingAuthentication, ActionListener.wrap(tuple -> { 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 566bc20966b3b..5d94ae5e8a61f 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 @@ -73,7 +73,11 @@ private void authenticateAndCreateToken(CreateTokenRequest request, ActionListen authenticationService.authenticate(CreateTokenAction.NAME, request, authToken, ActionListener.wrap(authentication -> { request.getPassword().close(); - createToken(request, authentication, originatingAuthentication, true, listener); + if (authentication != null) { + createToken(request, authentication, originatingAuthentication, true, listener); + } else { + listener.onFailure(new UnsupportedOperationException("cannot create token if authentication is not allowed")); + } }, e -> { // clear the request password request.getPassword().close(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java index e9756925ce6de..734fde7cca937 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java @@ -135,8 +135,9 @@ class Authenticator { private final AuditableRequest request; private final User fallbackUser; - + private final List defaultOrderedRealmList; private final ActionListener listener; + private RealmRef authenticatedBy = null; private RealmRef lookedupBy = null; private AuthenticationToken authenticationToken = null; @@ -154,6 +155,7 @@ class Authenticator { private Authenticator(AuditableRequest auditableRequest, User fallbackUser, ActionListener listener) { this.request = auditableRequest; this.fallbackUser = fallbackUser; + this.defaultOrderedRealmList = realms.asList(); this.listener = listener; } @@ -172,27 +174,33 @@ private Authenticator(AuditableRequest auditableRequest, User fallbackUser, Acti * */ private void authenticateAsync() { - lookForExistingAuthentication((authentication) -> { - if (authentication != null) { - listener.onResponse(authentication); - } else { - tokenService.getAndValidateToken(threadContext, ActionListener.wrap(userToken -> { - if (userToken != null) { - writeAuthToContext(userToken.getAuthentication()); - } else { - extractToken(this::consumeToken); - } - }, e -> { - if (e instanceof ElasticsearchSecurityException && + if (defaultOrderedRealmList.isEmpty()) { + // this happens when the license state changes between the call to authenticate and the actual invocation + // to get the realm list + listener.onResponse(null); + } else { + lookForExistingAuthentication((authentication) -> { + if (authentication != null) { + listener.onResponse(authentication); + } else { + tokenService.getAndValidateToken(threadContext, ActionListener.wrap(userToken -> { + if (userToken != null) { + writeAuthToContext(userToken.getAuthentication()); + } else { + extractToken(this::consumeToken); + } + }, e -> { + if (e instanceof ElasticsearchSecurityException && tokenService.isExpiredTokenException((ElasticsearchSecurityException) e) == false) { - // intentionally ignore the returned exception; we call this primarily - // for the auditing as we already have a purpose built exception - request.tamperedRequest(); - } - listener.onFailure(e); - })); - } - }); + // intentionally ignore the returned exception; we call this primarily + // for the auditing as we already have a purpose built exception + request.tamperedRequest(); + } + listener.onFailure(e); + })); + } + }); + } } /** @@ -233,7 +241,7 @@ void extractToken(Consumer consumer) { if (authenticationToken != null) { action = () -> consumer.accept(authenticationToken); } else { - for (Realm realm : realms) { + for (Realm realm : defaultOrderedRealmList) { final AuthenticationToken token = realm.token(threadContext); if (token != null) { action = () -> consumer.accept(token); @@ -260,7 +268,6 @@ private void consumeToken(AuthenticationToken token) { handleNullToken(); } else { authenticationToken = token; - final List realmsList = realms.asList(); final Map> messages = new LinkedHashMap<>(); final BiConsumer> realmAuthenticatingConsumer = (realm, userListener) -> { if (realm.supports(authenticationToken)) { @@ -297,11 +304,12 @@ private void consumeToken(AuthenticationToken token) { userListener.onResponse(null); } }; + final IteratingActionListener authenticatingListener = new IteratingActionListener<>(ContextPreservingActionListener.wrapPreservingContext(ActionListener.wrap( (user) -> consumeUser(user, messages), (e) -> listener.onFailure(request.exceptionProcessingRequest(e, token))), threadContext), - realmAuthenticatingConsumer, realmsList, threadContext); + realmAuthenticatingConsumer, defaultOrderedRealmList, threadContext); try { authenticatingListener.run(); } catch (Exception e) { @@ -388,7 +396,7 @@ private void consumeUser(User user, Map> message * names of users that exist using a timing attack */ private void lookupRunAsUser(final User user, String runAsUsername, Consumer userConsumer) { - final RealmUserLookup lookup = new RealmUserLookup(realms.asList(), threadContext); + final RealmUserLookup lookup = new RealmUserLookup(defaultOrderedRealmList, threadContext); lookup.lookup(runAsUsername, ActionListener.wrap(tuple -> { if (tuple == null) { // the user does not exist, but we still create a User object, which will later be rejected by authz diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java index b59e656c5b92d..f4de04c729f5f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java @@ -187,12 +187,12 @@ protected Map initializeProfileFilters(Destructiv case "client": profileFilters.put(entry.getKey(), new ServerTransportFilter.ClientProfile(authcService, authzService, threadPool.getThreadContext(), extractClientCert, destructiveOperations, reservedRealmEnabled, - securityContext)); + securityContext, licenseState)); break; case "node": profileFilters.put(entry.getKey(), new ServerTransportFilter.NodeProfile(authcService, authzService, threadPool.getThreadContext(), extractClientCert, destructiveOperations, reservedRealmEnabled, - securityContext)); + securityContext, licenseState)); break; default: throw new IllegalStateException("unknown profile type: " + type); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java index fa8b094da7d0f..98ecef7ce62eb 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java @@ -19,6 +19,7 @@ import org.elasticsearch.action.admin.indices.open.OpenIndexAction; import org.elasticsearch.action.support.DestructiveOperations; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.transport.TaskTransportChannel; import org.elasticsearch.transport.TcpTransportChannel; import org.elasticsearch.transport.TransportChannel; @@ -78,10 +79,11 @@ class NodeProfile implements ServerTransportFilter { private final DestructiveOperations destructiveOperations; private final boolean reservedRealmEnabled; private final SecurityContext securityContext; + private final XPackLicenseState licenseState; NodeProfile(AuthenticationService authcService, AuthorizationService authzService, ThreadContext threadContext, boolean extractClientCert, DestructiveOperations destructiveOperations, - boolean reservedRealmEnabled, SecurityContext securityContext) { + boolean reservedRealmEnabled, SecurityContext securityContext, XPackLicenseState licenseState) { this.authcService = authcService; this.authzService = authzService; this.threadContext = threadContext; @@ -89,6 +91,7 @@ class NodeProfile implements ServerTransportFilter { this.destructiveOperations = destructiveOperations; this.reservedRealmEnabled = reservedRealmEnabled; this.securityContext = securityContext; + this.licenseState = licenseState; } @Override @@ -129,6 +132,7 @@ requests from all the nodes are attached with a user (either a serialize final Version version = transportChannel.getVersion().equals(Version.V_5_4_0) ? Version.CURRENT : transportChannel.getVersion(); authcService.authenticate(securityAction, request, (User)null, ActionListener.wrap((authentication) -> { + if (authentication != null) { if (reservedRealmEnabled && authentication.getVersion().before(Version.V_5_2_0) && KibanaUser.NAME.equals(authentication.getUser().authenticatedUser().principal())) { executeAsCurrentVersionKibanaUser(securityAction, request, transportChannel, listener, authentication); @@ -156,7 +160,12 @@ requests from all the nodes are attached with a user (either a serialize }); asyncAuthorizer.authorize(authzService); } - }, listener::onFailure)); + } else if (licenseState.isAuthAllowed() == false) { + listener.onResponse(null); + } else { + listener.onFailure(new IllegalStateException("no authentication present but auth is allowed")); + } + }, listener::onFailure)); } private void executeAsCurrentVersionKibanaUser(String securityAction, TransportRequest request, TransportChannel transportChannel, @@ -220,9 +229,9 @@ class ClientProfile extends NodeProfile { ClientProfile(AuthenticationService authcService, AuthorizationService authzService, ThreadContext threadContext, boolean extractClientCert, DestructiveOperations destructiveOperations, - boolean reservedRealmEnabled, SecurityContext securityContext) { + boolean reservedRealmEnabled, SecurityContext securityContext, XPackLicenseState licenseState) { super(authcService, authzService, threadContext, extractClientCert, destructiveOperations, reservedRealmEnabled, - securityContext); + securityContext, licenseState); } @Override diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/license/LicensingTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/license/LicensingTests.java index f2b0840ada806..c777514a3d820 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/license/LicensingTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/license/LicensingTests.java @@ -21,12 +21,12 @@ import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.transport.NoNodeAvailableException; import org.elasticsearch.client.transport.TransportClient; -import org.elasticsearch.cluster.routing.ShardRoutingState; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.discovery.DiscoveryModule; +import org.elasticsearch.license.License.OperationMode; import org.elasticsearch.node.MockNode; import org.elasticsearch.node.Node; import org.elasticsearch.plugins.Plugin; @@ -55,6 +55,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -69,7 +70,7 @@ @TestLogging("org.elasticsearch.cluster.service:TRACE,org.elasticsearch.discovery.zen:TRACE,org.elasticsearch.action.search:TRACE," + "org.elasticsearch.search:TRACE") public class LicensingTests extends SecurityIntegTestCase { - public static final String ROLES = + private static final String ROLES = SecuritySettingsSource.TEST_ROLE + ":\n" + " cluster: [ all ]\n" + " indices:\n" + @@ -92,7 +93,7 @@ public class LicensingTests extends SecurityIntegTestCase { " - names: 'b'\n" + " privileges: [all]\n"; - public static final String USERS_ROLES = + private static final String USERS_ROLES = SecuritySettingsSource.CONFIG_STANDARD_USER_ROLES + "role_a:user_a,user_b\n" + "role_b:user_b\n"; @@ -135,8 +136,8 @@ protected int maxNumberOfNodes() { } @Before - public void resetLicensing() { - enableLicensing(); + public void resetLicensing() throws InterruptedException { + enableLicensing(OperationMode.BASIC); } @After @@ -159,11 +160,7 @@ public void testEnableDisableBehaviour() throws Exception { assertEquals(DocWriteResponse.Result.CREATED, indexResponse.getResult()); refresh(); - // wait for all replicas to be started (to make sure that there are no more cluster state updates when we disable licensing) - assertBusy(() -> assertTrue(client().admin().cluster().prepareState().get().getState().routingTable() - .shardsWithState(ShardRoutingState.INITIALIZING).isEmpty())); - - Client client = internalCluster().transportClient(); + final Client client = internalCluster().transportClient(); disableLicensing(); @@ -277,7 +274,6 @@ public void testTransportClientAuthenticationByLicenseType() throws Exception { public void testNodeJoinWithoutSecurityExplicitlyEnabled() throws Exception { License.OperationMode mode = randomFrom(License.OperationMode.GOLD, License.OperationMode.PLATINUM, License.OperationMode.STANDARD); enableLicensing(mode); - ensureGreen(); final List unicastHostsList = internalCluster().masterClient().admin().cluster().nodesInfo(new NodesInfoRequest()).get() .getNodes().stream().map(n -> n.getTransport().getAddress().publishAddress().toString()).distinct() @@ -312,23 +308,64 @@ private static void assertElasticsearchSecurityException(ThrowingRunnable runnab assertThat(ee.status(), is(RestStatus.FORBIDDEN)); } - public static void disableLicensing() { - disableLicensing(License.OperationMode.BASIC); - } - - public static void disableLicensing(License.OperationMode operationMode) { - for (XPackLicenseState licenseState : internalCluster().getInstances(XPackLicenseState.class)) { - licenseState.update(operationMode, false, null); - } + private void disableLicensing() throws InterruptedException { + // This method first makes sure licensing is enabled everywhere so that we can execute + // monitoring actions to ensure we have a stable cluster and only then do we disable. + // This is done in an await busy since there is a chance that the enabling of the license + // is overwritten by some other cluster activity and the node throws an exception while we + // wait for things to stabilize! + final boolean success = awaitBusy(() -> { + try { + for (XPackLicenseState licenseState : internalCluster().getInstances(XPackLicenseState.class)) { + if (licenseState.isAuthAllowed() == false) { + enableLicensing(OperationMode.BASIC); + break; + } + } + + ensureGreen(); + ensureClusterSizeConsistency(); + ensureClusterStateConsistency(); + + // apply the disabling of the license once the cluster is stable + for (XPackLicenseState licenseState : internalCluster().getInstances(XPackLicenseState.class)) { + licenseState.update(OperationMode.BASIC, false, null); + } + } catch (Exception e) { + logger.error("Caught exception while disabling license", e); + return false; + } + return true; + }, 30L, TimeUnit.SECONDS); + assertTrue(success); } - public static void enableLicensing() { - enableLicensing(License.OperationMode.BASIC); - } - - public static void enableLicensing(License.OperationMode operationMode) { - for (XPackLicenseState licenseState : internalCluster().getInstances(XPackLicenseState.class)) { - licenseState.update(operationMode, true, null); - } + private void enableLicensing(License.OperationMode operationMode) throws InterruptedException { + // do this in an await busy since there is a chance that the enabling of the license is + // overwritten by some other cluster activity and the node throws an exception while we + // wait for things to stabilize! + final boolean success = awaitBusy(() -> { + try { + // first update the license so we can execute monitoring actions + for (XPackLicenseState licenseState : internalCluster().getInstances(XPackLicenseState.class)) { + licenseState.update(operationMode, true, null); + } + + ensureGreen(); + ensureClusterSizeConsistency(); + ensureClusterStateConsistency(); + + // re-apply the update in case any node received an updated cluster state that triggered the license state + // to change + for (XPackLicenseState licenseState : internalCluster().getInstances(XPackLicenseState.class)) { + licenseState.update(operationMode, true, null); + } + } catch (Exception e) { + logger.error("Caught exception while enabling license", e); + return false; + } + return true; + }, 30L, TimeUnit.SECONDS); + assertTrue(success); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java index becd6069a2526..35b44ba9579bf 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java @@ -109,6 +109,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; @@ -159,9 +160,9 @@ public void init() throws Exception { XPackLicenseState licenseState = mock(XPackLicenseState.class); when(licenseState.allowedRealmType()).thenReturn(XPackLicenseState.AllowedRealmType.ALL); when(licenseState.isAuthAllowed()).thenReturn(true); - realms = new TestRealms(Settings.EMPTY, TestEnvironment.newEnvironment(settings), Collections.emptyMap(), + realms = spy(new TestRealms(Settings.EMPTY, TestEnvironment.newEnvironment(settings), Collections.emptyMap(), licenseState, threadContext, mock(ReservedRealm.class), Arrays.asList(firstRealm, secondRealm), - Collections.singletonList(firstRealm)); + Collections.singletonList(firstRealm))); auditTrail = mock(AuditTrailService.class); client = mock(Client.class); @@ -264,6 +265,8 @@ public void testAuthenticateBothSupportSecondSucceeds() throws Exception { }, this::logAndFail)); assertTrue(completed.get()); verify(auditTrail).authenticationFailed(reqId, firstRealm.name(), token, "_action", message); + verify(realms).asList(); + verifyNoMoreInteractions(realms); } public void testAuthenticateFirstNotSupportingSecondSucceeds() throws Exception { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ServerTransportFilterTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ServerTransportFilterTests.java index 8624b780a9ced..6a9d33ae39ef2 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ServerTransportFilterTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ServerTransportFilterTests.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.transport.TransportChannel; import org.elasticsearch.transport.TransportRequest; @@ -311,13 +312,13 @@ private ServerTransportFilter.ClientProfile getClientFilter(boolean reservedReal Settings settings = Settings.builder().put("path.home", createTempDir()).build(); ThreadContext threadContext = new ThreadContext(settings); return new ServerTransportFilter.ClientProfile(authcService, authzService, threadContext, false, destructiveOperations, - reservedRealmEnabled, new SecurityContext(settings, threadContext)); + reservedRealmEnabled, new SecurityContext(settings, threadContext), new XPackLicenseState(settings)); } private ServerTransportFilter.NodeProfile getNodeFilter(boolean reservedRealmEnabled) throws IOException { Settings settings = Settings.builder().put("path.home", createTempDir()).build(); ThreadContext threadContext = new ThreadContext(settings); return new ServerTransportFilter.NodeProfile(authcService, authzService, threadContext, false, destructiveOperations, - reservedRealmEnabled, new SecurityContext(settings, threadContext)); + reservedRealmEnabled, new SecurityContext(settings, threadContext), new XPackLicenseState(settings)); } }