Don't clear realm-cache during security upgrade on basic license (elastic/elasticsearch#4806)

If a user has a basic license, but previously had a full-featured license (e.g. a trial license that expired) then they may have .security index that needs to be migrated even though security is not allowed under their license.

This change makes the clearing of the realm-cache conditional on the license state. If X-Pack is running on a license that does not allow auth, then, when the `logstash_system` user is disabled as part of an upgrade migration, the cache is not cleared.

This change also fix a bug whereby a mapping update could take place even if a data migration was in progress, which could cause the `logstash_system` user to be temporarily enabled when it ought not be.

Original commit: elastic/x-pack-elasticsearch@f272e2b19f
This commit is contained in:
Tim Vernum 2017-02-07 08:56:04 +11:00 committed by GitHub
parent 752356916d
commit 286d62f00e
8 changed files with 162 additions and 58 deletions

View File

@ -337,7 +337,7 @@ public class Security implements ActionPlugin, IngestPlugin, NetworkPlugin {
components.add(authzService);
components.add(new SecurityLifecycleService(settings, clusterService, threadPool, indexAuditTrail,
nativeUsersStore, nativeRolesStore, client));
nativeUsersStore, nativeRolesStore, licenseState, client));
ipFilter.set(new IPFilter(settings, auditTrailService, clusterService.getClusterSettings(), licenseState));
components.add(ipFilter.get());

View File

