Skip to content

Add "request.id" to file audit logs #35536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 27, 2018
Merged

Add "request.id" to file audit logs #35536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
95c4da1
Add "request.id" to file audit logs
tvernum Nov 14, 2018
c0c9639
Explicitly require request.id for Authorization
tvernum Nov 14, 2018
c79201a
Switch to random UUIDs for request.id
tvernum Nov 14, 2018
cba24fb
Merge branch 'master' into audit-request-id
tvernum Nov 21, 2018
bf400a9
Address feedback
tvernum Nov 21, 2018
d21d79c
Switch audit request.id to be a header, not transient
tvernum Nov 23, 2018
5879bcc
Merge branch 'master' into audit-request-id
tvernum Nov 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions x-pack/plugin/core/src/main/config/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ appender.audit_rolling.layout.pattern = {\
%varsNotEmpty{, "url.path":"%enc{%map{url.path}}{JSON}"}\
%varsNotEmpty{, "url.query":"%enc{%map{url.query}}{JSON}"}\
%varsNotEmpty{, "request.body":"%enc{%map{request.body}}{JSON}"}\
%varsNotEmpty{, "request.id":"%enc{%map{request.id}}{JSON}"}\
%varsNotEmpty{, "action":"%enc{%map{action}}{JSON}"}\
%varsNotEmpty{, "request.name":"%enc{%map{request.name}}{JSON}"}\
%varsNotEmpty{, "indices":%map{indices}}\
Expand Down Expand Up @@ -50,6 +51,7 @@ appender.audit_rolling.layout.pattern = {\
# "url.path" the URI component between the port and the query string; it is percent (URL) encoded
# "url.query" the URI component after the path and before the fragment; it is percent (URL) encoded
# "request.body" the content of the request body entity, JSON escaped
# "request.id" a synthentic identifier for the incoming request, this is unique per incoming request, and consistent across all audit events generated by that request
# "action" an action is the most granular operation that is authorized and this identifies it in a namespaced way (internal)
# "request.name" if the event is in connection to a transport message this is the name of the request class, similar to how rest requests are identified by the url path (internal)
# "indices" the array of indices that the "action" is acting upon
Expand Down
4 changes: 3 additions & 1 deletion ...org/elasticsearch/xpack/security/action/interceptor/IndicesAliasesRequestInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.xpack.core.security.authz.permission.Role;
import org.elasticsearch.xpack.core.security.support.Exceptions;
import org.elasticsearch.xpack.security.audit.AuditTrailService;
import org.elasticsearch.xpack.security.audit.AuditUtil;

import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -70,7 +71,8 @@ public void intercept(IndicesAliasesRequest request, Authentication authenticati
permissionsMap.computeIfAbsent(alias, userPermissions.indices()::allowedActionsMatcher);
if (Operations.subsetOf(aliasPermissions, indexPermissions) == false) {
// TODO we've already audited a access granted event so this is going to look ugly
auditTrailService.accessDenied(authentication, action, request, userPermissions.names());
auditTrailService.accessDenied(AuditUtil.extractRequestId(threadContext), authentication, action, request,
userPermissions.names());
throw Exceptions.authorizationError("Adding an alias is not allowed when the alias " +
"has more permissions than any of the indices");
}
Expand Down
4 changes: 3 additions & 1 deletion ...in/java/org/elasticsearch/xpack/security/action/interceptor/ResizeRequestInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import org.elasticsearch.xpack.core.security.support.Exceptions;
import org.elasticsearch.xpack.security.audit.AuditTrailService;

import static org.elasticsearch.xpack.security.audit.AuditUtil.extractRequestId;

public final class ResizeRequestInterceptor implements RequestInterceptor<ResizeRequest> {

private final ThreadContext threadContext;
Expand Down Expand Up @@ -59,7 +61,7 @@ public void intercept(ResizeRequest request, Authentication authentication, Role
userPermissions.indices().allowedActionsMatcher(request.getTargetIndexRequest().index());
if (Operations.subsetOf(targetIndexPermissions, sourceIndexPermissions) == false) {
// TODO we've already audited a access granted event so this is going to look ugly
auditTrailService.accessDenied(authentication, action, request, userPermissions.names());
auditTrailService.accessDenied(extractRequestId(threadContext), authentication, action, request, userPermissions.names());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

throw Exceptions.authorizationError("Resizing an index is not allowed when the target index " +
"has more permissions than the source index");
}
Expand Down
43 changes: 25 additions & 18 deletions x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,43 +18,50 @@ public interface AuditTrail {

String name();

void authenticationSuccess(String realm, User user, RestRequest request);
void authenticationSuccess(String requestId, String realm, User user, RestRequest request);

void authenticationSuccess(String realm, User user, String action, TransportMessage message);
void authenticationSuccess(String requestId, String realm, User user, String action, TransportMessage message);

void anonymousAccessDenied(String action, TransportMessage message);
void anonymousAccessDenied(String requestId, String action, TransportMessage message);

void anonymousAccessDenied(RestRequest request);
void anonymousAccessDenied(String requestId, RestRequest request);

void authenticationFailed(RestRequest request);
void authenticationFailed(String requestId, RestRequest request);

void authenticationFailed(String action, TransportMessage message);
void authenticationFailed(String requestId, String action, TransportMessage message);

void authenticationFailed(AuthenticationToken token, String action, TransportMessage message);
void authenticationFailed(String requestId, AuthenticationToken token, String action, TransportMessage message);

void authenticationFailed(AuthenticationToken token, RestRequest request);
void authenticationFailed(String requestId, AuthenticationToken token, RestRequest request);

void authenticationFailed(String realm, AuthenticationToken token, String action, TransportMessage message);
void authenticationFailed(String requestId, String realm, AuthenticationToken token, String action, TransportMessage message);

void authenticationFailed(String realm, AuthenticationToken token, RestRequest request);
void authenticationFailed(String requestId, String realm, AuthenticationToken token, RestRequest request);

void accessGranted(Authentication authentication, String action, TransportMessage message, String[] roleNames);
void accessGranted(String requestId, Authentication authentication, String action, TransportMessage message, String[] roleNames);

void accessDenied(Authentication authentication, String action, TransportMessage message, String[] roleNames);
void accessDenied(String requestId, Authentication authentication, String action, TransportMessage message, String[] roleNames);

void tamperedRequest(RestRequest request);
void tamperedRequest(String requestId, RestRequest request);

void tamperedRequest(String action, TransportMessage message);
void tamperedRequest(String requestId, String action, TransportMessage message);

void tamperedRequest(User user, String action, TransportMessage request);
void tamperedRequest(String requestId, User user, String action, TransportMessage request);

/**
* The {@link #connectionGranted(InetAddress, String, SecurityIpFilterRule)} and
* {@link #connectionDenied(InetAddress, String, SecurityIpFilterRule)} methods do not have a requestId because they related to a
* potentially long-lived TCP connection, not a single request. For both Transport and Rest connections, a single connection
* granted/denied event is generated even if that connection is used for multiple Elasticsearch actions (potentially as different users)
*/
void connectionGranted(InetAddress inetAddress, String profile, SecurityIpFilterRule rule);

void connectionDenied(InetAddress inetAddress, String profile, SecurityIpFilterRule rule);

void runAsGranted(Authentication authentication, String action, TransportMessage message, String[] roleNames);
void runAsGranted(String requestId, Authentication authentication, String action, TransportMessage message, String[] roleNames);

void runAsDenied(Authentication authentication, String action, TransportMessage message, String[] roleNames);
void runAsDenied(String requestId, Authentication authentication, String action, TransportMessage message, String[] roleNames);

void runAsDenied(String requestId, Authentication authentication, RestRequest request, String[] roleNames);

void runAsDenied(Authentication authentication, RestRequest request, String[] roleNames);
}
72 changes: 36 additions & 36 deletions ...ugin/security/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,136 +38,136 @@ public List<AuditTrail> getAuditTrails() {
}

@Override
public void authenticationSuccess(String realm, User user, RestRequest request) {
public void authenticationSuccess(String requestId, String realm, User user, RestRequest request) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.authenticationSuccess(realm, user, request);
auditTrail.authenticationSuccess(requestId, realm, user, request);
}
}
}

@Override
public void authenticationSuccess(String realm, User user, String action, TransportMessage message) {
public void authenticationSuccess(String requestId, String realm, User user, String action, TransportMessage message) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.authenticationSuccess(realm, user, action, message);
auditTrail.authenticationSuccess(requestId, realm, user, action, message);
}
}
}

@Override
public void anonymousAccessDenied(String action, TransportMessage message) {
public void anonymousAccessDenied(String requestId, String action, TransportMessage message) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.anonymousAccessDenied(action, message);
auditTrail.anonymousAccessDenied(requestId, action, message);
}
}
}

