[Backport 2.x] Redact sensitive URL parameters from audit logging (#4070

)

Backport e40efdc from #4067.

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Peter Nied <[email protected]>

src/integrationTest/java/org/opensearch/security/http/JwtAuthenticationWithUrlParamTests.java

@@ -21,8 +21,12 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.opensearch.test.framework.AuditCompliance;
+import org.opensearch.test.framework.AuditConfiguration;
+import org.opensearch.test.framework.AuditFilters;
 import org.opensearch.test.framework.JwtConfigBuilder;
 import org.opensearch.test.framework.TestSecurityConfig;
+import org.opensearch.test.framework.audit.AuditLogsRule;
 import org.opensearch.test.framework.cluster.ClusterManager;
 import org.opensearch.test.framework.cluster.LocalCluster;
 import org.opensearch.test.framework.cluster.TestRestClient;
@@ -36,9 +40,11 @@
 import static org.apache.http.HttpHeaders.AUTHORIZATION;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.opensearch.rest.RestRequest.Method.GET;
 import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
 import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.BASIC_AUTH_DOMAIN_ORDER;
 import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
+import static org.opensearch.test.framework.audit.AuditMessagePredicate.userAuthenticated;
 
 @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
 @ThreadLeakScope(ThreadLeakScope.Scope.NONE)
@@ -69,12 +75,19 @@ public class JwtAuthenticationWithUrlParamTests {
         new JwtConfigBuilder().jwtUrlParameter(TOKEN_URL_PARAM).signingKey(PUBLIC_KEY).subjectKey(CLAIM_USERNAME).rolesKey(CLAIM_ROLES)
     ).backend("noop");
 
+    @Rule
+    public AuditLogsRule auditLogsRule = new AuditLogsRule();
+
     @ClassRule
     public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
         .anonymousAuth(false)
         .nodeSettings(
             Map.of("plugins.security.restapi.roles_enabled", List.of("user_" + ADMIN_USER.getName() + "__" + ALL_ACCESS.getName()))
         )
+        .audit(
+            new AuditConfiguration(true).compliance(new AuditCompliance().enabled(true))
+                .filters(new AuditFilters().enabledRest(true).enabledTransport(true))
+        )
         .authc(AUTHC_HTTPBASIC_INTERNAL)
         .authc(JWT_AUTH_DOMAIN)
         .users(ADMIN_USER)
@@ -88,11 +101,16 @@ public void shouldAuthenticateWithJwtTokenInUrl_positive() {
         Header jwtToken = tokenFactory.generateValidToken(ADMIN_USER.getName());
         String jwtTokenValue = jwtToken.getValue();
         try (TestRestClient client = cluster.getRestClient()) {
-            HttpResponse response = client.getAuthInfo(Map.of(TOKEN_URL_PARAM, jwtTokenValue));
+            HttpResponse response = client.getAuthInfo(Map.of(TOKEN_URL_PARAM, jwtTokenValue, "verbose", "true"));
 
             response.assertStatusCode(200);
             String username = response.getTextFromJsonBody(POINTER_USERNAME);
             assertThat(username, equalTo(ADMIN_USER.getName()));
+            Map<String, String> expectedParams = Map.of("token", "REDACTED", "verbose", "true");
+
+            auditLogsRule.assertExactlyOne(
+                userAuthenticated(ADMIN_USER).withRestRequest(GET, "/_opendistro/_security/authinfo").withRestParams(expectedParams)
+            );
         }
     }
 

src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java

