Skip to content

Commit

Permalink
Redact sensitive URL parameters from audit logging (#4067)
Browse files Browse the repository at this point in the history 
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Co-authored-by: Peter Nied <[email protected]>
(cherry picked from commit e40efdc)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
@github-actions @peternied
github-actions[bot] and peternied committed Feb 23, 2024
1 parent 51e996d commit 4322b73
Show file tree
Hide file tree
Showing 11 changed files with 152 additions and 8 deletions.
20 changes: 19 additions & 1 deletion ...integrationTest/java/org/opensearch/security/http/JwtAuthenticationWithUrlParamTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
);
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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();
}
Expand Down Expand Up @@ -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;
Expand All @@ -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();
Expand Down
30 changes: 29 additions & 1 deletion src/integrationTest/java/org/opensearch/test/framework/audit/AuditMessagePredicate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -47,6 +49,7 @@ private AuditMessagePredicate(
AuditCategory category,
Origin requestLayer,
String restRequestPath,
Map<String, String> restParams,
String initiatingUser,
Method requestMethod,
String transportRequestType,
Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -110,6 +114,7 @@ public AuditMessagePredicate withLayer(Origin layer) {
category,
layer,
restRequestPath,
restParams,
initiatingUser,
requestMethod,
transportRequestType,
Expand All @@ -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,
Expand All @@ -138,6 +159,7 @@ public AuditMessagePredicate withInitiatingUser(String user) {
category,
requestLayer,
restRequestPath,
restParams,
user,
requestMethod,
transportRequestType,
Expand All @@ -156,6 +178,7 @@ public AuditMessagePredicate withRestMethod(Method method) {
category,
requestLayer,
restRequestPath,
restParams,
initiatingUser,
method,
transportRequestType,
Expand All @@ -170,6 +193,7 @@ public AuditMessagePredicate withTransportRequestType(String type) {
category,
requestLayer,
restRequestPath,
restParams,
initiatingUser,
requestMethod,
type,
Expand All @@ -184,6 +208,7 @@ public AuditMessagePredicate withEffectiveUser(String user) {
category,
requestLayer,
restRequestPath,
restParams,
initiatingUser,
requestMethod,
transportRequestType,
Expand All @@ -206,6 +231,7 @@ public AuditMessagePredicate withIndex(String indexName) {
category,
requestLayer,
restRequestPath,
restParams,
initiatingUser,
requestMethod,
transportRequestType,
Expand All @@ -220,6 +246,7 @@ public AuditMessagePredicate withPrivilege(String privilegeAction) {
category,
requestLayer,
restRequestPath,
restParams,
initiatingUser,
requestMethod,
transportRequestType,
Expand All @@ -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()));
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -199,6 +201,14 @@ public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest
);
}

@Override
public Set<String> getSensitiveUrlParams() {
if (jwtUrlParameter != null) {
return Set.of(jwtUrlParameter);

Check warning on line 207 in src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L207 was not covered by tests
}
return Collections.emptySet();

Check warning on line 209 in src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L209 was not covered by tests
}

@Override
public String getType() {
return "jwt";
Expand Down
38 changes: 38 additions & 0 deletions src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -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
) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -269,6 +276,7 @@ public static Filter from(Map<String, Object> properties) throws JsonProcessingE
ignoredAuditUsers,
ignoreAuditRequests,
ignoreHeaders,
new HashSet<>(),
disabledRestCategories,
disabledTransportCategories
);
Expand Down Expand Up @@ -314,6 +322,7 @@ public static Filter from(Settings settings) {
ignoredAuditUsers,
ignoreAuditRequests,
ignoreHeaders,
new HashSet<>(),
disabledRestCategories,
disabledTransportCategories
);
Expand Down Expand Up @@ -422,6 +431,21 @@ WildcardMatcher getIgnoredCustomHeadersMatcher() {
return ignoredCustomHeadersMatcher;
}

@VisibleForTesting
WildcardMatcher getIgnoredUrlParamsMatcher() {
return ignoredUrlParamsMatcher;

Check warning on line 436 in src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L436 was not covered by tests
}

/**
* 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
*
Expand All @@ -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;

Check warning on line 473 in src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L473 was not covered by tests
}
this.ignoredUrlParamsMatcher = WildcardMatcher.from(ignoredUrlParams);
this.ignoredUrlParams = ignoredUrlParams;
}

/**
* Disabled categories for REST API auditing
* @return set of categories
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -497,6 +533,8 @@ public String toString() {
+ ignoredAuditRequestsMatcher
+ ", ignoredCustomHeaders="
+ ignoredCustomHeadersMatcher
+ ", ignoredUrlParamsMatcher="
+ ignoredUrlParamsMatcher
+ '}';
}
}
Expand Down
Loading

0 comments on commit 4322b73

Please sign in to comment.