From 03fd79fe910e209cd092c92ad3a2507b8cc20d07 Mon Sep 17 00:00:00 2001 From: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Date: Fri, 5 Jan 2024 15:00:26 -0500 Subject: [PATCH] Add additional ignore_headers audit configuration setting (#3885) Signed-off-by: Stephen Crawford Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Co-authored-by: Craig Perkins --- .../test/framework/AuditFilters.java | 9 +++++ .../security/OpenSearchSecurityPlugin.java | 11 +++++- .../security/auditlog/config/AuditConfig.java | 38 +++++++++++++++++-- .../auditlog/impl/AbstractAuditLog.java | 5 --- .../security/auditlog/impl/AuditMessage.java | 9 +++-- .../security/support/ConfigConstants.java | 1 + .../config/AuditConfigFilterTest.java | 4 ++ .../config/AuditConfigSerializeTest.java | 7 ++++ .../auditlog/impl/AuditMessageTest.java | 24 +++++++++--- .../dlic/rest/api/AuditApiActionTest.java | 2 +- 10 files changed, 92 insertions(+), 18 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java b/src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java index f984becefa..087342eb6f 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java +++ b/src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java @@ -34,6 +34,8 @@ public class AuditFilters implements ToXContentObject { private List ignoreRequests; + private List ignoreHeaders; + private List disabledRestCategories; private List disabledTransportCategories; @@ -49,6 +51,7 @@ public AuditFilters() { this.ignoreUsers = Collections.emptyList(); this.ignoreRequests = Collections.emptyList(); + this.ignoreHeaders = Collections.emptyList(); this.disabledRestCategories = Collections.emptyList(); this.disabledTransportCategories = Collections.emptyList(); } @@ -93,6 +96,11 @@ public AuditFilters ignoreRequests(List ignoreRequests) { return this; } + public AuditFilters ignoreHeaders(List ignoreHeaders) { + this.ignoreHeaders = ignoreHeaders; + return this; + } + public AuditFilters disabledRestCategories(List disabledRestCategories) { this.disabledRestCategories = disabledRestCategories; return this; @@ -114,6 +122,7 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params xContentBuilder.field("exclude_sensitive_headers", excludeSensitiveHeaders); xContentBuilder.field("ignore_users", ignoreUsers); xContentBuilder.field("ignore_requests", ignoreRequests); + xContentBuilder.field("ignore_headers", ignoreHeaders); xContentBuilder.field("disabled_rest_categories", disabledRestCategories); xContentBuilder.field("disabled_transport_categories", disabledTransportCategories); xContentBuilder.endObject(); diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 3c04816c32..b0263e06d4 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1352,7 +1352,7 @@ public List> getSettings() { Function.identity(), Property.NodeScope ) - ); // not filtered here + ); settings.add( Setting.listSetting( ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_REQUESTS, @@ -1361,6 +1361,14 @@ public List> getSettings() { Property.NodeScope ) ); // not filtered here + settings.add( + Setting.listSetting( + ConfigConstants.SECURITY_AUDIT_IGNORE_HEADERS, + Collections.emptyList(), + Function.identity(), + Property.NodeScope + ) + ); settings.add( Setting.boolSetting( ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, @@ -1393,6 +1401,7 @@ public List> getSettings() { Property.NodeScope ); case IGNORE_REQUESTS: + case IGNORE_HEADERS: return Setting.listSetting( filterEntry.getKeyWithNamespace(), Collections.emptyList(), diff --git a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java index 2cffd93dfa..7b173099b5 100644 --- a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java +++ b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java @@ -62,7 +62,8 @@ * "ignore_users" : [ * "kibanaserver" * ], - * "ignore_requests" : [ ] + * "ignore_requests" : [ ], + * "ignore_headers" : [ ], * }, * "compliance" : { * "enabled": true, @@ -82,6 +83,7 @@ public class AuditConfig { public static final List DEFAULT_IGNORED_USERS = Collections.singletonList("kibanaserver"); + private static Set FIELDS = DefaultObjectMapper.getFields(AuditConfig.class); private AuditConfig() { @@ -138,8 +140,11 @@ public static class Filter { private final Set ignoredAuditUsers; @JsonProperty("ignore_requests") private final Set ignoredAuditRequests; + @JsonProperty("ignore_headers") + private final Set ignoredCustomHeaders; private final WildcardMatcher ignoredAuditUsersMatcher; private final WildcardMatcher ignoredAuditRequestsMatcher; + private final WildcardMatcher ignoredCustomHeadersMatcher; private final Set disabledRestCategories; private final Set disabledTransportCategories; @@ -153,6 +158,7 @@ public static class Filter { final boolean excludeSensitiveHeaders, final Set ignoredAuditUsers, final Set ignoredAuditRequests, + final Set ignoredCustomHeaders, final Set disabledRestCategories, final Set disabledTransportCategories ) { @@ -166,6 +172,8 @@ public static class Filter { this.ignoredAuditUsersMatcher = WildcardMatcher.from(ignoredAuditUsers); this.ignoredAuditRequests = ignoredAuditRequests; this.ignoredAuditRequestsMatcher = WildcardMatcher.from(ignoredAuditRequests); + this.ignoredCustomHeaders = ignoredCustomHeaders; + this.ignoredCustomHeadersMatcher = WildcardMatcher.from(ignoredCustomHeaders); this.disabledRestCategories = disabledRestCategories; this.disabledTransportCategories = disabledTransportCategories; } @@ -183,7 +191,8 @@ public enum FilterEntries { ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_TRANSPORT_CATEGORIES ), IGNORE_USERS("ignore_users", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_USERS), - IGNORE_REQUESTS("ignore_requests", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_REQUESTS); + IGNORE_REQUESTS("ignore_requests", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_REQUESTS), + IGNORE_HEADERS("ignore_headers", ConfigConstants.SECURITY_AUDIT_IGNORE_HEADERS); private final String key; private final String legacyKeyWithNamespace; @@ -246,6 +255,9 @@ public static Filter from(Map properties) throws JsonProcessingE final Set ignoreAuditRequests = ImmutableSet.copyOf( getOrDefault(properties, FilterEntries.IGNORE_REQUESTS.getKey(), Collections.emptyList()) ); + final Set ignoreHeaders = ImmutableSet.copyOf( + getOrDefault(properties, FilterEntries.IGNORE_HEADERS.getKey(), Collections.emptyList()) + ); return new Filter( isRestApiAuditEnabled, @@ -256,6 +268,7 @@ public static Filter from(Map properties) throws JsonProcessingE excludeSensitiveHeaders, ignoredAuditUsers, ignoreAuditRequests, + ignoreHeaders, disabledRestCategories, disabledTransportCategories ); @@ -290,7 +303,7 @@ public static Filter from(Settings settings) { ); final Set ignoredAuditUsers = fromSettingStringSet(settings, FilterEntries.IGNORE_USERS, DEFAULT_IGNORED_USERS); final Set ignoreAuditRequests = fromSettingStringSet(settings, FilterEntries.IGNORE_REQUESTS, Collections.emptyList()); - + final Set ignoreHeaders = fromSettingStringSet(settings, FilterEntries.IGNORE_HEADERS, Collections.emptyList()); return new Filter( isRestApiAuditEnabled, isTransportAuditEnabled, @@ -300,6 +313,7 @@ public static Filter from(Settings settings) { excludeSensitiveHeaders, ignoredAuditUsers, ignoreAuditRequests, + ignoreHeaders, disabledRestCategories, disabledTransportCategories ); @@ -403,6 +417,21 @@ WildcardMatcher getIgnoredAuditRequestsMatcher() { return ignoredAuditRequestsMatcher; } + @VisibleForTesting + WildcardMatcher getIgnoredCustomHeadersMatcher() { + return ignoredCustomHeadersMatcher; + } + + /** + * Check if the specified header is excluded from the audit + * + * @param header + * @return true if header should be excluded + */ + public boolean shouldExcludeHeader(String header) { + return ignoredCustomHeadersMatcher.test(header); + } + /** * Check if request is excluded from audit * @param action @@ -440,6 +469,7 @@ public void log(Logger logger) { logger.info("Index resolution is {} during request auditing.", resolveIndices ? "enabled" : "disabled"); logger.info("Sensitive headers auditing is {}.", excludeSensitiveHeaders ? "enabled" : "disabled"); logger.info("Auditing requests from {} users is disabled.", ignoredAuditUsersMatcher); + logger.info("Auditing request headers {} is disabled.", ignoredCustomHeadersMatcher); } @Override @@ -465,6 +495,8 @@ public String toString() { + ignoredAuditUsersMatcher + ", ignoreAuditRequests=" + ignoredAuditRequestsMatcher + + ", ignoredCustomHeaders=" + + ignoredCustomHeadersMatcher + '}'; } } diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java index d97adc358b..e5f314cd29 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java @@ -927,11 +927,6 @@ boolean checkRestFilter(final AuditCategory category, final String effectiveUser } return false; } - - // check rest audit enabled - // check category enabled - // check action - // check ignoreAuditUsers } protected abstract void save(final AuditMessage msg); diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java index 8b24a554d1..b57becc359 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java @@ -356,12 +356,15 @@ public void addRestParams(Map params) { } } - public void addRestHeaders(Map> headers, boolean excludeSensitiveHeaders) { + public void addRestHeaders(Map> headers, boolean excludeSensitiveHeaders, AuditConfig.Filter filter) { if (headers != null && !headers.isEmpty()) { final Map> headersClone = new HashMap<>(headers); if (excludeSensitiveHeaders) { headersClone.keySet().removeIf(AUTHORIZATION_HEADER); } + if (filter != null) { + headersClone.entrySet().removeIf(entry -> filter.shouldExcludeHeader(entry.getKey())); + } auditInfo.put(REST_REQUEST_HEADERS, headersClone); } } @@ -376,14 +379,14 @@ void addRestRequestInfo(final SecurityRequest request, final AuditConfig.Filter if (request != null) { final String path = request.path().toString(); addPath(path); - addRestHeaders(request.getHeaders(), filter.shouldExcludeSensitiveHeaders()); + addRestHeaders(request.getHeaders(), filter.shouldExcludeSensitiveHeaders(), filter); addRestParams(request.params()); addRestMethod(request.method()); if (filter.shouldLogRequestBody()) { if (!(request instanceof OpenSearchRequest)) { - // The request body is only avaliable on some request sources + // The request body is only available on some request sources return; } diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index f10dedade3..d4383c05de 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -165,6 +165,7 @@ public class ConfigConstants { ); public static final String OPENDISTRO_SECURITY_AUDIT_IGNORE_USERS = "opendistro_security.audit.ignore_users"; public static final String OPENDISTRO_SECURITY_AUDIT_IGNORE_REQUESTS = "opendistro_security.audit.ignore_requests"; + public static final String SECURITY_AUDIT_IGNORE_HEADERS = "plugins.security.audit.ignore_headers"; public static final String OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS = "opendistro_security.audit.resolve_bulk_requests"; public static final boolean OPENDISTRO_SECURITY_AUDIT_SSL_VERIFY_HOSTNAMES_DEFAULT = true; public static final boolean OPENDISTRO_SECURITY_AUDIT_SSL_ENABLE_SSL_CLIENT_AUTH_DEFAULT = false; diff --git a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java index e40e65549f..a28d940862 100644 --- a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java +++ b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java @@ -57,6 +57,7 @@ public void testDefault() { assertTrue(auditConfigFilter.shouldExcludeSensitiveHeaders()); assertSame(WildcardMatcher.NONE, auditConfigFilter.getIgnoredAuditRequestsMatcher()); assertEquals(defaultIgnoredUserMatcher, auditConfigFilter.getIgnoredAuditUsersMatcher()); + assertSame(WildcardMatcher.NONE, auditConfigFilter.getIgnoredCustomHeadersMatcher()); assertEquals(auditConfigFilter.getDisabledRestCategories(), defaultDisabledCategories); assertEquals(auditConfigFilter.getDisabledTransportCategories(), defaultDisabledCategories); } @@ -73,6 +74,7 @@ public void testConfig() { .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_EXCLUDE_SENSITIVE_HEADERS, false) .putList(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_REQUESTS, "test-request") .putList(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_USERS, "test-user") + .putList(ConfigConstants.SECURITY_AUDIT_IGNORE_HEADERS, "test-header") .putList( ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, BAD_HEADERS.toString(), @@ -95,6 +97,7 @@ public void testConfig() { assertFalse(auditConfigFilter.shouldExcludeSensitiveHeaders()); assertEquals(WildcardMatcher.from(Collections.singleton("test-user")), auditConfigFilter.getIgnoredAuditUsersMatcher()); assertEquals(WildcardMatcher.from(Collections.singleton("test-request")), auditConfigFilter.getIgnoredAuditRequestsMatcher()); + assertEquals(WildcardMatcher.from(Collections.singleton("test-header")), auditConfigFilter.getIgnoredCustomHeadersMatcher()); assertEquals(auditConfigFilter.getDisabledRestCategories(), EnumSet.of(BAD_HEADERS, SSL_EXCEPTION)); assertEquals(auditConfigFilter.getDisabledTransportCategories(), EnumSet.of(FAILED_LOGIN, MISSING_PRIVILEGES)); } @@ -121,6 +124,7 @@ public void testEmpty() { final Settings settings = Settings.builder() .putList(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_USERS, Collections.emptyList()) .putList(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_REQUESTS, Collections.emptyList()) + .putList(ConfigConstants.SECURITY_AUDIT_IGNORE_HEADERS, Collections.emptyList()) .putList(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, Collections.emptyList()) .putList(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_TRANSPORT_CATEGORIES, Collections.emptyList()) .build(); diff --git a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigSerializeTest.java b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigSerializeTest.java index 0b50c2ac20..b0b93afc54 100644 --- a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigSerializeTest.java +++ b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigSerializeTest.java @@ -72,6 +72,7 @@ public void testDefaultSerialize() throws IOException { .field("exclude_sensitive_headers", true) .field("ignore_users", Collections.singletonList("kibanaserver")) .field("ignore_requests", Collections.emptyList()) + .field("ignore_headers", Collections.emptyList()) .endObject() .startObject("compliance") .field("enabled", true) @@ -107,6 +108,7 @@ public void testDefaultDeserialize() throws IOException { assertTrue(audit.shouldExcludeSensitiveHeaders()); assertSame(WildcardMatcher.NONE, audit.getIgnoredAuditRequestsMatcher()); assertEquals(DEFAULT_IGNORED_USER, audit.getIgnoredAuditUsersMatcher()); + assertEquals(WildcardMatcher.NONE, audit.getIgnoredCustomHeadersMatcher()); assertFalse(compliance.shouldLogExternalConfig()); assertFalse(compliance.shouldLogInternalConfig()); assertFalse(compliance.shouldLogReadMetadataOnly()); @@ -133,6 +135,7 @@ public void testDeserialize() throws IOException { .field("exclude_sensitive_headers", true) .field("ignore_users", Collections.singletonList("test-user-1")) .field("ignore_requests", Collections.singletonList("test-request")) + .field("ignore_headers", Collections.singletonList("test-headers")) .endObject() .startObject("compliance") .field("enabled", true) @@ -196,6 +199,7 @@ public void testSerialize() throws IOException { true, ImmutableSet.of("ignore-user-1", "ignore-user-2"), ImmutableSet.of("ignore-request-1"), + ImmutableSet.of("test-header"), EnumSet.of(AuditCategory.FAILED_LOGIN, AuditCategory.GRANTED_PRIVILEGES), EnumSet.of(AUTHENTICATED) ); @@ -227,6 +231,7 @@ public void testSerialize() throws IOException { .field("exclude_sensitive_headers", true) .field("ignore_users", ImmutableList.of("ignore-user-1", "ignore-user-2")) .field("ignore_requests", Collections.singletonList("ignore-request-1")) + .field("ignore_headers", Collections.singletonList("test-header")) .endObject() .startObject("compliance") .field("enabled", true) @@ -269,6 +274,7 @@ public void testNullSerialize() throws IOException { .field("exclude_sensitive_headers", true) .field("ignore_users", ImmutableList.of("kibanaserver")) .field("ignore_requests", Collections.emptyList()) + .field("ignore_headers", Collections.emptyList()) .endObject() .startObject("compliance") .field("enabled", true) @@ -287,6 +293,7 @@ public void testNullSerialize() throws IOException { // act final String json = objectMapper.writeValueAsString(auditConfig); // assert + assertTrue(compareJson(jsonBuilder.toString(), json)); } diff --git a/src/test/java/org/opensearch/security/auditlog/impl/AuditMessageTest.java b/src/test/java/org/opensearch/security/auditlog/impl/AuditMessageTest.java index d915c02e55..3b7fc916ef 100644 --- a/src/test/java/org/opensearch/security/auditlog/impl/AuditMessageTest.java +++ b/src/test/java/org/opensearch/security/auditlog/impl/AuditMessageTest.java @@ -28,6 +28,7 @@ import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.security.auditlog.AuditLog; +import org.opensearch.security.auditlog.config.AuditConfig; import org.opensearch.security.securityconf.impl.CType; import static org.junit.Assert.assertEquals; @@ -60,32 +61,45 @@ public class AuditMessageTest { ); private AuditMessage message; + private AuditConfig auditConfig; @Before public void setUp() { final ClusterService clusterServiceMock = mock(ClusterService.class); when(clusterServiceMock.localNode()).thenReturn(mock(DiscoveryNode.class)); when(clusterServiceMock.getClusterName()).thenReturn(mock(ClusterName.class)); + auditConfig = mock(AuditConfig.class); + final AuditConfig.Filter auditFilter = mock(AuditConfig.Filter.class); + when(auditConfig.getFilter()).thenReturn(auditFilter); message = new AuditMessage(AuditCategory.AUTHENTICATED, clusterServiceMock, AuditLog.Origin.REST, AuditLog.Origin.REST); } @Test - public void testRestHeadersAreFiltered() { - message.addRestHeaders(TEST_REST_HEADERS, true); + public void testAuthorizationRestHeadersAreFiltered() { + when(auditConfig.getFilter().shouldExcludeHeader("test-header")).thenReturn(false); + message.addRestHeaders(TEST_REST_HEADERS, true, auditConfig.getFilter()); assertEquals(message.getAsMap().get(AuditMessage.REST_REQUEST_HEADERS), ImmutableMap.of("test-header", ImmutableList.of("test-4"))); } + @Test + public void testCustomRestHeadersAreFiltered() { + when(auditConfig.getFilter().shouldExcludeHeader("test-header")).thenReturn(true); + message.addRestHeaders(TEST_REST_HEADERS, true, auditConfig.getFilter()); + assertEquals(message.getAsMap().get(AuditMessage.REST_REQUEST_HEADERS), Map.of()); + } + @Test public void testRestHeadersNull() { - message.addRestHeaders(null, true); + message.addRestHeaders(null, true, null); assertNull(message.getAsMap().get(AuditMessage.REST_REQUEST_HEADERS)); - message.addRestHeaders(Collections.emptyMap(), true); + message.addRestHeaders(Collections.emptyMap(), true, null); assertNull(message.getAsMap().get(AuditMessage.REST_REQUEST_HEADERS)); } @Test public void testRestHeadersAreNotFiltered() { - message.addRestHeaders(TEST_REST_HEADERS, false); + when(auditConfig.getFilter().shouldExcludeHeader("test-header")).thenReturn(false); + message.addRestHeaders(TEST_REST_HEADERS, false, null); assertEquals(message.getAsMap().get(AuditMessage.REST_REQUEST_HEADERS), TEST_REST_HEADERS); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AuditApiActionTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AuditApiActionTest.java index b512ae2228..b3d916e8ed 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AuditApiActionTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AuditApiActionTest.java @@ -682,7 +682,7 @@ private String getTestPayload() { + "\"enable_rest\":true,\"disabled_rest_categories\":[\"AUTHENTICATED\"]," + "\"enable_transport\":true,\"disabled_transport_categories\":[\"SSL_EXCEPTION\"]," + "\"resolve_bulk_requests\":true,\"log_request_body\":true,\"resolve_indices\":true,\"exclude_sensitive_headers\":true," - + "\"ignore_users\":[\"test-user-1\"],\"ignore_requests\":[\"test-request\"]}," + + "\"ignore_users\":[\"test-user-1\"],\"ignore_requests\":[\"test-request\"], \"ignore_headers\":[\"\"]}," + "\"compliance\":{" + "\"enabled\":true," + "\"internal_config\":true,\"external_config\":true,"