Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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 @@
);
}

@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
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 @@
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 @@
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 @@
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 @@
ignoredAuditUsers,
ignoreAuditRequests,
ignoreHeaders,
new HashSet<>(),
disabledRestCategories,
disabledTransportCategories
);
Expand Down Expand Up @@ -314,6 +322,7 @@
ignoredAuditUsers,
ignoreAuditRequests,
ignoreHeaders,
new HashSet<>(),
disabledRestCategories,
disabledTransportCategories
);
Expand Down Expand Up @@ -422,6 +431,21 @@
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 @@
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 @@
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 @@
+ ignoredAuditRequestsMatcher
+ ", ignoredCustomHeaders="
+ ignoredCustomHeadersMatcher
+ ", ignoredUrlParamsMatcher="
+ ignoredUrlParamsMatcher
+ '}';
}
}
Expand Down
Loading
Loading