Fix audit logging to consistently include port number in origin.addre…

…ss (#86732) (#86968)

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

Closes #86694

docs/changelog/86732.yaml

@@ -0,0 +1,5 @@
+pr: 86732
+summary: Fix audit logging to consistently include port number in `origin.address`
+area: Audit
+type: bug
+issues: []

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

@@ -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"}
 ====
 
@@ -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"}
 ====
 

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

@@ -14,7 +14,6 @@
 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 {
@@ -66,14 +65,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,

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

@@ -18,7 +18,6 @@
 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;
@@ -150,10 +149,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(
@@ -362,14 +361,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);
             }

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

@@ -98,7 +98,6 @@
 import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule;
 
 import java.io.IOException;
-import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -915,7 +914,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")
@@ -932,7 +931,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")

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

@@ -286,15 +286,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;
     }
 

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

@@ -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;
@@ -259,7 +260,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);
@@ -273,7 +274,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);

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

@@ -1112,7 +1112,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 {
@@ -1122,7 +1122,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 {
@@ -1561,7 +1561,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 {
@@ -1571,7 +1571,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 {
@@ -1896,7 +1896,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 {
@@ -1906,7 +1906,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 {
@@ -2352,7 +2352,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 {
@@ -2362,7 +2362,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 {
@@ -2634,7 +2634,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 {
@@ -2644,7 +2644,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 {
@@ -2734,6 +2734,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);

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

@@ -2170,7 +2170,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);
 
@@ -2202,7 +2202,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);
 

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

@@ -291,8 +291,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());
@@ -306,8 +306,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());