Skip to content

Commit 1a98568

Browse files
albertzaharovitsalbertzaharovits
authored
AuditTrail correctly handle ReplicatedWriteRequest (#39925)
This fix deduplicates index names in `BulkShardRequests` and only audits the specific resolved index for every comprising `BulkItemRequest`.
1 parent f6c0b19 commit 1a98568

File tree

8 files changed

+175
-19
lines changed

8 files changed

+175
-19
lines changed

‎server/src/main/java/org/elasticsearch/action/bulk/BulkShardRequest.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626
import org.elasticsearch.index.shard.ShardId;
2727

2828
import java.io.IOException;
29-
import java.util.ArrayList;
30-
import java.util.List;
29+
import java.util.HashSet;
30+
import java.util.Set;
3131

3232
public class BulkShardRequest extends ReplicatedWriteRequest<BulkShardRequest> {
3333

@@ -48,7 +48,13 @@ public BulkItemRequest[] items() {
4848

4949
@Override
5050
public String[] indices() {
51-
List<String> indices = new ArrayList<>();
51+
// A bulk shard request encapsulates items targeted at a specific shard of an index.
52+
// However, items could be targeting aliases of the index, so the bulk request although
53+
// targeting a single concrete index shard might do so using several alias names.
54+
// These alias names have to be exposed by this method because authorization works with
55+
// aliases too, specifically, the item's target alias can be authorized but the concrete
56+
// index might not be.
57+
Set<String> indices = new HashSet<>(1);
5258
for (BulkItemRequest item : items) {
5359
if (item != null) {
5460
indices.add(item.index());

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
package org.elasticsearch.xpack.security.audit;
77

8+
import org.elasticsearch.common.transport.TransportAddress;
89
import org.elasticsearch.rest.RestRequest;
910
import org.elasticsearch.transport.TransportMessage;
1011
import org.elasticsearch.xpack.core.security.authc.Authentication;
@@ -72,4 +73,13 @@ void runAsDenied(String requestId, Authentication authentication, String action,
7273
void runAsDenied(String requestId, Authentication authentication, RestRequest request,
7374
AuthorizationInfo authorizationInfo);
7475

76+
/**
77+
* This is a "workaround" method to log index "access_granted" and "access_denied" events for actions not tied to a
78+
* {@code TransportMessage}, or when the connection is not 1:1, i.e. several audit events for an action associated with the same
79+
* message. It is currently only used to audit the resolved index (alias) name for each {@code BulkItemRequest} comprised by a
80+
* {@code BulkShardRequest}. We should strive to not use this and TODO refactor it out!
81+
*/
82+
void explicitIndexAccessEvent(String requestId, AuditLevel eventType, Authentication authentication, String action, String indices,
83+
String requestName, TransportAddress remoteAddress, AuthorizationInfo authorizationInfo);
84+
7585
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.security.audit;
77

88
import org.elasticsearch.common.component.AbstractComponent;
9+
import org.elasticsearch.common.transport.TransportAddress;
910
import org.elasticsearch.license.XPackLicenseState;
1011
import org.elasticsearch.rest.RestRequest;
1112
import org.elasticsearch.transport.TransportMessage;
@@ -223,4 +224,16 @@ public void runAsDenied(String requestId, Authentication authentication, RestReq
223224
}
224225
}
225226
}
227+
228+
@Override
229+
public void explicitIndexAccessEvent(String requestId, AuditLevel eventType, Authentication authentication, String action,
230+
String indices, String requestName, TransportAddress remoteAddress,
231+
AuthorizationInfo authorizationInfo) {
232+
if (licenseState.isAuditingAllowed()) {
233+
for (AuditTrail auditTrail : auditTrails) {
234+
auditTrail.explicitIndexAccessEvent(requestId, eventType, authentication, action, indices, requestName, remoteAddress,
235+
authorizationInfo);
236+
}
237+
}
238+
}
226239
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrail.java‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
4141
import org.elasticsearch.common.util.concurrent.EsExecutors;
4242
import org.elasticsearch.common.util.concurrent.ThreadContext;
43+
import org.elasticsearch.common.util.set.Sets;
4344
import org.elasticsearch.common.xcontent.XContentBuilder;
4445
import org.elasticsearch.common.xcontent.XContentFactory;
4546
import org.elasticsearch.common.xcontent.XContentType;
@@ -665,6 +666,30 @@ public void accessDenied(String requestId, Authentication authentication, String
665666
}
666667
}
667668

669+
@Override
670+
public void explicitIndexAccessEvent(String requestId, AuditLevel eventType, Authentication authentication, String action, String index,
671+
String requestName, TransportAddress remoteAddress, AuthorizationInfo authorizationInfo) {
672+
assert eventType == ACCESS_DENIED || eventType == AuditLevel.ACCESS_GRANTED || eventType == SYSTEM_ACCESS_GRANTED;
673+
final User user = authentication.getUser();
674+
final boolean isSystem = SystemUser.is(user) || XPackUser.is(user);
675+
if (isSystem && eventType == ACCESS_GRANTED) {
676+
eventType = SYSTEM_ACCESS_GRANTED;
677+
}
678+
if (events.contains(eventType)) {
679+
try {
680+
assert authentication.getAuthenticatedBy() != null;
681+
final String eventTypeString = eventType == ACCESS_DENIED ? "access_denied" : "access_granted";
682+
final String authRealmName = authentication.getAuthenticatedBy().getName();
683+
final String lookRealmName = authentication.getLookedUpBy() == null ? null : authentication.getLookedUpBy().getName();
684+
final String[] roleNames = (String[]) authorizationInfo.asMap().get(LoggingAuditTrail.PRINCIPAL_ROLES_FIELD_NAME);
685+
enqueue(message(eventTypeString, action, user, roleNames, new Tuple(authRealmName, lookRealmName), Sets.newHashSet(index),
686+
remoteAddress, requestName), eventTypeString);
687+
} catch (final Exception e) {
688+
logger.warn("failed to index audit event: [access_denied]", e);
689+
}
690+
}
691+
}
692+
668693
@Override
669694
public void tamperedRequest(String requestId, RestRequest request) {
670695
if (events.contains(TAMPERED_REQUEST)) {
@@ -773,10 +798,15 @@ public void runAsDenied(String requestId, Authentication authentication, RestReq
773798
private Message message(String type, @Nullable String action, @Nullable User user, @Nullable String[] roleNames,
774799
@Nullable Tuple<String, String> realms, @Nullable Set<String> indices, TransportMessage message)
775800
throws Exception {
801+
return message(type, action, user, roleNames, realms, indices, message.remoteAddress(), message.getClass().getSimpleName());
802+
}
776803

804+
private Message message(String type, @Nullable String action, @Nullable User user, @Nullable String[] roleNames,
805+
@Nullable Tuple<String, String> realms, @Nullable Set<String> indices,
806+
TransportAddress address, String requestName) throws Exception {
777807
Message msg = new Message().start();
778808
common("transport", type, msg.builder);
779-
originAttributes(message, msg.builder, clusterService.localNode(), threadPool.getThreadContext());
809+
originAttributes(address, msg.builder, clusterService.localNode(), threadPool.getThreadContext());
780810

781811
if (action != null) {
782812
msg.builder.field(Field.ACTION, action);
@@ -788,7 +818,7 @@ private Message message(String type, @Nullable String action, @Nullable User use
788818
if (indices != null) {
789819
msg.builder.array(Field.INDICES, indices.toArray(Strings.EMPTY_ARRAY));
790820
}
791-
msg.builder.field(Field.REQUEST, message.getClass().getSimpleName());
821+
msg.builder.field(Field.REQUEST, requestName);
792822

793823
return msg.end();
794824
}
@@ -943,6 +973,11 @@ private XContentBuilder common(String layer, String type, XContentBuilder builde
943973

944974
private static XContentBuilder originAttributes(TransportMessage message, XContentBuilder builder,
945975
DiscoveryNode localNode, ThreadContext threadContext) throws IOException {
976+
return originAttributes(message.remoteAddress(), builder, localNode, threadContext);
977+
}
978+
979+
private static XContentBuilder originAttributes(TransportAddress address, XContentBuilder builder,
980+
DiscoveryNode localNode, ThreadContext threadContext) throws IOException {
946981

947982
// first checking if the message originated in a rest call
948983
InetSocketAddress restAddress = RemoteHostHeader.restRemoteAddress(threadContext);
@@ -953,7 +988,6 @@ private static XContentBuilder originAttributes(TransportMessage message, XConte
953988
}
954989

955990
// we'll see if was originated in a remote node
956-
TransportAddress address = message.remoteAddress();
957991
if (address != null) {
958992
builder.field(Field.ORIGIN_TYPE, "transport");
959993
builder.field(Field.ORIGIN_ADDRESS,

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/DeprecatedLoggingAuditTrail.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,51 @@ localNodeInfo.prefix, originAttributes(threadContext, message, localNodeInfo), s
356356
}
357357
}
358358

359+
@Override
360+
public void explicitIndexAccessEvent(String requestId, AuditLevel eventType, Authentication authentication, String action, String index,
361+
String requestName, TransportAddress remoteAddress, AuthorizationInfo authorizationInfo) {
362+
assert eventType == ACCESS_DENIED || eventType == AuditLevel.ACCESS_GRANTED || eventType == SYSTEM_ACCESS_GRANTED;
363+
final String[] indices = index == null ? null : new String[] { index };
364+
final User user = authentication.getUser();
365+
final boolean isSystem = SystemUser.is(user) || XPackUser.is(user);
366+
if (isSystem && eventType == ACCESS_GRANTED) {
367+
eventType = SYSTEM_ACCESS_GRANTED;
368+
}
369+
if (events.contains(eventType)) {
370+
if (eventFilterPolicyRegistry.ignorePredicate()
371+
.test(new AuditEventMetaInfo(Optional.of(user), Optional.of(effectiveRealmName(authentication)),
372+
Optional.of(authorizationInfo), Optional.ofNullable(indices))) == false) {
373+
final String[] roleNames = (String[]) authorizationInfo.asMap().get(LoggingAuditTrail.PRINCIPAL_ROLES_FIELD_NAME);
374+
final StringBuilder logEntryBuilder = new StringBuilder();
375+
logEntryBuilder.append(localNodeInfo.prefix);
376+
logEntryBuilder.append("[transport] ");
377+
if (eventType == ACCESS_DENIED) {
378+
logEntryBuilder.append("[access_denied]\t");
379+
} else {
380+
logEntryBuilder.append("[access_granted]\t");
381+
}
382+
final String originAttributes = restOriginTag(threadContext).orElseGet(() -> {
383+
if (remoteAddress == null) {
384+
return localNodeInfo.localOriginTag;
385+
}
386+
return new StringBuilder("origin_type=[transport], origin_address=[")
387+
.append(NetworkAddress.format(remoteAddress.address().getAddress())).append("]").toString();
388+
});
389+
logEntryBuilder.append(originAttributes).append(", ");
390+
logEntryBuilder.append(subject(authentication)).append(", ");
391+
logEntryBuilder.append("roles=[").append(arrayToCommaDelimitedString(roleNames)).append("], ");
392+
logEntryBuilder.append("action=[").append(action).append("], ");
393+
logEntryBuilder.append("indices=[").append(index).append("], ");
394+
logEntryBuilder.append("request=[").append(requestName).append("]");
395+
final String opaqueId = threadContext.getHeader(Task.X_OPAQUE_ID);
396+
if (opaqueId != null) {
397+
logEntryBuilder.append(", opaque_id=[").append(opaqueId).append("]");
398+
}
399+
logger.info(logEntryBuilder.toString());
400+
}
401+
}
402+
}
403+
359404
@Override
360405
public void tamperedRequest(String requestId, RestRequest request) {
361406
if (events.contains(TAMPERED_REQUEST) && (eventFilterPolicyRegistry.ignorePredicate().test(AuditEventMetaInfo.EMPTY) == false)) {

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,46 @@ public void accessGranted(String requestId, Authentication authentication, Strin
452452
}
453453
}
454454

455+
@Override
456+
public void explicitIndexAccessEvent(String requestId, AuditLevel eventType, Authentication authentication, String action, String index,
457+
String requestName, TransportAddress remoteAddress, AuthorizationInfo authorizationInfo) {
458+
assert eventType == ACCESS_DENIED || eventType == AuditLevel.ACCESS_GRANTED || eventType == SYSTEM_ACCESS_GRANTED;
459+
final String[] indices = index == null ? null : new String[] { index };
460+
final User user = authentication.getUser();
461+
final boolean isSystem = SystemUser.is(user) || XPackUser.is(user);
462+
if (isSystem && eventType == ACCESS_GRANTED) {
463+
eventType = SYSTEM_ACCESS_GRANTED;
464+
}
465+
if (events.contains(eventType)) {
466+
if (eventFilterPolicyRegistry.ignorePredicate()
467+
.test(new AuditEventMetaInfo(Optional.of(user), Optional.of(effectiveRealmName(authentication)),
468+
Optional.of(authorizationInfo), Optional.ofNullable(indices))) == false) {
469+
final LogEntryBuilder logEntryBuilder = new LogEntryBuilder()
470+
.with(EVENT_TYPE_FIELD_NAME, TRANSPORT_ORIGIN_FIELD_VALUE)
471+
.with(EVENT_ACTION_FIELD_NAME, eventType == ACCESS_DENIED ? "access_denied" : "access_granted")
472+
.with(ACTION_FIELD_NAME, action)
473+
.with(REQUEST_NAME_FIELD_NAME, requestName)
474+
.withRequestId(requestId)
475+
.withSubject(authentication)
476+
.with(INDICES_FIELD_NAME, indices)
477+
.withOpaqueId(threadContext)
478+
.withXForwardedFor(threadContext)
479+
.with(authorizationInfo.asMap());
480+
final InetSocketAddress restAddress = RemoteHostHeader.restRemoteAddress(threadContext);
481+
if (restAddress != null) {
482+
logEntryBuilder
483+
.with(ORIGIN_TYPE_FIELD_NAME, REST_ORIGIN_FIELD_VALUE)
484+
.with(ORIGIN_ADDRESS_FIELD_NAME, NetworkAddress.format(restAddress));
485+
} else if (remoteAddress != null) {
486+
logEntryBuilder
487+
.with(ORIGIN_TYPE_FIELD_NAME, TRANSPORT_ORIGIN_FIELD_VALUE)
488+
.with(ORIGIN_ADDRESS_FIELD_NAME, NetworkAddress.format(remoteAddress.address()));
489+
}
490+
logger.info(logEntryBuilder.build());
491+
}
492+
}
493+
}
494+
455495
@Override
456496
public void accessDenied(String requestId, Authentication authentication, String action, TransportMessage message,
457497
AuthorizationInfo authorizationInfo) {

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import org.elasticsearch.xpack.core.security.user.User;
6565
import org.elasticsearch.xpack.core.security.user.XPackSecurityUser;
6666
import org.elasticsearch.xpack.core.security.user.XPackUser;
67+
import org.elasticsearch.xpack.security.audit.AuditLevel;
6768
import org.elasticsearch.xpack.security.audit.AuditTrailService;
6869
import org.elasticsearch.xpack.security.audit.AuditUtil;
6970
import org.elasticsearch.xpack.security.authz.interceptor.RequestInterceptor;
@@ -311,10 +312,12 @@ private void handleIndexActionAuthorizationResult(final IndexAuthorizationResult
311312
// if this is performing multiple actions on the index, then check each of those actions.
312313
assert request instanceof BulkShardRequest
313314
: "Action " + action + " requires " + BulkShardRequest.class + " but was " + request.getClass();
314-
authorizeBulkItems(requestInfo, authzInfo, authzEngine, resolvedIndicesAsyncSupplier, authorizedIndicesSupplier,
315-
metaData, requestId,
316-
ActionListener.wrap(ignore -> runRequestInterceptors(requestInfo, authzInfo, authorizationEngine, listener),
317-
listener::onFailure));
315+
authorizeBulkItems(requestInfo, authzInfo, authzEngine, resolvedIndicesAsyncSupplier, authorizedIndicesSupplier, metaData,
316+
requestId,
317+
wrapPreservingContext(
318+
ActionListener.wrap(ignore -> runRequestInterceptors(requestInfo, authzInfo, authorizationEngine, listener),
319+
listener::onFailure),
320+
threadContext));
318321
} else {
319322
runRequestInterceptors(requestInfo, authzInfo, authorizationEngine, listener);
320323
}
@@ -496,11 +499,12 @@ private void authorizeBulkItems(RequestInfo requestInfo, AuthorizationInfo authz
496499
for (BulkItemRequest item : request.items()) {
497500
final String resolvedIndex = resolvedIndexNames.get(item.index());
498501
final String itemAction = getAction(item);
499-
final IndicesAccessControl indicesAccessControl = actionToIndicesAccessControl.get(getAction(item));
502+
final IndicesAccessControl indicesAccessControl = actionToIndicesAccessControl.get(itemAction);
500503
final IndicesAccessControl.IndexAccessControl indexAccessControl
501504
= indicesAccessControl.getIndexPermissions(resolvedIndex);
502505
if (indexAccessControl == null || indexAccessControl.isGranted() == false) {
503-
auditTrail.accessDenied(requestId, authentication, itemAction, request, authzInfo);
506+
auditTrail.explicitIndexAccessEvent(requestId, AuditLevel.ACCESS_DENIED, authentication, itemAction,
507+
resolvedIndex, item.getClass().getSimpleName(), request.remoteAddress(), authzInfo);
504508
item.abort(resolvedIndex, denialException(authentication, itemAction, null));
505509
}
506510
}
@@ -522,7 +526,7 @@ private void authorizeBulkItems(RequestInfo requestInfo, AuthorizationInfo authz
522526
}, listener::onFailure));
523527
}
524528

525-
private IllegalArgumentException illegalArgument(String message) {
529+
private static IllegalArgumentException illegalArgument(String message) {
526530
assert false : message;
527531
return new IllegalArgumentException(message);
528532
}

0 commit comments

Comments
 (0)