@ -13,6 +13,7 @@ import org.elasticsearch.common.component.LifecycleListener;
import org.elasticsearch.common.inject.internal.Nullable;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.security.audit.index.IndexAuditTrail;
import org.elasticsearch.xpack.security.authc.esnative.NativeRealmMigrator;
@ -40,7 +41,7 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust
public SecurityLifecycleService(Settings settings, ClusterService clusterService, ThreadPool threadPool,
@Nullable IndexAuditTrail indexAuditTrail, NativeUsersStore nativeUserStore,
NativeRolesStore nativeRolesStore, InternalClient client) {
NativeRolesStore nativeRolesStore, XPackLicenseState licenseState, InternalClient client) {
super(settings);
this.settings = settings;
this.threadPool = threadPool;
@ -53,7 +54,8 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust
clusterService.addListener(this);
clusterService.addListener(nativeUserStore);
clusterService.addListener(nativeRolesStore);
clusterService.addListener(new SecurityTemplateService(settings, client, new NativeRealmMigrator(settings, nativeUserStore)));
final NativeRealmMigrator nativeRealmMigrator = new NativeRealmMigrator(settings, nativeUserStore, licenseState);
clusterService.addListener(new SecurityTemplateService(settings, client, nativeRealmMigrator));
clusterService.addLifecycleListener(new LifecycleListener() {
@Override

View File

@ -140,12 +140,13 @@ public class SecurityTemplateService extends AbstractComponent implements Cluste
});
return true;
} else {
andThen.run();
if (upgradeDataState.get() == UpgradeState.COMPLETE) {
andThen.run();
}
return false;
}
}
private void updateSecurityMapping() {
// only update the mapping if this is not already in progress
if (updateMappingPending.compareAndSet(false, true) ) {

View File

@ -12,6 +12,7 @@ import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.common.inject.internal.Nullable;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.xpack.security.SecurityTemplateService;
import org.elasticsearch.xpack.security.user.LogstashSystemUser;
@ -24,10 +25,12 @@ import org.elasticsearch.xpack.security.user.LogstashSystemUser;
public class NativeRealmMigrator {
private final NativeUsersStore nativeUsersStore;
private final XPackLicenseState licenseState;
private final Logger logger;
public NativeRealmMigrator(Settings settings, NativeUsersStore nativeUsersStore) {
public NativeRealmMigrator(Settings settings, NativeUsersStore nativeUsersStore, XPackLicenseState licenseState) {
this.nativeUsersStore = nativeUsersStore;
this.licenseState = licenseState;
this.logger = Loggers.getLogger(getClass(), settings);
}
@ -48,7 +51,10 @@ public class NativeRealmMigrator {
if (shouldDisableLogstashUser(previousVersion)) {
logger.info("Upgrading security from version [{}] - new reserved user [{}] will default to disabled",
previousVersion, LogstashSystemUser.NAME);
nativeUsersStore.ensureReservedUserIsDisabled(LogstashSystemUser.NAME, new ActionListener<Void>() {
// Only clear the cache is authentication is allowed by the current license
// otherwise the license management checks will prevent it from completing successfully.
final boolean clearCache = licenseState.isAuthAllowed();
nativeUsersStore.ensureReservedUserIsDisabled(LogstashSystemUser.NAME, clearCache, new ActionListener<Void>() {
@Override
public void onResponse(Void aVoid) {
listener.onResponse(true);

View File

@ -12,7 +12,6 @@ import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.action.DocWriteResponse.Result;
import org.elasticsearch.action.delete.DeleteRequest;
import org.elasticsearch.action.delete.DeleteResponse;
import org.elasticsearch.action.get.GetRequest;
@ -396,7 +395,7 @@ public class NativeUsersStore extends AbstractComponent implements ClusterStateL
}
if (ReservedRealm.isReserved(username, settings)) {
setReservedUserEnabled(username, enabled, refreshPolicy, listener);
setReservedUserEnabled(username, enabled, refreshPolicy, true, listener);
} else {
setRegularUserEnabled(username, enabled, refreshPolicy, listener);
}
@ -434,10 +433,10 @@ public class NativeUsersStore extends AbstractComponent implements ClusterStateL
}
}
void ensureReservedUserIsDisabled(final String username, final ActionListener<Void> listener) {
void ensureReservedUserIsDisabled(final String username, boolean clearCache, final ActionListener<Void> listener) {
getReservedUserInfo(username, ActionListener.wrap(userInfo -> {
if (userInfo == null || userInfo.enabled) {
setReservedUserEnabled(username, false, RefreshPolicy.IMMEDIATE, listener);
setReservedUserEnabled(username, false, RefreshPolicy.IMMEDIATE, clearCache, listener);
} else {
listener.onResponse(null);
}
@ -445,7 +444,7 @@ public class NativeUsersStore extends AbstractComponent implements ClusterStateL
}
private void setReservedUserEnabled(final String username, final boolean enabled, final RefreshPolicy refreshPolicy,
final ActionListener<Void> listener) {
boolean clearCache, final ActionListener<Void> listener) {
try {
client.prepareUpdate(SecurityTemplateService.SECURITY_INDEX_NAME, RESERVED_USER_DOC_TYPE, username)
.setDoc(Requests.INDEX_CONTENT_TYPE, User.Fields.ENABLED.getPreferredName(), enabled)
@ -456,7 +455,11 @@ public class NativeUsersStore extends AbstractComponent implements ClusterStateL
.execute(new ActionListener<UpdateResponse>() {
@Override
public void onResponse(UpdateResponse updateResponse) {
clearRealmCache(username, listener, null);
if (clearCache) {
clearRealmCache(username, listener, null);
} else {
listener.onResponse(null);
}
}
@Override

View File

@ -7,21 +7,43 @@ package org.elasticsearch.license;
import org.apache.http.message.BasicHeader;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionFuture;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsResponse;
import org.elasticsearch.action.admin.cluster.stats.ClusterStatsIndices;
import org.elasticsearch.action.admin.cluster.stats.ClusterStatsResponse;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest;
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse;
import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest;
import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateResponse;
import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.client.Client;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.transport.NoNodeAvailableException;
import org.elasticsearch.client.transport.TransportClient;
import org.elasticsearch.cluster.metadata.MappingMetaData;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.network.NetworkModule;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.SecurityIntegTestCase;
@ -31,18 +53,27 @@ import org.elasticsearch.transport.Transport;
import org.elasticsearch.xpack.TestXPackTransportClient;
import org.elasticsearch.xpack.XPackPlugin;
import org.elasticsearch.xpack.security.Security;
import org.elasticsearch.xpack.security.SecurityTemplateService;
import org.elasticsearch.xpack.security.action.user.GetUsersResponse;
import org.elasticsearch.xpack.security.authc.support.SecuredString;
import org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken;
import org.elasticsearch.xpack.security.client.SecurityClient;
import org.elasticsearch.xpack.template.TemplateUtils;
import org.junit.Before;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.is;
@ -50,37 +81,37 @@ import static org.hamcrest.Matchers.notNullValue;
public class LicensingTests extends SecurityIntegTestCase {
public static final String ROLES =
SecuritySettingsSource.DEFAULT_ROLE + ":\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [manage]\n" +
" - names: '/.*/'\n" +
" privileges: [write]\n" +
" - names: 'test'\n" +
" privileges: [read]\n" +
" - names: 'test1'\n" +
" privileges: [read]\n" +
"\n" +
"role_a:\n" +
" indices:\n" +
" - names: 'a'\n" +
" privileges: [all]\n" +
"\n" +
"role_b:\n" +
" indices:\n" +
" - names: 'b'\n" +
" privileges: [all]\n";
SecuritySettingsSource.DEFAULT_ROLE + ":\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [manage]\n" +
" - names: '/.*/'\n" +
" privileges: [write]\n" +
" - names: 'test'\n" +
" privileges: [read]\n" +
" - names: 'test1'\n" +
" privileges: [read]\n" +
"\n" +
"role_a:\n" +
" indices:\n" +
" - names: 'a'\n" +
" privileges: [all]\n" +
"\n" +
"role_b:\n" +
" indices:\n" +
" - names: 'b'\n" +
" privileges: [all]\n";
public static final String USERS =
SecuritySettingsSource.CONFIG_STANDARD_USER +
"user_a:{plain}passwd\n" +
"user_b:{plain}passwd\n";
SecuritySettingsSource.CONFIG_STANDARD_USER +
"user_a:{plain}passwd\n" +
"user_b:{plain}passwd\n";
public static final String USERS_ROLES =
SecuritySettingsSource.CONFIG_STANDARD_USER_ROLES +
"role_a:user_a,user_b\n" +
"role_b:user_b\n";
SecuritySettingsSource.CONFIG_STANDARD_USER_ROLES +
"role_a:user_a,user_b\n" +
"role_b:user_b\n";
@Override
protected String configRoles() {
@ -100,8 +131,8 @@ public class LicensingTests extends SecurityIntegTestCase {
@Override
public Settings nodeSettings(int nodeOrdinal) {
return Settings.builder().put(super.nodeSettings(nodeOrdinal))
.put(NetworkModule.HTTP_ENABLED.getKey(), true)
.build();
.put(NetworkModule.HTTP_ENABLED.getKey(), true)
.build();
}
@Override
@ -118,16 +149,16 @@ public class LicensingTests extends SecurityIntegTestCase {
public void testEnableDisableBehaviour() throws Exception {
IndexResponse indexResponse = index("test", "type", jsonBuilder()
.startObject()
.field("name", "value")
.endObject());
.startObject()
.field("name", "value")
.endObject());
assertEquals(DocWriteResponse.Result.CREATED, indexResponse.getResult());
indexResponse = index("test1", "type", jsonBuilder()
.startObject()
.field("name", "value1")
.endObject());
.startObject()
.field("name", "value1")
.endObject());
assertEquals(DocWriteResponse.Result.CREATED, indexResponse.getResult());
refresh();
@ -169,7 +200,7 @@ public class LicensingTests extends SecurityIntegTestCase {
// generate a new license with a mode that enables auth
License.OperationMode mode = randomFrom(License.OperationMode.GOLD, License.OperationMode.TRIAL,
License.OperationMode.PLATINUM, License.OperationMode.STANDARD);
License.OperationMode.PLATINUM, License.OperationMode.STANDARD);
enableLicensing(mode);
e = expectThrows(ResponseException.class, () -> getRestClient().performRequest("GET", "/"));
assertThat(e.getResponse().getStatusLine().getStatusCode(), is(401));
@ -211,7 +242,7 @@ public class LicensingTests extends SecurityIntegTestCase {
public void testTransportClientAuthenticationByLicenseType() throws Exception {
Settings.Builder builder = Settings.builder()
.put(internalCluster().transportClient().settings());
.put(internalCluster().transportClient().settings());
// remove user info
builder.remove(Security.USER_SETTING.getKey());
builder.remove(ThreadContext.PREFIX + "." + UsernamePasswordToken.BASIC_AUTH_HEADER);
@ -224,7 +255,7 @@ public class LicensingTests extends SecurityIntegTestCase {
// enable a license that enables security
License.OperationMode mode = randomFrom(License.OperationMode.GOLD, License.OperationMode.TRIAL,
License.OperationMode.PLATINUM, License.OperationMode.STANDARD);
License.OperationMode.PLATINUM, License.OperationMode.STANDARD);
enableLicensing(mode);
try (TransportClient client = new TestXPackTransportClient(builder.build())) {
@ -236,6 +267,55 @@ public class LicensingTests extends SecurityIntegTestCase {
}
}
public void testNativeRealmMigratorWorksUnderBasicLicense() throws Exception {
final String securityIndex = ".security";
final String reservedUserType = "reserved-user";
final String oldVersionThatRequiresMigration = Version.V_5_0_2.toString();
final Client client = internalCluster().transportClient();
final String template = TemplateUtils.loadTemplate("/" + SecurityTemplateService.SECURITY_TEMPLATE_NAME + ".json",
oldVersionThatRequiresMigration, Pattern.quote("${security.template.version}"));
PutIndexTemplateRequest putTemplateRequest = client.admin().indices()
.preparePutTemplate(SecurityTemplateService.SECURITY_TEMPLATE_NAME)
.setSource(new BytesArray(template.getBytes(StandardCharsets.UTF_8)), XContentType.JSON)
.request();
final PutIndexTemplateResponse putTemplateResponse = client.admin().indices().putTemplate(putTemplateRequest).actionGet();
assertThat(putTemplateResponse.isAcknowledged(), equalTo(true));
final CreateIndexRequest createIndexRequest = client.admin().indices().prepareCreate(securityIndex).request();
final CreateIndexResponse createIndexResponse = client.admin().indices().create(createIndexRequest).actionGet();
assertThat(createIndexResponse.isAcknowledged(), equalTo(true));
final Map<String, Object> templateMap = XContentHelper.convertToMap(JsonXContent.jsonXContent, template, false);
final Map<String, Object> mappings = (Map<String, Object>) templateMap.get("mappings");
final Map<String, Object> reservedUserMapping = (Map<String, Object>) mappings.get(reservedUserType);
final PutMappingRequest putMappingRequest = client.admin().indices()
.preparePutMapping(securityIndex).setSource(reservedUserMapping).setType(reservedUserType).request();
final PutMappingResponse putMappingResponse = client.admin().indices().putMapping(putMappingRequest).actionGet();
assertThat(putMappingResponse.isAcknowledged(), equalTo(true));
final GetMappingsRequest getMappingsRequest = client.admin().indices().prepareGetMappings(securityIndex).request();
final boolean upgradeOk = awaitBusy(() -> {
final GetMappingsResponse getMappingsResponse = client.admin().indices().getMappings(getMappingsRequest).actionGet();
final MappingMetaData metaData = getMappingsResponse.mappings().get(securityIndex).get(reservedUserType);
try {
Map<String, Object> meta = (Map<String, Object>) metaData.sourceAsMap().get("_meta");
return meta != null && Version.CURRENT.toString().equals(meta.get("security-version"));
} catch (IOException e) {
return false;
}
}, 1, TimeUnit.SECONDS);
assertThat(upgradeOk, equalTo(true));
final GetRequest getRequest = client.prepareGet(securityIndex, reservedUserType, "logstash_system").setRefresh(true).request();
final GetResponse getResponse = client.get(getRequest).actionGet();
assertThat(getResponse.isExists(), equalTo(true));
assertThat(getResponse.getFields(), equalTo(Collections.emptyMap()));
}
private static void assertElasticsearchSecurityException(ThrowingRunnable runnable) {
ElasticsearchSecurityException ee = expectThrows(ElasticsearchSecurityException.class, runnable);
assertThat(ee.getMetadata(LicenseUtils.EXPIRED_FEATURE_METADATA), hasItem(XPackPlugin.SECURITY));

View File

@ -217,8 +217,8 @@ public class SecurityTemplateServiceTests extends ESTestCase {
assertThat(securityTemplateService.upgradeDataState.get(), equalTo(UpgradeState.NOT_STARTED));
securityTemplateService.clusterChanged(new ClusterChangedEvent("test-event", clusterStateBuilder.build()
, EMPTY_CLUSTER_STATE));
securityTemplateService.clusterChanged(new ClusterChangedEvent("test-event", clusterStateBuilder.build(),
EMPTY_CLUSTER_STATE));
assertThat(migratorVersionRef.get(), equalTo(expectedOldVersion));
assertThat(migratorListenerRef.get(), notNullValue());
@ -241,7 +241,7 @@ public class SecurityTemplateServiceTests extends ESTestCase {
assertTrue(securityTemplateService.updateMappingPending.get());
// if we now simulate an error...
listener.onFailure(new Exception());
listener.onFailure(new Exception("Testing failure handling"));
assertFalse(securityTemplateService.updateMappingPending.get());
// ... we should be able to send a new update

View File

@ -12,6 +12,7 @@ import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.security.user.LogstashSystemUser;
import org.junit.Before;
@ -19,28 +20,38 @@ import org.mockito.Mockito;
import static org.hamcrest.Matchers.is;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
public class NativeRealmMigratorTests extends ESTestCase {
private Consumer<ActionListener<Void>> ensureDisabledHandler;
private NativeUsersStore nativeUsersStore;
private NativeRealmMigrator migrator;
private XPackLicenseState licenseState;
@Before
public void setupMocks() {
final boolean allowClearCache = randomBoolean();
ensureDisabledHandler = listener -> listener.onResponse(null);
nativeUsersStore = Mockito.mock(NativeUsersStore.class);
Mockito.doAnswer(invocation -> {
ActionListener<Void> listener = (ActionListener<Void>) invocation.getArguments()[1];
ActionListener<Void> listener = (ActionListener<Void>) invocation.getArguments()[2];
ensureDisabledHandler.accept(listener);
return null;
}).when(nativeUsersStore).ensureReservedUserIsDisabled(any(), any());
}).when(nativeUsersStore).ensureReservedUserIsDisabled(any(), eq(allowClearCache), any());
final Settings settings = Settings.EMPTY;
migrator = new NativeRealmMigrator(settings, nativeUsersStore);
licenseState = mock(XPackLicenseState.class);
when(licenseState.isAuthAllowed()).thenReturn(allowClearCache);
migrator = new NativeRealmMigrator(settings, nativeUsersStore, licenseState);
}
public void testNoChangeOnFreshInstall() throws Exception {
@ -71,7 +82,8 @@ public class NativeRealmMigratorTests extends ESTestCase {
private void verifyUpgradeDisablesLogstashSystemUser(Version fromVersion) throws ExecutionException, InterruptedException {
final PlainActionFuture<Boolean> future = doUpgrade(fromVersion);
verify(nativeUsersStore).ensureReservedUserIsDisabled(eq(LogstashSystemUser.NAME), any());
final boolean clearCache = licenseState.isAuthAllowed();
verify(nativeUsersStore).ensureReservedUserIsDisabled(eq(LogstashSystemUser.NAME), eq(clearCache), any());
verifyNoMoreInteractions(nativeUsersStore);
assertThat(future.get(), is(Boolean.TRUE));
}