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

Filter out session cookies sent by Spring and Spring Boot integrations #2593

Merged
merged 6 commits into from
Mar 14, 2023
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

- Fix timestamps of slow and frozen frames for profiles ([#2584](https://github.com/getsentry/sentry-java/pull/2584))
- Deprecate reportFullDisplayed in favor of reportFullyDisplayed ([#2585](https://github.com/getsentry/sentry-java/pull/2585))
- Filter out session cookies sent by Spring and Spring Boot integrations ([#2593](https://github.com/getsentry/sentry-java/pull/2593))
- We filter out some common cookies like JSESSIONID
- We also read the value from `server.servlet.session.cookie.name` and filter it out
- No longer send event / transaction to Sentry if `beforeSend` / `beforeSendTransaction` throws ([#2591](https://github.com/getsentry/sentry-java/pull/2591))
- Add version to sentryClientName used in auth header ([#2596](https://github.com/getsentry/sentry-java/pull/2596))
- Keep integration names from being obfuscated ([#2599](https://github.com/getsentry/sentry-java/pull/2599))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,26 @@

import com.jakewharton.nopen.annotation.Open;
import io.sentry.IHub;
import io.sentry.SentryLevel;
import io.sentry.protocol.Request;
import io.sentry.util.HttpUtils;
import io.sentry.util.Objects;
import io.sentry.util.UrlUtils;
import jakarta.servlet.ServletContext;
import jakarta.servlet.SessionCookieConfig;
import jakarta.servlet.http.HttpServletRequest;
import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@Open
public class SentryRequestResolver {
private final @NotNull IHub hub;
private volatile @Nullable List<String> extraSecurityCookies;

public SentryRequestResolver(final @NotNull IHub hub) {
this.hub = Objects.requireNonNull(hub, "options is required");
Expand All @@ -31,27 +36,73 @@ public SentryRequestResolver(final @NotNull IHub hub) {
UrlUtils.parse(httpRequest.getRequestURL().toString());
urlDetails.applyToRequest(sentryRequest);
sentryRequest.setQueryString(httpRequest.getQueryString());
sentryRequest.setHeaders(resolveHeadersMap(httpRequest));
final @NotNull List<String> additionalSecurityCookieNames =
extractSecurityCookieNamesOrUseCached(httpRequest);
sentryRequest.setHeaders(resolveHeadersMap(httpRequest, additionalSecurityCookieNames));

if (hub.getOptions().isSendDefaultPii()) {
sentryRequest.setCookies(toString(httpRequest.getHeaders("Cookie")));
String cookieName = HttpUtils.COOKIE_HEADER_NAME;
final @Nullable List<String> filteredHeaders =
HttpUtils.filterOutSecurityCookiesFromHeader(
httpRequest.getHeaders(cookieName), cookieName, additionalSecurityCookieNames);
sentryRequest.setCookies(toString(filteredHeaders));
}
return sentryRequest;
}

@NotNull
Map<String, String> resolveHeadersMap(final @NotNull HttpServletRequest request) {
Map<String, String> resolveHeadersMap(
final @NotNull HttpServletRequest request,
final @NotNull List<String> additionalSecurityCookieNames) {
final Map<String, String> headersMap = new HashMap<>();
for (String headerName : Collections.list(request.getHeaderNames())) {
// do not copy personal information identifiable headers
if (hub.getOptions().isSendDefaultPii() || !HttpUtils.containsSensitiveHeader(headerName)) {
headersMap.put(headerName, toString(request.getHeaders(headerName)));
final @Nullable List<String> filteredHeaders =
HttpUtils.filterOutSecurityCookiesFromHeader(
request.getHeaders(headerName), headerName, additionalSecurityCookieNames);
headersMap.put(headerName, toString(filteredHeaders));
}
}
return headersMap;
}

private static @Nullable String toString(final @Nullable Enumeration<String> enumeration) {
return enumeration != null ? String.join(",", Collections.list(enumeration)) : null;
private List<String> extractSecurityCookieNamesOrUseCached(
final @NotNull HttpServletRequest httpRequest) {
if (extraSecurityCookies == null) {
synchronized (SentryRequestResolver.class) {
if (extraSecurityCookies == null) {
extraSecurityCookies = extractSecurityCookieNames(httpRequest);
}
}
}

return extraSecurityCookies;
}

private List<String> extractSecurityCookieNames(final @NotNull HttpServletRequest httpRequest) {
sl0thentr0py marked this conversation as resolved.
Show resolved Hide resolved
try {
final @Nullable ServletContext servletContext = httpRequest.getServletContext();
if (servletContext != null) {
final @Nullable SessionCookieConfig sessionCookieConfig =
servletContext.getSessionCookieConfig();
if (sessionCookieConfig != null) {
final @Nullable String cookieName = sessionCookieConfig.getName();
if (cookieName != null) {
return Arrays.asList(cookieName);
}
}
}
} catch (Throwable t) {
hub.getOptions()
.getLogger()
.log(SentryLevel.WARNING, "Failed to extract session cookie name from request.", t);
}

return Collections.emptyList();
}

private static @Nullable String toString(final @Nullable List<String> list) {
return list != null ? String.join(",", list) : null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import io.sentry.util.Objects;
import io.sentry.util.UrlUtils;
import java.net.URI;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -36,7 +37,11 @@ public SentryRequestResolver(final @NotNull IHub hub) {
sentryRequest.setHeaders(resolveHeadersMap(httpRequest.getHeaders()));

if (hub.getOptions().isSendDefaultPii()) {
sentryRequest.setCookies(toString(httpRequest.getHeaders().get("Cookies")));
String headerName = HttpUtils.COOKIE_HEADER_NAME;
sentryRequest.setCookies(
toString(
HttpUtils.filterOutSecurityCookiesFromHeader(
httpRequest.getHeaders().get(headerName), headerName, Collections.emptyList())));
}
return sentryRequest;
}
Expand All @@ -46,9 +51,13 @@ Map<String, String> resolveHeadersMap(final HttpHeaders request) {
final Map<String, String> headersMap = new HashMap<>();
for (Map.Entry<String, List<String>> entry : request.entrySet()) {
// do not copy personal information identifiable headers
if (hub.getOptions().isSendDefaultPii()
|| !HttpUtils.containsSensitiveHeader(entry.getKey())) {
headersMap.put(entry.getKey(), toString(entry.getValue()));
String headerName = entry.getKey();
if (hub.getOptions().isSendDefaultPii() || !HttpUtils.containsSensitiveHeader(headerName)) {
headersMap.put(
headerName,
toString(
HttpUtils.filterOutSecurityCookiesFromHeader(
entry.getValue(), headerName, Collections.emptyList())));
}
}
return headersMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import io.sentry.SentryOptions.RequestSize.MEDIUM
import io.sentry.SentryOptions.RequestSize.NONE
import io.sentry.SentryOptions.RequestSize.SMALL
import jakarta.servlet.FilterChain
import jakarta.servlet.ServletContext
import jakarta.servlet.http.HttpServletRequest
import org.assertj.core.api.Assertions
import org.mockito.kotlin.any
Expand Down Expand Up @@ -150,24 +151,27 @@ class SentrySpringFilterTest {
}

@Test
fun `when sendDefaultPii is set to true, attaches cookies information to Scope request`() {
fun `when sendDefaultPii is set to true, attaches filtered cookies to Scope request`() {
val sentryOptions = SentryOptions().apply {
isSendDefaultPii = true
}

val listener = fixture.getSut(
request = MockMvcRequestBuilders
.get(URI.create("http://example.com?param1=xyz"))
.header("Cookie", "name=value")
.header("Cookie", "name2=value2")
.buildRequest(MockServletContext()),
.header("Cookie", "name=value; JSESSIONID=123; mysessioncookiename=789")
.header("Cookie", "name2=value2; SID=456")
.buildRequest(servletContextWithCustomCookieName("mysessioncookiename")),
options = sentryOptions
)

listener.doFilter(fixture.request, fixture.response, fixture.chain)

assertNotNull(fixture.scope.request) {
assertEquals("name=value,name2=value2", it.cookies)
val expectedCookieString =
"name=value; JSESSIONID=[Filtered]; mysessioncookiename=[Filtered],name2=value2; SID=[Filtered]"
assertEquals(expectedCookieString, it.cookies)
assertEquals(expectedCookieString, it.headers!!["Cookie"])
}
}

Expand Down Expand Up @@ -269,4 +273,8 @@ class SentrySpringFilterTest {
}
}
}

private fun servletContextWithCustomCookieName(name: String): ServletContext {
return MockServletContext().also { it.sessionCookieConfig.name = name }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,26 @@

import com.jakewharton.nopen.annotation.Open;
import io.sentry.IHub;
import io.sentry.SentryLevel;
import io.sentry.protocol.Request;
import io.sentry.util.HttpUtils;
import io.sentry.util.Objects;
import io.sentry.util.UrlUtils;
import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.servlet.ServletContext;
import javax.servlet.SessionCookieConfig;
import javax.servlet.http.HttpServletRequest;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@Open
public class SentryRequestResolver {
private final @NotNull IHub hub;
private volatile @Nullable List<String> extraSecurityCookies;

public SentryRequestResolver(final @NotNull IHub hub) {
this.hub = Objects.requireNonNull(hub, "options is required");
Expand All @@ -31,27 +36,73 @@ public SentryRequestResolver(final @NotNull IHub hub) {
UrlUtils.parse(httpRequest.getRequestURL().toString());
urlDetails.applyToRequest(sentryRequest);
sentryRequest.setQueryString(httpRequest.getQueryString());
sentryRequest.setHeaders(resolveHeadersMap(httpRequest));
final @NotNull List<String> additionalSecurityCookieNames =
extractSecurityCookieNamesOrUseCached(httpRequest);
sentryRequest.setHeaders(resolveHeadersMap(httpRequest, additionalSecurityCookieNames));

if (hub.getOptions().isSendDefaultPii()) {
sentryRequest.setCookies(toString(httpRequest.getHeaders("Cookie")));
String cookieName = HttpUtils.COOKIE_HEADER_NAME;
final @Nullable List<String> filteredHeaders =
HttpUtils.filterOutSecurityCookiesFromHeader(
httpRequest.getHeaders(cookieName), cookieName, additionalSecurityCookieNames);
sentryRequest.setCookies(toString(filteredHeaders));
}
return sentryRequest;
}

@NotNull
Map<String, String> resolveHeadersMap(final @NotNull HttpServletRequest request) {
Map<String, String> resolveHeadersMap(
final @NotNull HttpServletRequest request,
final @NotNull List<String> additionalSecurityCookieNames) {
final Map<String, String> headersMap = new HashMap<>();
for (String headerName : Collections.list(request.getHeaderNames())) {
// do not copy personal information identifiable headers
if (hub.getOptions().isSendDefaultPii() || !HttpUtils.containsSensitiveHeader(headerName)) {
headersMap.put(headerName, toString(request.getHeaders(headerName)));
final @Nullable List<String> filteredHeaders =
HttpUtils.filterOutSecurityCookiesFromHeader(
request.getHeaders(headerName), headerName, additionalSecurityCookieNames);
headersMap.put(headerName, toString(filteredHeaders));
}
}
return headersMap;
}

private static @Nullable String toString(final @Nullable Enumeration<String> enumeration) {
return enumeration != null ? String.join(",", Collections.list(enumeration)) : null;
private List<String> extractSecurityCookieNamesOrUseCached(
final @NotNull HttpServletRequest httpRequest) {
if (extraSecurityCookies == null) {
synchronized (SentryRequestResolver.class) {
if (extraSecurityCookies == null) {
extraSecurityCookies = extractSecurityCookieNames(httpRequest);
}
}
}

return extraSecurityCookies;
}

private List<String> extractSecurityCookieNames(final @NotNull HttpServletRequest httpRequest) {
try {
final @Nullable ServletContext servletContext = httpRequest.getServletContext();
if (servletContext != null) {
final @Nullable SessionCookieConfig sessionCookieConfig =
servletContext.getSessionCookieConfig();
if (sessionCookieConfig != null) {
final @Nullable String cookieName = sessionCookieConfig.getName();
if (cookieName != null) {
return Arrays.asList(cookieName);
}
}
}
} catch (Throwable t) {
hub.getOptions()
.getLogger()
.log(SentryLevel.WARNING, "Failed to extract session cookie name from request.", t);
}

return Collections.emptyList();
}

private static @Nullable String toString(final @Nullable List<String> list) {
return list != null ? String.join(",", list) : null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import io.sentry.util.Objects;
import io.sentry.util.UrlUtils;
import java.net.URI;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -36,7 +37,11 @@ public SentryRequestResolver(final @NotNull IHub hub) {
sentryRequest.setHeaders(resolveHeadersMap(httpRequest.getHeaders()));

if (hub.getOptions().isSendDefaultPii()) {
sentryRequest.setCookies(toString(httpRequest.getHeaders().get("Cookies")));
String headerName = HttpUtils.COOKIE_HEADER_NAME;
sentryRequest.setCookies(
toString(
HttpUtils.filterOutSecurityCookiesFromHeader(
httpRequest.getHeaders().get(headerName), headerName, Collections.emptyList())));
}
return sentryRequest;
}
Expand All @@ -46,9 +51,13 @@ Map<String, String> resolveHeadersMap(final HttpHeaders request) {
final Map<String, String> headersMap = new HashMap<>();
for (Map.Entry<String, List<String>> entry : request.entrySet()) {
// do not copy personal information identifiable headers
if (hub.getOptions().isSendDefaultPii()
|| !HttpUtils.containsSensitiveHeader(entry.getKey())) {
headersMap.put(entry.getKey(), toString(entry.getValue()));
String headerName = entry.getKey();
if (hub.getOptions().isSendDefaultPii() || !HttpUtils.containsSensitiveHeader(headerName)) {
headersMap.put(
headerName,
toString(
HttpUtils.filterOutSecurityCookiesFromHeader(
entry.getValue(), headerName, Collections.emptyList())));
}
}
return headersMap;
Expand Down
Loading