Skip to content

Commit f353be2

Browse files
albertzaharovitsalbertzaharovits
authored
Audit authn success in the SecurityRestFilter (#94120)
Authentication for the HTTP channel is changing to not have access to the contents of the HTTP request body. But auditing of authentication success still requires access to the request body. Consequently, auditing of authentication success is now separated from the authentication logic: auditing is invoked in the SecurityRestFilter#handleRequest, after the AuthenticationService has done its job earlier in the handling flow for the HTTP request.
1 parent a375393 commit f353be2

14 files changed

+169
-53
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -929,7 +929,11 @@ Collection<Object> createComponents(
929929
components.add(allRolesStore); // for SecurityInfoTransportAction and clear roles cache
930930
components.add(authzService);
931931

932-
final SecondaryAuthenticator secondaryAuthenticator = new SecondaryAuthenticator(securityContext.get(), authcService.get());
932+
final SecondaryAuthenticator secondaryAuthenticator = new SecondaryAuthenticator(
933+
securityContext.get(),
934+
authcService.get(),
935+
auditTrailService
936+
);
933937
this.secondayAuthc.set(secondaryAuthenticator);
934938
components.add(secondaryAuthenticator);
935939

@@ -1655,6 +1659,7 @@ public UnaryOperator<RestHandler> getRestHandlerInterceptor(ThreadContext thread
16551659
threadContext,
16561660
authcService.get(),
16571661
secondayAuthc.get(),
1662+
auditTrailService.get(),
16581663
handler,
16591664
extractClientCertificate
16601665
);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public interface AuditTrail {
2222

2323
String name();
2424

25-
void authenticationSuccess(String requestId, Authentication authentication, RestRequest request);
25+
void authenticationSuccess(RestRequest request);
2626

2727
void authenticationSuccess(String requestId, Authentication authentication, String action, TransportRequest transportRequest);
2828

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public String name() {
8080
}
8181

8282
@Override
83-
public void authenticationSuccess(String requestId, Authentication authentication, RestRequest request) {}
83+
public void authenticationSuccess(RestRequest request) {}
8484

8585
@Override
8686
public void authenticationSuccess(

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.elasticsearch.xcontent.XContentBuilder;
4242
import org.elasticsearch.xcontent.json.JsonStringEncoder;
4343
import org.elasticsearch.xcontent.json.JsonXContent;
44+
import org.elasticsearch.xpack.core.security.SecurityContext;
4445
import org.elasticsearch.xpack.core.security.action.Grant;
4546
import org.elasticsearch.xpack.core.security.action.apikey.BaseUpdateApiKeyRequest;
4647
import org.elasticsearch.xpack.core.security.action.apikey.BulkUpdateApiKeyAction;
@@ -95,6 +96,7 @@
9596
import org.elasticsearch.xpack.security.Security;
9697
import org.elasticsearch.xpack.security.audit.AuditLevel;
9798
import org.elasticsearch.xpack.security.audit.AuditTrail;
99+
import org.elasticsearch.xpack.security.audit.AuditUtil;
98100
import org.elasticsearch.xpack.security.authc.ApiKeyService;
99101
import org.elasticsearch.xpack.security.authc.service.ServiceAccountToken;
100102
import org.elasticsearch.xpack.security.rest.RemoteHostHeader;
@@ -121,6 +123,7 @@
121123
import java.util.stream.Stream;
122124

123125
import static java.util.Map.entry;
126+
import static org.elasticsearch.core.Strings.format;
124127
import static org.elasticsearch.xpack.core.security.SecurityField.setting;
125128
import static org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings.TOKEN_NAME_FIELD;
126129
import static org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings.TOKEN_SOURCE_FIELD;
@@ -366,6 +369,7 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
366369

367370
private final Logger logger;
368371
private final ThreadContext threadContext;
372+
private final SecurityContext securityContext;
369373
final EventFilterPolicyRegistry eventFilterPolicyRegistry;
370374
// package for testing
371375
volatile EnumSet<AuditLevel> events;
@@ -387,6 +391,7 @@ public LoggingAuditTrail(Settings settings, ClusterService clusterService, Threa
387391
this.events = parse(INCLUDE_EVENT_SETTINGS.get(settings), EXCLUDE_EVENT_SETTINGS.get(settings));
388392
this.includeRequestBody = INCLUDE_REQUEST_BODY.get(settings);
389393
this.threadContext = threadContext;
394+
this.securityContext = new SecurityContext(settings, threadContext);
390395
this.entryCommonFields = new EntryCommonFields(settings, null, clusterService);
391396
this.eventFilterPolicyRegistry = new EventFilterPolicyRegistry(settings);
392397
clusterService.addListener(this);
@@ -444,7 +449,24 @@ public LoggingAuditTrail(Settings settings, ClusterService clusterService, Threa
444449
}
445450

446451
@Override
447-
public void authenticationSuccess(String requestId, Authentication authentication, RestRequest request) {
452+
public void authenticationSuccess(RestRequest request) {
453+
final String requestId = AuditUtil.extractRequestId(securityContext.getThreadContext());
454+
if (requestId == null) {
455+
// should never happen
456+
throw new ElasticsearchSecurityException("Authenticated context must include request id");
457+
}
458+
final Authentication authentication;
459+
try {
460+
authentication = securityContext.getAuthentication();
461+
} catch (Exception e) {
462+
logger.error(() -> format("caught exception while trying to read authentication from request [%s]", request), e);
463+
tamperedRequest(requestId, request);
464+
throw new ElasticsearchSecurityException("rest request attempted to inject a user", e);
465+
}
466+
if (authentication == null) {
467+
// should never happen
468+
throw new ElasticsearchSecurityException("Context is not authenticated");
469+
}
448470
if (events.contains(AUTHENTICATION_SUCCESS)
449471
&& eventFilterPolicyRegistry.ignorePredicate()
450472
.test(
@@ -468,7 +490,7 @@ public void authenticationSuccess(String requestId, Authentication authenticatio
468490
.withAuthentication(authentication)
469491
.withRestOrigin(threadContext)
470492
.withRequestBody(request)
471-
.withThreadContext(threadContext)
493+
.withThreadContext(securityContext.getThreadContext())
472494
.build();
473495
}
474496
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
*/
77
package org.elasticsearch.xpack.security.authc;
88

9-
import org.apache.logging.log4j.LogManager;
10-
import org.apache.logging.log4j.Logger;
119
import org.elasticsearch.ElasticsearchSecurityException;
1210
import org.elasticsearch.action.ActionListener;
1311
import org.elasticsearch.common.cache.Cache;
@@ -67,7 +65,6 @@ public class AuthenticationService {
6765
TimeValue.timeValueHours(1L),
6866
Property.NodeScope
6967
);
70-
private static final Logger logger = LogManager.getLogger(AuthenticationService.class);
7168

7269
private final Realms realms;
7370
private final AuditTrailService auditTrailService;
@@ -383,7 +380,14 @@ static class AuditableRestRequest extends AuditableRequest {
383380

384381
@Override
385382
void authenticationSuccess(Authentication authentication) {
386-
auditTrail.authenticationSuccess(requestId, authentication, request);
383+
// REST requests are audited in the {@code SecurityRestFilter} because they need access to the request body
384+
// see {@code AuditTrail#authenticationSuccess(RestRequest)}
385+
// It's still valuable to keep the parent interface {@code AuditableRequest#AuthenticationSuccess(Authentication)} around
386+
// in order to audit authN success for transport requests for CCS. We may be able to find another way to audit that, which
387+
// doesn't rely on an `AuditableRequest` instance, but it's not trivial because we'd have to make sure to not audit
388+
// existing authentications. Separately, it's not easy to reconstruct another `AuditableRequest` outside the
389+
// `AuthenticationService` because that's tied to the audit `request.id` generation.
390+
// For more context see: https://github.com/elastic/elasticsearch/pull/94120#discussion_r1152804133
387391
}
388392

389393
@Override

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/SecondaryAuthenticator.java

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.elasticsearch.xpack.core.security.authc.Authentication;
2222
import org.elasticsearch.xpack.core.security.authc.support.SecondaryAuthentication;
2323
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
24+
import org.elasticsearch.xpack.security.audit.AuditTrailService;
2425
import org.elasticsearch.xpack.security.authc.AuthenticationService;
2526

2627
import java.util.function.Consumer;
@@ -39,14 +40,25 @@ public class SecondaryAuthenticator {
3940
private final Logger logger = LogManager.getLogger(SecondaryAuthenticator.class);
4041
private final SecurityContext securityContext;
4142
private final AuthenticationService authenticationService;
43+
private final AuditTrailService auditTrailService;
4244

43-
public SecondaryAuthenticator(Settings settings, ThreadContext threadContext, AuthenticationService authenticationService) {
44-
this(new SecurityContext(settings, threadContext), authenticationService);
45+
public SecondaryAuthenticator(
46+
Settings settings,
47+
ThreadContext threadContext,
48+
AuthenticationService authenticationService,
49+
AuditTrailService auditTrailService
50+
) {
51+
this(new SecurityContext(settings, threadContext), authenticationService, auditTrailService);
4552
}
4653

47-
public SecondaryAuthenticator(SecurityContext securityContext, AuthenticationService authenticationService) {
54+
public SecondaryAuthenticator(
55+
SecurityContext securityContext,
56+
AuthenticationService authenticationService,
57+
AuditTrailService auditTrailService
58+
) {
4859
this.securityContext = securityContext;
4960
this.authenticationService = authenticationService;
61+
this.auditTrailService = auditTrailService;
5062
}
5163

5264
/**
@@ -76,7 +88,10 @@ public void authenticateAndAttachToContext(RestRequest request, ActionListener<S
7688
// Use cases for secondary authentication are far more likely to want to fall back to the primary authentication if no secondary
7789
// auth is provided, so in that case we do no want to set anything in the context
7890
authenticate(
79-
authListener -> authenticationService.authenticate(request, false, authListener),
91+
authListener -> authenticationService.authenticate(request, false, authListener.delegateFailure((l, authentication) -> {
92+
auditTrailService.get().authenticationSuccess(request);
93+
l.onResponse(authentication);
94+
})),
8095
ActionListener.wrap(secondaryAuthentication -> {
8196
if (secondaryAuthentication != null) {
8297
secondaryAuthentication.writeToContext(threadContext);
@@ -121,16 +136,4 @@ private void authenticate(Consumer<ActionListener<Authentication>> authenticate,
121136
authenticate.accept(authenticationListener);
122137
}
123138
}
124-
125-
/**
126-
* Checks whether this thread context provides secondary authentication credentials.
127-
* This does not check whether the header contains valid credentials
128-
* - you must call {@link #authenticateAndAttachToContext} to validate the header.
129-
*
130-
* @return {@code true} if a secondary authentication header exists in the thread context.
131-
*/
132-
public boolean hasSecondaryAuthenticationHeader() {
133-
final String header = securityContext.getThreadContext().getHeader(SECONDARY_AUTH_HEADER_NAME);
134-
return Strings.isNullOrEmpty(header) == false;
135-
}
136139
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.elasticsearch.rest.RestRequestFilter;
2323
import org.elasticsearch.rest.RestResponse;
2424
import org.elasticsearch.rest.RestStatus;
25+
import org.elasticsearch.xpack.security.audit.AuditTrailService;
2526
import org.elasticsearch.xpack.security.authc.AuthenticationService;
2627
import org.elasticsearch.xpack.security.authc.support.SecondaryAuthenticator;
2728
import org.elasticsearch.xpack.security.transport.SSLEngineUtils;
@@ -38,6 +39,7 @@ public class SecurityRestFilter implements RestHandler {
3839
private final RestHandler restHandler;
3940
private final AuthenticationService authenticationService;
4041
private final SecondaryAuthenticator secondaryAuthenticator;
42+
private final AuditTrailService auditTrailService;
4143
private final boolean enabled;
4244
private final ThreadContext threadContext;
4345
private final boolean extractClientCertificate;
@@ -64,13 +66,15 @@ public SecurityRestFilter(
6466
ThreadContext threadContext,
6567
AuthenticationService authenticationService,
6668
SecondaryAuthenticator secondaryAuthenticator,
69+
AuditTrailService auditTrailService,
6770
RestHandler restHandler,
6871
boolean extractClientCertificate
6972
) {
7073
this.enabled = enabled;
7174
this.threadContext = threadContext;
7275
this.authenticationService = authenticationService;
7376
this.secondaryAuthenticator = secondaryAuthenticator;
77+
this.auditTrailService = auditTrailService;
7478
this.restHandler = restHandler;
7579
this.extractClientCertificate = extractClientCertificate;
7680
}
@@ -102,15 +106,16 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
102106
SSLEngineUtils.extractClientCertificates(logger, threadContext, httpChannel);
103107
}
104108

109+
final RestRequest wrappedRequest = maybeWrapRestRequest(request);
105110
RemoteHostHeader.process(request, threadContext);
106-
107-
authenticationService.authenticate(maybeWrapRestRequest(request), ActionListener.wrap(authentication -> {
111+
authenticationService.authenticate(wrappedRequest, ActionListener.wrap(authentication -> {
108112
if (authentication == null) {
109113
logger.trace("No authentication available for REST request [{}]", request.uri());
110114
} else {
111115
logger.trace("Authenticated REST request [{}] as {}", request.uri(), authentication);
112116
}
113-
secondaryAuthenticator.authenticateAndAttachToContext(request, ActionListener.wrap(secondaryAuthentication -> {
117+
auditTrailService.get().authenticationSuccess(wrappedRequest);
118+
secondaryAuthenticator.authenticateAndAttachToContext(wrappedRequest, ActionListener.wrap(secondaryAuthentication -> {
114119
if (secondaryAuthentication != null) {
115120
logger.trace("Found secondary authentication {} in REST request [{}]", secondaryAuthentication, request.uri());
116121
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,10 @@ public void testAuthenticationSuccessRest() throws Exception {
272272
.realmRef(new RealmRef("_look", "_type", "node", null))
273273
.build();
274274
final String requestId = randomAlphaOfLengthBetween(6, 12);
275-
service.get().authenticationSuccess(requestId, authentication, restRequest);
275+
service.get().authenticationSuccess(restRequest);
276276
verify(licenseState).isAllowed(Security.AUDITING_FEATURE);
277277
if (isAuditingAllowed) {
278-
verify(auditTrail).authenticationSuccess(requestId, authentication, restRequest);
278+
verify(auditTrail).authenticationSuccess(restRequest);
279279
} else {
280280
verifyNoMoreInteractions(auditTrail);
281281
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailFilterTests.java

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
4141
import org.elasticsearch.xpack.core.security.user.SystemUser;
4242
import org.elasticsearch.xpack.core.security.user.User;
43+
import org.elasticsearch.xpack.security.audit.AuditUtil;
4344
import org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail.AuditEventMetaInfo;
4445
import org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrailTests.MockRequest;
4546
import org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrailTests.RestContent;
@@ -1139,12 +1140,16 @@ public void testUsersFilter() throws Exception {
11391140
threadContext.stashContext();
11401141

11411142
// authentication Success
1142-
auditTrail.authenticationSuccess(randomAlphaOfLength(8), unfilteredAuthentication, getRestRequest());
1143+
AuditUtil.generateRequestId(threadContext);
1144+
unfilteredAuthentication.writeToContext(threadContext);
1145+
auditTrail.authenticationSuccess(getRestRequest());
11431146
assertThat("AuthenticationSuccess rest request: unfiltered user is filtered out", logOutput.size(), is(1));
11441147
logOutput.clear();
11451148
threadContext.stashContext();
11461149

1147-
auditTrail.authenticationSuccess(randomAlphaOfLength(8), filteredAuthentication, getRestRequest());
1150+
AuditUtil.generateRequestId(threadContext);
1151+
filteredAuthentication.writeToContext(threadContext);
1152+
auditTrail.authenticationSuccess(getRestRequest());
11481153
assertThat("AuthenticationSuccess rest request: filtered user is not filtered out", logOutput.size(), is(0));
11491154
logOutput.clear();
11501155
threadContext.stashContext();
@@ -1634,12 +1639,16 @@ public void testRealmsFilter() throws Exception {
16341639
threadContext.stashContext();
16351640

16361641
// authentication Success
1637-
auditTrail.authenticationSuccess(randomAlphaOfLength(8), createAuthentication(user, authUser, unfilteredRealm), getRestRequest());
1642+
AuditUtil.generateRequestId(threadContext);
1643+
createAuthentication(user, authUser, unfilteredRealm).writeToContext(threadContext);
1644+
auditTrail.authenticationSuccess(getRestRequest());
16381645
assertThat("AuthenticationSuccess rest request: unfiltered realm is filtered out", logOutput.size(), is(1));
16391646
logOutput.clear();
16401647
threadContext.stashContext();
16411648

1642-
auditTrail.authenticationSuccess(randomAlphaOfLength(8), createAuthentication(user, authUser, filteredRealm), getRestRequest());
1649+
AuditUtil.generateRequestId(threadContext);
1650+
createAuthentication(user, authUser, filteredRealm).writeToContext(threadContext);
1651+
auditTrail.authenticationSuccess(getRestRequest());
16431652
assertThat("AuthenticationSuccess rest request: filtered realm is not filtered out", logOutput.size(), is(0));
16441653
logOutput.clear();
16451654
threadContext.stashContext();
@@ -1950,7 +1959,9 @@ public void testRolesFilter() throws Exception {
19501959
threadContext.stashContext();
19511960

19521961
// authentication Success
1953-
auditTrail.authenticationSuccess(randomAlphaOfLength(8), authentication, getRestRequest());
1962+
AuditUtil.generateRequestId(threadContext);
1963+
authentication.writeToContext(threadContext);
1964+
auditTrail.authenticationSuccess(getRestRequest());
19541965
if (filterMissingRoles) {
19551966
assertThat("AuthenticationSuccess rest request: is not filtered out by the missing roles filter", logOutput.size(), is(0));
19561967
} else {
@@ -2426,7 +2437,9 @@ public void testIndicesFilter() throws Exception {
24262437
threadContext.stashContext();
24272438

24282439
// authentication Success
2429-
auditTrail.authenticationSuccess(randomAlphaOfLength(8), authentication, getRestRequest());
2440+
AuditUtil.generateRequestId(threadContext);
2441+
authentication.writeToContext(threadContext);
2442+
auditTrail.authenticationSuccess(getRestRequest());
24302443
if (filterMissingIndices) {
24312444
assertThat("AuthenticationSuccess rest request: is not filtered out by the missing indices filter", logOutput.size(), is(0));
24322445
} else {
@@ -2697,7 +2710,9 @@ public void testActionsFilter() throws Exception {
26972710
threadContext.stashContext();
26982711

26992712
// authentication Success
2700-
auditTrail.authenticationSuccess(randomAlphaOfLength(8), createAuthentication(user, authUser, "realm"), getRestRequest());
2713+
AuditUtil.generateRequestId(threadContext);
2714+
createAuthentication(user, authUser, "realm").writeToContext(threadContext);
2715+
auditTrail.authenticationSuccess(getRestRequest());
27012716
if (filterMissingAction) {
27022717
assertThat("AuthenticationSuccess rest request: not filtered out by the missing action filter", logOutput.size(), is(0));
27032718
} else {

0 commit comments

Comments
 (0)