@Override
public void anonymousAccessDenied(RestRequest request) {
public void anonymousAccessDenied(String requestId, RestRequest request) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.anonymousAccessDenied(request);
auditTrail.anonymousAccessDenied(requestId, request);
}
}
}

@Override
public void authenticationFailed(RestRequest request) {
public void authenticationFailed(String requestId, RestRequest request) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.authenticationFailed(request);
auditTrail.authenticationFailed(requestId, request);
}
}
}

@Override
public void authenticationFailed(String action, TransportMessage message) {
public void authenticationFailed(String requestId, String action, TransportMessage message) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.authenticationFailed(action, message);
auditTrail.authenticationFailed(requestId, action, message);
}
}
}

@Override
public void authenticationFailed(AuthenticationToken token, String action, TransportMessage message) {
public void authenticationFailed(String requestId, AuthenticationToken token, String action, TransportMessage message) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.authenticationFailed(token, action, message);
auditTrail.authenticationFailed(requestId, token, action, message);
}
}
}

@Override
public void authenticationFailed(String realm, AuthenticationToken token, String action, TransportMessage message) {
public void authenticationFailed(String requestId, String realm, AuthenticationToken token, String action, TransportMessage message) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.authenticationFailed(realm, token, action, message);
auditTrail.authenticationFailed(requestId, realm, token, action, message);
}
}
}

