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

Redact sensitive URL parameters from audit logging #4067

Merged
merged 23 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
642ac36
Redact JWT URL Parameter from the audit log
cwperks Jan 22, 2024
adab359
Make ignoreUrlParams static
cwperks Jan 22, 2024
e82d39f
WIP on redacting jwtUrlParameter
cwperks Jan 22, 2024
7ce47d3
Consume jwtUrlParameter
cwperks Jan 22, 2024
cb88036
Fix unconsumed parameter exception when authentication with jwtUrlPar…
cwperks Jan 23, 2024
61aa3c2
Create separate test suite
cwperks Jan 23, 2024
190df88
Add test that assert log entry for unauthed request
cwperks Jan 23, 2024
7b5e63b
Merge branch 'main' into fix-jwt-url-param
cwperks Jan 31, 2024
ab78f8e
getConsumedParams only on NettyRequest
cwperks Jan 31, 2024
df8fe59
Update src/main/java/org/opensearch/security/filter/OpenSearchRequest…
cwperks Feb 1, 2024
6a9330c
Use HashMap
cwperks Feb 1, 2024
ed081f1
Merge branch 'fix-jwt-url-param' into redact-jwt
cwperks Feb 6, 2024
aaa23a5
Resolve merge conflict
cwperks Feb 6, 2024
e3ed574
Remove option to disable url param redaction
cwperks Feb 6, 2024
56244cb
Add unit tests
cwperks Feb 6, 2024
75eba72
Redact sensitive url params from auditlog
cwperks Feb 6, 2024
222a2fc
Merge branch 'main' into redact-jwt
cwperks Feb 22, 2024
c4e49b2
Rebase
cwperks Feb 22, 2024
1939a03
Remove unneeded code
cwperks Feb 22, 2024
638599f
Back out JwtAuthenticationTests changes
cwperks Feb 22, 2024
161639b
Add generic way to define sensitive url params on http authenticator
cwperks Feb 22, 2024
e96f5b7
Ensure that other url params stay unredacted
cwperks Feb 22, 2024
be70a9c
Update expected payload
cwperks Feb 22, 2024
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)
cwperks marked this conversation as resolved.
Show resolved Hide resolved
);
}
}

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 @@ -15,9 +15,11 @@
import java.security.AccessController;
import java.security.PrivilegedAction;
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 @@ -194,6 +196,14 @@
);
}

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

Check warning on line 202 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#L202

Added line #L202 was not covered by tests
cwperks marked this conversation as resolved.
Show resolved Hide resolved
}
return Collections.emptySet();

Check warning on line 204 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#L204

Added line #L204 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