Fixed a bug in Permissions with multiple indices permission groups

The evalutation of the indices permission groups was wrong. Now, each index in the request is evaluated against all groups, such that:
  1. for each index, at least one group must grant the request
  2. all indices must be granted

  Along the way, also changed the audit logs structures such that:
  - moved the principal to "sit" next to the host
  - now, if we're logging an indices request, we also log the related indices (this provides more context to the actual request)

  Fixes elastic/elasticsearch#242

Original commit: elastic/x-pack-elasticsearch@95600d3148
This commit is contained in:
uboness 2014-10-22 19:14:46 +02:00
parent fa48c46813
commit 0777e8d94f
4 changed files with 276 additions and 56 deletions

View File

@ -5,6 +5,8 @@
*/
package org.elasticsearch.shield.audit.logfile;
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.logging.Loggers;
@ -41,69 +43,126 @@ public class LoggingAuditTrail implements AuditTrail {
@Override
public void anonymousAccess(String action, TransportMessage<?> message) {
if (logger.isDebugEnabled()) {
logger.debug("ANONYMOUS_ACCESS\thost=[{}], action=[{}], request=[{}]", message.remoteAddress(), action, message);
String indices = indices(message);
if (indices != null) {
if (logger.isDebugEnabled()) {
logger.debug("ANONYMOUS_ACCESS\thost=[{}], action=[{}], indices=[{}], request=[{}]", message.remoteAddress(), action, indices, message);
} else {
logger.warn("ANONYMOUS_ACCESS\thost=[{}], action=[{}], indices=[{}]", message.remoteAddress(), action, indices);
}
} else {
logger.warn("ANONYMOUS_ACCESS\thost=[{}], action=[{}]", message.remoteAddress(), action);
if (logger.isDebugEnabled()) {
logger.debug("ANONYMOUS_ACCESS\thost=[{}], action=[{}], request=[{}]", message.remoteAddress(), action, message);
} else {
logger.warn("ANONYMOUS_ACCESS\thost=[{}], action=[{}]", message.remoteAddress(), action);
}
}
}
@Override
public void authenticationFailed(AuthenticationToken token, String action, TransportMessage<?> message) {
if (logger.isDebugEnabled()) {
logger.debug("AUTHENTICATION_FAILED\thost=[{}], action=[{}], principal=[{}], request=[{}]", message.remoteAddress(), action, token.principal(), message);
String indices = indices(message);
if (indices != null) {
if (logger.isDebugEnabled()) {
logger.debug("AUTHENTICATION_FAILED\thost=[{}], principal=[{}], action=[{}], indices=[{}], request=[{}]", message.remoteAddress(), token.principal(), action, indices, message);
} else {
logger.error("AUTHENTICATION_FAILED\thost=[{}], principal=[{}], action=[{}], indices=[{}]", message.remoteAddress(), token.principal(), action, indices);
}
} else {
logger.error("AUTHENTICATION_FAILED\thost=[{}], action=[{}], principal=[{}]", message.remoteAddress(), action, token.principal());
if (logger.isDebugEnabled()) {
logger.debug("AUTHENTICATION_FAILED\thost=[{}], principal=[{}], action=[{}], request=[{}]", message.remoteAddress(), token.principal(), action, message);
} else {
logger.error("AUTHENTICATION_FAILED\thost=[{}], principal=[{}], action=[{}]", message.remoteAddress(), token.principal(), action);
}
}
}
@Override
public void authenticationFailed(AuthenticationToken token, RestRequest request) {
if (logger.isDebugEnabled()) {
logger.debug("AUTHENTICATION_FAILED\thost=[{}], URI=[{}], principal=[{}], request=[{}]", request.getRemoteAddress(), request.uri(), token.principal(), request);
logger.debug("AUTHENTICATION_FAILED\thost=[{}], principal=[{}], URI=[{}], request=[{}]", request.getRemoteAddress(), token.principal(), request.uri(), request);
} else {
logger.error("AUTHENTICATION_FAILED\thost=[{}], URI=[{}], principal=[{}]", request.getRemoteAddress(), request.uri(), token.principal());
logger.error("AUTHENTICATION_FAILED\thost=[{}], principal=[{}], URI=[{}]", request.getRemoteAddress(), token.principal(), request.uri());
}
}
@Override
public void authenticationFailed(String realm, AuthenticationToken token, String action, TransportMessage<?> message) {
if (logger.isTraceEnabled()) {
logger.trace("AUTHENTICATION_FAILED[{}]\thost=[{}], action=[{}], principal=[{}], request=[{}]", realm, message.remoteAddress(), action, token.principal(), message);
String indices = indices(message);
if (indices != null) {
logger.trace("AUTHENTICATION_FAILED[{}]\thost=[{}], principal=[{}], action=[{}], indices=[{}], request=[{}]", realm, message.remoteAddress(), token.principal(), action, indices, message);
} else {
logger.trace("AUTHENTICATION_FAILED[{}]\thost=[{}], principal=[{}], action=[{}], request=[{}]", realm, message.remoteAddress(), token.principal(), action, message);
}
}
}
@Override
public void authenticationFailed(String realm, AuthenticationToken token, RestRequest request) {
if (logger.isTraceEnabled()) {
logger.trace("AUTHENTICATION_FAILED[{}]\thost=[{}], URI=[{}], principal=[{}], request=[{}]", realm, request.getRemoteAddress(), request.uri(), token.principal(), request);
logger.trace("AUTHENTICATION_FAILED[{}]\thost=[{}], principal=[{}], URI=[{}], request=[{}]", realm, request.getRemoteAddress(), token.principal(), request.uri(), request);
}
}
@Override
public void accessGranted(User user, String action, TransportMessage<?> message) {
if (logger.isDebugEnabled()) {
logger.debug("ACCESS_GRANTED\thost=[{}], action=[{}], principal=[{}], request=[{}]", message.remoteAddress(), action, user.principal(), message);
String indices = indices(message);
if (indices != null) {
if (logger.isDebugEnabled()) {
logger.debug("ACCESS_GRANTED\thost=[{}], principal=[{}], action=[{}], indices=[{}], request=[{}]", message.remoteAddress(), user.principal(), action, indices, message);
} else {
logger.info("ACCESS_GRANTED\thost=[{}], principal=[{}], action=[{}], indices=[{}]", message.remoteAddress(), user.principal(), action, indices);
}
} else {
logger.info("ACCESS_GRANTED\thost=[{}], action=[{}], principal=[{}]", message.remoteAddress(), action, user.principal());
if (logger.isDebugEnabled()) {
logger.debug("ACCESS_GRANTED\thost=[{}], principal=[{}], action=[{}], request=[{}]", message.remoteAddress(), user.principal(), action, message);
} else {
logger.info("ACCESS_GRANTED\thost=[{}], principal=[{}], action=[{}]", message.remoteAddress(), user.principal(), action);
}
}
}
@Override
public void accessDenied(User user, String action, TransportMessage<?> message) {
if (logger.isDebugEnabled()) {
logger.debug("ACCESS_DENIED\thost=[{}], action=[{}], principal=[{}], request=[{}]", message.remoteAddress(), action, user.principal(), message);
String indices = indices(message);
if (indices != null) {
if (logger.isDebugEnabled()) {
logger.debug("ACCESS_DENIED\thost=[{}], principal=[{}], action=[{}], indices=[{}], request=[{}]", message.remoteAddress(), user.principal(), action, indices, message);
} else {
logger.error("ACCESS_DENIED\thost=[{}], principal=[{}], action=[{}], indices=[{}]", message.remoteAddress(), user.principal(), action, indices);
}
} else {
logger.error("ACCESS_DENIED\thost=[{}], action=[{}], principal=[{}]", message.remoteAddress(), action, user.principal());
if (logger.isDebugEnabled()) {
logger.debug("ACCESS_DENIED\thost=[{}], principal=[{}], action=[{}], request=[{}]", message.remoteAddress(), user.principal(), action, message);
} else {
logger.error("ACCESS_DENIED\thost=[{}], principal=[{}], action=[{}]", message.remoteAddress(), user.principal(), action);
}
}
}
@Override
public void tamperedRequest(User user, String action, TransportRequest request) {
if (logger.isDebugEnabled()) {
logger.debug("TAMPERED REQUEST\thost=[{}], action=[{}], principal=[{}], request=[{}]", request.remoteAddress(), action, user.principal(), request);
String indices = indices(request);
if (indices != null) {
if (logger.isDebugEnabled()) {
logger.debug("TAMPERED REQUEST\thost=[{}], principal=[{}], action=[{}], indices=[{}], request=[{}]", request.remoteAddress(), user.principal(), action, indices, request);
} else {
logger.error("TAMPERED REQUEST\thost=[{}], principal=[{}], action=[{}], indices=[{}]", request.remoteAddress(), user.principal(), action, indices);
}
} else {
logger.error("TAMPERED REQUEST\thost=[{}], action=[{}], principal=[{}]", request.remoteAddress(), action, user.principal());
if (logger.isDebugEnabled()) {
logger.debug("TAMPERED REQUEST\thost=[{}], principal=[{}], action=[{}], request=[{}]", request.remoteAddress(), user.principal(), action, request);
} else {
logger.error("TAMPERED REQUEST\thost=[{}], principal=[{}], action=[{}]", request.remoteAddress(), user.principal(), action);
}
}
}
private static String indices(TransportMessage message) {
if (message instanceof IndicesRequest) {
return Strings.arrayToCommaDelimitedString(((IndicesRequest) message).indices());
}
return null;
}
}

View File

@ -9,7 +9,6 @@ import org.elasticsearch.action.CompositeIndicesRequest;
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.base.Predicate;
import org.elasticsearch.common.cache.Cache;
import org.elasticsearch.common.cache.CacheBuilder;
import org.elasticsearch.common.cache.CacheLoader;
import org.elasticsearch.common.cache.LoadingCache;
@ -233,12 +232,25 @@ public interface Permission {
}
}
for (Group group : groups) {
if (group.check(action, indices)) {
return true;
if (indices == null) {
return true;
}
// for every index, at least one group should match it... otherwise denied
for (String index : indices) {
boolean grant = false;
for (Group group : groups) {
if (group.check(action, index)) {
grant = true;
break;
}
}
if (!grant) {
return false;
}
}
return false;
return true;
}
public static class Group {
@ -264,21 +276,8 @@ public interface Permission {
return indices;
}
public boolean check(String action, Set<String> indices) {
if (!actionMatcher.apply(action)) {
return false;
}
if (indices != null) {
for (String index : indices) {
if (!indexNameMatcher.apply(index)) {
return false;
}
}
}
return true;
public boolean check(String action, String index) {
return actionMatcher.apply(action) && !(index != null && !indexNameMatcher.apply(index));
}

View File

@ -0,0 +1,98 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.shield;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.shield.authz.AuthorizationException;
import org.elasticsearch.shield.test.ShieldIntegrationTest;
import org.junit.Test;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.query.QueryBuilders.indicesQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
import static org.hamcrest.Matchers.*;
/**
*
*/
public class MultipleIndicesPermissionsTests extends ShieldIntegrationTest {
public static final String ROLES = "user:\n" +
" cluster: all\n" +
" indices:\n" +
" '.*': manage\n" +
" '.*': write\n" +
" 'test': read\n" +
" 'test1': read\n";
@Override
protected String configRole() {
return ROLES;
}
@Test
public void testDifferetCombinationsOfIndices() throws Exception {
IndexResponse indexResponse = index("test", "type", jsonBuilder()
.startObject()
.field("name", "value")
.endObject());
assertThat(indexResponse.isCreated(), is(true));
indexResponse = index("test1", "type", jsonBuilder()
.startObject()
.field("name", "value1")
.endObject());
assertThat(indexResponse.isCreated(), is(true));
refresh();
Client client = internalCluster().transportClient();
// no specifying an index, should replace indices with the permitted ones (test & test1)
SearchResponse searchResponse = client.prepareSearch().setQuery(matchAllQuery()).get();
assertNoFailures(searchResponse);
assertHitCount(searchResponse, 2);
searchResponse = client.prepareSearch().setQuery(indicesQuery(matchAllQuery(), "test1")).get();
assertNoFailures(searchResponse);
assertHitCount(searchResponse, 2);
// _all should expand to all the permitted indices
searchResponse = client.prepareSearch("_all").setQuery(matchAllQuery()).get();
assertNoFailures(searchResponse);
assertHitCount(searchResponse, 2);
// wildcards should expand to all the permitted indices
searchResponse = client.prepareSearch("test*").setQuery(matchAllQuery()).get();
assertNoFailures(searchResponse);
assertHitCount(searchResponse, 2);
// specifying a permitted index, should only return results from that index
searchResponse = client.prepareSearch("test1").setQuery(indicesQuery(matchAllQuery(), "test1")).get();
assertNoFailures(searchResponse);
assertHitCount(searchResponse, 1);
// specifying a forbidden index, should throw an authorization exception
try {
client.prepareSearch("test2").setQuery(indicesQuery(matchAllQuery(), "test1")).get();
fail("expected an authorization exception when searching a forbidden index");
} catch (AuthorizationException ae) {
// expected
}
try {
client.prepareSearch("test", "test2").setQuery(matchAllQuery()).get();
fail("expected an authorization exception when one of mulitple indices is forbidden");
} catch (AuthorizationException ae) {
// expected
}
}
}

View File

@ -5,6 +5,8 @@
*/
package org.elasticsearch.shield.audit.logfile;
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.common.transport.LocalTransportAddress;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.shield.User;
@ -32,18 +34,27 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
for (Level level : Level.values()) {
CapturingLogger logger = new CapturingLogger(level);
LoggingAuditTrail auditTrail = new LoggingAuditTrail(logger);
auditTrail.anonymousAccess("_action", new MockMessage());
TransportMessage message = randomBoolean() ? new MockMessage() : new MockIndicesRequest();
auditTrail.anonymousAccess("_action", message);
switch (level) {
case ERROR:
assertEmptyLog(logger);
break;
case WARN:
case INFO:
assertMsg(logger, Level.WARN, "ANONYMOUS_ACCESS\thost=[local[_host]], action=[_action]");
if (message instanceof IndicesRequest) {
assertMsg(logger, Level.WARN, "ANONYMOUS_ACCESS\thost=[local[_host]], action=[_action], indices=[idx1,idx2]");
} else {
assertMsg(logger, Level.WARN, "ANONYMOUS_ACCESS\thost=[local[_host]], action=[_action]");
}
break;
case DEBUG:
case TRACE:
assertMsg(logger, Level.DEBUG, "ANONYMOUS_ACCESS\thost=[local[_host]], action=[_action], request=[mock-message]");
if (message instanceof IndicesRequest) {
assertMsg(logger, Level.DEBUG, "ANONYMOUS_ACCESS\thost=[local[_host]], action=[_action], indices=[idx1,idx2], request=[mock-message]");
} else {
assertMsg(logger, Level.DEBUG, "ANONYMOUS_ACCESS\thost=[local[_host]], action=[_action], request=[mock-message]");
}
}
}
}
@ -53,16 +64,25 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
for (Level level : Level.values()) {
CapturingLogger logger = new CapturingLogger(level);
LoggingAuditTrail auditTrail = new LoggingAuditTrail(logger);
auditTrail.authenticationFailed(new MockToken(), "_action", new MockMessage());
TransportMessage message = randomBoolean() ? new MockMessage() : new MockIndicesRequest();
auditTrail.authenticationFailed(new MockToken(), "_action", message);
switch (level) {
case ERROR:
case WARN:
case INFO:
assertMsg(logger, Level.ERROR, "AUTHENTICATION_FAILED\thost=[local[_host]], action=[_action], principal=[_principal]");
if (message instanceof IndicesRequest) {
assertMsg(logger, Level.ERROR, "AUTHENTICATION_FAILED\thost=[local[_host]], principal=[_principal], action=[_action], indices=[idx1,idx2]");
} else {
assertMsg(logger, Level.ERROR, "AUTHENTICATION_FAILED\thost=[local[_host]], principal=[_principal], action=[_action]");
}
break;
case DEBUG:
case TRACE:
assertMsg(logger, Level.DEBUG, "AUTHENTICATION_FAILED\thost=[local[_host]], action=[_action], principal=[_principal], request=[mock-message]");
if (message instanceof IndicesRequest) {
assertMsg(logger, Level.DEBUG, "AUTHENTICATION_FAILED\thost=[local[_host]], principal=[_principal], action=[_action], indices=[idx1,idx2], request=[mock-message]");
} else {
assertMsg(logger, Level.DEBUG, "AUTHENTICATION_FAILED\thost=[local[_host]], principal=[_principal], action=[_action], request=[mock-message]");
}
}
}
}
@ -81,11 +101,11 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
case ERROR:
case WARN:
case INFO:
assertMsg(logger, Level.ERROR, "AUTHENTICATION_FAILED\thost=[_hostname:9200], URI=[_uri], principal=[_principal]");
assertMsg(logger, Level.ERROR, "AUTHENTICATION_FAILED\thost=[_hostname:9200], principal=[_principal], URI=[_uri]");
break;
case DEBUG:
case TRACE:
assertMsg(logger, Level.DEBUG, "AUTHENTICATION_FAILED\thost=[_hostname:9200], URI=[_uri], principal=[_principal], request=[rest_request]");
assertMsg(logger, Level.DEBUG, "AUTHENTICATION_FAILED\thost=[_hostname:9200], principal=[_principal], URI=[_uri], request=[rest_request]");
}
}
}
@ -95,7 +115,8 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
for (Level level : Level.values()) {
CapturingLogger logger = new CapturingLogger(level);
LoggingAuditTrail auditTrail = new LoggingAuditTrail(logger);
auditTrail.authenticationFailed("_realm", new MockToken(), "_action", new MockMessage());
TransportMessage message = randomBoolean() ? new MockMessage() : new MockIndicesRequest();
auditTrail.authenticationFailed("_realm", new MockToken(), "_action", message);
switch (level) {
case ERROR:
case WARN:
@ -104,7 +125,11 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
assertEmptyLog(logger);
break;
case TRACE:
assertMsg(logger, Level.TRACE, "AUTHENTICATION_FAILED[_realm]\thost=[local[_host]], action=[_action], principal=[_principal], request=[mock-message]");
if (message instanceof IndicesRequest) {
assertMsg(logger, Level.TRACE, "AUTHENTICATION_FAILED[_realm]\thost=[local[_host]], principal=[_principal], action=[_action], indices=[idx1,idx2], request=[mock-message]");
} else {
assertMsg(logger, Level.TRACE, "AUTHENTICATION_FAILED[_realm]\thost=[local[_host]], principal=[_principal], action=[_action], request=[mock-message]");
}
}
}
}
@ -127,7 +152,7 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
assertEmptyLog(logger);
break;
case TRACE:
assertMsg(logger, Level.TRACE, "AUTHENTICATION_FAILED[_realm]\thost=[_hostname:9200], URI=[_uri], principal=[_principal], request=[rest_request]");
assertMsg(logger, Level.TRACE, "AUTHENTICATION_FAILED[_realm]\thost=[_hostname:9200], principal=[_principal], URI=[_uri], request=[rest_request]");
}
}
}
@ -137,18 +162,27 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
for (Level level : Level.values()) {
CapturingLogger logger = new CapturingLogger(level);
LoggingAuditTrail auditTrail = new LoggingAuditTrail(logger);
auditTrail.accessGranted(new User.Simple("_username", "r1"), "_action", new MockMessage());
TransportMessage message = randomBoolean() ? new MockMessage() : new MockIndicesRequest();
auditTrail.accessGranted(new User.Simple("_username", "r1"), "_action", message);
switch (level) {
case ERROR:
case WARN:
assertEmptyLog(logger);
break;
case INFO:
assertMsg(logger, Level.INFO, "ACCESS_GRANTED\thost=[local[_host]], action=[_action], principal=[_username]");
if (message instanceof IndicesRequest) {
assertMsg(logger, Level.INFO, "ACCESS_GRANTED\thost=[local[_host]], principal=[_username], action=[_action], indices=[idx1,idx2]");
} else {
assertMsg(logger, Level.INFO, "ACCESS_GRANTED\thost=[local[_host]], principal=[_username], action=[_action]");
}
break;
case DEBUG:
case TRACE:
assertMsg(logger, Level.DEBUG, "ACCESS_GRANTED\thost=[local[_host]], action=[_action], principal=[_username], request=[mock-message]");
if (message instanceof IndicesRequest) {
assertMsg(logger, Level.DEBUG, "ACCESS_GRANTED\thost=[local[_host]], principal=[_username], action=[_action], indices=[idx1,idx2], request=[mock-message]");
} else {
assertMsg(logger, Level.DEBUG, "ACCESS_GRANTED\thost=[local[_host]], principal=[_username], action=[_action], request=[mock-message]");
}
}
}
}
@ -158,16 +192,25 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
for (Level level : Level.values()) {
CapturingLogger logger = new CapturingLogger(level);
LoggingAuditTrail auditTrail = new LoggingAuditTrail(logger);
auditTrail.accessDenied(new User.Simple("_username", "r1"), "_action", new MockMessage());
TransportMessage message = randomBoolean() ? new MockMessage() : new MockIndicesRequest();
auditTrail.accessDenied(new User.Simple("_username", "r1"), "_action", message);
switch (level) {
case ERROR:
case WARN:
case INFO:
assertMsg(logger, Level.ERROR, "ACCESS_DENIED\thost=[local[_host]], action=[_action], principal=[_username]");
if (message instanceof IndicesRequest) {
assertMsg(logger, Level.ERROR, "ACCESS_DENIED\thost=[local[_host]], principal=[_username], action=[_action], indices=[idx1,idx2]");
} else {
assertMsg(logger, Level.ERROR, "ACCESS_DENIED\thost=[local[_host]], principal=[_username], action=[_action]");
}
break;
case DEBUG:
case TRACE:
assertMsg(logger, Level.DEBUG, "ACCESS_DENIED\thost=[local[_host]], action=[_action], principal=[_username], request=[mock-message]");
if (message instanceof IndicesRequest) {
assertMsg(logger, Level.DEBUG, "ACCESS_DENIED\thost=[local[_host]], principal=[_username], action=[_action], indices=[idx1,idx2], request=[mock-message]");
} else {
assertMsg(logger, Level.DEBUG, "ACCESS_DENIED\thost=[local[_host]], principal=[_username], action=[_action], request=[mock-message]");
}
}
}
}
@ -194,6 +237,27 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
}
}
private static class MockIndicesRequest extends TransportMessage<MockIndicesRequest> implements IndicesRequest {
private MockIndicesRequest() {
remoteAddress(new LocalTransportAddress("_host"));
}
@Override
public String[] indices() {
return new String[] { "idx1", "idx2" };
}
@Override
public IndicesOptions indicesOptions() {
return IndicesOptions.strictExpandOpenAndForbidClosed();
}
@Override
public String toString() {
return "mock-message";
}
}
private static class MockToken implements AuthenticationToken {
@Override