@Override
public void authenticationFailed(AuthenticationToken token, RestRequest request) {
public void authenticationFailed(String requestId, AuthenticationToken token, RestRequest request) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.authenticationFailed(token, request);
auditTrail.authenticationFailed(requestId, token, request);
}
}
}

@Override
public void authenticationFailed(String realm, AuthenticationToken token, RestRequest request) {
public void authenticationFailed(String requestId, String realm, AuthenticationToken token, RestRequest request) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.authenticationFailed(realm, token, request);
auditTrail.authenticationFailed(requestId, realm, token, request);
}
}
}

@Override
public void accessGranted(Authentication authentication, String action, TransportMessage message, String[] roleNames) {
public void accessGranted(String requestId, Authentication authentication, String action, TransportMessage msg, String[] roleNames) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.accessGranted(authentication, action, message, roleNames);
auditTrail.accessGranted(requestId, authentication, action, msg, roleNames);
}
}
}

@Override
public void accessDenied(Authentication authentication, String action, TransportMessage message, String[] roleNames) {
public void accessDenied(String requestId, Authentication authentication, String action, TransportMessage message, String[] roleNames) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.accessDenied(authentication, action, message, roleNames);
auditTrail.accessDenied(requestId, authentication, action, message, roleNames);
}
}
}

@Override
public void tamperedRequest(RestRequest request) {
public void tamperedRequest(String requestId, RestRequest request) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.tamperedRequest(request);
auditTrail.tamperedRequest(requestId, request);
}
}
}

