Skip to content

Commit b7a6b02

Browse files
slobodanadamovicslobodanadamovic
authored
Fix audit logging to consistently include port number in origin.address (#86732) (#86969)
This commit changes audit logging of `connection_denied` and `connection_granted` events in order to include a port number. Closes #86694 (cherry picked from commit 954d288) # Conflicts: # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java
1 parent e8aee39 commit b7a6b02

File tree

10 files changed

+45
-36
lines changed

10 files changed

+45
-36
lines changed

‎docs/changelog/86732.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 86732
2+
summary: Fix audit logging to consistently include port number in `origin.address`
3+
area: Audit
4+
type: bug
5+
issues: []

‎x-pack/docs/en/security/auditing/event-types.asciidoc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ Logged when an incoming TCP connection does not pass the
188188
[source,js]
189189
{"type":"audit", "timestamp":"2020-12-30T21:47:31,526+0200", "node.id":
190190
"0RMNyghkQYCc_gVd1G6tZQ", "event.type":"ip_filter", "event.action":
191-
"connection_denied", "origin.type":"rest", "origin.address":"10.10.0.20",
191+
"connection_denied", "origin.type":"rest", "origin.address":"10.10.0.20:52314",
192192
"transport.profile":".http", "rule":"deny 10.10.0.0/16"}
193193
====
194194

@@ -203,7 +203,7 @@ for a specific profile.
203203
[source,js]
204204
{"type":"audit", "timestamp":"2020-12-30T21:47:31,526+0200", "node.id":
205205
"0RMNyghkQYCc_gVd1G6tZQ", "event.type":"ip_filter", "event.action":
206-
"connection_granted", "origin.type":"rest", "origin.address":"::1",
206+
"connection_granted", "origin.type":"rest", "origin.address":"[::1]:52314",
207207
"transport.profile":".http", "rule":"allow ::1,127.0.0.1"}
208208
====
209209

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
1616
import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule;
1717

18-
import java.net.InetAddress;
18+
import java.net.InetSocketAddress;
1919

2020
public interface AuditTrail {
2121

@@ -66,14 +66,14 @@ void accessDenied(
6666
void tamperedRequest(String requestId, Authentication authentication, String action, TransportRequest transportRequest);
6767

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

76-
void connectionDenied(InetAddress inetAddress, String profile, SecurityIpFilterRule rule);
76+
void connectionDenied(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule);
7777

7878
void runAsGranted(
7979
String requestId,

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import org.elasticsearch.xpack.security.Security;
2020
import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule;
2121

22-
import java.net.InetAddress;
22+
import java.net.InetSocketAddress;
2323
import java.time.Duration;
2424
import java.time.Instant;
2525
import java.util.Collections;
@@ -150,10 +150,10 @@ public void tamperedRequest(String requestId, String action, TransportRequest tr
150150
public void tamperedRequest(String requestId, Authentication authentication, String action, TransportRequest transportRequest) {}
151151

152152
@Override
153-
public void connectionGranted(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {}
153+
public void connectionGranted(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {}
154154

155155
@Override
156-
public void connectionDenied(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {}
156+
public void connectionDenied(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {}
157157

158158
@Override
159159
public void runAsGranted(
@@ -362,14 +362,14 @@ public void tamperedRequest(String requestId, Authentication authentication, Str
362362
}
363363

364364
@Override
365-
public void connectionGranted(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {
365+
public void connectionGranted(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {
366366
for (AuditTrail auditTrail : auditTrails) {
367367
auditTrail.connectionGranted(inetAddress, profile, rule);
368368
}
369369
}
370370

371371
@Override
372-
public void connectionDenied(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {
372+
public void connectionDenied(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {
373373
for (AuditTrail auditTrail : auditTrails) {
374374
auditTrail.connectionDenied(inetAddress, profile, rule);
375375
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@
9090
import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule;
9191

9292
import java.io.IOException;
93-
import java.net.InetAddress;
9493
import java.net.InetSocketAddress;
9594
import java.util.Arrays;
9695
import java.util.Collections;
@@ -879,7 +878,7 @@ public void tamperedRequest(String requestId, Authentication authentication, Str
879878
}
880879

881880
@Override
882-
public void connectionGranted(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {
881+
public void connectionGranted(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {
883882
if (events.contains(CONNECTION_GRANTED) && eventFilterPolicyRegistry.ignorePredicate().test(AuditEventMetaInfo.EMPTY) == false) {
884883
new LogEntryBuilder().with(EVENT_TYPE_FIELD_NAME, IP_FILTER_ORIGIN_FIELD_VALUE)
885884
.with(EVENT_ACTION_FIELD_NAME, "connection_granted")
@@ -896,7 +895,7 @@ public void connectionGranted(InetAddress inetAddress, String profile, SecurityI
896895
}
897896

898897
@Override
899-
public void connectionDenied(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {
898+
public void connectionDenied(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {
900899
if (events.contains(CONNECTION_DENIED) && eventFilterPolicyRegistry.ignorePredicate().test(AuditEventMetaInfo.EMPTY) == false) {
901900
new LogEntryBuilder().with(EVENT_TYPE_FIELD_NAME, IP_FILTER_ORIGIN_FIELD_VALUE)
902901
.with(EVENT_ACTION_FIELD_NAME, "connection_denied")

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,15 +288,15 @@ public boolean accept(String profile, InetSocketAddress peerAddress) {
288288
if (rule.matches(peerAddress)) {
289289
boolean isAllowed = rule.ruleType() == IpFilterRuleType.ACCEPT;
290290
if (isAllowed) {
291-
auditTrail.connectionGranted(peerAddress.getAddress(), profile, rule);
291+
auditTrail.connectionGranted(peerAddress, profile, rule);
292292
} else {
293-
auditTrail.connectionDenied(peerAddress.getAddress(), profile, rule);
293+
auditTrail.connectionDenied(peerAddress, profile, rule);
294294
}
295295
return isAllowed;
296296
}
297297
}
298298

299-
auditTrail.connectionGranted(peerAddress.getAddress(), profile, DEFAULT_PROFILE_ACCEPT_ALL);
299+
auditTrail.connectionGranted(peerAddress, profile, DEFAULT_PROFILE_ACCEPT_ALL);
300300
return true;
301301
}
302302

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.junit.Before;
2828

2929
import java.net.InetAddress;
30+
import java.net.InetSocketAddress;
3031
import java.time.Duration;
3132
import java.time.Instant;
3233
import java.util.ArrayList;
@@ -260,7 +261,7 @@ public void testAccessDenied() throws Exception {
260261
}
261262

262263
public void testConnectionGranted() throws Exception {
263-
InetAddress inetAddress = InetAddress.getLoopbackAddress();
264+
InetSocketAddress inetAddress = new InetSocketAddress(InetAddress.getLoopbackAddress(), randomIntBetween(0, 65535));
264265
SecurityIpFilterRule rule = randomBoolean() ? SecurityIpFilterRule.ACCEPT_ALL : IPFilter.DEFAULT_PROFILE_ACCEPT_ALL;
265266
service.get().connectionGranted(inetAddress, "client", rule);
266267
verify(licenseState).isAllowed(Security.AUDITING_FEATURE);
@@ -274,7 +275,7 @@ public void testConnectionGranted() throws Exception {
274275
}
275276

276277
public void testConnectionDenied() throws Exception {
277-
InetAddress inetAddress = InetAddress.getLoopbackAddress();
278+
InetSocketAddress inetAddress = new InetSocketAddress(InetAddress.getLoopbackAddress(), randomIntBetween(0, 65535));
278279
SecurityIpFilterRule rule = new SecurityIpFilterRule(false, "_all");
279280
service.get().connectionDenied(inetAddress, "client", rule);
280281
verify(licenseState).isAllowed(Security.AUDITING_FEATURE);

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,7 +1101,7 @@ public void testUsersFilter() throws Exception {
11011101
threadContext.stashContext();
11021102

11031103
// connection denied
1104-
auditTrail.connectionDenied(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
1104+
auditTrail.connectionDenied(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
11051105
if (filterMissingUser) {
11061106
assertThat("Connection denied: is not filtered out by the missing user filter", logOutput.size(), is(0));
11071107
} else {
@@ -1111,7 +1111,7 @@ public void testUsersFilter() throws Exception {
11111111
threadContext.stashContext();
11121112

11131113
// connection granted
1114-
auditTrail.connectionGranted(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
1114+
auditTrail.connectionGranted(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
11151115
if (filterMissingUser) {
11161116
assertThat("Connection granted: is not filtered out by the missing user filter", logOutput.size(), is(0));
11171117
} else {
@@ -1550,7 +1550,7 @@ public void testRealmsFilter() throws Exception {
15501550
threadContext.stashContext();
15511551

15521552
// connection denied
1553-
auditTrail.connectionDenied(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
1553+
auditTrail.connectionDenied(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
15541554
if (filterMissingRealm) {
15551555
assertThat("Connection denied: is not filtered out by the missing realm filter", logOutput.size(), is(0));
15561556
} else {
@@ -1560,7 +1560,7 @@ public void testRealmsFilter() throws Exception {
15601560
threadContext.stashContext();
15611561

15621562
// connection granted
1563-
auditTrail.connectionGranted(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
1563+
auditTrail.connectionGranted(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
15641564
if (filterMissingRealm) {
15651565
assertThat("Connection granted: is not filtered out by the missing realm filter", logOutput.size(), is(0));
15661566
} else {
@@ -1885,7 +1885,7 @@ public void testRolesFilter() throws Exception {
18851885
threadContext.stashContext();
18861886

18871887
// connection denied
1888-
auditTrail.connectionDenied(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
1888+
auditTrail.connectionDenied(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
18891889
if (filterMissingRoles) {
18901890
assertThat("Connection denied: is not filtered out by the missing roles filter", logOutput.size(), is(0));
18911891
} else {
@@ -1895,7 +1895,7 @@ public void testRolesFilter() throws Exception {
18951895
threadContext.stashContext();
18961896

18971897
// connection granted
1898-
auditTrail.connectionGranted(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
1898+
auditTrail.connectionGranted(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
18991899
if (filterMissingRoles) {
19001900
assertThat("Connection granted: is not filtered out by the missing roles filter", logOutput.size(), is(0));
19011901
} else {
@@ -2341,7 +2341,7 @@ public void testIndicesFilter() throws Exception {
23412341
threadContext.stashContext();
23422342

23432343
// connection denied
2344-
auditTrail.connectionDenied(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
2344+
auditTrail.connectionDenied(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
23452345
if (filterMissingIndices) {
23462346
assertThat("Connection denied: not filtered out by missing indices filter", logOutput.size(), is(0));
23472347
} else {
@@ -2351,7 +2351,7 @@ public void testIndicesFilter() throws Exception {
23512351
threadContext.stashContext();
23522352

23532353
// connection granted
2354-
auditTrail.connectionGranted(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
2354+
auditTrail.connectionGranted(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
23552355
if (filterMissingIndices) {
23562356
assertThat("Connection granted: not filtered out by missing indices filter", logOutput.size(), is(0));
23572357
} else {
@@ -2623,7 +2623,7 @@ public void testActionsFilter() throws Exception {
26232623
threadContext.stashContext();
26242624

26252625
// connection denied
2626-
auditTrail.connectionDenied(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
2626+
auditTrail.connectionDenied(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
26272627
if (filterMissingAction) {
26282628
assertThat("Connection denied: not filtered out by the missing action filter", logOutput.size(), is(0));
26292629
} else {
@@ -2633,7 +2633,7 @@ public void testActionsFilter() throws Exception {
26332633
threadContext.stashContext();
26342634

26352635
// connection granted
2636-
auditTrail.connectionGranted(InetAddress.getLoopbackAddress(), "default", new SecurityIpFilterRule(false, "_all"));
2636+
auditTrail.connectionGranted(randomLoopbackInetSocketAddress(), "default", new SecurityIpFilterRule(false, "_all"));
26372637
if (filterMissingAction) {
26382638
assertThat("Connection granted: not filtered out by the missing action filter", logOutput.size(), is(0));
26392639
} else {
@@ -2723,6 +2723,10 @@ public void testActionsFilter() throws Exception {
27232723
threadContext.stashContext();
27242724
}
27252725

2726+
private InetSocketAddress randomLoopbackInetSocketAddress() {
2727+
return new InetSocketAddress(InetAddress.getLoopbackAddress(), randomIntBetween(0, 65535));
2728+
}
2729+
27262730
private <T> List<T> randomListFromLengthBetween(List<T> l, int min, int max) {
27272731
assert (min >= 0) && (min <= max) && (max <= l.size());
27282732
final int len = randomIntBetween(min, max);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2049,7 +2049,7 @@ public void testTamperedRequestWithUser() throws Exception {
20492049
}
20502050

20512051
public void testConnectionDenied() throws Exception {
2052-
final InetAddress inetAddress = InetAddress.getLoopbackAddress();
2052+
final InetSocketAddress inetAddress = new InetSocketAddress(InetAddress.getLoopbackAddress(), randomIntBetween(0, 65535));
20532053
final SecurityIpFilterRule rule = new SecurityIpFilterRule(false, "_all");
20542054
final String profile = randomBoolean() ? IPFilter.HTTP_PROFILE_NAME : randomAlphaOfLengthBetween(1, 6);
20552055

@@ -2081,7 +2081,7 @@ public void testConnectionDenied() throws Exception {
20812081
}
20822082

20832083
public void testConnectionGranted() throws Exception {
2084-
final InetAddress inetAddress = InetAddress.getLoopbackAddress();
2084+
final InetSocketAddress inetAddress = new InetSocketAddress(InetAddress.getLoopbackAddress(), randomIntBetween(0, 65535));
20852085
final SecurityIpFilterRule rule = IPFilter.DEFAULT_PROFILE_ACCEPT_ALL;
20862086
final String profile = randomBoolean() ? IPFilter.HTTP_PROFILE_NAME : randomAlphaOfLengthBetween(1, 6);
20872087

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/filter/IPFilterTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,8 @@ public void testThatNodeStartsWithIPFilterDisabled() throws Exception {
296296
private void assertAddressIsAllowedForProfile(String profile, String... inetAddresses) {
297297
for (String inetAddress : inetAddresses) {
298298
String message = String.format(Locale.ROOT, "Expected address %s to be allowed", inetAddress);
299-
InetAddress address = InetAddresses.forString(inetAddress);
300-
assertTrue(message, ipFilter.accept(profile, new InetSocketAddress(address, 0)));
299+
InetSocketAddress address = new InetSocketAddress(InetAddresses.forString(inetAddress), 0);
300+
assertTrue(message, ipFilter.accept(profile, address));
301301
ArgumentCaptor<SecurityIpFilterRule> ruleCaptor = ArgumentCaptor.forClass(SecurityIpFilterRule.class);
302302
verify(auditTrail).connectionGranted(eq(address), eq(profile), ruleCaptor.capture());
303303
assertNotNull(ruleCaptor.getValue());
@@ -311,8 +311,8 @@ private void assertAddressIsAllowed(String... inetAddresses) {
311311
private void assertAddressIsDeniedForProfile(String profile, String... inetAddresses) {
312312
for (String inetAddress : inetAddresses) {
313313
String message = String.format(Locale.ROOT, "Expected address %s to be denied", inetAddress);
314-
InetAddress address = InetAddresses.forString(inetAddress);
315-
assertFalse(message, ipFilter.accept(profile, new InetSocketAddress(address, 0)));
314+
InetSocketAddress address = new InetSocketAddress(InetAddresses.forString(inetAddress), 0);
315+
assertFalse(message, ipFilter.accept(profile, address));
316316
ArgumentCaptor<SecurityIpFilterRule> ruleCaptor = ArgumentCaptor.forClass(SecurityIpFilterRule.class);
317317
verify(auditTrail).connectionDenied(eq(address), eq(profile), ruleCaptor.capture());
318318
assertNotNull(ruleCaptor.getValue());

0 commit comments

Comments
 (0)