Skip to content

Commit

Permalink
Fix audit logging to consistently include port number in origin.addre…
Browse files Browse the repository at this point in the history 
…ss (elastic#86732)

This commit changes audit logging of `connection_denied`
and `connection_granted` events in order to include a port number.

Closes elastic#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
  • Loading branch information
@slobodanadamovic
slobodanadamovic committed May 20, 2022
1 parent e8aee39 commit 8b375a3
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 36 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/86732.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 86732
summary: Fix audit logging to consistently include port number in `origin.address`
area: Audit
type: bug
issues: []
4 changes: 2 additions & 2 deletions x-pack/docs/en/security/auditing/event-types.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ Logged when an incoming TCP connection does not pass the
[source,js]
{"type":"audit", "timestamp":"2020-12-30T21:47:31,526+0200", "node.id":
"0RMNyghkQYCc_gVd1G6tZQ", "event.type":"ip_filter", "event.action":
"connection_denied", "origin.type":"rest", "origin.address":"10.10.0.20",
"connection_denied", "origin.type":"rest", "origin.address":"10.10.0.20:52314",
"transport.profile":".http", "rule":"deny 10.10.0.0/16"}
====

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

Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule;

import java.net.InetAddress;
import java.net.InetSocketAddress;

public interface AuditTrail {

Expand Down Expand Up @@ -66,14 +66,14 @@ void accessDenied(
void tamperedRequest(String requestId, Authentication authentication, String action, TransportRequest transportRequest);

/**
* The {@link #connectionGranted(InetAddress, String, SecurityIpFilterRule)} and
* {@link #connectionDenied(InetAddress, String, SecurityIpFilterRule)} methods do not have a requestId because they related to a
* The {@link #connectionGranted(InetSocketAddress, String, SecurityIpFilterRule)} and
* {@link #connectionDenied(InetSocketAddress, 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 connectionGranted(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule);

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

void runAsGranted(
String requestId,
Expand Down
10 changes: 5 additions & 5 deletions ...ugin/security/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import org.elasticsearch.xpack.security.Security;
import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule;

import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.time.Duration;
import java.time.Instant;
import java.util.Collections;
Expand Down Expand Up @@ -150,10 +150,10 @@ public void tamperedRequest(String requestId, String action, TransportRequest tr
public void tamperedRequest(String requestId, Authentication authentication, String action, TransportRequest transportRequest) {}

@Override
public void connectionGranted(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {}
public void connectionGranted(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {}

@Override
public void connectionDenied(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {}
public void connectionDenied(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {}

@Override
public void runAsGranted(
Expand Down Expand Up @@ -362,14 +362,14 @@ public void tamperedRequest(String requestId, Authentication authentication, Str
}

@Override
public void connectionGranted(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {
public void connectionGranted(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.connectionGranted(inetAddress, profile, rule);
}
}

@Override
public void connectionDenied(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {
public void connectionDenied(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {
for (AuditTrail auditTrail : auditTrails) {
auditTrail.connectionDenied(inetAddress, profile, rule);
}
Expand Down
5 changes: 2 additions & 3 deletions ...urity/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@
import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule;

import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -879,7 +878,7 @@ public void tamperedRequest(String requestId, Authentication authentication, Str
}

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

@Override
public void connectionDenied(InetAddress inetAddress, String profile, SecurityIpFilterRule rule) {
public void connectionDenied(InetSocketAddress inetAddress, String profile, SecurityIpFilterRule rule) {
if (events.contains(CONNECTION_DENIED) && eventFilterPolicyRegistry.ignorePredicate().test(AuditEventMetaInfo.EMPTY) == false) {
new LogEntryBuilder().with(EVENT_TYPE_FIELD_NAME, IP_FILTER_ORIGIN_FIELD_VALUE)
.with(EVENT_ACTION_FIELD_NAME, "connection_denied")
Expand Down
6 changes: 3 additions & 3 deletions ...in/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,15 +288,15 @@ public boolean accept(String profile, InetSocketAddress peerAddress) {
if (rule.matches(peerAddress)) {
boolean isAllowed = rule.ruleType() == IpFilterRuleType.ACCEPT;
if (isAllowed) {
auditTrail.connectionGranted(peerAddress.getAddress(), profile, rule);
auditTrail.connectionGranted(peerAddress, profile, rule);
} else {
auditTrail.connectionDenied(peerAddress.getAddress(), profile, rule);
auditTrail.connectionDenied(peerAddress, profile, rule);
}
return isAllowed;
}
}

auditTrail.connectionGranted(peerAddress.getAddress(), profile, DEFAULT_PROFILE_ACCEPT_ALL);
auditTrail.connectionGranted(peerAddress, profile, DEFAULT_PROFILE_ACCEPT_ALL);
return true;
}

Expand Down
5 changes: 3 additions & 2 deletions ...security/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.junit.Before;

import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
Expand Down Expand Up @@ -260,7 +261,7 @@ public void testAccessDenied() throws Exception {
}

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

public void testConnectionDenied() throws Exception {
InetAddress inetAddress = InetAddress.getLoopbackAddress();
InetSocketAddress inetAddress = new InetSocketAddress(InetAddress.getLoopbackAddress(), randomIntBetween(0, 65535));
SecurityIpFilterRule rule = new SecurityIpFilterRule(false, "_all");
service.get().connectionDenied(inetAddress, "client", rule);
verify(licenseState).isAllowed(Security.AUDITING_FEATURE);
Expand Down
24 changes: 14 additions & 10 deletions ...est/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailFilterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,7 @@ public void testUsersFilter() throws Exception {
threadContext.stashContext();

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

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

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

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

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

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

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

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

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

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

private InetSocketAddress randomLoopbackInetSocketAddress() {
return new InetSocketAddress(InetAddress.getLoopbackAddress(), randomIntBetween(0, 65535));
}

private <T> List<T> randomListFromLengthBetween(List<T> l, int min, int max) {
assert (min >= 0) && (min <= max) && (max <= l.size());
final int len = randomIntBetween(min, max);
Expand Down
4 changes: 2 additions & 2 deletions .../src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2049,7 +2049,7 @@ public void testTamperedRequestWithUser() throws Exception {
}

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

Expand Down Expand Up @@ -2081,7 +2081,7 @@ public void testConnectionDenied() throws Exception {
}

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

Expand Down
8 changes: 4 additions & 4 deletions ...curity/src/test/java/org/elasticsearch/xpack/security/transport/filter/IPFilterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,8 @@ public void testThatNodeStartsWithIPFilterDisabled() throws Exception {
private void assertAddressIsAllowedForProfile(String profile, String... inetAddresses) {
for (String inetAddress : inetAddresses) {
String message = String.format(Locale.ROOT, "Expected address %s to be allowed", inetAddress);
InetAddress address = InetAddresses.forString(inetAddress);
assertTrue(message, ipFilter.accept(profile, new InetSocketAddress(address, 0)));
InetSocketAddress address = new InetSocketAddress(InetAddresses.forString(inetAddress), 0);
assertTrue(message, ipFilter.accept(profile, address));
ArgumentCaptor<SecurityIpFilterRule> ruleCaptor = ArgumentCaptor.forClass(SecurityIpFilterRule.class);
verify(auditTrail).connectionGranted(eq(address), eq(profile), ruleCaptor.capture());
assertNotNull(ruleCaptor.getValue());
Expand All @@ -311,8 +311,8 @@ private void assertAddressIsAllowed(String... inetAddresses) {
private void assertAddressIsDeniedForProfile(String profile, String... inetAddresses) {
for (String inetAddress : inetAddresses) {
String message = String.format(Locale.ROOT, "Expected address %s to be denied", inetAddress);
InetAddress address = InetAddresses.forString(inetAddress);
assertFalse(message, ipFilter.accept(profile, new InetSocketAddress(address, 0)));
InetSocketAddress address = new InetSocketAddress(InetAddresses.forString(inetAddress), 0);
assertFalse(message, ipFilter.accept(profile, address));
ArgumentCaptor<SecurityIpFilterRule> ruleCaptor = ArgumentCaptor.forClass(SecurityIpFilterRule.class);
verify(auditTrail).connectionDenied(eq(address), eq(profile), ruleCaptor.capture());
assertNotNull(ruleCaptor.getValue());
Expand Down

0 comments on commit 8b375a3

Please sign in to comment.