@Override
public void tamperedRequest(String action, TransportMessage message) {
public void tamperedRequest(String requestId, String action, TransportMessage message) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.tamperedRequest(action, message);
auditTrail.tamperedRequest(requestId, action, message);
}
}
}

@Override
public void tamperedRequest(User user, String action, TransportMessage request) {
public void tamperedRequest(String requestId, User user, String action, TransportMessage request) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.tamperedRequest(user, action, request);
auditTrail.tamperedRequest(requestId, user, action, request);
}
}
}
Expand All @@ -191,28 +191,28 @@ public void connectionDenied(InetAddress inetAddress, String profile, SecurityIp
}

@Override
public void runAsGranted(Authentication authentication, String action, TransportMessage message, String[] roleNames) {
public void runAsGranted(String requestId, Authentication authentication, String action, TransportMessage message, String[] roleNames) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.runAsGranted(authentication, action, message, roleNames);
auditTrail.runAsGranted(requestId, authentication, action, message, roleNames);
}
}
}

@Override
public void runAsDenied(Authentication authentication, String action, TransportMessage message, String[] roleNames) {
public void runAsDenied(String requestId, Authentication authentication, String action, TransportMessage message, String[] roleNames) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.runAsDenied(authentication, action, message, roleNames);
auditTrail.runAsDenied(requestId, authentication, action, message, roleNames);
}
}
}

@Override
public void runAsDenied(Authentication authentication, RestRequest request, String[] roleNames) {
public void runAsDenied(String requestId, Authentication authentication, RestRequest request, String[] roleNames) {
if (licenseState.isAuditingAllowed()) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.runAsDenied(authentication, request, roleNames);
auditTrail.runAsDenied(requestId, authentication, request, roleNames);
}
}
}
Expand Down
35 changes: 35 additions & 0 deletions x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/AuditUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
package org.elasticsearch.xpack.security.audit;

import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.transport.TransportMessage;
Expand All @@ -17,6 +20,8 @@

public class AuditUtil {

private static final String AUDIT_REQUEST_ID = "_xpack_audit_request_id";

public static String restRequestContent(RestRequest request) {
if (request.hasContent()) {
try {
Expand All @@ -38,4 +43,34 @@ public static Set<String> indices(TransportMessage message) {
private static Set<String> arrayToSetOrNull(String[] indices) {
return indices == null ? null : new HashSet<>(Arrays.asList(indices));
}

public static String generateRequestId(ThreadContext threadContext) {
return generateRequestId(threadContext, true);
}

public static String getOrGenerateRequestId(ThreadContext threadContext) {
final String requestId = extractRequestId(threadContext);
if (Strings.isEmpty(requestId)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can get rid of this.

Copy link
Contributor

@albertzaharovits albertzaharovits Nov 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in: be explicit when we generate these.
I know it would be a bummer if we hit an assertion about an event not assigned to a request, but I think there's value if we catch such an error, because such an event might reveal hidden bugs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not really feasible to do that unfortunately.
When we process a transport action, we simply don't/can't know whether that action was spawned from an existing rest/transport request or whether it's standalone.

Well, I guess we could refactor a lot of stuff to make that explicit, but it's not right now.
Maybe, between system context, and withOrigin we could pull something together to know when a transport action is isolated from any originating request, but it's going to be fragile.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

return generateRequestId(threadContext, false);
}
return requestId;
}

private static String generateRequestId(ThreadContext threadContext, boolean checkExisting) {
if (checkExisting) {
final String existing = extractRequestId(threadContext);
if (existing != null) {
throw new IllegalStateException("Cannot generate a new audit request id - existing id ["
+ existing + "] already registered");
}
}
final String requestId = UUIDs.randomBase64UUID();
// Store as a header (not transient) so that it is passed over the network if this request requires execution on other nodes
threadContext.putHeader(AUDIT_REQUEST_ID, requestId);
return requestId;
}

public static String extractRequestId(ThreadContext threadContext) {
return threadContext.getHeader(AUDIT_REQUEST_ID);
}
}
Loading