@@ -35,6 +35,7 @@ public class AuditFilters implements ToXContentObject {
     private List<String> ignoreRequests;
 
     private List<String> ignoreHeaders;
+    private List<String> ignoreUrlParams;
 
     private List<String> disabledRestCategories;
 
@@ -52,6 +53,7 @@ public AuditFilters() {
         this.ignoreUsers = Collections.emptyList();
         this.ignoreRequests = Collections.emptyList();
         this.ignoreHeaders = Collections.emptyList();
+        this.ignoreUrlParams = Collections.emptyList();
         this.disabledRestCategories = Collections.emptyList();
         this.disabledTransportCategories = Collections.emptyList();
     }
@@ -101,6 +103,11 @@ public AuditFilters ignoreHeaders(List<String> ignoreHeaders) {
         return this;
     }
 
+    public AuditFilters ignoreUrlParams(List<String> ignoreUrlParams) {
+        this.ignoreUrlParams = ignoreUrlParams;
+        return this;
+    }
+
     public AuditFilters disabledRestCategories(List<String> disabledRestCategories) {
         this.disabledRestCategories = disabledRestCategories;
         return this;
@@ -123,6 +130,7 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params
         xContentBuilder.field("ignore_users", ignoreUsers);
         xContentBuilder.field("ignore_requests", ignoreRequests);
         xContentBuilder.field("ignore_headers", ignoreHeaders);
+        xContentBuilder.field("ignore_url_params", ignoreUrlParams);
         xContentBuilder.field("disabled_rest_categories", disabledRestCategories);
         xContentBuilder.field("disabled_transport_categories", disabledTransportCategories);
         xContentBuilder.endObject();

src/integrationTest/java/org/opensearch/test/framework/audit/AuditMessagePredicate.java

@@ -29,13 +29,15 @@
 import static org.opensearch.security.auditlog.impl.AuditCategory.MISSING_PRIVILEGES;
 import static org.opensearch.security.auditlog.impl.AuditMessage.REQUEST_LAYER;
 import static org.opensearch.security.auditlog.impl.AuditMessage.RESOLVED_INDICES;
+import static org.opensearch.security.auditlog.impl.AuditMessage.REST_REQUEST_PARAMS;
 import static org.opensearch.security.auditlog.impl.AuditMessage.REST_REQUEST_PATH;
 
 public class AuditMessagePredicate implements Predicate<AuditMessage> {
 
     private final AuditCategory category;
     private final Origin requestLayer;
     private final String restRequestPath;
+    private final Map<String, String> restParams;
     private final String initiatingUser;
     private final Method requestMethod;
     private final String transportRequestType;
@@ -47,6 +49,7 @@ private AuditMessagePredicate(
         AuditCategory category,
         Origin requestLayer,
         String restRequestPath,
+        Map<String, String> restParams,
         String initiatingUser,
         Method requestMethod,
         String transportRequestType,
@@ -57,6 +60,7 @@ private AuditMessagePredicate(
         this.category = category;
         this.requestLayer = requestLayer;
         this.restRequestPath = restRequestPath;
+        this.restParams = restParams;
         this.initiatingUser = initiatingUser;
         this.requestMethod = requestMethod;
         this.transportRequestType = transportRequestType;
@@ -66,7 +70,7 @@ private AuditMessagePredicate(
     }
 
     private AuditMessagePredicate(AuditCategory category) {
-        this(category, null, null, null, null, null, null, null, null);
+        this(category, null, null, null, null, null, null, null, null, null);
     }
 
     public static AuditMessagePredicate auditPredicate(AuditCategory category) {
@@ -110,6 +114,7 @@ public AuditMessagePredicate withLayer(Origin layer) {
             category,
             layer,
             restRequestPath,
+            restParams,
             initiatingUser,
             requestMethod,
             transportRequestType,
@@ -124,6 +129,22 @@ public AuditMessagePredicate withRequestPath(String path) {
             category,
             requestLayer,
             path,
+            restParams,
+            initiatingUser,
+            requestMethod,
+            transportRequestType,
+            effectiveUser,
+            index,
+            privilege
+        );
+    }
+
+    public AuditMessagePredicate withRestParams(Map<String, String> params) {
+        return new AuditMessagePredicate(
+            category,
+            requestLayer,
+            restRequestPath,
+            params,
             initiatingUser,
             requestMethod,
             transportRequestType,
@@ -138,6 +159,7 @@ public AuditMessagePredicate withInitiatingUser(String user) {
             category,
             requestLayer,
             restRequestPath,
+            restParams,
             user,
             requestMethod,
             transportRequestType,
@@ -156,6 +178,7 @@ public AuditMessagePredicate withRestMethod(Method method) {
             category,
             requestLayer,
             restRequestPath,
+            restParams,
             initiatingUser,
             method,
             transportRequestType,
@@ -170,6 +193,7 @@ public AuditMessagePredicate withTransportRequestType(String type) {
             category,
             requestLayer,
             restRequestPath,
+            restParams,
             initiatingUser,
             requestMethod,
             type,
@@ -184,6 +208,7 @@ public AuditMessagePredicate withEffectiveUser(String user) {
             category,
             requestLayer,
             restRequestPath,
+            restParams,
             initiatingUser,
             requestMethod,
             transportRequestType,
@@ -206,6 +231,7 @@ public AuditMessagePredicate withIndex(String indexName) {
             category,
             requestLayer,
             restRequestPath,
+            restParams,
             initiatingUser,
             requestMethod,
             transportRequestType,
@@ -220,6 +246,7 @@ public AuditMessagePredicate withPrivilege(String privilegeAction) {
             category,
             requestLayer,
             restRequestPath,
+            restParams,
             initiatingUser,
             requestMethod,
             transportRequestType,
@@ -235,6 +262,7 @@ public boolean test(AuditMessage auditMessage) {
         predicates.add(audit -> Objects.isNull(category) || category.equals(audit.getCategory()));
         predicates.add(audit -> Objects.isNull(requestLayer) || requestLayer.equals(audit.getAsMap().get(REQUEST_LAYER)));
         predicates.add(audit -> Objects.isNull(restRequestPath) || restRequestPath.equals(audit.getAsMap().get(REST_REQUEST_PATH)));
+        predicates.add(audit -> Objects.isNull(restParams) || restParams.equals(auditMessage.getAsMap().get(REST_REQUEST_PARAMS)));
         predicates.add(audit -> Objects.isNull(initiatingUser) || initiatingUser.equals(audit.getInitiatingUser()));
         predicates.add(audit -> Objects.isNull(requestMethod) || requestMethod.equals(audit.getRequestMethod()));
         predicates.add(audit -> Objects.isNull(transportRequestType) || transportRequestType.equals(audit.getRequestType()));

src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java

@@ -20,9 +20,11 @@
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.X509EncodedKeySpec;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.Set;
 import java.util.regex.Pattern;
 
 import org.apache.http.HttpStatus;
@@ -199,6 +201,14 @@ public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest
         );
     }
 
+    @Override
+    public Set<String> getSensitiveUrlParams() {
+        if (jwtUrlParameter != null) {
+            return Set.of(jwtUrlParameter);
+        }
+        return Collections.emptySet();
+    }
+
     @Override
     public String getType() {
         return "jwt";

src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java

@@ -12,6 +12,7 @@
 package org.opensearch.security.auditlog.config;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -142,9 +143,12 @@ public static class Filter {
         private final Set<String> ignoredAuditRequests;
         @JsonProperty("ignore_headers")
         private final Set<String> ignoredCustomHeaders;
+        @JsonProperty("ignore_url_params")
+        private Set<String> ignoredUrlParams;
         private final WildcardMatcher ignoredAuditUsersMatcher;
         private final WildcardMatcher ignoredAuditRequestsMatcher;
         private final WildcardMatcher ignoredCustomHeadersMatcher;
+        private WildcardMatcher ignoredUrlParamsMatcher;
         private final Set<AuditCategory> disabledRestCategories;
         private final Set<AuditCategory> disabledTransportCategories;
 
@@ -159,6 +163,7 @@ public static class Filter {
             final Set<String> ignoredAuditUsers,
             final Set<String> ignoredAuditRequests,
             final Set<String> ignoredCustomHeaders,
+            final Set<String> ignoredUrlParams,
             final Set<AuditCategory> disabledRestCategories,
             final Set<AuditCategory> disabledTransportCategories
         ) {
@@ -174,6 +179,8 @@ public static class Filter {
             this.ignoredAuditRequestsMatcher = WildcardMatcher.from(ignoredAuditRequests);
             this.ignoredCustomHeaders = ignoredCustomHeaders;
             this.ignoredCustomHeadersMatcher = WildcardMatcher.from(ignoredCustomHeaders);
+            this.ignoredUrlParams = ignoredUrlParams;
+            this.ignoredUrlParamsMatcher = WildcardMatcher.from(ignoredUrlParams);
             this.disabledRestCategories = disabledRestCategories;
             this.disabledTransportCategories = disabledTransportCategories;
         }
@@ -269,6 +276,7 @@ public static Filter from(Map<String, Object> properties) throws JsonProcessingE
                 ignoredAuditUsers,
                 ignoreAuditRequests,
                 ignoreHeaders,
+                new HashSet<>(),
                 disabledRestCategories,
                 disabledTransportCategories
             );
@@ -314,6 +322,7 @@ public static Filter from(Settings settings) {
                 ignoredAuditUsers,
                 ignoreAuditRequests,
                 ignoreHeaders,
+                new HashSet<>(),
                 disabledRestCategories,
                 disabledTransportCategories
             );
@@ -422,6 +431,21 @@ WildcardMatcher getIgnoredCustomHeadersMatcher() {
             return ignoredCustomHeadersMatcher;
         }
 
+        @VisibleForTesting
+        WildcardMatcher getIgnoredUrlParamsMatcher() {
+            return ignoredUrlParamsMatcher;
+        }
+
+        /**
+         * Check if the specified url param is excluded from the audit
+         *
+         * @param param
+         * @return true if header should be excluded
+         */
+        public boolean shouldExcludeUrlParam(String param) {
+            return ignoredUrlParamsMatcher.test(param);
+        }
+
         /**
          * Check if the specified header is excluded from the audit
          *
@@ -441,6 +465,17 @@ public boolean isRequestAuditDisabled(String action) {
             return ignoredAuditRequestsMatcher.test(action);
         }
 
+        /**
+         * URL Params to redact for auditing
+         */
+        public void setIgnoredUrlParams(Set<String> ignoredUrlParams) {
+            if (ignoredUrlParams == null) {
+                return;
+            }
+            this.ignoredUrlParamsMatcher = WildcardMatcher.from(ignoredUrlParams);
+            this.ignoredUrlParams = ignoredUrlParams;
+        }
+
         /**
          * Disabled categories for REST API auditing
          * @return set of categories
@@ -470,6 +505,7 @@ public void log(Logger logger) {
             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);
+            logger.info("Auditing request url params {} is disabled.", ignoredUrlParamsMatcher);
         }
 
         @Override
@@ -497,6 +533,8 @@ public String toString() {
                 + ignoredAuditRequestsMatcher
                 + ", ignoredCustomHeaders="
                 + ignoredCustomHeadersMatcher
+                + ", ignoredUrlParamsMatcher="
+                + ignoredUrlParamsMatcher
                 + '}';
         }
     }